From b2e90480edf5d845f01fa3848a78f7aa4bde4a2e Mon Sep 17 00:00:00 2001 From: eugene Date: Tue, 10 Aug 2021 16:59:17 -0400 Subject: [PATCH] htlcswitch: extend ChannelLink interface with ShutdownIfChannelClean This allows a caller to ensure to optimistically shut down the link if the channel is clean. If the channel is not clean, an error is returned and the link continues functioning as normal. The caller should also call RemoveLink to ensure that the link isn't seen as usable within the switch. --- htlcswitch/interfaces.go | 5 +++ htlcswitch/link.go | 61 +++++++++++++++++++++++++--- htlcswitch/link_test.go | 85 +++++++++++++++++++++++++++++++++++++++ htlcswitch/linkfailure.go | 3 ++ htlcswitch/mock.go | 1 + 5 files changed, 149 insertions(+), 6 deletions(-) diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index d2d93232d..b899e6a90 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -88,6 +88,11 @@ type ChannelUpdateHandler interface { // MayAddOutgoingHtlc returns an error if we may not add an outgoing // htlc to the channel. MayAddOutgoingHtlc() error + + // ShutdownIfChannelClean shuts the link down if the channel state is + // clean. This can be used with dynamic commitment negotiation or coop + // close negotiation which require a clean channel state. + ShutdownIfChannelClean() error } // ChannelLink is an interface which represents the subsystem for managing the diff --git a/htlcswitch/link.go b/htlcswitch/link.go index eed8ba316..2760c42bc 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -301,6 +301,13 @@ type localUpdateAddMsg struct { err chan error } +// shutdownReq contains an error channel that will be used by the channelLink +// to send an error if shutdown failed. If shutdown succeeded, the channel will +// be closed. +type shutdownReq struct { + err chan error +} + // channelLink is the service which drives a channel's commitment update // state-machine. In the event that an HTLC needs to be propagated to another // link, the forward handler from config is used which sends HTLC to the @@ -369,6 +376,10 @@ type channelLink struct { // sub-systems with the latest set of active HTLC's on our channel. htlcUpdates chan *contractcourt.ContractUpdate + // shutdownRequest is a channel that the channelLink will listen on to + // service shutdown requests from ShutdownIfChannelClean calls. + shutdownRequest chan *shutdownReq + // updateFeeTimer is the timer responsible for updating the link's // commitment fee every time it fires. updateFeeTimer *time.Timer @@ -414,12 +425,13 @@ func NewChannelLink(cfg ChannelLinkConfig, channel: channel, shortChanID: channel.ShortChanID(), // TODO(roasbeef): just do reserve here? - htlcUpdates: make(chan *contractcourt.ContractUpdate), - hodlMap: make(map[channeldb.CircuitKey]hodlHtlc), - hodlQueue: queue.NewConcurrentQueue(10), - log: build.NewPrefixLog(logPrefix, log), - quit: make(chan struct{}), - localUpdateAdd: make(chan *localUpdateAddMsg), + htlcUpdates: make(chan *contractcourt.ContractUpdate), + shutdownRequest: make(chan *shutdownReq), + hodlMap: make(map[channeldb.CircuitKey]hodlHtlc), + hodlQueue: queue.NewConcurrentQueue(10), + log: build.NewPrefixLog(logPrefix, log), + quit: make(chan struct{}), + localUpdateAdd: make(chan *localUpdateAddMsg), } } @@ -1176,6 +1188,20 @@ func (l *channelLink) htlcManager() { return } + case req := <-l.shutdownRequest: + // If the channel is clean, we send nil on the err chan + // and return to prevent the htlcManager goroutine from + // processing any more updates. The full link shutdown + // will be triggered by RemoveLink in the peer. + if l.channel.IsChannelClean() { + req.err <- nil + return + } + + // Otherwise, the channel has lingering updates, send + // an error and continue. + req.err <- ErrLinkFailedShutdown + case <-l.quit: return } @@ -2434,6 +2460,29 @@ func (l *channelLink) HandleChannelUpdate(message lnwire.Message) { l.mailBox.AddMessage(message) } +// ShutdownIfChannelClean triggers a link shutdown if the channel is in a clean +// state and errors if the channel has lingering updates. +// +// NOTE: Part of the ChannelUpdateHandler interface. +func (l *channelLink) ShutdownIfChannelClean() error { + errChan := make(chan error, 1) + + select { + case l.shutdownRequest <- &shutdownReq{ + err: errChan, + }: + case <-l.quit: + return ErrLinkShuttingDown + } + + select { + case err := <-errChan: + return err + case <-l.quit: + return ErrLinkShuttingDown + } +} + // updateChannelFee updates the commitment fee-per-kw on this channel by // committing to an update_fee message. func (l *channelLink) updateChannelFee(feePerKw chainfee.SatPerKWeight) error { diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index e685a3e21..d228b9509 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -6532,6 +6532,91 @@ func TestPendingCommitTicker(t *testing.T) { } } +// TestShutdownIfChannelClean tests that a link will exit the htlcManager loop +// if and only if the underlying channel state is clean. +func TestShutdownIfChannelClean(t *testing.T) { + t.Parallel() + + const chanAmt = btcutil.SatoshiPerBitcoin * 5 + const chanReserve = btcutil.SatoshiPerBitcoin * 1 + aliceLink, bobChannel, batchTicker, start, cleanUp, _, err := + newSingleLinkTestHarness(chanAmt, chanReserve) + require.NoError(t, err) + + var ( + coreLink = aliceLink.(*channelLink) + aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + ) + + shutdownAssert := func(expectedErr error) { + err = aliceLink.ShutdownIfChannelClean() + if expectedErr != nil { + require.Error(t, err, expectedErr) + } else { + require.NoError(t, err) + } + } + + err = start() + require.NoError(t, err) + defer cleanUp() + + ctx := linkTestContext{ + t: t, + aliceLink: aliceLink, + bobChannel: bobChannel, + aliceMsgs: aliceMsgs, + } + + // First send an HTLC from Bob to Alice and assert that the link can't + // be shutdown while the update is outstanding. + htlc := generateHtlc(t, coreLink, 0) + + // <---add----- + ctx.sendHtlcBobToAlice(htlc) + // <---sig----- + ctx.sendCommitSigBobToAlice(1) + // ----rev----> + ctx.receiveRevAndAckAliceToBob() + shutdownAssert(ErrLinkFailedShutdown) + + // ----sig----> + ctx.receiveCommitSigAliceToBob(1) + shutdownAssert(ErrLinkFailedShutdown) + + // <---rev----- + ctx.sendRevAndAckBobToAlice() + shutdownAssert(ErrLinkFailedShutdown) + + // ---settle--> + ctx.receiveSettleAliceToBob() + shutdownAssert(ErrLinkFailedShutdown) + + // ----sig----> + ctx.receiveCommitSigAliceToBob(0) + shutdownAssert(ErrLinkFailedShutdown) + + // <---rev----- + ctx.sendRevAndAckBobToAlice() + shutdownAssert(ErrLinkFailedShutdown) + + // <---sig----- + ctx.sendCommitSigBobToAlice(0) + shutdownAssert(ErrLinkFailedShutdown) + + // ----rev----> + ctx.receiveRevAndAckAliceToBob() + shutdownAssert(nil) + + // Now that the link has exited the htlcManager loop, attempt to + // trigger the batch ticker. It should not be possible. + select { + case batchTicker <- time.Now(): + t.Fatalf("expected batch ticker to be inactive") + case <-time.After(5 * time.Second): + } +} + // assertFailureCode asserts that an error is of type ClearTextError and that // the failure code is as expected. func assertFailureCode(t *testing.T, err error, code lnwire.FailCode) { diff --git a/htlcswitch/linkfailure.go b/htlcswitch/linkfailure.go index 32aa83ade..2f4be531e 100644 --- a/htlcswitch/linkfailure.go +++ b/htlcswitch/linkfailure.go @@ -5,6 +5,9 @@ import "github.com/go-errors/errors" var ( // ErrLinkShuttingDown signals that the link is shutting down. ErrLinkShuttingDown = errors.New("link shutting down") + + // ErrLinkFailedShutdown signals that a requested shutdown failed. + ErrLinkFailedShutdown = errors.New("link failed to shutdown") ) // errorCode encodes the possible types of errors that will make us fail the diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index fb97c54e9..fd2964465 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -757,6 +757,7 @@ func (f *mockChannelLink) ChannelPoint() *wire.OutPoint { return func (f *mockChannelLink) Stop() {} func (f *mockChannelLink) EligibleToForward() bool { return f.eligible } func (f *mockChannelLink) MayAddOutgoingHtlc() error { return nil } +func (f *mockChannelLink) ShutdownIfChannelClean() error { return nil } func (f *mockChannelLink) setLiveShortChanID(sid lnwire.ShortChannelID) { f.shortChanID = sid } func (f *mockChannelLink) UpdateShortChanID() (lnwire.ShortChannelID, error) { f.eligible = true