From 646444cdfe8096c27ad7dfa31938e97652ab8e90 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 28 Aug 2023 11:13:25 +0200 Subject: [PATCH 1/2] chanfunding: fix PSBT funding for taproot chans --- lnwallet/chanfunding/psbt_assembler.go | 38 ++++++++------------------ 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/lnwallet/chanfunding/psbt_assembler.go b/lnwallet/chanfunding/psbt_assembler.go index e509d5ce5..f1e1f2c95 100644 --- a/lnwallet/chanfunding/psbt_assembler.go +++ b/lnwallet/chanfunding/psbt_assembler.go @@ -1,7 +1,6 @@ package chanfunding import ( - "crypto/sha256" "errors" "fmt" "sync" @@ -178,23 +177,26 @@ func (i *PsbtIntent) FundingParams() (btcutil.Address, int64, *psbt.Packet, // The funding output needs to be known already at this point, which // means we need to have the local and remote multisig keys bound // already. - witnessScript, out, err := i.FundingOutput() + _, out, err := i.FundingOutput() if err != nil { return nil, 0, nil, fmt.Errorf("unable to create funding "+ "output: %v", err) } - witnessScriptHash := sha256.Sum256(witnessScript) - // Encode the address in the human readable bech32 format. - addr, err := btcutil.NewAddressWitnessScriptHash( - witnessScriptHash[:], i.netParams, - ) + script, err := txscript.ParsePkScript(out.PkScript) + if err != nil { + return nil, 0, nil, fmt.Errorf("unable to parse funding "+ + "output script: %w", err) + } + + // Encode the address in the human-readable bech32 format. + addr, err := script.Address(i.netParams) if err != nil { return nil, 0, nil, fmt.Errorf("unable to encode address: %v", err) } - // We'll also encode the address/amount in a machine readable raw PSBT + // We'll also encode the address/amount in a machine-readable raw PSBT // format. If the user supplied a base PSBT, we'll add the output to // that one, otherwise we'll create a new one. packet := i.BasePsbt @@ -597,8 +599,8 @@ func verifyAllInputsSegWit(txIns []*wire.TxIn, ins []psbt.PInput) error { txIn := txIns[idx] txOut := utxo.TxOut[txIn.PreviousOutPoint.Index] - if !isSegWitScript(txOut.PkScript) && - !isSegWitScript(in.RedeemScript) { + if !txscript.IsWitnessProgram(txOut.PkScript) && + !txscript.IsWitnessProgram(in.RedeemScript) { return fmt.Errorf("input %d is non-SegWit "+ "spend or missing redeem script", idx) @@ -614,19 +616,3 @@ func verifyAllInputsSegWit(txIns []*wire.TxIn, ins []psbt.PInput) error { return nil } - -// isSegWitScript returns true if the given pkScript can be parsed successfully -// as a SegWit v0 spend. -func isSegWitScript(pkScript []byte) bool { - if len(pkScript) == 0 { - return false - } - - parsed, err := txscript.ParsePkScript(pkScript) - if err != nil { - return false - } - - return parsed.Class() == txscript.WitnessV0PubKeyHashTy || - parsed.Class() == txscript.WitnessV0ScriptHashTy -} From 590dc0c3b3c7b5cce711f127b6c2e8d16b7f9caf Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 28 Aug 2023 13:09:52 +0200 Subject: [PATCH 2/2] itest: unify PSBT channel funding tests, add taproot With this commit we unify three existing PSBT channel funding integration tests into a single one with the goal of testing all three cases with simple taproot channels as well. --- itest/list_on_test.go | 8 -- itest/lnd_psbt_test.go | 152 ++++++++++++++++++++++++++------ itest/lnd_remote_signer_test.go | 5 +- lntest/harness.go | 18 ++++ 4 files changed, 149 insertions(+), 34 deletions(-) diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 613e572fe..e2a603130 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -269,14 +269,6 @@ var allTestCases = []*lntest.TestCase{ Name: "psbt channel funding", TestFunc: testPsbtChanFunding, }, - { - Name: "psbt channel funding external", - TestFunc: testPsbtChanFundingExternal, - }, - { - Name: "psbt channel funding single step", - TestFunc: testPsbtChanFundingSingleStep, - }, { Name: "sign psbt", TestFunc: testSignPsbt, diff --git a/itest/lnd_psbt_test.go b/itest/lnd_psbt_test.go index ffa680d75..3bf1cfa04 100644 --- a/itest/lnd_psbt_test.go +++ b/itest/lnd_psbt_test.go @@ -2,6 +2,7 @@ package itest import ( "bytes" + "testing" "time" "github.com/btcsuite/btcd/btcec/v2" @@ -27,19 +28,116 @@ import ( // by using a Partially Signed Bitcoin Transaction that funds the channel // multisig funding output. func testPsbtChanFunding(ht *lntest.HarnessTest) { - // First, we'll create two new nodes that we'll use to open channels - // between for this test. Dave gets some coins that will be used to - // fund the PSBT, just to make sure that Carol has an empty wallet. - carol := ht.NewNode("carol", nil) - dave := ht.NewNode("dave", nil) + const ( + burnAddr = "bcrt1qxsnqpdc842lu8c0xlllgvejt6rhy49u6fmpgyz" + ) - runPsbtChanFunding(ht, carol, dave) + testCases := []struct { + name string + commitmentType lnrpc.CommitmentType + private bool + }{ + { + name: "anchors", + commitmentType: lnrpc.CommitmentType_ANCHORS, + private: false, + }, + { + name: "simple taproot", + commitmentType: lnrpc.CommitmentType_SIMPLE_TAPROOT, + + // Set this to true once simple taproot channels can be + // announced to the network. + private: true, + }, + } + + for _, tc := range testCases { + tc := tc + + success := ht.T.Run(tc.name, func(tt *testing.T) { + st := ht.Subtest(tt) + + args := lntest.NodeArgsForCommitType(tc.commitmentType) + + // First, we'll create two new nodes that we'll use to + // open channels between for this test. Dave gets some + // coins that will be used to fund the PSBT, just to + // make sure that Carol has an empty wallet. + carol := st.NewNode("carol", args) + dave := st.NewNode("dave", args) + + // We just send enough funds to satisfy the anchor + // channel reserve for 5 channels (50k sats). + st.FundCoins(50_000, carol) + st.FundCoins(50_000, dave) + + st.RunTestCase(&lntest.TestCase{ + Name: tc.name, + TestFunc: func(sst *lntest.HarnessTest) { + runPsbtChanFunding( + sst, carol, dave, tc.private, + tc.commitmentType, + ) + }, + }) + + // Empty out the wallets so there aren't any lingering + // coins. + sendAllCoinsConfirm(st, carol, burnAddr) + sendAllCoinsConfirm(st, dave, burnAddr) + + // Now we test the second scenario. Again, we just send + // enough funds to satisfy the anchor channel reserve + // for 5 channels (50k sats). + st.FundCoins(50_000, carol) + st.FundCoins(50_000, dave) + + st.RunTestCase(&lntest.TestCase{ + Name: tc.name, + TestFunc: func(sst *lntest.HarnessTest) { + runPsbtChanFundingExternal( + sst, carol, dave, tc.private, + tc.commitmentType, + ) + }, + }) + + // Empty out the wallets a last time, so there aren't + // any lingering coins. + sendAllCoinsConfirm(st, carol, burnAddr) + sendAllCoinsConfirm(st, dave, burnAddr) + + // The last test case tests the anchor channel reserve + // itself, so we need empty wallets. + st.RunTestCase(&lntest.TestCase{ + Name: tc.name, + TestFunc: func(sst *lntest.HarnessTest) { + runPsbtChanFundingSingleStep( + sst, carol, dave, tc.private, + tc.commitmentType, + ) + }, + }) + }) + if !success { + // Log failure time to help relate the lnd logs to the + // failure. + ht.Logf("Failure time: %v", time.Now().Format( + "2006-01-02 15:04:05.000", + )) + + break + } + } } // runPsbtChanFunding makes sure a channel can be opened between carol and dave // by using a Partially Signed Bitcoin Transaction that funds the channel // multisig funding output. -func runPsbtChanFunding(ht *lntest.HarnessTest, carol, dave *node.HarnessNode) { +func runPsbtChanFunding(ht *lntest.HarnessTest, carol, dave *node.HarnessNode, + private bool, commitType lnrpc.CommitmentType) { + const chanSize = funding.MaxBtcFundingAmount ht.FundCoins(btcutil.SatoshiPerBitcoin, dave) @@ -71,6 +169,8 @@ func runPsbtChanFunding(ht *lntest.HarnessTest, carol, dave *node.HarnessNode) { }, }, }, + Private: private, + CommitmentType: commitType, }, ) @@ -89,6 +189,10 @@ func runPsbtChanFunding(ht *lntest.HarnessTest, carol, dave *node.HarnessNode) { }, }, }, + // We haven't started Alice with the explicit params to + // support the current commit type, so we'll just use + // the default for this channel. That also allows us to + // test batches of different channel types. }, ) @@ -224,18 +328,14 @@ func runPsbtChanFunding(ht *lntest.HarnessTest, carol, dave *node.HarnessNode) { ht.CloseChannel(carol, chanPoint2) } -// testPsbtChanFundingExternal makes sure a channel can be opened between carol +// runPsbtChanFundingExternal makes sure a channel can be opened between carol // and dave by using a Partially Signed Bitcoin Transaction that funds the // channel multisig funding output and is fully funded by an external third // party. -func testPsbtChanFundingExternal(ht *lntest.HarnessTest) { - const chanSize = funding.MaxBtcFundingAmount +func runPsbtChanFundingExternal(ht *lntest.HarnessTest, carol, + dave *node.HarnessNode, private bool, commitType lnrpc.CommitmentType) { - // First, we'll create two new nodes that we'll use to open channels - // between for this test. Both these nodes have an empty wallet as Alice - // will be funding the channel. - carol := ht.NewNode("carol", nil) - dave := ht.NewNode("dave", nil) + const chanSize = funding.MaxBtcFundingAmount // Before we start the test, we'll ensure both sides are connected so // the funding flow can be properly executed. @@ -265,6 +365,8 @@ func testPsbtChanFundingExternal(ht *lntest.HarnessTest) { }, }, }, + Private: private, + CommitmentType: commitType, }, ) @@ -283,6 +385,10 @@ func testPsbtChanFundingExternal(ht *lntest.HarnessTest) { }, }, }, + // We haven't started Alice with the explicit params to + // support the current commit type, so we'll just use + // the default for this channel. That also allows us to + // test batches of different channel types. }, ) @@ -406,21 +512,15 @@ func testPsbtChanFundingExternal(ht *lntest.HarnessTest) { ht.CloseChannel(carol, chanPoint2) } -// testPsbtChanFundingSingleStep checks whether PSBT funding works also when +// runPsbtChanFundingSingleStep checks whether PSBT funding works also when // the wallet of both nodes are empty and one of them uses PSBT and an external // wallet to fund the channel while creating reserve output in the same // transaction. -func testPsbtChanFundingSingleStep(ht *lntest.HarnessTest) { +func runPsbtChanFundingSingleStep(ht *lntest.HarnessTest, carol, + dave *node.HarnessNode, private bool, commitType lnrpc.CommitmentType) { + const chanSize = funding.MaxBtcFundingAmount - args := lntest.NodeArgsForCommitType(lnrpc.CommitmentType_ANCHORS) - - // First, we'll create two new nodes that we'll use to open channels - // between for this test. But in this case both nodes have an empty - // wallet. - carol := ht.NewNode("carol", args) - dave := ht.NewNode("dave", args) - alice := ht.Alice ht.FundCoins(btcutil.SatoshiPerBitcoin, alice) @@ -458,6 +558,8 @@ func testPsbtChanFundingSingleStep(ht *lntest.HarnessTest) { }, }, }, + Private: private, + CommitmentType: commitType, }, ) diff --git a/itest/lnd_remote_signer_test.go b/itest/lnd_remote_signer_test.go index 45f806cfd..c3cd260c5 100644 --- a/itest/lnd_remote_signer_test.go +++ b/itest/lnd_remote_signer_test.go @@ -111,7 +111,10 @@ func testRemoteSigner(ht *lntest.HarnessTest) { name: "psbt", randomSeed: true, fn: func(tt *lntest.HarnessTest, wo, carol *node.HarnessNode) { - runPsbtChanFunding(tt, carol, wo) + runPsbtChanFunding( + tt, carol, wo, false, + lnrpc.CommitmentType_LEGACY, + ) runSignPsbtSegWitV0P2WKH(tt, wo) runSignPsbtSegWitV1KeySpendBip86(tt, wo) runSignPsbtSegWitV1KeySpendRootHash(tt, wo) diff --git a/lntest/harness.go b/lntest/harness.go index 3c4374303..5915abed7 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -1467,6 +1467,7 @@ func (h *HarnessTest) OpenChannelPsbt(srcNode, destNode *node.HarnessNode, SpendUnconfirmed: p.SpendUnconfirmed, MinHtlcMsat: int64(p.MinHtlc), FundingShim: p.FundingShim, + CommitmentType: p.CommitmentType, } respStream := srcNode.RPC.OpenChannel(req) @@ -1476,6 +1477,23 @@ func (h *HarnessTest) OpenChannelPsbt(srcNode, destNode *node.HarnessNode, upd, ok := resp.Update.(*lnrpc.OpenStatusUpdate_PsbtFund) require.Truef(h, ok, "expected PSBT funding update, got %v", resp) + // Make sure the channel funding address has the correct type for the + // given commitment type. + fundingAddr, err := btcutil.DecodeAddress( + upd.PsbtFund.FundingAddress, harnessNetParams, + ) + require.NoError(h, err) + + switch p.CommitmentType { + case lnrpc.CommitmentType_SIMPLE_TAPROOT: + require.IsType(h, &btcutil.AddressTaproot{}, fundingAddr) + + default: + require.IsType( + h, &btcutil.AddressWitnessScriptHash{}, fundingAddr, + ) + } + return respStream, upd.PsbtFund.Psbt }