txscript: modify TweakTaprootPrivKey to operate on private key copy

In this commit, we fix an inadvertent mutation bug that would at times
cause the private key passed into the tweak function to actually be
*modified* in place.

We fix this by accepting the value instead of a pointer. The actual
private key struct itself contains no pointer fields, so this is
effectively a deep copy via dereference.

We also add a new unit test that fails w/o this change, to show that the
private key was indeed being modified.

txscript/sign.go

@@ -82,7 +82,7 @@ func RawTxInTaprootSignature(tx *wire.MsgTx, sigHashes *TxSigHashes, idx int,
 
 	// Before we sign the sighash, we'll need to apply the taptweak to the
 	// private key based on the tapScriptRootHash.
-	privKeyTweak := TweakTaprootPrivKey(key, tapScriptRootHash)
+	privKeyTweak := TweakTaprootPrivKey(*key, tapScriptRootHash)
 
 	// With the sighash constructed, we can sign it with the specified
 	// private key.

txscript/taproot.go

@@ -296,12 +296,12 @@ func ComputeTaprootKeyNoScript(internalKey *btcec.PublicKey) *btcec.PublicKey {
 // but on the private key instead. The final key is derived as: privKey +
 // h_tapTweak(internalKey || merkleRoot) % N, where N is the order of the
 // secp256k1 curve, and merkleRoot is the root hash of the tapscript tree.
-func TweakTaprootPrivKey(privKey *btcec.PrivateKey,
+func TweakTaprootPrivKey(privKey btcec.PrivateKey,
 	scriptRoot []byte) *btcec.PrivateKey {
 
 	// If the corresponding public key has an odd y coordinate, then we'll
 	// negate the private key as specified in BIP 341.
-	privKeyScalar := &privKey.Key
+	privKeyScalar := privKey.Key
 	pubKeyBytes := privKey.PubKey().SerializeCompressed()
 	if pubKeyBytes[0] == secp.PubKeyFormatCompressedOdd {
 		privKeyScalar.Negate()

txscript/taproot_test.go

@@ -166,8 +166,8 @@ func TestControlBlockParsing(t *testing.T) {
 // key, then generating a public key from that. This test a quickcheck test to
 // assert the following invariant:
 //
-// * taproot_tweak_pubkey(pubkey_gen(seckey), h)[1] ==
-//   pubkey_gen(taproot_tweak_seckey(seckey, h))
+//   - taproot_tweak_pubkey(pubkey_gen(seckey), h)[1] ==
+//     pubkey_gen(taproot_tweak_seckey(seckey, h))
 func TestTaprootScriptSpendTweak(t *testing.T) {
 	t.Parallel()
 
@@ -186,7 +186,7 @@ func TestTaprootScriptSpendTweak(t *testing.T) {
 		tweakedPub := ComputeTaprootOutputKey(privKey.PubKey(), x[:])
 
 		// Now we'll generate the corresponding tweaked private key.
-		tweakedPriv := TweakTaprootPrivKey(privKey, x[:])
+		tweakedPriv := TweakTaprootPrivKey(*privKey, x[:])
 
 		// The public key for this private key should be the same as
 		// the tweaked public key we generate above.
@@ -204,6 +204,42 @@ func TestTaprootScriptSpendTweak(t *testing.T) {
 
 }
 
+// TestTaprootTweakNoMutation tests that the underlying private key passed into
+// TweakTaprootPrivKey is never mutated.
+func TestTaprootTweakNoMutation(t *testing.T) {
+	t.Parallel()
+
+	// Assert that given a random tweak, and a random private key, that if
+	// we tweak the private key it remains unaffected.
+	f := func(privBytes, tweak [32]byte) bool {
+		privKey, _ := btcec.PrivKeyFromBytes(privBytes[:])
+
+		// Now we'll generate the corresponding tweaked private key.
+		tweakedPriv := TweakTaprootPrivKey(*privKey, tweak[:])
+
+		// The tweaked private key and the original private key should
+		// NOT be the same.
+		if *privKey == *tweakedPriv {
+			t.Logf("private key was mutated")
+			return false
+		}
+
+		// We shuold be able to re-derive the private key from raw
+		// bytes and have that match up again.
+		privKeyCopy, _ := btcec.PrivKeyFromBytes(privBytes[:])
+		if *privKey != *privKeyCopy {
+			t.Logf("private doesn't match")
+			return false
+		}
+
+		return true
+	}
+
+	if err := quick.Check(f, nil); err != nil {
+		t.Fatalf("private key modified: %v", err)
+	}
+}
+
 // TestTaprootConstructKeyPath tests the key spend only taproot construction.
 func TestTaprootConstructKeyPath(t *testing.T) {
 	checkPath := func(branch uint32, expectedAddresses []string) {