From 5f9c1b902f8f824a6beceacb9541b3e20838b0cf Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jun 2022 11:14:54 -0700 Subject: [PATCH 1/8] lnwire+feature: add awareness of option_shutdown_anysegwit In this commit, we add awareness of the option_shutdown_anysegwit that permits both sides to send newer segwit based addresses. This'll eventually enable us to send taproot addresses for co-op close. --- feature/default_sets.go | 4 ++++ lnwire/features.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/feature/default_sets.go b/feature/default_sets.go index 7956f6932..1b1fd1f10 100644 --- a/feature/default_sets.go +++ b/feature/default_sets.go @@ -79,4 +79,8 @@ var defaultSetDesc = setDesc{ SetInit: {}, // I SetNodeAnn: {}, // N }, + lnwire.ShutdownAnySegwitOptional: { + SetInit: {}, // I + SetNodeAnn: {}, // N + }, } diff --git a/lnwire/features.go b/lnwire/features.go index 55ee5bd10..c386fd37b 100644 --- a/lnwire/features.go +++ b/lnwire/features.go @@ -129,6 +129,18 @@ const ( // transactions, which also imply anchor commitments. AnchorsZeroFeeHtlcTxOptional FeatureBit = 23 + // ShutdownAnySegwitRequired is an required feature bit that signals + // that the sender is able to properly handle/parse segwit witness + // programs up to version 16. This enables utilization of Taproot + // addresses for cooperative closure addresses. + ShutdownAnySegwitRequired FeatureBit = 26 + + // ShutdownAnySegwitOptional is an optional feature bit that signals + // that the sender is able to properly handle/parse segwit witness + // programs up to version 16. This enables utilization of Taproot + // addresses for cooperative closure addresses. + ShutdownAnySegwitOptional FeatureBit = 27 + // AMPRequired is a required feature bit that signals that the receiver // of a payment supports accepts spontaneous payments, i.e. // sender-generated preimages according to BOLT XX. @@ -266,6 +278,8 @@ var Features = map[FeatureBit]string{ ScidAliasOptional: "scid-alias", ZeroConfRequired: "zero-conf", ZeroConfOptional: "zero-conf", + ShutdownAnySegwitRequired: "shutdown-any-segwit", + ShutdownAnySegwitOptional: "shutdown-any-segwit", } // RawFeatureVector represents a set of feature bits as defined in BOLT-09. A From c79ffc07ceb63429382ac8bd8dadd6ee7bb397fb Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jun 2022 11:16:02 -0700 Subject: [PATCH 2/8] lnwallet: export ValidateUpfrontShutdown and restrict allowed addrs In this commit, we catch up our logic with the latest version of the spec that removed support for normal p2kh and p2sh addresses for co-op closes, in order to make dust calculations more uniform. --- lnwallet/wallet.go | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index f3868695f..8ee8cabc6 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -1329,7 +1329,7 @@ func (l *LightningWallet) handleContributionMsg(req *addContributionMsg) { shutdown := req.contribution.UpfrontShutdown if len(shutdown) > 0 { // Validate the shutdown script. - if !validateUpfrontShutdown(shutdown, &l.Cfg.NetParams) { + if !ValidateUpfrontShutdown(shutdown, &l.Cfg.NetParams) { req.err <- fmt.Errorf("invalid shutdown script") return } @@ -1660,7 +1660,7 @@ func (l *LightningWallet) handleSingleContribution(req *addSingleContributionMsg shutdown := req.contribution.UpfrontShutdown if len(shutdown) > 0 { // Validate the shutdown script. - if !validateUpfrontShutdown(shutdown, &l.Cfg.NetParams) { + if !ValidateUpfrontShutdown(shutdown, &l.Cfg.NetParams) { req.err <- fmt.Errorf("invalid shutdown script") return } @@ -2087,8 +2087,8 @@ func (l *LightningWallet) WithCoinSelectLock(f func() error) error { // state hints from the root to be used for a new channel. The obfuscator is // generated via the following computation: // -// * sha256(initiatorKey || responderKey)[26:] -// * where both keys are the multi-sig keys of the respective parties +// - sha256(initiatorKey || responderKey)[26:] +// -- where both keys are the multi-sig keys of the respective parties // // The first 6 bytes of the resulting hash are used as the state hint. func DeriveStateHintObfuscator(key1, key2 *btcec.PublicKey) [StateHintSize]byte { @@ -2254,24 +2254,37 @@ func (s *shimKeyRing) DeriveNextKey(keyFam keychain.KeyFamily) (keychain.KeyDesc return *fundingKeys.LocalKey, nil } -// validateUpfrontShutdown checks whether the provided upfront_shutdown_script +// ValidateUpfrontShutdown checks whether the provided upfront_shutdown_script // is of a valid type that we accept. -func validateUpfrontShutdown(shutdown lnwire.DeliveryAddress, +func ValidateUpfrontShutdown(shutdown lnwire.DeliveryAddress, params *chaincfg.Params) bool { // We don't need to worry about a large UpfrontShutdownScript since it // was already checked in lnwire when decoding from the wire. scriptClass, _, _, _ := txscript.ExtractPkScriptAddrs(shutdown, params) - switch scriptClass { - case txscript.PubKeyHashTy, - txscript.WitnessV0PubKeyHashTy, - txscript.ScriptHashTy, - txscript.WitnessV0ScriptHashTy: - // The above four types are permitted according to BOLT#02. - // Everything else is disallowed. + switch { + case scriptClass == txscript.WitnessV0PubKeyHashTy, + scriptClass == txscript.WitnessV0ScriptHashTy, + scriptClass == txscript.WitnessV1TaprootTy: + + // The above three types are permitted according to BOLT#02 and + // BOLT#05. Everything else is disallowed. return true + // In this case, we don't know about the actual script template, but it + // might be a witness program with versions 2-16. So we'll check that + // now + case txscript.IsWitnessProgram(shutdown): + version, _, err := txscript.ExtractWitnessProgramInfo(shutdown) + if err != nil { + walletLog.Warnf("unable to extract witness program "+ + "version (script=%x): %v", shutdown, err) + return false + } + + return version >= 1 && version <= 16 + default: return false } From a61b6c25b33b310872b97283bf17e341bc986452 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jun 2022 11:17:20 -0700 Subject: [PATCH 3/8] lnwallet/chanclose: update ProcessCloseMsg to check co-op close addrs We only want to allow p2wkh, p2tr, and p2wsh addresses, so we'll utilize the newly public wallet function to restrict this. --- lnwallet/chancloser/chancloser.go | 51 ++++++++++---- lnwallet/chancloser/chancloser_test.go | 92 +++++++++++++++++++------- peer/brontide.go | 3 +- 3 files changed, 109 insertions(+), 37 deletions(-) diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index a2e6226c5..1af162f7d 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -43,6 +43,10 @@ var ( // responder. ErrProposalExeceedsMaxFee = fmt.Errorf("latest fee proposal exceeds " + "max fee") + + // ErrInvalidShutdownScript is returned when we receive an address from + // a peer that isn't either a p2wsh or p2tr address. + ErrInvalidShutdownScript = fmt.Errorf("invalid shutdown script") ) // closeState represents all the possible states the channel closer state @@ -153,6 +157,9 @@ type ChanCloseCfg struct { // willing to pay to close the channel. MaxFee chainfee.SatPerKWeight + // ChainParams holds the parameters of the chain that we're active on. + ChainParams *chaincfg.Params + // Quit is a channel that should be sent upon in the occasion the state // machine should cease all progress and shutdown. Quit chan struct{} @@ -359,17 +366,33 @@ func (c *ChanCloser) NegotiationHeight() uint32 { return c.negotiationHeight } -// maybeMatchScript attempts to match the script provided in our peer's -// shutdown message with the upfront shutdown script we have on record. If no -// upfront shutdown script was set, we do not need to enforce option upfront -// shutdown, so the function returns early. If an upfront script is set, we -// check whether it matches the script provided by our peer. If they do not -// match, we use the disconnect function provided to disconnect from the peer. -func maybeMatchScript(disconnect func() error, upfrontScript, - peerScript lnwire.DeliveryAddress) error { +// validateShutdownScript attempts to match and validate the script provided in +// our peer's shutdown message with the upfront shutdown script we have on +// record. For any script specified, we also make sure it matches our +// requirements. If no upfront shutdown script was set, we do not need to +// enforce option upfront shutdown, so the function returns early. If an +// upfront script is set, we check whether it matches the script provided by +// our peer. If they do not match, we use the disconnect function provided to +// disconnect from the peer. +func validateShutdownScript(disconnect func() error, upfrontScript, + peerScript lnwire.DeliveryAddress, netParams *chaincfg.Params) error { - // If no upfront shutdown script was set, return early because we do not - // need to enforce closure to a specific script. + // Either way, we'll make sure that the script passed meets our + // standards. The upfrontScript should have already been checked at an + // earlier stage, but we'll repeat the check here for defense in depth. + if len(upfrontScript) != 0 { + if !lnwallet.ValidateUpfrontShutdown(upfrontScript, netParams) { + return ErrInvalidShutdownScript + } + } + if len(peerScript) != 0 { + if !lnwallet.ValidateUpfrontShutdown(peerScript, netParams) { + return ErrInvalidShutdownScript + } + } + + // If no upfront shutdown script was set, return early because we do + // not need to enforce closure to a specific script. if len(upfrontScript) == 0 { return nil } @@ -435,9 +458,9 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, // If the remote node opened the channel with option upfront shutdown // script, check that the script they provided matches. - if err := maybeMatchScript( + if err := validateShutdownScript( c.cfg.Disconnect, c.cfg.Channel.RemoteUpfrontShutdownScript(), - shutdownMsg.Address, + shutdownMsg.Address, c.cfg.ChainParams, ); err != nil { return nil, false, err } @@ -494,8 +517,10 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, // If the remote node opened the channel with option upfront shutdown // script, check that the script they provided matches. - if err := maybeMatchScript(c.cfg.Disconnect, + if err := validateShutdownScript( + c.cfg.Disconnect, c.cfg.Channel.RemoteUpfrontShutdownScript(), shutdownMsg.Address, + c.cfg.ChainParams, ); err != nil { return nil, false, err } diff --git a/lnwallet/chancloser/chancloser_test.go b/lnwallet/chancloser/chancloser_test.go index f3d2d4bd6..6d194398c 100644 --- a/lnwallet/chancloser/chancloser_test.go +++ b/lnwallet/chancloser/chancloser_test.go @@ -1,12 +1,14 @@ package chancloser import ( - "crypto/rand" + "bytes" "fmt" "testing" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet/chainfee" @@ -14,48 +16,58 @@ import ( "github.com/stretchr/testify/require" ) -// randDeliveryAddress generates a random delivery address for testing. -func randDeliveryAddress(t *testing.T) lnwire.DeliveryAddress { - // Generate an address of maximum length. - da := lnwire.DeliveryAddress(make([]byte, 34)) - - _, err := rand.Read(da) - require.NoError(t, err, "cannot generate random address") - - return da -} - // TestMaybeMatchScript tests that the maybeMatchScript errors appropriately // when an upfront shutdown script is set and the script provided does not // match, and does not error in any other case. func TestMaybeMatchScript(t *testing.T) { t.Parallel() - addr1 := randDeliveryAddress(t) - addr2 := randDeliveryAddress(t) + pubHash := bytes.Repeat([]byte{0x0}, 20) + scriptHash := bytes.Repeat([]byte{0x0}, 32) - tests := []struct { + p2wkh, err := txscript.NewScriptBuilder().AddOp(txscript.OP_0). + AddData(pubHash).Script() + require.NoError(t, err) + + p2wsh, err := txscript.NewScriptBuilder().AddOp(txscript.OP_0). + AddData(scriptHash).Script() + require.NoError(t, err) + + p2tr, err := txscript.NewScriptBuilder().AddOp(txscript.OP_1). + AddData(scriptHash).Script() + require.NoError(t, err) + + p2OtherV1, err := txscript.NewScriptBuilder().AddOp(txscript.OP_1). + AddData(pubHash).Script() + require.NoError(t, err) + + invalidFork, err := txscript.NewScriptBuilder().AddOp(txscript.OP_NOP). + AddData(scriptHash).Script() + require.NoError(t, err) + + type testCase struct { name string shutdownScript lnwire.DeliveryAddress upfrontScript lnwire.DeliveryAddress expectedErr error - }{ + } + tests := []testCase{ { name: "no upfront shutdown set, script ok", - shutdownScript: addr1, + shutdownScript: p2wkh, upfrontScript: []byte{}, expectedErr: nil, }, { name: "upfront shutdown set, script ok", - shutdownScript: addr1, - upfrontScript: addr1, + shutdownScript: p2wkh, + upfrontScript: p2wkh, expectedErr: nil, }, { name: "upfront shutdown set, script not ok", - shutdownScript: addr1, - upfrontScript: addr2, + shutdownScript: p2wkh, + upfrontScript: p2wsh, expectedErr: ErrUpfrontShutdownScriptMismatch, }, { @@ -64,6 +76,40 @@ func TestMaybeMatchScript(t *testing.T) { upfrontScript: []byte{}, expectedErr: nil, }, + { + name: "p2tr is ok", + shutdownScript: p2tr, + }, + { + name: "segwit v1 is ok", + shutdownScript: p2OtherV1, + }, + { + name: "invalid script not allowed", + shutdownScript: invalidFork, + expectedErr: ErrInvalidShutdownScript, + }, + } + + // All future segwit softforks should also be ok. + futureForks := []byte{ + txscript.OP_1, txscript.OP_2, txscript.OP_3, txscript.OP_4, + txscript.OP_5, txscript.OP_6, txscript.OP_7, txscript.OP_8, + txscript.OP_9, txscript.OP_10, txscript.OP_11, txscript.OP_12, + txscript.OP_13, txscript.OP_14, txscript.OP_15, txscript.OP_16, + } + for _, witnessVersion := range futureForks { + p2FutureFork, err := txscript.NewScriptBuilder().AddOp(witnessVersion). + AddData(scriptHash).Script() + require.NoError(t, err) + + opString, err := txscript.DisasmString([]byte{witnessVersion}) + require.NoError(t, err) + + tests = append(tests, testCase{ + name: fmt.Sprintf("witness_version=%v", opString), + shutdownScript: p2FutureFork, + }) } for _, test := range tests { @@ -72,9 +118,9 @@ func TestMaybeMatchScript(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - err := maybeMatchScript( + err := validateShutdownScript( func() error { return nil }, test.upfrontScript, - test.shutdownScript, + test.shutdownScript, &chaincfg.SimNetParams, ) if err != test.expectedErr { diff --git a/peer/brontide.go b/peer/brontide.go index ea6396f08..ee8c7907c 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -2716,7 +2716,8 @@ func (p *Brontide) createChanCloser(channel *lnwallet.LightningChannel, Disconnect: func() error { return p.cfg.DisconnectPeer(p.IdentityKey()) }, - Quit: p.quit, + ChainParams: &p.cfg.Wallet.Cfg.NetParams, + Quit: p.quit, }, deliveryScript, fee, From 99d37f254ef4488e5200dd4da5cfbed82495610a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jun 2022 11:17:51 -0700 Subject: [PATCH 4/8] funding: send taproot addrs as upfront shutdown if ShutdownAnySegwitOptional is active --- funding/manager.go | 57 +++++++++++++++++++++++------------------ funding/manager_test.go | 8 +++--- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index cb948a4b6..681183487 100644 --- a/funding/manager.go +++ b/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 @@ -1486,16 +1486,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( @@ -3689,7 +3680,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. @@ -3721,7 +3712,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 @@ -3780,18 +3776,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 @@ -4270,3 +4256,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) +} diff --git a/funding/manager_test.go b/funding/manager_test.go index 93b450a73..0b353d531 100644 --- a/funding/manager_test.go +++ b/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", From fffad49ad15d71e23612705ead3f0675c77d9b24 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 10 Jun 2022 11:18:33 -0700 Subject: [PATCH 5/8] peer: send taproot addrs during co-op close based on new feature bit If the ShutdownAnySegwitOptional option is active, then we can safely send these newer addresses. --- peer/brontide.go | 16 +++++++++++++++- peer/brontide_test.go | 6 ++++++ peer/test_utils.go | 17 +++++++---------- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/peer/brontide.go b/peer/brontide.go index ee8c7907c..b9741fae2 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -657,6 +657,13 @@ func (p *Brontide) initGossipSync() { } } +// taprootShutdownAllowed returns true if both parties have negotiated the +// shutdown-any-segwit feature. +func (p *Brontide) taprootShutdownAllowed() bool { + return p.RemoteFeatures().HasFeature(lnwire.ShutdownAnySegwitOptional) && + p.LocalFeatures().HasFeature(lnwire.ShutdownAnySegwitOptional) +} + // QuitSignal is a method that should return a channel which will be sent upon // or closed once the backing peer exits. This allows callers using the // interface to cancel any processing in the event the backing implementation @@ -2244,8 +2251,15 @@ func (p *Brontide) ChannelSnapshots() []*channeldb.ChannelSnapshot { // genDeliveryScript returns a new script to be used to send our funds to in // the case of a cooperative channel close negotiation. func (p *Brontide) genDeliveryScript() ([]byte, error) { + // We'll send a normal p2wkh address unless we've negotiated the + // shutdown-any-segwit feature. + addrType := lnwallet.WitnessPubKey + if p.taprootShutdownAllowed() { + addrType = lnwallet.TaprootPubkey + } + deliveryAddr, err := p.cfg.Wallet.NewAddress( - lnwallet.WitnessPubKey, false, lnwallet.DefaultAccountName, + addrType, false, lnwallet.DefaultAccountName, ) if err != nil { return nil, err diff --git a/peer/brontide_test.go b/peer/brontide_test.go index 9a657e79f..38ef2e5e7 100644 --- a/peer/brontide_test.go +++ b/peer/brontide_test.go @@ -55,6 +55,8 @@ func TestPeerChannelClosureAcceptFeeResponder(t *testing.T) { mockLink := newMockUpdateHandler(chanID) mockSwitch.links = append(mockSwitch.links, mockLink) + dummyDeliveryScript := genScript(t, p2wshAddress) + // We send a shutdown request to Alice. She will now be the responding // node in this shutdown procedure. We first expect Alice to answer // this shutdown request with a Shutdown message. @@ -156,6 +158,8 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { mockLink := newMockUpdateHandler(chanID) mockSwitch.links = append(mockSwitch.links, mockLink) + dummyDeliveryScript := genScript(t, p2wshAddress) + // We make Alice send a shutdown request. updateChan := make(chan interface{}, 1) errChan := make(chan error, 1) @@ -281,6 +285,7 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { // Bob sends a shutdown request to Alice. She will now be the responding // node in this shutdown procedure. We first expect Alice to answer this // Shutdown request with a Shutdown message. + dummyDeliveryScript := genScript(t, p2wshAddress) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: lnwire.NewShutdown(chanID, @@ -492,6 +497,7 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { aliceDeliveryScript := shutdownMsg.Address // Bob will answer the Shutdown message with his own Shutdown. + dummyDeliveryScript := genScript(t, p2wshAddress) respShutdown := lnwire.NewShutdown(chanID, dummyDeliveryScript) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, diff --git a/peer/test_utils.go b/peer/test_utils.go index f80705d55..abebf5260 100644 --- a/peer/test_utils.go +++ b/peer/test_utils.go @@ -45,9 +45,6 @@ const ( ) var ( - // Just use some arbitrary bytes as delivery script. - dummyDeliveryScript = channels.AlicesPrivKey - testKeyLoc = keychain.KeyLocator{Family: keychain.KeyFamilyNodeKey} ) @@ -368,26 +365,26 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, } cfg := &Config{ - Addr: cfgAddr, - PubKeyBytes: pubKey, - ErrorBuffer: errBuffer, - ChainIO: chainIO, - Switch: mockSwitch, - + Addr: cfgAddr, + PubKeyBytes: pubKey, + ErrorBuffer: errBuffer, + ChainIO: chainIO, + Switch: mockSwitch, ChanActiveTimeout: chanActiveTimeout, InterceptSwitch: htlcswitch.NewInterceptableSwitch( nil, testCltvRejectDelta, false, ), - ChannelDB: dbAlice.ChannelStateDB(), FeeEstimator: estimator, Wallet: wallet, ChainNotifier: notifier, ChanStatusMgr: chanStatusMgr, + Features: lnwire.NewFeatureVector(nil, lnwire.Features), DisconnectPeer: func(b *btcec.PublicKey) error { return nil }, } alicePeer := NewBrontide(*cfg) + alicePeer.remoteFeatures = lnwire.NewFeatureVector(nil, lnwire.Features) chanID := lnwire.NewChanIDFromOutPoint(channelAlice.ChannelPoint()) alicePeer.activeChannels[chanID] = channelAlice From 89529fbb4feedecaf0af301386de549cf649d5d8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 4 Aug 2022 19:09:39 -0700 Subject: [PATCH 6/8] feature+lncfg: add config option to turn of anysegwit --- feature/manager.go | 8 ++++++++ lncfg/protocol.go | 10 ++++++++++ lncfg/protocol_rpctest.go | 10 ++++++++++ sample-lnd.conf | 4 ++++ server.go | 2 ++ 5 files changed, 34 insertions(+) diff --git a/feature/manager.go b/feature/manager.go index 6d7787950..26a3d4a31 100644 --- a/feature/manager.go +++ b/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. diff --git a/lncfg/protocol.go b/lncfg/protocol.go index 0e1bd80d6..03dfa71e9 100644 --- a/lncfg/protocol.go +++ b/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 +} diff --git a/lncfg/protocol_rpctest.go b/lncfg/protocol_rpctest.go index 651e2606e..d2d4b197f 100644 --- a/lncfg/protocol_rpctest.go +++ b/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 +} diff --git a/sample-lnd.conf b/sample-lnd.conf index 5904918ab..aed084167 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -1191,6 +1191,10 @@ litecoin.node=ltcd ; option-scid-alias flag to also be set. ; protocol.zero-conf=true +; Set to disable support for using P2TR addresses (and beyond) for co-op +; closing. +; protocol.no-any-segwit + [db] ; The selected database backend. The current default backend is "bolt". lnd diff --git a/server.go b/server.go index 990c02352..0d2adc7b8 100644 --- a/server.go +++ b/server.go @@ -536,6 +536,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, NoKeysend: !cfg.AcceptKeySend, NoOptionScidAlias: !cfg.ProtocolOptions.ScidAlias(), NoZeroConf: !cfg.ProtocolOptions.ZeroConf(), + NoAnySegwit: cfg.ProtocolOptions.NoAnySegwit(), }) if err != nil { return nil, err @@ -1577,6 +1578,7 @@ func (s *server) signAliasUpdate(u *lnwire.ChannelUpdate) (*ecdsa.Signature, // - diskCheck // - tlsHealthCheck // - torController, only created when tor is enabled. +// // If a health check has been disabled by setting attempts to 0, our monitor // will not run it. func (s *server) createLivenessMonitor(cfg *Config, cc *chainreg.ChainControl) { From 3bf2790e219eb9c63396deb17b29b9e469df099a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 4 Aug 2022 19:09:56 -0700 Subject: [PATCH 7/8] lntest/itest: add new itest to cover taproot co-op close --- lntest/itest/lnd_taproot_test.go | 81 +++++++++++++++++++++++++++ lntest/itest/lnd_test_list_on_test.go | 4 ++ 2 files changed, 85 insertions(+) diff --git a/lntest/itest/lnd_taproot_test.go b/lntest/itest/lnd_taproot_test.go index 20f080404..348eb640c 100644 --- a/lntest/itest/lnd_taproot_test.go +++ b/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!") +} diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 71d6df0d4..39f6b24cd 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/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, + }, } From cd7c705db25e0946b2d081dd013673b02aca7175 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 10 Aug 2022 18:44:01 -0700 Subject: [PATCH 8/8] docs/release-notes: add release notes entry --- docs/release-notes/release-notes-0.15.1.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/release-notes/release-notes-0.15.1.md b/docs/release-notes/release-notes-0.15.1.md index ffedc822d..685afddf3 100644 --- a/docs/release-notes/release-notes-0.15.1.md +++ b/docs/release-notes/release-notes-0.15.1.md @@ -20,6 +20,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`