diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index 3a352ad0..a0fff549 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -131,6 +131,8 @@ you. * Locally force closed channels are now [kept in the channel.backup file until their time lock has fully matured](https://github.com/lightningnetwork/lnd/pull/5528). +* [Cooperative closes optimistically shutdown the associated `link` before closing the channel.](https://github.com/lightningnetwork/lnd/pull/5618) + ## Build System * [A new pre-submit check has been diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index d2d93232..b899e6a9 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 eed8ba31..2760c42b 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 e685a3e2..d228b950 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 32aa83ad..2f4be531 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 fb97c54e..fd296446 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 diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 0fe643bc..2eaa8328 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -79,11 +79,6 @@ type ChanCloseCfg struct { // Channel is the channel that should be closed. Channel *lnwallet.LightningChannel - // UnregisterChannel is a function closure that allows the ChanCloser to - // unregister a channel. Once this has been done, no further HTLC's should - // be routed through the channel. - UnregisterChannel func(lnwire.ChannelID) - // BroadcastTx broadcasts the passed transaction to the network. BroadcastTx func(*wire.MsgTx, string) error @@ -212,8 +207,6 @@ func (c *ChanCloser) initChanShutdown() (*lnwire.Shutdown, error) { // closing script. shutdown := lnwire.NewShutdown(c.cid, c.localDeliveryScript) - // TODO(roasbeef): err if channel has htlc's? - // Before closing, we'll attempt to send a disable update for the channel. // We do so before closing the channel as otherwise the current edge policy // won't be retrievable from the graph. @@ -222,10 +215,6 @@ func (c *ChanCloser) initChanShutdown() (*lnwire.Shutdown, error) { c.chanPoint, err) } - // Before returning the shutdown message, we'll unregister the channel to - // ensure that it isn't seen as usable within the system. - c.cfg.UnregisterChannel(c.cid) - // Before continuing, mark the channel as cooperatively closed with a nil // txn. Even though we haven't negotiated the final txn, this guarantees // that our listchannels rpc will be externally consistent, and reflect diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 56991bd3..9143238b 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -4470,6 +4470,50 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, return nil } +// IsChannelClean returns true if neither side has pending commitments, neither +// side has HTLC's, and all updates are locked in irrevocably. Internally, it +// utilizes the oweCommitment function by calling it for local and remote +// evaluation. We check if we have a pending commitment for our local state +// since this function may be called by sub-systems that are not the link (e.g. +// the rpcserver), and the ReceiveNewCommitment & RevokeCurrentCommitment calls +// are not atomic, even though link processing ensures no updates can happen in +// between. +func (lc *LightningChannel) IsChannelClean() bool { + lc.RLock() + defer lc.RUnlock() + + // Check whether we have a pending commitment for our local state. + if lc.localCommitChain.hasUnackedCommitment() { + return false + } + + // Check whether our counterparty has a pending commitment for their + // state. + if lc.remoteCommitChain.hasUnackedCommitment() { + return false + } + + // We call ActiveHtlcs to ensure there are no HTLCs on either + // commitment. + if len(lc.channelState.ActiveHtlcs()) != 0 { + return false + } + + // Now check that both local and remote commitments are signing the + // same updates. + if lc.oweCommitment(true) { + return false + } + + if lc.oweCommitment(false) { + return false + } + + // If we reached this point, the channel has no HTLCs and both + // commitments sign the same updates. + return true +} + // OweCommitment returns a boolean value reflecting whether we need to send // out a commitment signature because there are outstanding local updates and/or // updates in the local commit tx that aren't reflected in the remote commit tx diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 6f190ade..c201cc78 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -9669,3 +9669,150 @@ func TestMayAddOutgoingHtlcZeroValue(t *testing.T) { require.NoError(t, aliceChannel.MayAddOutgoingHtlc()) require.NoError(t, bobChannel.MayAddOutgoingHtlc()) } + +// TestIsChannelClean tests that IsChannelClean returns the expected values +// in different channel states. +func TestIsChannelClean(t *testing.T) { + t.Parallel() + + aliceChannel, bobChannel, cleanUp, err := CreateTestChannels( + channeldb.ZeroHtlcTxFeeBit, + ) + require.NoError(t, err) + defer cleanUp() + + // Channel state should be clean at the start of the test. + assertCleanOrDirty(true, aliceChannel, bobChannel, t) + + // Assert that neither side considers the channel clean when alice + // sends an htlc. + // ---add---> + htlc, preimage := createHTLC(0, lnwire.MilliSatoshi(5000000)) + _, err = aliceChannel.AddHTLC(htlc, nil) + require.NoError(t, err) + _, err = bobChannel.ReceiveHTLC(htlc) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // Assert that the channel remains dirty until the HTLC is completely + // removed from both commitments. + + // ---sig---> + aliceSig, aliceHtlcSigs, _, err := aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <---rev--- + bobRevocation, _, err := bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <---sig--- + bobSig, bobHtlcSigs, _, err := bobChannel.SignNextCommitment() + require.NoError(t, err) + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // ---rev---> + aliceRevocation, _, err := aliceChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <--settle-- + err = bobChannel.SettleHTLC(preimage, 0, nil, nil, nil) + require.NoError(t, err) + err = aliceChannel.ReceiveHTLCSettle(preimage, 0) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <---sig--- + bobSig, bobHtlcSigs, _, err = bobChannel.SignNextCommitment() + require.NoError(t, err) + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // ---rev---> + aliceRevocation, _, err = aliceChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // ---sig---> + aliceSig, aliceHtlcSigs, _, err = aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <---rev--- + bobRevocation, _, err = bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + assertCleanOrDirty(true, aliceChannel, bobChannel, t) + + // Now we check that update_fee is handled and state is dirty until it + // is completely locked in. + // ---fee---> + fee := chainfee.SatPerKWeight(333) + err = aliceChannel.UpdateFee(fee) + require.NoError(t, err) + err = bobChannel.ReceiveUpdateFee(fee) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // ---sig---> + aliceSig, aliceHtlcSigs, _, err = aliceChannel.SignNextCommitment() + require.NoError(t, err) + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <---rev--- + bobRevocation, _, err = bobChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // <---sig--- + bobSig, bobHtlcSigs, _, err = bobChannel.SignNextCommitment() + require.NoError(t, err) + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + require.NoError(t, err) + assertCleanOrDirty(false, aliceChannel, bobChannel, t) + + // The state should finally be clean after alice sends her revocation. + // ---rev---> + aliceRevocation, _, err = aliceChannel.RevokeCurrentCommitment() + require.NoError(t, err) + _, _, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation) + require.NoError(t, err) + assertCleanOrDirty(true, aliceChannel, bobChannel, t) +} + +// assertCleanOrDirty is a helper function that asserts that both channels are +// clean if clean is true, and dirty if clean is false. +func assertCleanOrDirty(clean bool, alice, bob *LightningChannel, + t *testing.T) { + + t.Helper() + + if clean { + require.True(t, alice.IsChannelClean()) + require.True(t, bob.IsChannelClean()) + return + } + + require.False(t, alice.IsChannelClean()) + require.False(t, bob.IsChannelClean()) +} diff --git a/peer/brontide.go b/peer/brontide.go index fbd45efd..3cde7c07 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -1184,11 +1184,9 @@ func waitUntilLinkActive(p *Brontide, // The link may already be active by this point, and we may have missed the // ActiveLinkEvent. Check if the link exists. - links, _ := p.cfg.Switch.GetLinksByInterface(p.cfg.PubKeyBytes) - for _, link := range links { - if link.ChanID() == cid { - return link - } + link := p.fetchLinkFromKeyAndCid(cid) + if link != nil { + return link } // If the link is nil, we must wait for it to be active. @@ -1216,16 +1214,7 @@ func waitUntilLinkActive(p *Brontide, // The link shouldn't be nil as we received an // ActiveLinkEvent. If it is nil, we return nil and the // calling function should catch it. - links, _ = p.cfg.Switch.GetLinksByInterface( - p.cfg.PubKeyBytes, - ) - for _, link := range links { - if link.ChanID() == cid { - return link - } - } - - return nil + return p.fetchLinkFromKeyAndCid(cid) case <-p.quit: return nil @@ -2409,12 +2398,11 @@ func (p *Brontide) fetchActiveChanCloser(chanID lnwire.ChannelID) ( // cooperative channel closure. chanCloser, ok := p.activeChanCloses[chanID] if !ok { - // If we need to create a chan closer for the first time, then - // we'll check to ensure that the channel is even in the proper - // state to allow a co-op channel closure. - if len(channel.ActiveHtlcs()) != 0 { - return nil, fmt.Errorf("cannot co-op close " + - "channel w/ active htlcs") + // Optimistically try a link shutdown, erroring out if it + // failed. + if err := p.tryLinkShutdown(chanID); err != nil { + peerLog.Errorf("failed link shutdown: %v", err) + return nil, err } // We'll create a valid closing state machine in order to @@ -2454,9 +2442,8 @@ func (p *Brontide) fetchActiveChanCloser(chanID lnwire.ChannelID) ( chanCloser = chancloser.NewChanCloser( chancloser.ChanCloseCfg{ - Channel: channel, - UnregisterChannel: p.cfg.Switch.RemoveLink, - BroadcastTx: p.cfg.Wallet.PublishTransaction, + Channel: channel, + BroadcastTx: p.cfg.Wallet.PublishTransaction, DisableChannel: func(chanPoint wire.OutPoint) error { return p.cfg.ChanStatusMgr.RequestDisable(chanPoint, false) }, @@ -2570,11 +2557,18 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) { return } + // Optimistically try a link shutdown, erroring out if it + // failed. + if err := p.tryLinkShutdown(chanID); err != nil { + peerLog.Errorf("failed link shutdown: %v", err) + req.Err <- err + return + } + chanCloser := chancloser.NewChanCloser( chancloser.ChanCloseCfg{ - Channel: channel, - UnregisterChannel: p.cfg.Switch.RemoveLink, - BroadcastTx: p.cfg.Wallet.PublishTransaction, + Channel: channel, + BroadcastTx: p.cfg.Wallet.PublishTransaction, DisableChannel: func(chanPoint wire.OutPoint) error { return p.cfg.ChanStatusMgr.RequestDisable(chanPoint, false) }, @@ -2701,6 +2695,55 @@ func (p *Brontide) handleLinkFailure(failure linkFailureReport) { } } +// tryLinkShutdown attempts to fetch a target link from the switch, calls +// ShutdownIfChannelClean to optimistically trigger a link shutdown, and +// removes the link from the switch. It returns an error if any step failed. +func (p *Brontide) tryLinkShutdown(cid lnwire.ChannelID) error { + // Fetch the appropriate link and call ShutdownIfChannelClean to ensure + // no other updates can occur. + chanLink := p.fetchLinkFromKeyAndCid(cid) + + // If the link happens to be nil, return ErrChannelNotFound so we can + // ignore the close message. + if chanLink == nil { + return ErrChannelNotFound + } + + // Else, the link exists, so attempt to trigger shutdown. If this + // fails, we'll send an error message to the remote peer. + if err := chanLink.ShutdownIfChannelClean(); err != nil { + return err + } + + // Next, we remove the link from the switch to shut down all of the + // link's goroutines and remove it from the switch's internal maps. We + // don't call WipeChannel as the channel must still be in the + // activeChannels map to process coop close messages. + p.cfg.Switch.RemoveLink(cid) + + return nil +} + +// fetchLinkFromKeyAndCid fetches a link from the switch via the remote's +// public key and the channel id. +func (p *Brontide) fetchLinkFromKeyAndCid( + cid lnwire.ChannelID) htlcswitch.ChannelUpdateHandler { + + var chanLink htlcswitch.ChannelUpdateHandler + + // We don't need to check the error here, and can instead just loop + // over the slice and return nil. + links, _ := p.cfg.Switch.GetLinksByInterface(p.cfg.PubKeyBytes) + for _, link := range links { + if link.ChanID() == cid { + chanLink = link + break + } + } + + return chanLink +} + // finalizeChanClosure performs the final clean up steps once the cooperative // closure transaction has been fully broadcast. The finalized closing state // machine should be passed in. Once the transaction has been sufficiently diff --git a/peer/brontide_test.go b/peer/brontide_test.go index 2141a945..01a5f0c8 100644 --- a/peer/brontide_test.go +++ b/peer/brontide_test.go @@ -39,8 +39,10 @@ func TestPeerChannelClosureAcceptFeeResponder(t *testing.T) { } broadcastTxChan := make(chan *wire.MsgTx) + mockSwitch := &mockMessageSwitch{} + alicePeer, bobChan, cleanUp, err := createTestPeer( - notifier, broadcastTxChan, noUpdate, + notifier, broadcastTxChan, noUpdate, mockSwitch, ) if err != nil { t.Fatalf("unable to create test channels: %v", err) @@ -49,6 +51,9 @@ func TestPeerChannelClosureAcceptFeeResponder(t *testing.T) { chanID := lnwire.NewChanIDFromOutPoint(bobChan.ChannelPoint()) + mockLink := newMockUpdateHandler(chanID) + mockSwitch.links = append(mockSwitch.links, mockLink) + // 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. @@ -142,14 +147,20 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { } broadcastTxChan := make(chan *wire.MsgTx) + mockSwitch := &mockMessageSwitch{} + alicePeer, bobChan, cleanUp, err := createTestPeer( - notifier, broadcastTxChan, noUpdate, + notifier, broadcastTxChan, noUpdate, mockSwitch, ) if err != nil { t.Fatalf("unable to create test channels: %v", err) } defer cleanUp() + chanID := lnwire.NewChanIDFromOutPoint(bobChan.ChannelPoint()) + mockLink := newMockUpdateHandler(chanID) + mockSwitch.links = append(mockSwitch.links, mockLink) + // We make Alice send a shutdown request. updateChan := make(chan interface{}, 1) errChan := make(chan error, 1) @@ -179,7 +190,6 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { aliceDeliveryScript := shutdownMsg.Address // Bob will respond with his own Shutdown message. - chanID := shutdownMsg.ChannelID alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: lnwire.NewShutdown(chanID, @@ -264,8 +274,10 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { } broadcastTxChan := make(chan *wire.MsgTx) + mockSwitch := &mockMessageSwitch{} + alicePeer, bobChan, cleanUp, err := createTestPeer( - notifier, broadcastTxChan, noUpdate, + notifier, broadcastTxChan, noUpdate, mockSwitch, ) if err != nil { t.Fatalf("unable to create test channels: %v", err) @@ -274,6 +286,9 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { chanID := lnwire.NewChanIDFromOutPoint(bobChan.ChannelPoint()) + mockLink := newMockUpdateHandler(chanID) + mockSwitch.links = append(mockSwitch.links, mockLink) + // 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. @@ -458,14 +473,20 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { } broadcastTxChan := make(chan *wire.MsgTx) + mockSwitch := &mockMessageSwitch{} + alicePeer, bobChan, cleanUp, err := createTestPeer( - notifier, broadcastTxChan, noUpdate, + notifier, broadcastTxChan, noUpdate, mockSwitch, ) if err != nil { t.Fatalf("unable to create test channels: %v", err) } defer cleanUp() + chanID := lnwire.NewChanIDFromOutPoint(bobChan.ChannelPoint()) + mockLink := newMockUpdateHandler(chanID) + mockSwitch.links = append(mockSwitch.links, mockLink) + // We make the initiator send a shutdown request. updateChan := make(chan interface{}, 1) errChan := make(chan error, 1) @@ -496,7 +517,6 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { aliceDeliveryScript := shutdownMsg.Address // Bob will answer the Shutdown message with his own Shutdown. - chanID := lnwire.NewChanIDFromOutPoint(bobChan.ChannelPoint()) respShutdown := lnwire.NewShutdown(chanID, dummyDeliveryScript) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, @@ -793,20 +813,27 @@ func TestCustomShutdownScript(t *testing.T) { } broadcastTxChan := make(chan *wire.MsgTx) + mockSwitch := &mockMessageSwitch{} + // Open a channel. alicePeer, bobChan, cleanUp, err := createTestPeer( notifier, broadcastTxChan, test.update, + mockSwitch, ) if err != nil { t.Fatalf("unable to create test channels: %v", err) } defer cleanUp() + chanPoint := bobChan.ChannelPoint() + chanID := lnwire.NewChanIDFromOutPoint(chanPoint) + mockLink := newMockUpdateHandler(chanID) + mockSwitch.links = append(mockSwitch.links, mockLink) + // Request initiator to cooperatively close the channel, with // a specified delivery address. updateChan := make(chan interface{}, 1) errChan := make(chan error, 1) - chanPoint := bobChan.ChannelPoint() closeCommand := htlcswitch.ChanClose{ CloseType: htlcswitch.CloseRegular, ChanPoint: chanPoint, diff --git a/peer/test_utils.go b/peer/test_utils.go index b6cf16b0..3ce1cbe0 100644 --- a/peer/test_utils.go +++ b/peer/test_utils.go @@ -54,7 +54,8 @@ var noUpdate = func(a, b *channeldb.OpenChannel) {} // an updateChan function which can be used to modify the default values on // the channel states for each peer. func createTestPeer(notifier chainntnfs.ChainNotifier, - publTx chan *wire.MsgTx, updateChan func(a, b *channeldb.OpenChannel)) ( + publTx chan *wire.MsgTx, updateChan func(a, b *channeldb.OpenChannel), + mockSwitch *mockMessageSwitch) ( *Brontide, *lnwallet.LightningChannel, func(), error) { aliceKeyPriv, aliceKeyPub := btcec.PrivKeyFromBytes( @@ -306,7 +307,11 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, }, } - htlcSwitch := &mockMessageSwitch{} + // If mockSwitch is not set by the caller, set it to the default as the + // caller does not need to control it. + if mockSwitch == nil { + mockSwitch = &mockMessageSwitch{} + } nodeSignerAlice := netann.NewNodeSigner(aliceKeySigner) @@ -349,7 +354,7 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, PubKeyBytes: pubKey, ErrorBuffer: errBuffer, ChainIO: chainIO, - Switch: htlcSwitch, + Switch: mockSwitch, ChanActiveTimeout: chanActiveTimeout, InterceptSwitch: htlcswitch.NewInterceptableSwitch(nil), @@ -375,7 +380,9 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, // mockMessageSwitch is a mock implementation of the messageSwitch interface // used for testing without relying on a *htlcswitch.Switch in unit tests. -type mockMessageSwitch struct{} +type mockMessageSwitch struct { + links []htlcswitch.ChannelUpdateHandler +} // BestHeight currently returns a dummy value. func (m *mockMessageSwitch) BestHeight() uint32 { @@ -397,13 +404,44 @@ func (m *mockMessageSwitch) CreateAndAddLink(cfg htlcswitch.ChannelLinkConfig, return nil } -// GetLinksByInterface currently returns dummy values. +// GetLinksByInterface returns the active links. func (m *mockMessageSwitch) GetLinksByInterface(pub [33]byte) ( []htlcswitch.ChannelUpdateHandler, error) { - return nil, nil + return m.links, nil } +// mockUpdateHandler is a mock implementation of the ChannelUpdateHandler +// interface. It is used in mockMessageSwitch's GetLinksByInterface method. +type mockUpdateHandler struct { + cid lnwire.ChannelID +} + +// newMockUpdateHandler creates a new mockUpdateHandler. +func newMockUpdateHandler(cid lnwire.ChannelID) *mockUpdateHandler { + return &mockUpdateHandler{ + cid: cid, + } +} + +// HandleChannelUpdate currently does nothing. +func (m *mockUpdateHandler) HandleChannelUpdate(msg lnwire.Message) {} + +// ChanID returns the mockUpdateHandler's cid. +func (m *mockUpdateHandler) ChanID() lnwire.ChannelID { return m.cid } + +// Bandwidth currently returns a dummy value. +func (m *mockUpdateHandler) Bandwidth() lnwire.MilliSatoshi { return 0 } + +// EligibleToForward currently returns a dummy value. +func (m *mockUpdateHandler) EligibleToForward() bool { return false } + +// MayAddOutgoingHtlc currently returns nil. +func (m *mockUpdateHandler) MayAddOutgoingHtlc() error { return nil } + +// ShutdownIfChannelClean currently returns nil. +func (m *mockUpdateHandler) ShutdownIfChannelClean() error { return nil } + type mockMessageConn struct { t *testing.T