Merge pull request #6633 from Roasbeef/segwit-any-shutdown

multi: send P2TR addrs by default for co-op close, add new `option_any_segwit` feature bit

Author: Roasbeef

docs/release-notes/release-notes-0.15.1.md

@@ -24,6 +24,8 @@
 [`lnd` will now refuse to start if it detects the full node backned does not
 support Tapoot](https://github.com/lightningnetwork/lnd/pull/6798).
 
+[`lnd` will now use taproot addresses for co-op closes if the remote peer
+supports the feature.](https://github.com/lightningnetwork/lnd/pull/6633)
 
 ## `lncli`
 

feature/default_sets.go

@@ -79,4 +79,8 @@ var defaultSetDesc = setDesc{
 		SetInit:    {}, // I
 		SetNodeAnn: {}, // N
 	},
+	lnwire.ShutdownAnySegwitOptional: {
+		SetInit:    {}, // I
+		SetNodeAnn: {}, // N
+	},
 }

feature/manager.go

@@ -40,6 +40,10 @@ type Config struct {
 	// channels. This should be used instead of NoOptionScidAlias to still
 	// keep option-scid-alias support.
 	NoZeroConf bool
+
+	// NoAnySegwit unsets any bits that signal support for using other
+	// segwit witness versions for co-op closes.
+	NoAnySegwit bool
 }
 
 // Manager is responsible for generating feature vectors for different requested
@@ -142,6 +146,10 @@ func newManager(cfg Config, desc setDesc) (*Manager, error) {
 			raw.Unset(lnwire.ZeroConfOptional)
 			raw.Unset(lnwire.ZeroConfRequired)
 		}
+		if cfg.NoAnySegwit {
+			raw.Unset(lnwire.ShutdownAnySegwitOptional)
+			raw.Unset(lnwire.ShutdownAnySegwitRequired)
+		}
 
 		// Ensure that all of our feature sets properly set any
 		// dependent features.

funding/manager.go

@@ -131,7 +131,7 @@ var (
 // via a local signal such as RPC.
 //
 // TODO(roasbeef): actually use the context package
-//  * deadlines, etc.
+//   - deadlines, etc.
 type reservationWithCtx struct {
 	reservation *lnwallet.ChannelReservation
 	peer        lnpeer.Peer
@@ -1483,16 +1483,7 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer,
 	// (if any) in lieu of user input.
 	shutdown, err := getUpfrontShutdownScript(
 		f.cfg.EnableUpfrontShutdown, peer, acceptorResp.UpfrontShutdown,
-		func() (lnwire.DeliveryAddress, error) {
-			addr, err := f.cfg.Wallet.NewAddress(
-				lnwallet.WitnessPubKey, false,
-				lnwallet.DefaultAccountName,
-			)
-			if err != nil {
-				return nil, err
-			}
-			return txscript.PayToAddrScript(addr)
-		},
+		f.selectShutdownScript,
 	)
 	if err != nil {
 		f.failFundingFlow(
@@ -3686,7 +3677,7 @@ func (f *Manager) InitFundingWorkflow(msg *InitFundingMsg) {
 // upfront shutdown scripts automatically.
 func getUpfrontShutdownScript(enableUpfrontShutdown bool, peer lnpeer.Peer,
 	script lnwire.DeliveryAddress,
-	getScript func() (lnwire.DeliveryAddress, error)) (lnwire.DeliveryAddress,
+	getScript func(bool) (lnwire.DeliveryAddress, error)) (lnwire.DeliveryAddress,
 	error) {
 
 	// Check whether the remote peer supports upfront shutdown scripts.
@@ -3718,7 +3709,12 @@ func getUpfrontShutdownScript(enableUpfrontShutdown bool, peer lnpeer.Peer,
 		return nil, nil
 	}
 
-	return getScript()
+	// We can safely send a taproot address iff, both sides have negotiated
+	// the shutdown-any-segwit feature.
+	taprootOK := peer.RemoteFeatures().HasFeature(lnwire.ShutdownAnySegwitOptional) &&
+		peer.LocalFeatures().HasFeature(lnwire.ShutdownAnySegwitOptional)
+
+	return getScript(taprootOK)
 }
 
 // handleInitFundingMsg creates a channel reservation within the daemon's
@@ -3777,18 +3773,8 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) {
 	// address from the wallet if our node is configured to set shutdown
 	// address by default).
 	shutdown, err := getUpfrontShutdownScript(
-		f.cfg.EnableUpfrontShutdown, msg.Peer,
-		msg.ShutdownScript,
-		func() (lnwire.DeliveryAddress, error) {
-			addr, err := f.cfg.Wallet.NewAddress(
-				lnwallet.WitnessPubKey, false,
-				lnwallet.DefaultAccountName,
-			)
-			if err != nil {
-				return nil, err
-			}
-			return txscript.PayToAddrScript(addr)
-		},
+		f.cfg.EnableUpfrontShutdown, msg.Peer, msg.ShutdownScript,
+		f.selectShutdownScript,
 	)
 	if err != nil {
 		msg.Err <- err
@@ -4267,3 +4253,24 @@ func (f *Manager) deleteChannelOpeningState(chanPoint *wire.OutPoint) error {
 		outpointBytes.Bytes(),
 	)
 }
+
+// selectShutdownScript selects the shutdown script we should send to the peer.
+// If we can use taproot, then we prefer that, otherwise we'll use a p2wkh
+// script.
+func (f *Manager) selectShutdownScript(taprootOK bool,
+) (lnwire.DeliveryAddress, error) {
+
+	addrType := lnwallet.WitnessPubKey
+	if taprootOK {
+		addrType = lnwallet.TaprootPubkey
+	}
+
+	addr, err := f.cfg.Wallet.NewAddress(
+		addrType, false, lnwallet.DefaultAccountName,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return txscript.PayToAddrScript(addr)
+}

funding/manager_test.go

@@ -3331,13 +3331,13 @@ func TestGetUpfrontShutdownScript(t *testing.T) {
 	upfrontScript := []byte("upfront script")
 	generatedScript := []byte("generated script")
 
-	getScript := func() (lnwire.DeliveryAddress, error) {
+	getScript := func(_ bool) (lnwire.DeliveryAddress, error) {
 		return generatedScript, nil
 	}
 
 	tests := []struct {
 		name           string
-		getScript      func() (lnwire.DeliveryAddress, error)
+		getScript      func(bool) (lnwire.DeliveryAddress, error)
 		upfrontScript  lnwire.DeliveryAddress
 		peerEnabled    bool
 		localEnabled   bool
@@ -3628,14 +3628,14 @@ func TestFundingManagerUpfrontShutdown(t *testing.T) {
 			pkscript: []byte("\xa9\x14\xfe\x44\x10\x65\xb6\x53" +
 				"\x22\x31\xde\x2f\xac\x56\x31\x52\x20\x5e" +
 				"\xc4\xf5\x9c\x74\x87"),
-			expectErr: false,
+			expectErr: true,
 		},
 		{
 			name: "p2pkh script",
 			pkscript: []byte("\x76\xa9\x14\x64\x1a\xd5\x05\x1e" +
 				"\xdd\x97\x02\x9a\x00\x3f\xe9\xef\xb2\x93" +
 				"\x59\xfc\xee\x40\x9d\x88\xac"),
-			expectErr: false,
+			expectErr: true,
 		},
 		{
 			name: "p2wpkh script",

lncfg/protocol.go

@@ -38,6 +38,10 @@ type ProtocolOptions struct {
 	// OptionZeroConf should be set if we want to signal the zero-conf
 	// feature bit.
 	OptionZeroConf bool `long:"zero-conf" description:"enable support for zero-conf channels, must have option-scid-alias set also"`
+
+	// NoOptionAnySegwit should be set to true if we don't want to use any
+	// Taproot (and beyond) addresses for co-op closing.
+	NoOptionAnySegwit bool `long:"no-any-segwit" description:"disallow using any segiwt witness version as a co-op close address"`
 }
 
 // Wumbo returns true if lnd should permit the creation and acceptance of wumbo
@@ -67,3 +71,9 @@ func (l *ProtocolOptions) ScidAlias() bool {
 func (l *ProtocolOptions) ZeroConf() bool {
 	return l.OptionZeroConf
 }
+
+// NoAnySegwit returns true if we don't signal that we understand other newer
+// segwit witness versions for co-op close addresses.
+func (l *ProtocolOptions) NoAnySegwit() bool {
+	return l.NoOptionAnySegwit
+}

lncfg/protocol_rpctest.go

@@ -39,6 +39,10 @@ type ProtocolOptions struct {
 	// OptionZeroConf should be set if we want to signal the zero-conf
 	// feature bit.
 	OptionZeroConf bool `long:"zero-conf" description:"enable support for zero-conf channels, must have option-scid-alias set also"`
+
+	// NoOptionAnySegwit should be set to true if we don't want to use any
+	// Taproot (and beyond) addresses for co-op closing.
+	NoOptionAnySegwit bool `long:"no-any-segwit" description:"disallow using any segiwt witness version as a co-op close address"`
 }
 
 // Wumbo returns true if lnd should permit the creation and acceptance of wumbo
@@ -68,3 +72,9 @@ func (l *ProtocolOptions) ScidAlias() bool {
 func (l *ProtocolOptions) ZeroConf() bool {
 	return l.OptionZeroConf
 }
+
+// NoAnySegwit returns true if we don't signal that we understand other newer
+// segwit witness versions for co-op close addresses.
+func (l *ProtocolOptions) NoAnySegwit() bool {
+	return l.NoOptionAnySegwit
+}

lntest/itest/lnd_taproot_test.go

@@ -11,8 +11,10 @@ import (
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcec/v2/schnorr"
 	"github.com/btcsuite/btcd/btcutil"
+	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/txscript"
 	"github.com/btcsuite/btcd/wire"
+	"github.com/lightningnetwork/lnd/funding"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/chainrpc"
@@ -1498,3 +1500,82 @@ func createMuSigSessions(ctx context.Context, t *harnessTest,
 
 	return internalKey, combinedKey, sessResp1, sessResp2, sessResp3
 }
+
+// assertTaprootDeliveryUsed returns true if a Taproot addr was used in the
+// co-op close transaction.
+func assertTaprootDeliveryUsed(net *lntest.NetworkHarness,
+	t *harnessTest, closingTxid *chainhash.Hash) bool {
+
+	tx, err := net.Miner.Client.GetRawTransaction(closingTxid)
+	require.NoError(t.t, err, "unable to get closing tx")
+
+	for _, txOut := range tx.MsgTx().TxOut {
+		if !txscript.IsPayToTaproot(txOut.PkScript) {
+			return false
+		}
+	}
+
+	return true
+}
+
+// testTaprootCoopClose asserts that if both peers signal ShutdownAnySegwit,
+// then a taproot closing addr is used. Otherwise, we shouldn't expect one to
+// be used.
+func testTaprootCoopClose(net *lntest.NetworkHarness, t *harnessTest) {
+	// We'll start by making two new nodes, and funding a channel between
+	// them.
+	carol := net.NewNode(t.t, "Carol", nil)
+	defer shutdownAndAssert(net, t, carol)
+
+	net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol)
+
+	dave := net.NewNode(t.t, "Dave", nil)
+	defer shutdownAndAssert(net, t, dave)
+
+	net.EnsureConnected(t.t, carol, dave)
+
+	chanAmt := funding.MaxBtcFundingAmount
+	pushAmt := btcutil.Amount(100000)
+	satPerVbyte := btcutil.Amount(1)
+
+	// We'll now open a channel between Carol and Dave.
+	chanPoint := openChannelAndAssert(
+		t, net, carol, dave,
+		lntest.OpenChannelParams{
+			Amt:         chanAmt,
+			PushAmt:     pushAmt,
+			SatPerVByte: satPerVbyte,
+		},
+	)
+
+	// We'll now close out the channel and obtain the closing TXID.
+	closingTxid := closeChannelAndAssert(t, net, carol, chanPoint, false)
+
+	// We expect that the closing transaction only has P2TR addresses.
+	require.True(t.t, assertTaprootDeliveryUsed(net, t, closingTxid),
+		"taproot addr not used!")
+
+	// Now we'll bring Eve into the mix, Eve is running older software that
+	// doesn't understand Taproot.
+	eveArgs := []string{"--protocol.no-any-segwit"}
+	eve := net.NewNode(t.t, "Eve", eveArgs)
+	defer shutdownAndAssert(net, t, eve)
+
+	net.EnsureConnected(t.t, carol, eve)
+
+	// We'll now open up a chanel again between Carol and Eve.
+	chanPoint = openChannelAndAssert(
+		t, net, carol, eve,
+		lntest.OpenChannelParams{
+			Amt:         chanAmt,
+			PushAmt:     pushAmt,
+			SatPerVByte: satPerVbyte,
+		},
+	)
+
+	// We'll now close out this channel and expect that no Taproot
+	// addresses are used in the co-op close transaction.
+	closingTxid = closeChannelAndAssert(t, net, carol, chanPoint, false)
+	require.False(t.t, assertTaprootDeliveryUsed(net, t, closingTxid),
+		"taproot addr shouldn't be used!")
+}

lntest/itest/lnd_test_list_on_test.go

@@ -431,4 +431,8 @@ var allTestCases = []*testCase{
 		name: "nonstd sweep",
 		test: testNonstdSweep,
 	},
+	{
+		name: "taproot coop close",
+		test: testTaprootCoopClose,
+	},
 }