From 785790abed70d3602915485bc8d22dbcc3ab9928 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Tue, 6 Feb 2024 19:11:26 +0200 Subject: [PATCH] peer/lnwallet: persist shutdown info on send In this commit, we start persisting shutdown info when we send the Shutdown message. When starting up a link, we also first check if we have previously persisted Shutdown info and if we have, we start the link in shutdown mode meaning that it will not accept any new outgoing HTLC additions and it will queue the shutdown message after any pending CommitSig has been sent. --- itest/lnd_coop_close_with_htlcs_test.go | 45 +++++------- lnwallet/chancloser/chancloser.go | 11 +++ peer/brontide.go | 96 +++++++++++++++++++++---- 3 files changed, 114 insertions(+), 38 deletions(-) diff --git a/itest/lnd_coop_close_with_htlcs_test.go b/itest/lnd_coop_close_with_htlcs_test.go index 25c1e1764..4c437cd32 100644 --- a/itest/lnd_coop_close_with_htlcs_test.go +++ b/itest/lnd_coop_close_with_htlcs_test.go @@ -186,7 +186,7 @@ func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest) { DeliveryAddress: newAddr.Address, }) - // Assert that both nodes now see this channel as inactive. + // Assert that both nodes see the channel as waiting for close. ht.AssertChannelInactive(bob, chanPoint) ht.AssertChannelInactive(alice, chanPoint) @@ -196,42 +196,35 @@ func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest) { ht.AssertConnected(alice, bob) - // Show that the channel is seen as active again by Alice and Bob. - // - // NOTE: This is a bug and will be fixed in an upcoming commit. - ht.AssertChannelActive(alice, chanPoint) - ht.AssertChannelActive(bob, chanPoint) + // Show that both nodes still see the channel as waiting for close after + // the restart. + ht.AssertChannelInactive(bob, chanPoint) + ht.AssertChannelInactive(alice, chanPoint) - // Let's settle the invoice. + // Settle the invoice. alice.RPC.SettleInvoice(preimage[:]) // Wait for the channel to appear in the waiting closed list. - // - // NOTE: this will time out at the moment since there is a bug that - // results in shutdown not properly being re-started after a reconnect. err := wait.Predicate(func() bool { pendingChansResp := alice.RPC.PendingChannels() waitingClosed := pendingChansResp.WaitingCloseChannels return len(waitingClosed) == 1 }, defaultTimeout) + require.NoError(ht, err) - // We assert here that there is a timeout error. This will be fixed in - // an upcoming commit. - require.Error(ht, err) + // Wait for the close tx to be in the Mempool and then mine 6 blocks + // to confirm the close. + closingTx := ht.AssertClosingTxInMempool( + chanPoint, lnrpc.CommitmentType_LEGACY, + ) + ht.MineBlocksAndAssertNumTxes(6, 1) - // Since the channel closure did not continue, we need to re-init the - // close. - closingTXID := ht.CloseChannel(alice, chanPoint) - - // To further demonstrate the extent of the bug, we inspect the closing - // transaction here to show that the delivery address that Alice - // specified in her original close request is not the one that ended up - // being used. - // - // NOTE: this is a bug that will be fixed in an upcoming commit. + // Finally, we inspect the closing transaction here to show that the + // delivery address that Alice specified in her original close request + // is the one that ended up being used in the final closing transaction. tx := alice.RPC.GetTransaction(&walletrpc.GetTransactionRequest{ - Txid: closingTXID.String(), + Txid: closingTx.TxHash().String(), }) require.Len(ht, tx.OutputDetails, 2) @@ -245,6 +238,6 @@ func coopCloseWithHTLCsWithRestart(ht *lntest.HarnessTest) { } require.NotNil(ht, outputDetail) - // Show that the address used is not the one she requested. - require.NotEqual(ht, outputDetail.Address, newAddr.Address) + // Show that the address used is the one she requested. + require.Equal(ht, outputDetail.Address, newAddr.Address) } diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 19c9248ec..97f5f4a63 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -356,6 +356,17 @@ func (c *ChanCloser) initChanShutdown() (*lnwire.Shutdown, error) { chancloserLog.Infof("ChannelPoint(%v): sending shutdown message", c.chanPoint) + // At this point, we persist any relevant info regarding the Shutdown + // message we are about to send in order to ensure that if a + // re-establish occurs then we will re-send the same Shutdown message. + shutdownInfo := channeldb.NewShutdownInfo( + c.localDeliveryScript, c.locallyInitiated, + ) + err := c.cfg.Channel.MarkShutdownSent(shutdownInfo) + if err != nil { + return nil, err + } + return shutdown, nil } diff --git a/peer/brontide.go b/peer/brontide.go index c15845d93..6692ee3c1 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -27,6 +27,7 @@ import ( "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/discovery" "github.com/lightningnetwork/lnd/feature" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/htlcswitch/hodl" @@ -986,6 +987,59 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( continue } + shutdownInfo, err := lnChan.State().ShutdownInfo() + if err != nil && !errors.Is(err, channeldb.ErrNoShutdownInfo) { + return nil, err + } + + var ( + shutdownMsg fn.Option[lnwire.Shutdown] + shutdownInfoErr error + ) + shutdownInfo.WhenSome(func(info channeldb.ShutdownInfo) { + // Compute an ideal fee. + feePerKw, err := p.cfg.FeeEstimator.EstimateFeePerKW( + p.cfg.CoopCloseTargetConfs, + ) + if err != nil { + shutdownInfoErr = fmt.Errorf("unable to "+ + "estimate fee: %w", err) + + return + } + + chanCloser, err := p.createChanCloser( + lnChan, info.DeliveryScript.Val, feePerKw, nil, + info.LocalInitiator.Val, + ) + if err != nil { + shutdownInfoErr = fmt.Errorf("unable to "+ + "create chan closer: %w", err) + + return + } + + chanID := lnwire.NewChanIDFromOutPoint( + &lnChan.State().FundingOutpoint, + ) + + p.activeChanCloses[chanID] = chanCloser + + // Create the Shutdown message. + shutdown, err := chanCloser.ShutdownChan() + if err != nil { + delete(p.activeChanCloses, chanID) + shutdownInfoErr = err + + return + } + + shutdownMsg = fn.Some[lnwire.Shutdown](*shutdown) + }) + if shutdownInfoErr != nil { + return nil, shutdownInfoErr + } + // Subscribe to the set of on-chain events for this channel. chainEvents, err := p.cfg.ChainArb.SubscribeChannelEvents( *chanPoint, @@ -996,7 +1050,7 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( err = p.addLink( chanPoint, lnChan, forwardingPolicy, chainEvents, - true, + true, shutdownMsg, ) if err != nil { return nil, fmt.Errorf("unable to add link %v to "+ @@ -1014,7 +1068,7 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint, lnChan *lnwallet.LightningChannel, forwardingPolicy *models.ForwardingPolicy, chainEvents *contractcourt.ChainEventSubscription, - syncStates bool) error { + syncStates bool, shutdownMsg fn.Option[lnwire.Shutdown]) error { // onChannelFailure will be called by the link in case the channel // fails for some reason. @@ -1083,6 +1137,7 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint, NotifyInactiveLinkEvent: p.cfg.ChannelNotifier.NotifyInactiveLinkEvent, HtlcNotifier: p.cfg.HtlcNotifier, GetAliases: p.cfg.GetAliases, + PreviouslySentShutdown: shutdownMsg, } // Before adding our new link, purge the switch of any pending or live @@ -2802,15 +2857,32 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( return nil, nil } - // As mentioned above, we don't re-create the delivery script. - deliveryScript := c.LocalShutdownScript - if len(deliveryScript) == 0 { - var err error - deliveryScript, err = p.genDeliveryScript() - if err != nil { - p.log.Errorf("unable to gen delivery script: %v", - err) - return nil, fmt.Errorf("close addr unavailable") + var deliveryScript []byte + + shutdownInfo, err := c.ShutdownInfo() + switch { + // We have previously stored the delivery script that we need to use + // in the shutdown message. Re-use this script. + case err == nil: + shutdownInfo.WhenSome(func(info channeldb.ShutdownInfo) { + deliveryScript = info.DeliveryScript.Val + }) + + // An error other than ErrNoShutdownInfo was returned + case err != nil && !errors.Is(err, channeldb.ErrNoShutdownInfo): + return nil, err + + case errors.Is(err, channeldb.ErrNoShutdownInfo): + deliveryScript = c.LocalShutdownScript + if len(deliveryScript) == 0 { + var err error + deliveryScript, err = p.genDeliveryScript() + if err != nil { + p.log.Errorf("unable to gen delivery script: "+ + "%v", err) + + return nil, fmt.Errorf("close addr unavailable") + } } } @@ -3905,7 +3977,7 @@ func (p *Brontide) addActiveChannel(c *lnpeer.NewChannel) error { // Create the link and add it to the switch. err = p.addLink( chanPoint, lnChan, initialPolicy, chainEvents, - shouldReestablish, + shouldReestablish, fn.None[lnwire.Shutdown](), ) if err != nil { return fmt.Errorf("can't register new channel link(%v) with "+