Merge pull request #7932 from guggero/psbt-funding-taproot-chans

psbt: fix channel funding address for simple taproot channels

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,

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
+	const (
-	// between for this test. Dave gets some coins that will be used to
+		burnAddr = "bcrt1qxsnqpdc842lu8c0xlllgvejt6rhy49u6fmpgyz"
-	// fund the PSBT, just to make sure that Carol has an empty wallet.
+	)
-	carol := ht.NewNode("carol", nil)
-	dave := ht.NewNode("dave", nil)
 
-	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) {
+func runPsbtChanFundingExternal(ht *lntest.HarnessTest, carol,
-	const chanSize = funding.MaxBtcFundingAmount
+	dave *node.HarnessNode, private bool, commitType lnrpc.CommitmentType) {
 
-	// First, we'll create two new nodes that we'll use to open channels
+	const chanSize = funding.MaxBtcFundingAmount
-	// 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)
 
 	// 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,
 		},
 	)
 

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)

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
 }
 

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.
+	script, err := txscript.ParsePkScript(out.PkScript)
-	addr, err := btcutil.NewAddressWitnessScriptHash(
+	if err != nil {
-		witnessScriptHash[:], i.netParams,
+		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) &&
+			if !txscript.IsWitnessProgram(txOut.PkScript) &&
-				!isSegWitScript(in.RedeemScript) {
+				!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
-}