Merge pull request #7332 from Roasbeef/sign-output-raw-bug-fix

[1/?] - lnwallet: ensure that SignOutputRaw can sign w/ non-default sighash f…
lightningnetwork · Jan 26, 2023 · 292551f · 292551f
2 parents 9187d46 + be04709
commit 292551f
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 22 deletions.
diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md
@@ -100,6 +100,11 @@ current gossip sync query status.
 * [Sign/Verify messages and signatures for single
   addresses](https://github.com/lightningnetwork/lnd/pull/7231).
 
+* [A bug has been fixed within the `SignOutputRaw` call for taproot
+  signatures](https://github.com/lightningnetwork/lnd/pull/7332).  The
+  `SignOutputRaw` call will now properly work for taproot signatures with a
+  non-default sighash.
+
 ## Wallet
 
 * [Allows Taproot public keys and tap scripts to be imported as watch-only

diff --git a/go.mod b/go.mod
@@ -3,7 +3,7 @@ module github.com/lightningnetwork/lnd
 require (
 	github.com/NebulousLabs/go-upnp v0.0.0-20180202185039-29b680b06c82
 	github.com/Yawning/aez v0.0.0-20211027044916-e49e68abd344
-	github.com/btcsuite/btcd v0.23.4
+	github.com/btcsuite/btcd v0.23.5-0.20230125025938-be056b0a0b2f
 	github.com/btcsuite/btcd/btcec/v2 v2.2.2
 	github.com/btcsuite/btcd/btcutil v1.1.3
 	github.com/btcsuite/btcd/btcutil/psbt v1.1.5

diff --git a/go.sum b/go.sum
@@ -78,8 +78,9 @@ github.com/btcsuite/btcd v0.22.0-beta.0.20220316175102-8d5c75c28923/go.mod h1:ta
 github.com/btcsuite/btcd v0.23.0/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY=
 github.com/btcsuite/btcd v0.23.1/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY=
 github.com/btcsuite/btcd v0.23.3/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY=
-github.com/btcsuite/btcd v0.23.4 h1:IzV6qqkfwbItOS/sg/aDfPDsjPP8twrCOE2R93hxMlQ=
 github.com/btcsuite/btcd v0.23.4/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY=
+github.com/btcsuite/btcd v0.23.5-0.20230125025938-be056b0a0b2f h1:UJ/S/pV25+YsK0CJRJh8RDpTgy5h1oXWjOd4fp+opvY=
+github.com/btcsuite/btcd v0.23.5-0.20230125025938-be056b0a0b2f/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY=
 github.com/btcsuite/btcd/btcec/v2 v2.1.0/go.mod h1:2VzYrv4Gm4apmbVVsSq5bqf1Ec8v56E48Vt0Y/umPgA=
 github.com/btcsuite/btcd/btcec/v2 v2.1.1/go.mod h1:ctjw4H1kknNJmRN4iP1R7bTQ+v3GJkZBd6mui8ZsAZE=
 github.com/btcsuite/btcd/btcec/v2 v2.1.3/go.mod h1:ctjw4H1kknNJmRN4iP1R7bTQ+v3GJkZBd6mui8ZsAZE=

diff --git a/lntest/itest/lnd_signer_test.go b/lntest/itest/lnd_signer_test.go
@@ -217,7 +217,7 @@ func runSignOutputRaw(ht *lntemp.HarnessTest, alice *node.HarnessNode) {
 	assertSignOutputRaw(
 		ht, alice, targetPubKey, &signrpc.KeyDescriptor{
 			RawKeyBytes: keyDesc.RawKeyBytes,
-		},
+		}, txscript.SigHashAll,
 	)
 
 	// Now try again, this time only with the (0 index!) key locator.
@@ -227,7 +227,7 @@ func runSignOutputRaw(ht *lntemp.HarnessTest, alice *node.HarnessNode) {
 				KeyFamily: keyDesc.KeyLoc.KeyFamily,
 				KeyIndex:  keyDesc.KeyLoc.KeyIndex,
 			},
-		},
+		}, txscript.SigHashAll,
 	)
 
 	// And now test everything again with a new key where we know the index
@@ -245,7 +245,7 @@ func runSignOutputRaw(ht *lntemp.HarnessTest, alice *node.HarnessNode) {
 	assertSignOutputRaw(
 		ht, alice, targetPubKey, &signrpc.KeyDescriptor{
 			RawKeyBytes: keyDesc.RawKeyBytes,
-		},
+		}, txscript.SigHashAll,
 	)
 
 	// Now try again, this time only with the key locator.
@@ -255,7 +255,17 @@ func runSignOutputRaw(ht *lntemp.HarnessTest, alice *node.HarnessNode) {
 				KeyFamily: keyDesc.KeyLoc.KeyFamily,
 				KeyIndex:  keyDesc.KeyLoc.KeyIndex,
 			},
-		},
+		}, txscript.SigHashAll,
+	)
+
+	// Finally, we'll try again, but this time with a non-default sighash.
+	assertSignOutputRaw(
+		ht, alice, targetPubKey, &signrpc.KeyDescriptor{
+			KeyLoc: &signrpc.KeyLocator{
+				KeyFamily: keyDesc.KeyLoc.KeyFamily,
+				KeyIndex:  keyDesc.KeyLoc.KeyIndex,
+			},
+		}, txscript.SigHashSingle,
 	)
 }
 
@@ -264,7 +274,8 @@ func runSignOutputRaw(ht *lntemp.HarnessTest, alice *node.HarnessNode) {
 // SignOutputRaw RPC with the key descriptor provided.
 func assertSignOutputRaw(ht *lntemp.HarnessTest,
 	alice *node.HarnessNode, targetPubKey *btcec.PublicKey,
-	keyDesc *signrpc.KeyDescriptor) {
+	keyDesc *signrpc.KeyDescriptor,
+	sigHash txscript.SigHashType) {
 
 	pubKeyHash := btcutil.Hash160(targetPubKey.SerializeCompressed())
 	targetAddr, err := btcutil.NewAddressWitnessPubKeyHash(
@@ -326,14 +337,14 @@ func assertSignOutputRaw(ht *lntemp.HarnessTest,
 			},
 			InputIndex:    0,
 			KeyDesc:       keyDesc,
-			Sighash:       uint32(txscript.SigHashAll),
+			Sighash:       uint32(sigHash),
 			WitnessScript: targetScript,
 		}},
 	}
 	signResp := alice.RPC.SignOutputRaw(signReq)
 
 	tx.TxIn[0].Witness = wire.TxWitness{
-		append(signResp.RawSigs[0], byte(txscript.SigHashAll)),
+		append(signResp.RawSigs[0], byte(sigHash)),
 		targetPubKey.SerializeCompressed(),
 	}
 

diff --git a/lntest/itest/lnd_taproot_test.go b/lntest/itest/lnd_taproot_test.go
@@ -48,7 +48,13 @@ func testTaproot(ht *lntemp.HarnessTest) {
 	testTaprootSendCoinsKeySpendBip86(ht, ht.Alice)
 	testTaprootComputeInputScriptKeySpendBip86(ht, ht.Alice)
 	testTaprootSignOutputRawScriptSpend(ht, ht.Alice)
+	testTaprootSignOutputRawScriptSpend(
+		ht, ht.Alice, txscript.SigHashSingle,
+	)
 	testTaprootSignOutputRawKeySpendBip86(ht, ht.Alice)
+	testTaprootSignOutputRawKeySpendBip86(
+		ht, ht.Alice, txscript.SigHashSingle,
+	)
 	testTaprootSignOutputRawKeySpendRootHash(ht, ht.Alice)
 
 	testTaprootMuSig2KeySpendBip86(ht, ht.Alice)
@@ -219,7 +225,7 @@ func testTaprootComputeInputScriptKeySpendBip86(ht *lntemp.HarnessTest,
 // testTaprootSignOutputRawScriptSpend tests sending to and spending from p2tr
 // script addresses using the script path with the SignOutputRaw RPC.
 func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
-	alice *node.HarnessNode) {
+	alice *node.HarnessNode, sigHashType ...txscript.SigHashType) {
 
 	// For the next step, we need a public key. Let's use a special family
 	// for this.
@@ -260,7 +266,18 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
 		input.TaprootSignatureWitnessSize, tapscript,
 	)
 	estimator.AddP2WKHOutput()
+
 	estimatedWeight := int64(estimator.Weight())
+	sigHash := txscript.SigHashDefault
+	if len(sigHashType) != 0 {
+		sigHash = sigHashType[0]
+
+		// If a non-default sighash is used, then we'll need to add an
+		// extra byte to account for the sighash that doesn't exist in
+		// the default case.
+		estimatedWeight++
+	}
+
 	requiredFee := feeRate.FeeForWeight(estimatedWeight)
 
 	tx := wire.NewMsgTx(2)
@@ -290,7 +307,7 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
 			Output:        utxoInfo[0],
 			InputIndex:    0,
 			KeyDesc:       keyDesc,
-			Sighash:       uint32(txscript.SigHashDefault),
+			Sighash:       uint32(sigHash),
 			WitnessScript: leaf2.Script,
 			SignMethod:    signMethodTapscript,
 		}},
@@ -309,7 +326,7 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
 			Output:        utxoInfo[0],
 			InputIndex:    0,
 			KeyDesc:       keyDesc,
-			Sighash:       uint32(txscript.SigHashDefault),
+			Sighash:       uint32(sigHash),
 			WitnessScript: leaf2.Script,
 		}},
 		PrevOutputs: utxoInfo,
@@ -327,7 +344,7 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
 			Output:        utxoInfo[0],
 			InputIndex:    0,
 			KeyDesc:       keyDesc,
-			Sighash:       uint32(txscript.SigHashDefault),
+			Sighash:       uint32(sigHash),
 			WitnessScript: leaf2.Script,
 			SignMethod:    signMethodTapscript,
 		}},
@@ -339,10 +356,12 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
 	controlBlockBytes, err := tapscript.ControlBlock.ToBytes()
 	require.NoError(ht, err)
 
+	sig := signResp.RawSigs[0]
+	if len(sigHashType) != 0 {
+		sig = append(sig, byte(sigHashType[0]))
+	}
 	tx.TxIn[0].Witness = wire.TxWitness{
-		signResp.RawSigs[0],
-		leaf2.Script,
-		controlBlockBytes,
+		sig, leaf2.Script, controlBlockBytes,
 	}
 
 	// Serialize, weigh and publish the TX now, then make sure the
@@ -364,7 +383,7 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
 // also be spent using the key spend path through the SignOutputRaw RPC using a
 // BIP0086 key spend only commitment.
 func testTaprootSignOutputRawKeySpendBip86(ht *lntemp.HarnessTest,
-	alice *node.HarnessNode) {
+	alice *node.HarnessNode, sigHashType ...txscript.SigHashType) {
 
 	// For the next step, we need a public key. Let's use a special family
 	// for this.
@@ -393,10 +412,15 @@ func testTaprootSignOutputRawKeySpendBip86(ht *lntemp.HarnessTest,
 		ht, alice, lnrpc.AddressType_WITNESS_PUBKEY_HASH,
 	)
 
+	sigHash := txscript.SigHashDefault
+	if len(sigHashType) != 0 {
+		sigHash = sigHashType[0]
+	}
+
 	// Create fee estimation for a p2tr input and p2wkh output.
 	feeRate := chainfee.SatPerKWeight(12500)
 	estimator := input.TxWeightEstimator{}
-	estimator.AddTaprootKeySpendInput(txscript.SigHashDefault)
+	estimator.AddTaprootKeySpendInput(sigHash)
 	estimator.AddP2WKHOutput()
 	estimatedWeight := int64(estimator.Weight())
 	requiredFee := feeRate.FeeForWeight(estimatedWeight)
@@ -425,16 +449,18 @@ func testTaprootSignOutputRawKeySpendBip86(ht *lntemp.HarnessTest,
 			InputIndex:  0,
 			KeyDesc:     keyDesc,
 			SingleTweak: dummyKeyTweak[:],
-			Sighash:     uint32(txscript.SigHashDefault),
+			Sighash:     uint32(sigHash),
 			SignMethod:  signMethodBip86,
 		}},
 		PrevOutputs: utxoInfo,
 	}
 	signResp := alice.RPC.SignOutputRaw(signReq)
 
-	tx.TxIn[0].Witness = wire.TxWitness{
-		signResp.RawSigs[0],
+	sig := signResp.RawSigs[0]
+	if len(sigHashType) != 0 {
+		sig = append(sig, byte(sigHash))
 	}
+	tx.TxIn[0].Witness = wire.TxWitness{sig}
 
 	// Serialize, weigh and publish the TX now, then make sure the
 	// coins are sent and confirmed to the final sweep destination address.
@@ -1506,6 +1532,8 @@ func publishTxAndConfirmSweep(ht *lntemp.HarnessTest, node *node.HarnessNode,
 	tx *wire.MsgTx, estimatedWeight int64,
 	spendRequest *chainrpc.SpendRequest, sweepAddr string) {
 
+	ht.Helper()
+
 	// Before we publish the tx that spends the p2tr transaction, we want to
 	// register a spend listener that we expect to fire after mining the
 	// block.

diff --git a/lnwallet/btcwallet/signer.go b/lnwallet/btcwallet/signer.go
@@ -401,9 +401,19 @@ func (b *BtcWallet) SignOutputRaw(tx *wire.MsgTx,
 			if err != nil {
 				return nil, err
 			}
+
+		default:
+			return nil, fmt.Errorf("unknown sign method: %v",
+				signDesc.SignMethod)
 		}
 
-		sig, err := schnorr.ParseSignature(rawSig)
+		// The signature returned above might have a sighash flag
+		// attached if a non-default type was used. We'll slice this
+		// off if it exists to ensure we can properly parse the raw
+		// signature.
+		sig, err := schnorr.ParseSignature(
+			rawSig[:schnorr.SignatureSize],
+		)
 		if err != nil {
 			return nil, err
 		}