From ec55831229a2f1699422e97a9f43e0975bdd49d9 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Tue, 28 Nov 2023 16:05:33 -0800 Subject: [PATCH] htlcswitch+peer: remove ShutdownIfChannelClean --- htlcswitch/interfaces.go | 5 --- htlcswitch/link.go | 53 ------------------------ htlcswitch/link_test.go | 88 ---------------------------------------- htlcswitch/mock.go | 1 - peer/test_utils.go | 3 -- 5 files changed, 150 deletions(-) diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index f39de2a73..4b3b5d9b4 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -134,11 +134,6 @@ type ChannelUpdateHandler interface { // parameter. MayAddOutgoingHtlc(lnwire.MilliSatoshi) 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 - // EnableAdds sets the ChannelUpdateHandler state to allow // UpdateAddHtlc's in the specified direction. It returns an error if // the state already allowed those adds. diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 1b30a9b00..c321a7e83 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -273,13 +273,6 @@ type ChannelLinkConfig struct { GetAliases func(base lnwire.ShortChannelID) []lnwire.ShortChannelID } -// 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 @@ -340,10 +333,6 @@ type channelLink struct { // by the HTLC switch. downstream chan *htlcPacket - // 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 @@ -458,7 +447,6 @@ func NewChannelLink(cfg ChannelLinkConfig, cfg: cfg, channel: channel, shortChanID: channel.ShortChanID(), - shutdownRequest: make(chan *shutdownReq), hodlMap: make(map[models.CircuitKey]hodlHtlc), hodlQueue: queue.NewConcurrentQueue(10), log: build.NewPrefixLog(logPrefix, log), @@ -1446,24 +1434,6 @@ func (l *channelLink) htlcManager() { ) } - 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 - } - - l.log.Infof("Channel is in an unclean state " + - "(lingering updates), graceful shutdown of " + - "channel link not possible") - - // Otherwise, the channel has lingering updates, send - // an error and continue. - req.err <- ErrLinkFailedShutdown - case <-l.quit: return } @@ -3020,29 +2990,6 @@ 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 1c6e2d92a..cac26a6de 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -6666,94 +6666,6 @@ 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, _, err := - newSingleLinkTestHarness(t, 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) - - 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) - - // There is currently no controllable breakpoint between Alice - // receiving the CommitSig and her sending out the RevokeAndAck. As - // soon as the RevokeAndAck is generated, the channel becomes clean. - // This can happen right after the CommitSig is received, so there is - // no shutdown assertion here. - // <---sig----- - ctx.sendCommitSigBobToAlice(0) - - // ----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): - } -} - // TestPipelineSettle tests that a link should only pipeline a settle if the // related add is fully locked-in meaning it is on both sides' commitment txns. func TestPipelineSettle(t *testing.T) { diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 6d21a9519..5c7722a54 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -899,7 +899,6 @@ func (f *mockChannelLink) ChannelPoint() *wire.OutPoint { return func (f *mockChannelLink) Stop() {} func (f *mockChannelLink) EligibleToForward() bool { return f.eligible } func (f *mockChannelLink) MayAddOutgoingHtlc(lnwire.MilliSatoshi) error { return nil } -func (f *mockChannelLink) ShutdownIfChannelClean() error { return nil } func (f *mockChannelLink) setLiveShortChanID(sid lnwire.ShortChannelID) { f.shortChanID = sid } func (f *mockChannelLink) IsUnadvertised() bool { return f.unadvertised } func (f *mockChannelLink) UpdateShortChanID() (lnwire.ShortChannelID, error) { diff --git a/peer/test_utils.go b/peer/test_utils.go index 8c5105ae8..e4b5d6086 100644 --- a/peer/test_utils.go +++ b/peer/test_utils.go @@ -485,9 +485,6 @@ func (m *mockUpdateHandler) EligibleToForward() bool { return false } // MayAddOutgoingHtlc currently returns nil. func (m *mockUpdateHandler) MayAddOutgoingHtlc(lnwire.MilliSatoshi) error { return nil } -// ShutdownIfChannelClean currently returns nil. -func (m *mockUpdateHandler) ShutdownIfChannelClean() error { return nil } - type mockMessageConn struct { t *testing.T