From 9b422acffeaccb30e43bded9df9d4c22dd5898c9 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 23 Jan 2023 21:13:00 -0800 Subject: [PATCH 1/5] build: update to latest version of btcd --- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 66bed097d..8428c3c2b 100644 --- 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 index a6cae7986..02a68c1fb 100644 --- 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= From 843c62b59da8fb68ad9684f42c3a544d9b272ca8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 16 Jan 2023 19:55:21 -0800 Subject: [PATCH 2/5] lnwallet: ensure that SignOutputRaw can sign w/ non-default sighash for schnorr sigs Before this commitment, we'd end up failing in `schnorr.ParseSignature` if a non-default sighash was used. To fix that, we'll slice the signature to only pass in the sig w/o the sighash flag. --- lnwallet/btcwallet/signer.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lnwallet/btcwallet/signer.go b/lnwallet/btcwallet/signer.go index 91ee53e80..9522cefc4 100644 --- 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 } From 8ab598e40c8ef6dfdd73c724873a244b97f6bfdb Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 23 Jan 2023 21:07:00 -0800 Subject: [PATCH 3/5] lntest/itest: make publishTxAndConfirmSweep a test helper Calling ht.Helper() means we'll get the proper line number if an error hapens. --- lntest/itest/lnd_taproot_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lntest/itest/lnd_taproot_test.go b/lntest/itest/lnd_taproot_test.go index 192ded4cd..684b9d1b1 100644 --- a/lntest/itest/lnd_taproot_test.go +++ b/lntest/itest/lnd_taproot_test.go @@ -1506,6 +1506,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. From 3d8f284af7a274436241514c2e02b08f822ffb07 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 23 Jan 2023 21:08:02 -0800 Subject: [PATCH 4/5] lntest/itest: update taproot signing itest to use default and non-default sighashes Without the prior commit, this test fails with: ``` rpc error: code = Unknown desc = malformed signature: too long: 65 > 64 ``` --- lntest/itest/lnd_signer_test.go | 25 +++++++++++----- lntest/itest/lnd_taproot_test.go | 50 ++++++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/lntest/itest/lnd_signer_test.go b/lntest/itest/lnd_signer_test.go index 15b7903f3..ce16240a3 100644 --- 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 index 684b9d1b1..1cbb82355 100644 --- 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. From be047096c2e83782a94bf0e522b5271d7611129d Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 23 Jan 2023 21:11:06 -0800 Subject: [PATCH 5/5] docs/release-notes: add entry for taproot sighash bug fix --- docs/release-notes/release-notes-0.16.0.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 571224548..11bc8c31b 100644 --- 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