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