diff --git a/channeldb/channel.go b/channeldb/channel.go index 58956620ec..d28bda3839 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -224,6 +224,9 @@ const ( // A tlv type definition used to serialize and deserialize the // confirmed ShortChannelID for a zero-conf channel. realScidType tlv.Type = 4 + + // sentShutdown field. + sentShutdownType tlv.Type = 5 ) // indexStatus is an enum-like type that describes what state the @@ -822,6 +825,10 @@ type OpenChannel struct { // default ShortChannelID. This is only set for zero-conf channels. confirmedScid lnwire.ShortChannelID + // sentShutdown denotes whether or not we've sent the Shutdown message + // for this channel. + sentShutdown bool + // TODO(roasbeef): eww Db *ChannelStateDB @@ -1364,6 +1371,96 @@ func (c *OpenChannel) SecondCommitmentPoint() (*btcec.PublicKey, error) { return input.ComputeCommitmentPoint(revocation[:]), nil } +// MarkShutdownSent is used to mark that we've sent a Shutdown message for this +// channel. This is so we can retransmit Shutdown. +func (c *OpenChannel) MarkShutdownSent() error { + c.Lock() + defer c.Unlock() + + if err := kvdb.Update(c.Db.backend, func(tx kvdb.RwTx) error { + chanBucket, err := fetchChanBucketRw( + tx, c.IdentityPub, &c.FundingOutpoint, c.ChainHash, + ) + if err != nil { + return err + } + + channel, err := fetchOpenChannel( + chanBucket, &c.FundingOutpoint, + ) + if err != nil { + return err + } + + channel.sentShutdown = true + + return putOpenChannel(chanBucket, channel) + }, func() {}); err != nil { + return err + } + + c.sentShutdown = true + + return nil +} + +// HasSentShutdown returns whether or not we've ever sent Shutdown for this +// channel. For nodes upgrading to 0.16.0, this will initially be false even if +// a Shutdown has been sent. This doesn't matter since this function is only +// used when restarting a ChannelLink and pre-0.16.0 nodes only sent Shutdown +// after the link was permanently stopped. +func (c *OpenChannel) HasSentShutdown() bool { + c.RLock() + defer c.RUnlock() + + return c.sentShutdown +} + +// PersistDeliveryScript is used during the cooperative close flow to persist +// a script sent in Shutdown when we did not set an upfront shutdown script +// during the funding flow. +func (c *OpenChannel) PersistDeliveryScript( + deliveryScript lnwire.DeliveryAddress) error { + + c.Lock() + defer c.Unlock() + + if err := kvdb.Update(c.Db.backend, func(tx kvdb.RwTx) error { + chanBucket, err := fetchChanBucketRw( + tx, c.IdentityPub, &c.FundingOutpoint, c.ChainHash, + ) + if err != nil { + return err + } + + channel, err := fetchOpenChannel( + chanBucket, &c.FundingOutpoint, + ) + if err != nil { + return err + } + + channel.LocalShutdownScript = deliveryScript + + return putOpenChannel(chanBucket, channel) + }, func() {}); err != nil { + return err + } + + c.LocalShutdownScript = deliveryScript + + return nil +} + +// GetLocalShutdownScript fetches the local shutdown script with the read +// mutex. +func (c *OpenChannel) GetLocalShutdownScript() lnwire.DeliveryAddress { + c.RLock() + defer c.RUnlock() + + return c.LocalShutdownScript +} + // ChanSyncMsg returns the ChannelReestablish message that should be sent upon // reconnection with the remote peer that we're maintaining this channel with. // The information contained within this message is necessary to re-sync our @@ -3691,6 +3788,12 @@ func putChanInfo(chanBucket kvdb.RwBucket, channel *OpenChannel) error { localBalance := uint64(channel.InitialLocalBalance) remoteBalance := uint64(channel.InitialRemoteBalance) + // Convert sentShutdown to a uint8. + var sentShutdownVal uint8 + if channel.sentShutdown { + sentShutdownVal = 1 + } + // Create the tlv stream. tlvStream, err := tlv.NewStream( // Write the RevocationKeyLocator as the first entry in a tlv @@ -3705,6 +3808,9 @@ func putChanInfo(chanBucket kvdb.RwBucket, channel *OpenChannel) error { initialRemoteBalanceType, &remoteBalance, ), MakeScidRecord(realScidType, &channel.confirmedScid), + tlv.MakePrimitiveRecord( + sentShutdownType, &sentShutdownVal, + ), ) if err != nil { return err @@ -3906,6 +4012,8 @@ func fetchChanInfo(chanBucket kvdb.RBucket, channel *OpenChannel) error { var ( localBalance uint64 remoteBalance uint64 + + sentShutdownVal uint8 ) // Create the tlv stream. @@ -3922,6 +4030,9 @@ func fetchChanInfo(chanBucket kvdb.RBucket, channel *OpenChannel) error { initialRemoteBalanceType, &remoteBalance, ), MakeScidRecord(realScidType, &channel.confirmedScid), + tlv.MakePrimitiveRecord( + sentShutdownType, &sentShutdownVal, + ), ) if err != nil { return err @@ -3935,6 +4046,11 @@ func fetchChanInfo(chanBucket kvdb.RBucket, channel *OpenChannel) error { channel.InitialLocalBalance = lnwire.MilliSatoshi(localBalance) channel.InitialRemoteBalance = lnwire.MilliSatoshi(remoteBalance) + // Populate the sentShutdown field. + if sentShutdownVal == 1 { + channel.sentShutdown = true + } + channel.Packager = NewChannelPackager(channel.ShortChannelID) // Finally, read the optional shutdown scripts. diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index 8e2a1c43b3..c98870592a 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -132,10 +132,9 @@ 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 + // NotifyShouldShutdown is used by the Switch to inform the link that + // it should begin the shutdown flow. + NotifyShouldShutdown(req *ChanClose) error } // ChannelLink is an interface which represents the subsystem for managing the diff --git a/htlcswitch/link.go b/htlcswitch/link.go index c3c824afc1..41557465e9 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -62,6 +62,14 @@ const ( DefaultMaxLinkFeeAllocation float64 = 0.5 ) +var ( + // ErrUpfrontShutdownScriptMismatch is returned when a peer or end user + // provides a cooperative close script which does not match the upfront + // shutdown script previously set for that party. + ErrUpfrontShutdownScriptMismatch = fmt.Errorf("shutdown script does " + + "not match upfront shutdown script") +) + // ForwardingPolicy describes the set of constraints that a given ChannelLink // is to adhere to when forwarding HTLC's. For each incoming HTLC, this set of // constraints will be consulted in order to ensure that adequate fees are @@ -303,13 +311,29 @@ type ChannelLinkConfig struct { // GetAliases is used by the link and switch to fetch the set of // aliases for a given link. 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 + // DeliveryAddr is the address to use in Shutdown if an upfront script + // has not been set. + DeliveryAddr lnwire.DeliveryAddress + + // NotifySendingShutdown is used by the link to notify an outside + // subsystem that Shutdown is about to be sent. This is not used during + // retransmission. It takes a quit chan that is used as a precaution to + // avoid deadlock. + NotifySendingShutdown func(req *ChanClose, quit chan struct{}) + + // NotifyCoopReady is used by the link to notify an outside subsystem + // that the channel has entered a clean state and ClosingSigned + // negotiation may begin. The link will begin shutting down once this + // is called. It takes a quit chan that is used as a precaution to + // avoid deadlock. + NotifyCoopReady func(chanPoint *wire.OutPoint, quit chan struct{}) + + // RetransmitShutdown is a boolean that indicates whether the link + // should retransmit a Shutdown during the Reestablish flow. This + // automatically starts the link in "shutdown mode" so that new HTLCs + // are not accepted or sent. + RetransmitShutdown bool } // channelLink is the service which drives a channel's commitment update @@ -373,8 +397,30 @@ type channelLink struct { downstream chan *htlcPacket // shutdownRequest is a channel that the channelLink will listen on to - // service shutdown requests from ShutdownIfChannelClean calls. - shutdownRequest chan *shutdownReq + // service shutdown requests from NotifyShouldShutdown calls. + shutdownRequest chan *ChanClose + + // localCloseReq stores the local ChanClose request to pass to the + // peer.Brontide when we're done with the Shutdown phase. + localCloseReq *ChanClose + + // shutdownInit is a bool that is set when we've initiated a coop + // close. + shutdownInit bool + + // shutdownReceived is a bool that is set when we've received a + // Shutdown message from the remote peer. + // + // The mutex is only accessed if: + // - a write is occurring + // - a read is occurring from hasReceivedShutdown + // all reads from the htlcManager goroutine should be race-free. + shutdownReceived bool + shutdownReceivedMtx sync.RWMutex + + // shutdownSent is a bool signalling that we've sent a Shutdown message + // to the peer. + shutdownSent bool // updateFeeTimer is the timer responsible for updating the link's // commitment fee every time it fires. @@ -420,7 +466,7 @@ func NewChannelLink(cfg ChannelLinkConfig, cfg: cfg, channel: channel, shortChanID: channel.ShortChanID(), - shutdownRequest: make(chan *shutdownReq), + shutdownRequest: make(chan *ChanClose), hodlMap: make(map[channeldb.CircuitKey]hodlHtlc), hodlQueue: queue.NewConcurrentQueue(10), log: build.NewPrefixLog(logPrefix, log), @@ -590,6 +636,18 @@ func (l *channelLink) markReestablished() { atomic.StoreInt32(&l.reestablished, 1) } +// hasReceivedShutdown returns true if the link has received a Shutdown message +// from the remote peer. It acquires the mutex as this is called outside of the +// htlcManager goroutine. +// +// NOTE: This is only used in tests to avoid a data race when accessing the +// private shutdownReceived boolean. +func (l *channelLink) hasReceivedShutdown() bool { + l.shutdownReceivedMtx.RLock() + defer l.shutdownReceivedMtx.RUnlock() + return l.shutdownReceived +} + // IsUnadvertised returns true if the underlying channel is unadvertised. func (l *channelLink) IsUnadvertised() bool { state := l.channel.State() @@ -760,6 +818,26 @@ func (l *channelLink) syncChanStates() error { } } + // If we have been signaled to retransmit Shutdown, we'll do so + // here. + if l.cfg.RetransmitShutdown { + // Recreate the Shutdown message. + shutdownMsg := lnwire.NewShutdown( + l.ChanID(), + l.channel.LocalUpfrontShutdownScript(), + ) + + // Flip the shutdownInit and shutdownSent bools. + l.shutdownInit = true + l.shutdownSent = true + + err = l.cfg.Peer.SendMessage(false, shutdownMsg) + if err != nil { + return fmt.Errorf("unable to re-send "+ + "Shutdown: %v", err) + } + } + // In any case, we'll then process their ChanSync message. l.log.Info("received re-establishment message from remote side") @@ -1150,6 +1228,132 @@ func (l *channelLink) htlcManager() { "PendingLocalUpdateCount") } + // If we are attempting to shutdown the link to cooperatively + // close and the channel is clear of our pending updates and we + // haven't already sent a Shutdown, we'll send one now. + if (l.shutdownInit || l.shutdownReceived) && !l.shutdownSent && + l.channel.PendingLocalUpdateCount() == 0 { + + // Create the Shutdown we'll send over. Send the local + // upfront shutdown script if it exists, and the + // provided delivery address otherwise. This is used in + // case we did not initiate the coop close. + upfrontAddr := l.channel.LocalUpfrontShutdownScript() + if len(upfrontAddr) != 0 { + l.cfg.DeliveryAddr = upfrontAddr + } else { + // Persist the delivery script for + // retransmission. + err := l.channel.State().PersistDeliveryScript( + l.cfg.DeliveryAddr, + ) + if err != nil { + l.fail(LinkFailureError{ + code: ErrInternalError, + }, "failed persisting script: %v", err) + return + } + } + + // Mark that we've sent the Shutdown message. This is + // used for retransmission. + err := l.channel.State().MarkShutdownSent() + if err != nil { + l.fail(LinkFailureError{ + code: ErrInternalError, + }, "failed marking shutdown: %v", err) + return + } + + // Notify peer.Brontide that we've sent the Shutdown + // message so that the coop close state machine can + // advance. + closeReq := l.localCloseReq + if closeReq == nil { + // If we are not the initiator of the coop + // close, we'll create a ChanClose struct. We + // won't listen on the error or update chans. + // If there's an error, we'll eventually stop + // the link anyways. The Updates chan is + // currently unused, but is added to prevent a + // future change from hanging if it attempts to + // send along the chan. + closeType := contractcourt.CloseRegular + chanPoint := l.ChannelPoint() + deliveryAddr := l.cfg.DeliveryAddr + errChan := make(chan error, 1) + updatesChan := make(chan interface{}, 2) + + // Set the TargetFeePerKw to ensure that if a + // bug occurs and this struct is used, that the + // zero fee isn't used instead. + feeEst := l.cfg.FeeEstimator + feePerKw, err := feeEst.EstimateFeePerKW(6) + if err != nil { + l.fail(LinkFailureError{ + code: ErrInternalError, + }, "failed fetching fee: %v", err) + return + } + + closeReq = &ChanClose{ + CloseType: closeType, + ChanPoint: chanPoint, + DeliveryScript: deliveryAddr, + Err: errChan, + Updates: updatesChan, + TargetFeePerKw: feePerKw, + } + } else { + // The script may have changed so set the + // DeliveryScript to the link's DeliveryAddr. + closeReq.DeliveryScript = l.cfg.DeliveryAddr + } + + // This is called before actually sending Shutdown so + // that the coop close state machine does not end up + // racing with the ClosingSigned from the peer: + // + // Current flow: + // - State machine receives our Shutdown message + // - Send Shutdown message to peer + // - Peer replies with ClosingSigned + // + // Racing flow: + // - Send shutdown message to peer + // - Peer replies with ClosingSigned + // - State machine receives our Shutdown message + l.cfg.NotifySendingShutdown(closeReq, l.quit) + + shutdownMsg := lnwire.NewShutdown( + l.ChanID(), l.cfg.DeliveryAddr, + ) + + err = l.cfg.Peer.SendMessage(false, shutdownMsg) + if err != nil { + // We can just let the peer.Brontide object + // stop the link. + l.log.Errorf("failed sending shutdown to "+ + "peer: %v", err) + } + + l.shutdownSent = true + } + + // If we've sent and received Shutdown and the channel is + // clean, then we'll notify peer.Brontide that the channel can + // be coop closed and stop the link. + if l.shutdownSent && l.shutdownReceived && + l.channel.IsChannelClean() { + + l.cfg.NotifyCoopReady(l.ChannelPoint(), l.quit) + + // The peer.Brontide that has access to this link will + // call RemoveLink which will stop this link. Returning + // here ensures no more channel updates can occur. + return + } + select { // Our update fee timer has fired, so we'll check the network // fee to see if we should adjust our commitment fee. @@ -1272,18 +1476,13 @@ 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 - } - - // Otherwise, the channel has lingering updates, send - // an error and continue. - req.err <- ErrLinkFailedShutdown + // This is a local shutdown request, so we set the + // shutdownInit bool and wait until there are no more + // updates to send. We should not forward HTLC's to our + // peer. Only cancels and update_fee messages are + // allowed. The peer.Brontide + l.shutdownInit = true + l.localCloseReq = req case <-l.quit: return @@ -1425,6 +1624,18 @@ func (l *channelLink) handleDownstreamUpdateAdd(pkt *htlcPacket) error { return nil } + // If the remote peer has sent a Shutdown or we have initiated the + // shutdown process (but may not have sent Shutdown yet), we'll fail + // the Add as we're either waiting for a window to send a Shutdown + // message or have sent Shutdown and are attempting to clear the + // channel state completely. + if l.shutdownInit || l.shutdownReceived { + l.log.Debugf("Failing downstream add HTLC since we're in " + + "the shutdown phase of coop close") + l.mailBox.FailAdd(pkt) + return ErrLinkCoopClosing + } + // A new payment has been initiated via the downstream channel, // so we add the new HTLC to our local log, then update the // commitment chains. @@ -1729,6 +1940,15 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { switch msg := msg.(type) { case *lnwire.UpdateAddHTLC: + // If the peer has already sent Shutdown, fail the link. + if l.shutdownReceived { + l.fail( + LinkFailureError{code: ErrInvalidUpdate}, + "peer sent add after shutdown", + ) + return + } + // We just received an add request from an upstream peer, so we // add it to our state machine, then add the HTLC to our // "settle" list in the event that we know the preimage. @@ -2106,6 +2326,28 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { "ChannelPoint(%v): received error from peer: %v", l.channel.ChannelPoint(), msg.Error(), ) + + case *lnwire.Shutdown: + // We've received a Shutdown message from the remote peer. + // We'll set the shutdownReceived bool and cancel back any new + // HTLC's received after this point instead of forwarding them. + if l.shutdownReceived { + // Fail the link if a duplicate shutdown is received. + // This is also checked in peer.Brontide, but done here + // to properly stop the link. + l.fail( + LinkFailureError{code: ErrRemoteError}, + "ChannelPoint(%v): received dupe shutdown "+ + "from peer", l.channel.ChannelPoint(), + ) + return + } + + // Otherwise, set the shutdownReceived bool. + l.shutdownReceivedMtx.Lock() + l.shutdownReceived = true + l.shutdownReceivedMtx.Unlock() + default: l.log.Warnf("received unknown message of type %T", msg) } @@ -2708,27 +2950,62 @@ 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. +// NotifyShouldShutdown signals that the link should begin Shutdown. This is +// not called when retransmitting Shutdown. // // NOTE: Part of the ChannelUpdateHandler interface. -func (l *channelLink) ShutdownIfChannelClean() error { - errChan := make(chan error, 1) +func (l *channelLink) NotifyShouldShutdown(req *ChanClose) error { + // Determine if upfront shutdown has been violated. If it hasn't, we'll + // replace the DeliveryAddr with the user-provided one. + deliveryAddr, err := chooseDeliveryScript( + l.channel.LocalUpfrontShutdownScript(), req.DeliveryScript, + ) + if err != nil { + return err + } - select { - case l.shutdownRequest <- &shutdownReq{ - err: errChan, - }: - case <-l.quit: - return ErrLinkShuttingDown + if len(deliveryAddr) != 0 { + l.cfg.DeliveryAddr = deliveryAddr } select { - case err := <-errChan: - return err + case l.shutdownRequest <- req: case <-l.quit: return ErrLinkShuttingDown } + + return nil +} + +// chooseDeliveryScript is a utility function to determine which delivery +// script to use, or to error if a passed script does not match the upfront +// shutdown script. Nil may be returned if both arguments are nil. +func chooseDeliveryScript(upfront, + requested lnwire.DeliveryAddress) (lnwire.DeliveryAddress, error) { + + // If no upfront shutdown script was provided, return the requested + // address (which may be nil). + if len(upfront) == 0 { + return requested, nil + } + + // If the user did not request a custom shutdown script, return the + // upfront address. + if len(requested) == 0 { + return upfront, nil + } + + // If both an upfront shutdown script and a custom close script were + // provided, error if the user provided shutdown script does not match + // the upfront shutdown script (because closing out to a different + // script would violate upfront shutdown). + if !bytes.Equal(upfront, requested) { + return nil, ErrUpfrontShutdownScriptMismatch + } + + // The user requested shutdown script matches the upfront shutdown + // script, so we can return it without error. + return upfront, nil } // updateChannelFee updates the commitment fee-per-kw on this channel by diff --git a/htlcswitch/link_isolated_test.go b/htlcswitch/link_isolated_test.go index 7204a58737..1dd6be9962 100644 --- a/htlcswitch/link_isolated_test.go +++ b/htlcswitch/link_isolated_test.go @@ -255,6 +255,36 @@ func (l *linkTestContext) receiveFailAliceToBob() { } } +// sendShutdownBobToAlice makes Bob generate a Shutdown and give it to Alice. +func (l *linkTestContext) sendShutdownBobToAlice() { + l.t.Helper() + + bobDelivery := genScript(l.t, p2wshAddress) + + shutdownMsg := lnwire.NewShutdown(l.aliceLink.ChanID(), bobDelivery) + l.aliceLink.HandleChannelUpdate(shutdownMsg) +} + +// receiveShutdownAliceToBob waits for Alice to send a Shutdown to Bob and +// returns it. +func (l *linkTestContext) receiveShutdownAlice() *lnwire.Shutdown { + l.t.Helper() + + var msg lnwire.Message + select { + case msg = <-l.aliceMsgs: + case <-time.After(15 * time.Second): + l.t.Fatalf("did not receive message") + } + + shutdown, ok := msg.(*lnwire.Shutdown) + if !ok { + l.t.Fatalf("expected Shutdown, got %T", msg) + } + + return shutdown +} + // assertNoMsgFromAlice asserts that Alice hasn't sent a message. Before // calling, make sure that Alice has had the opportunity to send the message. func (l *linkTestContext) assertNoMsgFromAlice(timeout time.Duration) { diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index d2387ff138..dd02bbdcab 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -16,7 +16,9 @@ import ( "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/davecgh/go-spew/spew" "github.com/go-errors/errors" @@ -6333,9 +6335,228 @@ 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) { +// TestShutdownRetransmit tests that the link is able to properly retransmit a +// Shutdown message if it restarts. +func TestShutdownRetransmit(t *testing.T) { + t.Parallel() + + // Create the three hop network even though we'll only be using Alice + // and Bob. + const aliceInitialBalance = btcutil.SatoshiPerBitcoin * 3 + channels, _, err := createClusterChannels( + t, aliceInitialBalance, btcutil.SatoshiPerBitcoin*5, + ) + require.NoError(t, err) + + n := newThreeHopNetwork(t, channels.aliceToBob, channels.bobToAlice, + channels.bobToCarol, channels.carolToBob, testStartingHeight) + + // Set up the message interceptors to ensure the proper messages are + // sent and received in the correct order. + chanID := n.aliceChannelLink.ChanID() + messages := []expectedMessage{ + {"alice", "bob", &lnwire.ChannelReestablish{}, false}, + {"bob", "alice", &lnwire.ChannelReestablish{}, false}, + + {"alice", "bob", &lnwire.FundingLocked{}, false}, + {"bob", "alice", &lnwire.FundingLocked{}, false}, + + {"alice", "bob", &lnwire.Shutdown{}, false}, + {"bob", "alice", &lnwire.Shutdown{}, false}, + } + n.aliceServer.intersect(createInterceptorFunc("[alice] <-- [bob]", + "alice", messages, chanID, false)) + n.bobServer.intersect(createInterceptorFunc("[alice] --> [bob]", "bob", + messages, chanID, false)) + + closeReqChan := make(chan *ChanClose, 2) + notifyShutdown := func(req *ChanClose, quit chan struct{}) { + select { + case closeReqChan <- req: + default: + } + } + + coopReadyChan := make(chan struct{}, 2) + notifyCoopReady := func(chanPoint *wire.OutPoint, quit chan struct{}) { + select { + case coopReadyChan <- struct{}{}: + default: + } + } + + // Set the closure functions for Alice and Bob. + n.aliceChannelLink.cfg.NotifySendingShutdown = notifyShutdown + n.aliceChannelLink.cfg.NotifyCoopReady = notifyCoopReady + n.firstBobChannelLink.cfg.NotifySendingShutdown = notifyShutdown + n.firstBobChannelLink.cfg.NotifyCoopReady = notifyCoopReady + + // Set the RetransmitShutdown config option for Alice. + n.aliceChannelLink.cfg.RetransmitShutdown = true + + err = n.start() + require.NoError(t, err) + defer n.stop() + defer func() { + _ = n.feeEstimator.Stop() + }() +} + +// TestCustomShutdownScript tests that it's possible to set a custom shutdown +// script when using the NotifyShouldShutdown call. +func TestCustomShutdownScript(t *testing.T) { + script := genScript(t, p2wshAddress) + + tests := []struct { + name string + + setShutdown bool + + userCloseScript lnwire.DeliveryAddress + + expectedScript lnwire.DeliveryAddress + + expectedError error + }{ + { + name: "User set script", + userCloseScript: script, + expectedScript: script, + }, + { + name: "No user set script", + }, + { + name: "Shutdown set, no user script", + setShutdown: true, + expectedScript: script, + }, + { + name: "Shutdown set, user script matches", + setShutdown: true, + userCloseScript: script, + expectedScript: script, + }, + { + name: "Shutdown set, user script different", + setShutdown: true, + userCloseScript: []byte("different addr"), + expectedError: ErrUpfrontShutdownScriptMismatch, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + customShutdownScriptInternal( + t, test.setShutdown, test.userCloseScript, + test.expectedScript, test.expectedError, + ) + }) + } +} + +// customShutdownScriptInternal is the internal test function of +// TestCustomShutdownScript. +func customShutdownScriptInternal(t *testing.T, setShutdown bool, + userCloseScript, expectedScript lnwire.DeliveryAddress, + expectedError error) { + + const chanAmt = btcutil.SatoshiPerBitcoin * 5 + const chanReserve = btcutil.SatoshiPerBitcoin * 1 + aliceLink, bobChannel, _, start, _, err := + newSingleLinkTestHarness(t, chanAmt, chanReserve) + require.NoError(t, err) + + var ( + coreLink = aliceLink.(*channelLink) + aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + ) + + // Start Alice's switch. + err = coreLink.cfg.Switch.Start() + require.NoError(t, err) + + closeReqChan := make(chan *ChanClose) + coreLink.cfg.NotifySendingShutdown = func(req *ChanClose, + quit chan struct{}) { + + select { + case closeReqChan <- req: + case <-quit: + } + } + + // Set the shutdown script to the p2wshAddress if setShutdown is true. + if setShutdown { + script := genScript(t, p2wshAddress) + coreLink.channel.State().LocalShutdownScript = script + } + + err = start() + require.NoError(t, err) + + ctx := linkTestContext{ + t: t, + aliceLink: aliceLink, + bobChannel: bobChannel, + aliceMsgs: aliceMsgs, + } + + closeReq := &ChanClose{ + CloseType: contractcourt.CloseRegular, + ChanPoint: bobChannel.ChannelPoint(), + TargetFeePerKw: 300, + Updates: make(chan interface{}, 2), + Err: make(chan error, 1), + } + + // Set the user's close script if one was provided. + if len(userCloseScript) != 0 { + closeReq.DeliveryScript = userCloseScript + } + + // If there is no expectedScript and the user-close script doesn't + // exist, set the script to a random one. + if len(expectedScript) == 0 && len(userCloseScript) == 0 { + closeReq.DeliveryScript = genScript(t, p2wshAddress) + } + + err = aliceLink.NotifyShouldShutdown(closeReq) + if expectedError != nil { + require.Equal(t, expectedError, err) + return + } + require.NoError(t, err) + + // Ensure we receive the NotifySendingShutdown event. + select { + case req := <-closeReqChan: + // Only check if the expectedScript was set. + if len(expectedScript) != 0 { + isEqual := bytes.Equal( + req.DeliveryScript, expectedScript, + ) + require.True(t, isEqual) + } + case <-time.After(5 * time.Second): + t.Fatalf("listening for close req failed") + } + + // ----shutdown----> + aliceShutdown := ctx.receiveShutdownAlice() + // Only check if the expectedScript was set. + if len(expectedScript) != 0 { + require.True( + t, bytes.Equal(aliceShutdown.Address, expectedScript), + ) + } +} + +// TestSuccessfulShutdownResp tests that the link can properly handle the happy +// path when being on the responding side of Shutdown. +func TestSuccessfulShutdownResp(t *testing.T) { t.Parallel() const chanAmt = btcutil.SatoshiPerBitcoin * 5 @@ -6347,14 +6568,30 @@ func TestShutdownIfChannelClean(t *testing.T) { var ( coreLink = aliceLink.(*channelLink) aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + aliceEst = coreLink.cfg.FeeEstimator.(*mockFeeEstimator) ) - shutdownAssert := func(expectedErr error) { - err = aliceLink.ShutdownIfChannelClean() - if expectedErr != nil { - require.Error(t, err, expectedErr) - } else { - require.NoError(t, err) + // Start Alice's switch. + err = coreLink.cfg.Switch.Start() + require.NoError(t, err) + + closeReqChan := make(chan *ChanClose) + coreLink.cfg.NotifySendingShutdown = func(req *ChanClose, + quit chan struct{}) { + + select { + case closeReqChan <- req: + case <-quit: + } + } + + coopReadyChan := make(chan struct{}) + coreLink.cfg.NotifyCoopReady = func(chanPoint *wire.OutPoint, + quit chan struct{}) { + + select { + case coopReadyChan <- struct{}{}: + case <-quit: } } @@ -6368,52 +6605,501 @@ func TestShutdownIfChannelClean(t *testing.T) { 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) + // Alice will send an HTLC to Bob before he sends Shutdown. + aliceHtlc1, _ := generateHtlcAndInvoice(t, 0) + // ----add----> + ctx.sendHtlcAliceToBob(0, aliceHtlc1) + ctx.receiveHtlcAliceToBob() - // <---add----- - ctx.sendHtlcBobToAlice(htlc) - // <---sig----- + // <----shutdown---- + ctx.sendShutdownBobToAlice() + + // ----sig----> + select { + case batchTicker <- time.Now(): + case <-time.After(5 * time.Second): + t.Fatalf("batch ticker should have ticked") + } + ctx.receiveCommitSigAliceToBob(1) + + // Give Alice a fee estimate. + select { + case aliceEst.byteFeeIn <- 400: + case <-time.After(time.Second * 5): + t.Fatalf("alice didn't query for the new network fee") + } + + var ok bool + select { + case req := <-closeReqChan: + ok = bytes.Equal(req.DeliveryScript, coreLink.cfg.DeliveryAddr) + require.True(t, ok) + case <-time.After(5 * time.Second): + t.Fatalf("listening on close req failed") + } + + // Wait to receive Alice's Shutdown. + aliceShutdown := ctx.receiveShutdownAlice() + ok = bytes.Equal(aliceShutdown.Address, coreLink.cfg.DeliveryAddr) + require.True(t, ok) + + // <----rev---- + ctx.sendRevAndAckBobToAlice() + // <----sig---- ctx.sendCommitSigBobToAlice(1) // ----rev----> ctx.receiveRevAndAckAliceToBob() - shutdownAssert(ErrLinkFailedShutdown) + // Bob will now fail the HTLC backwards and once the channel state is + // clean, Alice will issue the NotifyCoopReady event. + err = bobChannel.FailHTLC(0, []byte("nop"), nil, nil, nil) + require.NoError(t, err) + failMsg := &lnwire.UpdateFailHTLC{ + ID: 0, + Reason: lnwire.OpaqueReason([]byte("nop")), + } + + // <----fail---- + aliceLink.HandleChannelUpdate(failMsg) + + // <----sig---- + ctx.sendCommitSigBobToAlice(0) + // ----rev----> + ctx.receiveRevAndAckAliceToBob() + // ----sig----> + ctx.receiveCommitSigAliceToBob(0) + // <----rev---- + ctx.sendRevAndAckBobToAlice() + + // Ensure we receive the NotifyCoopReady event. + select { + case <-coopReadyChan: + case <-time.After(5 * time.Second): + t.Fatalf("listening for close req failed") + } + + // It should not be possible to tick the BatchTicker anymore. + select { + case batchTicker <- time.Now(): + t.Fatalf("expected batch ticker to be inactive") + case <-time.After(5 * time.Second): + } +} + +// TestAddAfterShutdown tests that the link will fail if it receives an HTLC +// from the peer after receiving Shutdown. +func TestAddAfterShutdown(t *testing.T) { + t.Parallel() + + const chanAmt = btcutil.SatoshiPerBitcoin * 5 + const chanReserve = btcutil.SatoshiPerBitcoin * 1 + aliceLink, bobChannel, _, start, _, err := + newSingleLinkTestHarness(t, chanAmt, chanReserve) + require.NoError(t, err) + + var ( + coreLink = aliceLink.(*channelLink) + aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + ) + + // Start Alice's switch. + err = coreLink.cfg.Switch.Start() + require.NoError(t, err) + + closeReqChan := make(chan *ChanClose) + coreLink.cfg.NotifySendingShutdown = func(req *ChanClose, + quit chan struct{}) { + + select { + case closeReqChan <- req: + case <-quit: + } + } + + err = start() + require.NoError(t, err) + + ctx := linkTestContext{ + t: t, + aliceLink: aliceLink, + bobChannel: bobChannel, + aliceMsgs: aliceMsgs, + } + + linkErrors := make(chan LinkFailureError, 1) + + // Modify OnChannelFailure so we are notified when the link fails. + coreLink.cfg.OnChannelFailure = func(_ lnwire.ChannelID, + _ lnwire.ShortChannelID, linkErr LinkFailureError) { + + linkErrors <- linkErr + } + + // Alice will send Bob an HTLC so she doesn't close the link after + // sending Shutdown + aliceHtlc1, _ := generateHtlcAndInvoice(t, 0) + // ----add----> + ctx.sendHtlcAliceToBob(0, aliceHtlc1) + ctx.receiveHtlcAliceToBob() + // <----shutdown---- + ctx.sendShutdownBobToAlice() + + // Wait until Alice receives the Shutdown message in the link. + err = wait.NoError(func() error { + if coreLink.hasReceivedShutdown() { + return nil + } + + return fmt.Errorf("alice hasn't received Shutdown") + }, time.Second*5) + require.NoError(t, err) + + // If Alice receives an HTLC from the switch, she will simply fail it + // backwards. + amount := lnwire.NewMSatFromSatoshis(100_000) + + htlcAmt, totalTimelock, hops := generateHops( + amount, testStartingHeight, coreLink, + ) + blob, err := generateRoute(hops...) + require.NoError(t, err) + + _, switchHtlc, pid, err := generatePayment( + amount, htlcAmt, totalTimelock, blob, + ) + require.NoError(t, err) + + err = coreLink.cfg.Switch.SendHTLC( + aliceLink.ShortChanID(), pid, switchHtlc, + ) + require.NoError(t, err) + + resultChan, err := coreLink.cfg.Switch.GetPaymentResult( + pid, switchHtlc.PaymentHash, newMockDeobfuscator(), + ) + require.NoError(t, err) + + var ( + result *PaymentResult + ok bool + ) + + select { + case result, ok = <-resultChan: + require.True(t, ok) + case <-time.After(10 * time.Second): + t.Fatalf("no result arrived") + } + + assertFailureCode( + t, result.Error, lnwire.CodeTemporaryChannelFailure, + ) + + // If Alice receives an HTLC from Bob, the link will be failed. + bobHtlc := generateHtlc(t, coreLink, 0) + // <----add---- + ctx.sendHtlcBobToAlice(bobHtlc) + + select { + case linkErr := <-linkErrors: + require.Equal(t, linkErr.code, ErrInvalidUpdate) + case <-time.After(time.Second * 5): + t.Fatalf("expected to receive a link error") + } +} + +// TestDuplicateShutdown tests that the link will fail if it receives a second +// Shutdown from the remote peer. +func TestDuplicateShutdown(t *testing.T) { + t.Parallel() + + const chanAmt = btcutil.SatoshiPerBitcoin * 5 + const chanReserve = btcutil.SatoshiPerBitcoin * 1 + aliceLink, bobChannel, _, start, _, err := + newSingleLinkTestHarness(t, chanAmt, chanReserve) + require.NoError(t, err) + + var ( + coreLink = aliceLink.(*channelLink) + aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + aliceEst = coreLink.cfg.FeeEstimator.(*mockFeeEstimator) + ) + + // Start Alice's switch. + err = coreLink.cfg.Switch.Start() + require.NoError(t, err) + + closeReqChan := make(chan *ChanClose) + coreLink.cfg.NotifySendingShutdown = func(req *ChanClose, + quit chan struct{}) { + + select { + case closeReqChan <- req: + case <-quit: + } + } + + err = start() + require.NoError(t, err) + + ctx := linkTestContext{ + t: t, + aliceLink: aliceLink, + bobChannel: bobChannel, + aliceMsgs: aliceMsgs, + } + + linkErrors := make(chan LinkFailureError, 1) + + // Modify OnChannelFailure so we are notified when the link fails. + coreLink.cfg.OnChannelFailure = func(_ lnwire.ChannelID, + _ lnwire.ShortChannelID, linkErr LinkFailureError) { + + linkErrors <- linkErr + } + + // Bob will send an HTLC so that Alice's link doesn't immediately close + // the link after sending Shutdown. + bobHtlc := generateHtlc(t, coreLink, 0) + // <----add---- + ctx.sendHtlcBobToAlice(bobHtlc) + // <----shutdown---- + ctx.sendShutdownBobToAlice() + + // Give Alice a fee estimate. + select { + case aliceEst.byteFeeIn <- 400: + case <-time.After(time.Second * 5): + t.Fatalf("alice didn't query for the new network fee") + } + + select { + case <-closeReqChan: + case <-time.After(5 * time.Second): + t.Fatalf("expected to receive NotifySendingShutdown event") + } + + // Wait to receive the Shutdown from Alice. + // ----shutdown----> + _ = ctx.receiveShutdownAlice() + + // Bob sends a duplicate Shutdown + // <----shutdown---- + ctx.sendShutdownBobToAlice() + + select { + case linkErr := <-linkErrors: + require.Equal(t, linkErr.code, ErrRemoteError) + case <-time.After(time.Second * 5): + t.Fatalf("expected to receive a link error") + } +} + +// TestShutdownInit tests that the link will send Shutdown once there are no +// pending HTLCs. It also tests that no HTLC's will be forwarded through us to +// the peer. +func TestShutdownInit(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 + ) + + // Start Alice's switch. + err = coreLink.cfg.Switch.Start() + require.NoError(t, err) + + // Populate the shutdown functions as they're not defined by default + // with the test harness. + closeReqChan := make(chan *ChanClose) + coreLink.cfg.NotifySendingShutdown = func(req *ChanClose, + quit chan struct{}) { + + select { + case closeReqChan <- req: + case <-quit: + } + } + + coopReadyChan := make(chan struct{}) + coreLink.cfg.NotifyCoopReady = func(chanPoint *wire.OutPoint, + quit chan struct{}) { + + select { + case coopReadyChan <- struct{}{}: + case <-quit: + } + } + + err = start() + require.NoError(t, err) + + ctx := linkTestContext{ + t: t, + aliceLink: aliceLink, + bobChannel: bobChannel, + aliceMsgs: aliceMsgs, + } + + // First Alice will send an HTLC to Bob before initiating Shutdown. We + // verify that Shutdown is only sent after the HTLC is signed for. + aliceHtlc1, _ := generateHtlcAndInvoice(t, 0) + + // ----add----> + ctx.sendHtlcAliceToBob(0, aliceHtlc1) + ctx.receiveHtlcAliceToBob() + + // Create the ChanClose to pass to Alice's link. + aliceScript := genScript(t, p2wshAddress) + closeReq := &ChanClose{ + CloseType: contractcourt.CloseRegular, + ChanPoint: bobChannel.ChannelPoint(), + TargetFeePerKw: 300, + DeliveryScript: aliceScript, + Updates: make(chan interface{}, 2), + Err: make(chan error, 1), + } + err = aliceLink.NotifyShouldShutdown(closeReq) + require.NoError(t, err) + + // If Alice receives an HTLC from the switch after NotifyShouldShutdown + // is called, she'll fail it backwards. + amount := lnwire.NewMSatFromSatoshis(100_000) + + htlcAmt, totalTimelock, hops := generateHops( + amount, testStartingHeight, coreLink, + ) + blob, err := generateRoute(hops...) + require.NoError(t, err) + + _, switchHtlc, pid, err := generatePayment( + amount, htlcAmt, totalTimelock, blob, + ) + require.NoError(t, err) + + err = coreLink.cfg.Switch.SendHTLC( + aliceLink.ShortChanID(), pid, switchHtlc, + ) + require.NoError(t, err) + + resultChan, err := coreLink.cfg.Switch.GetPaymentResult( + pid, switchHtlc.PaymentHash, newMockDeobfuscator(), + ) + require.NoError(t, err) + + var ( + result *PaymentResult + ok bool + ) + + select { + case result, ok = <-resultChan: + require.True(t, ok) + case <-time.After(10 * time.Second): + t.Fatalf("no result arrived") + } + + assertFailureCode( + t, result.Error, lnwire.CodeTemporaryChannelFailure, + ) + + // After Alice sends CommitSig, she'll send Shutdown. + select { + case batchTicker <- time.Now(): + case <-time.After(5 * time.Second): + t.Fatalf("batch ticker should have ticked") + } // ----sig----> ctx.receiveCommitSigAliceToBob(1) - shutdownAssert(ErrLinkFailedShutdown) - // <---rev----- + // Ensure we receive the NotifySendingShutdown event. + select { + case req := <-closeReqChan: + require.True(t, bytes.Equal(req.DeliveryScript, aliceScript)) + case <-time.After(5 * time.Second): + t.Fatalf("listening for close req failed") + } + + // ----shutdown----> + aliceShutdown := ctx.receiveShutdownAlice() + require.True(t, bytes.Equal(aliceShutdown.Address, aliceScript)) + + // <----rev---- ctx.sendRevAndAckBobToAlice() - shutdownAssert(ErrLinkFailedShutdown) - // ---settle--> - ctx.receiveSettleAliceToBob() - shutdownAssert(ErrLinkFailedShutdown) + // <----sig---- + ctx.sendCommitSigBobToAlice(1) + + // ----rev----> + ctx.receiveRevAndAckAliceToBob() + // Now Bob will send an HTLC through to Alice. + bobHtlc := generateHtlc(t, coreLink, 0) + // <----add---- + ctx.sendHtlcBobToAlice(bobHtlc) + // The link code does not verify that the peer has no pending updates. + // This is easier and the channel will eventually come to a stopping + // point anyways given that we'll reject any new HTLC's after this + // point. + // <----shutdown---- + ctx.sendShutdownBobToAlice() + // <----sig---- + ctx.sendCommitSigBobToAlice(2) + // ----rev----> + ctx.receiveRevAndAckAliceToBob() // ----sig----> - ctx.receiveCommitSigAliceToBob(0) - shutdownAssert(ErrLinkFailedShutdown) + ctx.receiveCommitSigAliceToBob(2) + // <----rev---- + ctx.sendRevAndAckBobToAlice() - // <---rev----- + // Alice should send a settle to Bob. + // ----settle----> + ctx.receiveSettleAliceToBob() + // ----sig----> + ctx.receiveCommitSigAliceToBob(1) + // <----rev---- ctx.sendRevAndAckBobToAlice() - shutdownAssert(ErrLinkFailedShutdown) + // <----sig---- + ctx.sendCommitSigBobToAlice(1) + // ----rev----> + ctx.receiveRevAndAckAliceToBob() - // 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) + // Bob will fail back the remaining HTLC and Alice should call + // NotifyCoopReady. + err = bobChannel.FailHTLC(0, []byte("nop"), nil, nil, nil) + require.NoError(t, err) + failMsg := &lnwire.UpdateFailHTLC{ + ID: 0, + Reason: lnwire.OpaqueReason([]byte("nop")), + } + + // <----fail---- + aliceLink.HandleChannelUpdate(failMsg) + // <----sig---- + ctx.sendCommitSigBobToAlice(0) // ----rev----> ctx.receiveRevAndAckAliceToBob() - shutdownAssert(nil) + // ----sig----> + ctx.receiveCommitSigAliceToBob(0) + // <----rev---- + ctx.sendRevAndAckBobToAlice() + + // Ensure we receive the NotifyCoopReady event. + select { + case <-coopReadyChan: + case <-time.After(5 * time.Second): + t.Fatalf("listening for close req failed") + } - // Now that the link has exited the htlcManager loop, attempt to - // trigger the batch ticker. It should not be possible. + // It should not be possible to tick the BatchTicker anymore. select { case batchTicker <- time.Now(): t.Fatalf("expected batch ticker to be inactive") @@ -6421,6 +7107,107 @@ func TestShutdownIfChannelClean(t *testing.T) { } } +// genScript creates a script paying out to the address provided, which must +// be a valid address. +func genScript(t *testing.T, address string) lnwire.DeliveryAddress { + // Generate an address which can be used for testing. + deliveryAddr, err := btcutil.DecodeAddress( + address, + &chaincfg.TestNet3Params, + ) + require.NoError(t, err, "invalid delivery address") + + script, err := txscript.PayToAddrScript(deliveryAddr) + require.NoError(t, err, "cannot create script") + + return script +} + +var ( + // p2SHAddress is a valid pay to script hash address. + p2SHAddress = "2NBFNJTktNa7GZusGbDbGKRZTxdK9VVez3n" + + // p2wshAddress is a valid pay to witness script hash address. + p2wshAddress = "bc1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3qccfmv3" +) + +// TestChooseDeliveryScript tests that chooseDeliveryScript correctly errors +// when upfront and user set scripts that do not match are provided, allows +// matching values and returns appropriate values in the case where one or none +// are set. +func TestChooseDeliveryScript(t *testing.T) { + // generate non-zero scripts for testing. + script1 := genScript(t, p2SHAddress) + script2 := genScript(t, p2wshAddress) + + tests := []struct { + name string + userScript lnwire.DeliveryAddress + shutdownScript lnwire.DeliveryAddress + expectedScript lnwire.DeliveryAddress + expectedError error + }{ + { + name: "Neither set", + userScript: nil, + shutdownScript: nil, + expectedScript: nil, + expectedError: nil, + }, + { + name: "Both set and equal", + userScript: script1, + shutdownScript: script1, + expectedScript: script1, + expectedError: nil, + }, + { + name: "Both set and not equal", + userScript: script1, + shutdownScript: script2, + expectedScript: nil, + expectedError: ErrUpfrontShutdownScriptMismatch, + }, + { + name: "Only upfront script", + userScript: nil, + shutdownScript: script1, + expectedScript: script1, + expectedError: nil, + }, + { + name: "Only user script", + userScript: script2, + shutdownScript: nil, + expectedScript: script2, + expectedError: nil, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + script, err := chooseDeliveryScript( + test.shutdownScript, test.userScript, + ) + if err != test.expectedError { + t.Fatalf( + "Expected: %v, got: %v", + test.expectedError, err, + ) + } + + if !bytes.Equal(script, test.expectedScript) { + t.Fatalf( + "Expected: %x, got: %x", + test.expectedScript, script, + ) + } + }) + } +} + // 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/linkfailure.go b/htlcswitch/linkfailure.go index 1f454a7bb4..c1452c7521 100644 --- a/htlcswitch/linkfailure.go +++ b/htlcswitch/linkfailure.go @@ -8,6 +8,10 @@ var ( // ErrLinkFailedShutdown signals that a requested shutdown failed. ErrLinkFailedShutdown = errors.New("link failed to shutdown") + + // ErrLinkCoopClosing signals that the link is in the shutdown phase of + // the coop close flow. + ErrLinkCoopClosing = errors.New("link coop closing") ) // errorCode encodes the possible types of errors that will make us fail the diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 0c9bd303f8..cfb149705b 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -616,6 +616,8 @@ func (s *mockServer) readHandler(message lnwire.Message) error { targetChan = msg.ChanID case *lnwire.UpdateFee: targetChan = msg.ChanID + case *lnwire.Shutdown: + targetChan = msg.ChannelID default: return fmt.Errorf("unknown message type: %T", msg) } @@ -878,13 +880,15 @@ 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) { f.eligible = true return f.shortChanID, nil } +func (f *mockChannelLink) NotifyShouldShutdown(req *ChanClose) error { + return nil +} var _ ChannelLink = (*mockChannelLink)(nil) diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index c04a42df20..780c401788 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -132,11 +132,6 @@ type Config struct { // settle is eventually received. FwdingLog ForwardingLog - // LocalChannelClose kicks-off the workflow to execute a cooperative or - // forced unilateral closure of the channel initiated by a local - // subsystem. - LocalChannelClose func(pubKey []byte, request *ChanClose) - // DB is the database backend that will be used to back the switch's // persistent circuit map. DB kvdb.Backend @@ -1805,11 +1800,14 @@ out: } s.indexMtx.RUnlock() - peerPub := link.Peer().PubKey() log.Debugf("Requesting local channel close: peer=%v, "+ "chan_id=%x", link.Peer(), chanID[:]) - go s.cfg.LocalChannelClose(peerPub[:], req) + // Tell link to shutdown. If an error is received, + // we'll pass it to the caller. + if err := link.NotifyShouldShutdown(req); err != nil { + req.Err <- err + } case resolutionMsg := <-s.resolutionMsgs: // We'll persist the resolution message to the Switch's diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index e4568fc0e9..586ddd704c 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -498,6 +498,8 @@ func getChanID(msg lnwire.Message) (lnwire.ChannelID, error) { chanID = msg.ChanID case *lnwire.UpdateFee: chanID = msg.ChanID + case *lnwire.Shutdown: + chanID = msg.ChannelID default: return chanID, fmt.Errorf("unknown type: %T", msg) } diff --git a/lntest/itest/lnd_coopclose_test.go b/lntest/itest/lnd_coopclose_test.go new file mode 100644 index 0000000000..64455d27f1 --- /dev/null +++ b/lntest/itest/lnd_coopclose_test.go @@ -0,0 +1,146 @@ +package itest + +import ( + "context" + "fmt" + "time" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" + "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntest" + "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/stretchr/testify/require" +) + +// testCoopClose tests that the coop close process adheres to the BOLT#02 +// specification. +func testCoopClose(net *lntest.NetworkHarness, t *harnessTest) { + ctxb := context.Background() + + chanReq := lntest.OpenChannelParams{ + Amt: 300000, + } + + chanPoint := openChannelAndAssert( + t, net, net.Alice, net.Bob, chanReq, + ) + + // Create a hodl invoice for Bob. This will allow us to exercise the + // Shutdown logic. + var ( + preimage = lntypes.Preimage{1, 2, 3} + payHash = preimage.Hash() + ) + + invoiceReq := &invoicesrpc.AddHoldInvoiceRequest{ + Value: 30000, + CltvExpiry: 40, + Hash: payHash[:], + } + + ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + bobInvoice, err := net.Bob.AddHoldInvoice(ctxt, invoiceReq) + require.NoError(t.t, err) + + // Alice will now pay this invoice and get a single HTLC locked-in on + // each of the commitment transactions. + _, err = net.Alice.RouterClient.SendPaymentV2( + ctxb, &routerrpc.SendPaymentRequest{ + PaymentRequest: bobInvoice.PaymentRequest, + TimeoutSeconds: 60, + FeeLimitMsat: noFeeLimitMsat, + }, + ) + require.NoError(t.t, err) + + waitForInvoiceAccepted(t, net.Bob, payHash) + + // Alice and Bob should both have a single HTLC locked in. + nodes := []*lntest.HarnessNode{net.Alice, net.Bob} + err = wait.NoError(func() error { + return assertActiveHtlcs(nodes, payHash[:]) + }, defaultTimeout) + require.NoError(t.t, err) + + closeReq := &lnrpc.CloseChannelRequest{ + ChannelPoint: chanPoint, + } + + // Alice will initiate the coop close and we'll obtain the close + // channel client to assert that the channel is closed once the htlc is + // settled. + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + closeRespStream, err := net.Alice.CloseChannel(ctxt, closeReq) + require.NoError(t.t, err) + + time.Sleep(time.Second * 10) + + // Bob will now settle the invoice. + settle := &invoicesrpc.SettleInvoiceMsg{ + Preimage: preimage[:], + } + _, err = net.Bob.SettleInvoice(ctxt, settle) + require.NoError(t.t, err) + + // The coop close negotiation should now complete. + var closeTxid *chainhash.Hash + err = wait.NoError(func() error { + closeResp, err := closeRespStream.Recv() + if err != nil { + return fmt.Errorf("unable to Recv() from stream: %v", + err) + } + + pendingClose, ok := closeResp.Update.(*lnrpc.CloseStatusUpdate_ClosePending) + if !ok { + return fmt.Errorf("expected channel close update, "+ + "instead got %v", pendingClose) + } + + closeTxid, err = chainhash.NewHash( + pendingClose.ClosePending.Txid, + ) + if err != nil { + return fmt.Errorf("unable to decode closeTxid: %v", + err) + } + + coopTx, err := waitForTxInMempool( + net.Miner.Client, minerMempoolTimeout, + ) + if err != nil { + return err + } + + if !coopTx.IsEqual(closeTxid) { + return fmt.Errorf("mempool tx is not coop close tx") + } + + return nil + }, channelCloseTimeout) + require.NoError(t.t, err) + + // The closing transaction will be mined and Alice and Bob should both + // see the channel as successfully resolved. + block := mineBlocks(t, net, 1, 1)[0] + assertTxInBlock(t, block, closeTxid) + + // Sleep again to ensure the ChannelArbitrator can advance to + // StateFullyResolved. + time.Sleep(10 * time.Second) + + aliceReq := &lnrpc.ClosedChannelsRequest{} + aliceClosedList, err := net.Alice.ClosedChannels(ctxt, aliceReq) + require.NoError(t.t, err, "alice list closed channels") + require.Len(t.t, aliceClosedList.Channels, 1, "alice closed channels") + + bobReq := &lnrpc.ClosedChannelsRequest{} + bobClosedList, err := net.Bob.ClosedChannels(ctxt, bobReq) + require.NoError(t.t, err, "bob list closed channels") + require.Len(t.t, bobClosedList.Channels, 1, "bob closed channels") +} diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index fe5ac63784..3831ca9927 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -423,4 +423,8 @@ var allTestCases = []*testCase{ name: "open channel fee policy", test: testOpenChannelUpdateFeePolicy, }, + { + name: "coop close", + test: testCoopClose, + }, } diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 2ddf479500..74a092abf7 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -2,6 +2,7 @@ package chancloser import ( "bytes" + "errors" "fmt" "github.com/btcsuite/btcd/btcutil" @@ -33,12 +34,6 @@ var ( // message while it is in an unknown state. ErrInvalidState = fmt.Errorf("invalid state") - // ErrUpfrontShutdownScriptMismatch is returned when a peer or end user - // provides a cooperative close script which does not match the upfront - // shutdown script previously set for that party. - ErrUpfrontShutdownScriptMismatch = fmt.Errorf("shutdown script does not " + - "match upfront shutdown script") - // ErrProposalExeceedsMaxFee is returned when as the initiator, the // latest fee proposal sent by the responder exceed our max fee. // responder. @@ -48,6 +43,23 @@ var ( // ErrInvalidShutdownScript is returned when we receive an address from // a peer that isn't either a p2wsh or p2tr address. ErrInvalidShutdownScript = fmt.Errorf("invalid shutdown script") + + // ErrCloseTypeChanged is returned when the counterparty attempts to + // change the negotiation type from fee-range to legacy or vice versa. + ErrCloseTypeChanged = fmt.Errorf("close type changed") + + // ErrNoRangeOverlap is returned when there is no overlap between our + // and our counterparty's fee_range. + ErrNoRangeOverlap = fmt.Errorf("no range overlap") + + // ErrFeeNotInOverlap is returned when the counterparty sends a fee that + // is not in the overlapping fee_range. + ErrFeeNotInOverlap = fmt.Errorf("fee not in overlap") + + // ErrFeeRangeViolation is returned when the fundee receives a bad + // FeeRange from the funder after the fundee has sent their one and + // only FeeRange to the funder. + ErrFeeRangeViolation = fmt.Errorf("fee range violation") ) // closeState represents all the possible states the channel closer state @@ -60,14 +72,13 @@ const ( // closeIdle is the initial starting state. In this state, the state // machine has been instantiated, but no state transitions have been // attempted. If a state machine receives a message while in this state, - // then it is the responder to an initiated cooperative channel closure. + // then either we have initiated coop close or the remote peer has. closeIdle closeState = iota - // closeShutdownInitiated is the state that's transitioned to once the - // initiator of a closing workflow sends the shutdown message. At this - // point, they're waiting for the remote party to respond with their own - // shutdown message. After which, they'll both enter the fee negotiation - // phase. + // closeShutdownInitiated is the state that's transitioned to once + // either we or the remote party has sent a Shutdown message. At this + // point we'll be waiting for the recipient (which may be us) to + // respond. After which, we'll enter the fee negotiation phase. closeShutdownInitiated // closeFeeNegotiation is the third, and most persistent state. Both @@ -225,6 +236,9 @@ type ChanCloser struct { // idealFeeRate is our ideal fee rate. idealFeeRate chainfee.SatPerKWeight + // idealFeeRange is our ideal fee range. + idealFeeRange *lnwire.FeeRange + // lastFeeProposal is the last fee that we proposed to the remote party. // We'll use this as a pivot point to ratchet our next offer up, down, or // simply accept the remote party's prior offer. @@ -256,6 +270,37 @@ type ChanCloser struct { // locallyInitiated is true if we initiated the channel close. locallyInitiated bool + + // channelClean is true if the peer.Brontide has signaled that the + // channel is no longer operational and is in a clean state. + channelClean bool + + // peerClosingSigned stores a single ClosingSigned that is received + // while the channel is still not in a clean state. + peerClosingSigned *lnwire.ClosingSigned + + // receivedLocalShutdown stores whether or not we've processed a + // Shutdown message from ourselves. + receivedLocalShutdown bool + + // receivedRemoteShutdown stores whether or not we've processed a + // Shutdown message from the remote peer. + receivedRemoteShutdown bool + + // cleanOnRecv means that the channel is already clean, but we are + // waiting for the peer's Shutdown and will call ChannelClean when we + // receive it. This is only used when we restart the connection. + cleanOnRecv bool + + // legacyNegotiation means that legacy negotiation has been initiated. + // This is used so that the remote can't change from legacy to range + // based negotiation. + legacyNegotiation bool + + // rangeNegotiation means that range-based negotiation has been + // initiated. This is used so the remote can't change from range-based + // to legacy negotiation. + rangeNegotiation bool } // calcCoopCloseFee computes an "ideal" absolute co-op close fee given the @@ -302,7 +347,8 @@ func (d *SimpleCoopFeeEstimator) EstimateFee(chanType channeldb.ChannelType, // be populated iff, we're the initiator of this closing request. func NewChanCloser(cfg ChanCloseCfg, deliveryScript []byte, idealFeePerKw chainfee.SatPerKWeight, negotiationHeight uint32, - closeReq *htlcswitch.ChanClose, locallyInitiated bool) *ChanCloser { + closeReq *htlcswitch.ChanClose, locallyInitiated, + cleanOnRecv bool) *ChanCloser { cid := lnwire.NewChanIDFromOutPoint(cfg.Channel.ChannelPoint()) return &ChanCloser{ @@ -316,6 +362,7 @@ func NewChanCloser(cfg ChanCloseCfg, deliveryScript []byte, localDeliveryScript: deliveryScript, priorFeeOffers: make(map[btcutil.Amount]*lnwire.ClosingSigned), locallyInitiated: locallyInitiated, + cleanOnRecv: cleanOnRecv, } } @@ -354,69 +401,84 @@ func (c *ChanCloser) initFeeBaseline() { ) } + // Calculate the minimum fee we'll accept for the fee range. + minFeeSats := c.cfg.FeeEstimator.EstimateFee( + 0, localTxOut, remoteTxOut, chainfee.FeePerKwFloor, + ) + + // Populate the fee range. If minFeeSats is greater than idealFeeSat, + // use idealFeeSat as the minimum. This may happen since FeePerKwFloor + // uses 253 sat/kw instead of 250 sat/kw. + c.idealFeeRange = &lnwire.FeeRange{ + MaxFeeSats: c.maxFee, + } + + if minFeeSats > c.idealFeeSat { + c.idealFeeRange.MinFeeSats = c.idealFeeSat + } else { + c.idealFeeRange.MinFeeSats = minFeeSats + } + chancloserLog.Infof("Ideal fee for closure of ChannelPoint(%v) "+ "is: %v sat (max_fee=%v sat)", c.cfg.Channel.ChannelPoint(), int64(c.idealFeeSat), int64(c.maxFee)) } -// initChanShutdown begins the shutdown process by un-registering the channel, -// and creating a valid shutdown message to our target delivery address. -func (c *ChanCloser) initChanShutdown() (*lnwire.Shutdown, error) { - // With both items constructed we'll now send the shutdown message for this - // particular channel, advertising a shutdown request to our desired - // closing script. - shutdown := lnwire.NewShutdown(c.cid, c.localDeliveryScript) - - // 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. - if err := c.cfg.DisableChannel(c.chanPoint); err != nil { - chancloserLog.Warnf("Unable to disable channel %v on close: %v", - c.chanPoint, err) +// SetLocalScript sets the localDeliveryScript. This doesn't need a mutex since +// operations usually run in the same calling thread. +func (c *ChanCloser) SetLocalScript(local lnwire.DeliveryAddress) { + c.localDeliveryScript = local +} + +// ChannelClean is used by the caller to notify the ChanCloser that the channel +// is in a clean state and ClosingSigned negotiation can begin. Any +// ClosingSigned received before then will be stored in a variable instead of +// being processed. +func (c *ChanCloser) ChannelClean() ([]lnwire.Message, bool, error) { + // Return an error if the state transition is invalid. + if c.state != closeFeeNegotiation { + return nil, false, fmt.Errorf("channel clean, but not in " + + "state closeFeeNegotiation") } - // 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 - // that the channel is being shutdown by the time the closing request - // returns. + // Set the fee baseline now that we know the remote's delivery script. + c.initFeeBaseline() + + c.channelClean = true + + // 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 that the channel is being shutdown by the time the + // closing request returns. err := c.cfg.Channel.MarkCoopBroadcasted(nil, c.locallyInitiated) if err != nil { - return nil, err + return nil, false, err } - chancloserLog.Infof("ChannelPoint(%v): sending shutdown message", - c.chanPoint) - - return shutdown, nil -} + if c.cfg.Channel.IsInitiator() { + closeSigned, err := c.proposeCloseSigned(c.idealFeeSat) + if err != nil { + return nil, false, err + } -// ShutdownChan is the first method that's to be called by the initiator of the -// cooperative channel closure. This message returns the shutdown message to -// send to the remote party. Upon completion, we enter the -// closeShutdownInitiated phase as we await a response. -func (c *ChanCloser) ShutdownChan() (*lnwire.Shutdown, error) { - // If we attempt to shutdown the channel for the first time, and we're not - // in the closeIdle state, then the caller made an error. - if c.state != closeIdle { - return nil, ErrChanAlreadyClosing + return []lnwire.Message{closeSigned}, false, err } - chancloserLog.Infof("ChannelPoint(%v): initiating shutdown", c.chanPoint) + // We may not have the peer's ClosingSigned at this point. If we don't, + // we'll return early. We don't return an error, since this is a valid + // state to be in. + if c.peerClosingSigned == nil { + return nil, false, nil + } - shutdownMsg, err := c.initChanShutdown() + // Process the peer's ClosingSigned. + msgs, closeFin, err := c.ProcessCloseMsg(c.peerClosingSigned, true) if err != nil { - return nil, err + return nil, false, err } - // With the opening steps complete, we'll transition into the - // closeShutdownInitiated state. In this state, we'll wait until the other - // party sends their version of the shutdown message. - c.state = closeShutdownInitiated - - // Finally, we'll return the shutdown message to the caller so it can send - // it to the remote peer. - return shutdownMsg, nil + return msgs, closeFin, nil } // ClosingTx returns the fully signed, final closing transaction. @@ -499,7 +561,7 @@ func validateShutdownScript(disconnect func() error, upfrontScript, return err } - return ErrUpfrontShutdownScriptMismatch + return htlcswitch.ErrUpfrontShutdownScriptMismatch } return nil @@ -510,13 +572,13 @@ func validateShutdownScript(disconnect func() error, upfrontScript, // the next set of messages to be sent, and a bool indicating if the fee // negotiation process has completed. If the second value is true, then this // means the ChanCloser can be garbage collected. -func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, - bool, error) { +func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message, remote bool) ( + []lnwire.Message, bool, error) { switch c.state { - // If we're in the close idle state, and we're receiving a channel closure - // related message, then this indicates that we're on the receiving side of - // an initiated channel closure. + // If we're in the closeIdle state, and we're processing a channel + // close message, either the remote peer sent us a Shutdown or we're + // initiating the coop close process. case closeIdle: // First, we'll assert that we have a channel shutdown message, // as otherwise, this is an attempted invalid state transition. @@ -526,80 +588,70 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, "have %v", spew.Sdump(msg)) } - // As we're the responder to this shutdown (the other party + // Populate the Shutdown bool to catch if we receive two from + // ourselves or the peer. Receiving a second from ourselves + // shouldn't be possible, but we catch it anyways. + if remote { + c.receivedRemoteShutdown = true + } else { + c.receivedLocalShutdown = true + } + + // If we're the responder to this shutdown (the other party // wants to close), we'll check if this is a frozen channel or // not. If the channel is frozen and we were not also the // initiator of the channel opening, then we'll deny their close // attempt. chanInitiator := c.cfg.Channel.IsInitiator() - if !chanInitiator { - absoluteThawHeight, err := c.cfg.Channel.AbsoluteThawHeight() + if !chanInitiator && remote { + thawHeight, err := c.cfg.Channel.AbsoluteThawHeight() if err != nil { return nil, false, err } - if c.negotiationHeight < absoluteThawHeight { + if c.negotiationHeight < thawHeight { return nil, false, fmt.Errorf("initiator "+ "attempting to co-op close frozen "+ "ChannelPoint(%v) (current_height=%v, "+ "thaw_height=%v)", c.chanPoint, - c.negotiationHeight, absoluteThawHeight) + c.negotiationHeight, thawHeight) } } - // If the remote node opened the channel with option upfront shutdown - // script, check that the script they provided matches. - if err := validateShutdownScript( - c.cfg.Disconnect, c.cfg.Channel.RemoteUpfrontShutdownScript(), - shutdownMsg.Address, c.cfg.ChainParams, - ); err != nil { - return nil, false, err - } - - // Once we have checked that the other party has not violated option - // upfront shutdown we set their preference for delivery address. We'll - // use this when we craft the closure transaction. - c.remoteDeliveryScript = shutdownMsg.Address + // If this message is from the remote node and they opened the + // channel with option upfront shutdown script, check that the + // script they provided matches. + if remote { + if err := validateShutdownScript( + c.cfg.Disconnect, + c.cfg.Channel.RemoteUpfrontShutdownScript(), + shutdownMsg.Address, c.cfg.ChainParams, + ); err != nil { + return nil, false, err + } - // Now that we know their desried delivery script, we can - // compute what our max/ideal fee will be. - c.initFeeBaseline() + // Once we have checked that the other party has not + // violated option upfront shutdown we set their + // preference for delivery address. We'll use this when + // we craft the closure transaction. + c.remoteDeliveryScript = shutdownMsg.Address + } - // We'll generate a shutdown message of our own to send across the - // wire. - localShutdown, err := c.initChanShutdown() + // We'll attempt to send a disable update for the channel. This + // way, we shouldn't get as many forwards while we're trying to + // wind down the link. + err := c.cfg.DisableChannel(c.chanPoint) if err != nil { - return nil, false, err + chancloserLog.Warnf("Unable to disable channel %v on "+ + "close: %v", c.chanPoint, err) } - chancloserLog.Infof("ChannelPoint(%v): responding to shutdown", - c.chanPoint) + c.state = closeShutdownInitiated - msgsToSend := make([]lnwire.Message, 0, 2) - msgsToSend = append(msgsToSend, localShutdown) - - // After the other party receives this message, we'll actually start - // the final stage of the closure process: fee negotiation. So we'll - // update our internal state to reflect this, so we can handle the next - // message sent. - c.state = closeFeeNegotiation - - // We'll also craft our initial close proposal in order to keep the - // negotiation moving, but only if we're the negotiator. - if chanInitiator { - closeSigned, err := c.proposeCloseSigned(c.idealFeeSat) - if err != nil { - return nil, false, err - } - msgsToSend = append(msgsToSend, closeSigned) - } - - // We'll return both sets of messages to send to the remote party to - // kick off the fee negotiation process. - return msgsToSend, false, nil + return nil, false, nil - // If we just initiated a channel shutdown, and we receive a new message, - // then this indicates the other party is ready to shutdown as well. In - // this state we'll send our first signature. + // If we are processing a Shutdown message and we're in this state, + // we are ready to start the signing process. If we are the channel + // initiator, we'll send our first signature. case closeShutdownInitiated: // First, we'll assert that we have a channel shutdown message. // Otherwise, this is an attempted invalid state transition. @@ -609,42 +661,47 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, "have %v", spew.Sdump(msg)) } - // If the remote node opened the channel with option upfront shutdown - // script, check that the script they provided matches. - if err := validateShutdownScript( - c.cfg.Disconnect, - c.cfg.Channel.RemoteUpfrontShutdownScript(), shutdownMsg.Address, - c.cfg.ChainParams, - ); err != nil { - return nil, false, err + // Check that we're not receiving two Shutdowns. + if remote && c.receivedRemoteShutdown { + return nil, false, errors.New("received duplicate " + + "Shutdown from peer") + } else if !remote && c.receivedLocalShutdown { + return nil, false, errors.New("received duplicate " + + "Shutdown from ourselves") } - // Now that we know this is a valid shutdown message and address, we'll - // record their preferred delivery closing script. - c.remoteDeliveryScript = shutdownMsg.Address + // If the remote node sent Shutdown and option upfront shutdown + // script was negotiated, check that the script they provided + // matches. + if remote { + if err := validateShutdownScript( + c.cfg.Disconnect, + c.cfg.Channel.RemoteUpfrontShutdownScript(), + shutdownMsg.Address, c.cfg.ChainParams, + ); err != nil { + return nil, false, err + } + + // Now that we know this is a valid shutdown message + // and address, we'll record their preferred delivery + // closing script. + c.remoteDeliveryScript = shutdownMsg.Address + } // At this point, we can now start the fee negotiation state, by // constructing and sending our initial signature for what we think the // closing transaction should look like. c.state = closeFeeNegotiation - // Now that we know their desried delivery script, we can - // compute what our max/ideal fee will be. - c.initFeeBaseline() - - chancloserLog.Infof("ChannelPoint(%v): shutdown response received, "+ - "entering fee negotiation", c.chanPoint) + chancloserLog.Infof("ChannelPoint(%v): entering fee "+ + "negotiation", c.chanPoint) - // Starting with our ideal fee rate, we'll create an initial closing - // proposal, but only if we're the initiator, as otherwise, the other - // party will send their initial proposal first. - if c.cfg.Channel.IsInitiator() { - closeSigned, err := c.proposeCloseSigned(c.idealFeeSat) - if err != nil { - return nil, false, err - } - - return []lnwire.Message{closeSigned}, false, nil + // If the cleanOnRecv bool is set, then we should call + // ChannelClean. It's not possible to be in the finished state + // at this point. The local message is always processed first, + // so the remote MUST be sending the message here. + if c.cleanOnRecv && remote { + return c.ChannelClean() } return nil, false, nil @@ -661,39 +718,45 @@ func (c *ChanCloser) ProcessCloseMsg(msg lnwire.Message) ([]lnwire.Message, "instead have %v", spew.Sdump(msg)) } + if !c.channelClean { + // If we are the initiator, they should not be sending + // ClosingSigned here, as we haven't even sent ours. + if c.cfg.Channel.IsInitiator() { + return nil, false, errors.New("remote peer " + + "sent ClosingSigned first when they " + + "are not the channel initiator") + } + + // We are not the initiator. If an outside subsystem + // hasn't given us the notification that the channel is + // clean, don't process this message and instead store + // in a buffer. We store this even though it may be + // before the link is actually clean, but we can't know + // for sure. It will be processed immediately when the + // ChanCloser is notified that the channel is clean. + c.peerClosingSigned = closeSignedMsg + return nil, false, nil + } + // We'll compare the proposed total fee, to what we've proposed during // the negotiations. If it doesn't match any of our prior offers, then // we'll attempt to ratchet the fee closer to remoteProposedFee := closeSignedMsg.FeeSatoshis if _, ok := c.priorFeeOffers[remoteProposedFee]; !ok { - // We'll now attempt to ratchet towards a fee deemed acceptable by - // both parties, factoring in our ideal fee rate, and the last - // proposed fee by both sides. - feeProposal := calcCompromiseFee(c.chanPoint, c.idealFeeSat, - c.lastFeeProposal, remoteProposedFee, - ) - if c.cfg.Channel.IsInitiator() && feeProposal > c.maxFee { - return nil, false, fmt.Errorf("%w: %v > %v", - ErrProposalExeceedsMaxFee, feeProposal, - c.maxFee) - } - - // With our new fee proposal calculated, we'll craft a new close - // signed signature to send to the other party so we can continue - // the fee negotiation process. - closeSigned, err := c.proposeCloseSigned(feeProposal) + response, err := c.handleRemoteProposal(closeSignedMsg) if err != nil { + // If an error was returned, no message was + // returned. Bubble up the error. return nil, false, err } - // If the compromise fee doesn't match what the peer proposed, then - // we'll return this latest close signed message so we can continue - // negotiation. - if feeProposal != remoteProposedFee { - chancloserLog.Debugf("ChannelPoint(%v): close tx fee "+ - "disagreement, continuing negotiation", c.chanPoint) - return []lnwire.Message{closeSigned}, false, nil + // If a response was returned, bubble up the response. + if response != nil { + return []lnwire.Message{response}, false, nil } + + // Else, no response or error was returned so we can + // finish up negotiation. } chancloserLog.Infof("ChannelPoint(%v) fee of %v accepted, ending "+ @@ -799,7 +862,9 @@ func (c *ChanCloser) proposeCloseSigned(fee btcutil.Amount) (*lnwire.ClosingSign // We'll assemble a ClosingSigned message using this information and return // it to the caller so we can kick off the final stage of the channel // closure process. - closeSignedMsg := lnwire.NewClosingSigned(c.cid, fee, parsedSig) + closeSignedMsg := lnwire.NewClosingSigned( + c.cid, fee, parsedSig, c.idealFeeRange, + ) // We'll also save this close signed, in the case that the remote party // accepts our offer. This way, we don't have to re-sign. @@ -808,6 +873,175 @@ func (c *ChanCloser) proposeCloseSigned(fee btcutil.Amount) (*lnwire.ClosingSign return closeSignedMsg, nil } +// handleRemoteProposal sanity checks the remote's ClosingSigned message and +// tries to determine an acceptable fee to reply with. It may return: +// - a message and no error (continue negotiation) +// - no message and an error (negotiation failed) +// - no message and no error (indicating we agree with the remote's fee) +func (c *ChanCloser) handleRemoteProposal( + remoteMsg *lnwire.ClosingSigned) (lnwire.Message, error) { + + isFunder := c.cfg.Channel.IsInitiator() + + // If FeeRange is set, perform FeeRange-specific checks. + if remoteMsg.FeeRange != nil { + // If legacyNegotiation is already set, fail outright. + if c.legacyNegotiation { + return nil, ErrCloseTypeChanged + } + + // Set rangeNegotiation to true. + c.rangeNegotiation = true + + // Get the intersection of our two FeeRanges if one exists. + overlap := c.idealFeeRange.GetOverlap(remoteMsg.FeeRange) + + if isFunder { + // If the fundee replies with a FeeRange, there must be + // overlap with our FeeRange. + // + // BOLT#02: + // - otherwise (it is not the funder) + // - ... + // - otherwise + // - MUST propose a fee_satoshis in the overlap + // between received and (about-to-be) sent + // fee_range. + if overlap == nil { + return nil, ErrNoRangeOverlap + } + + // This is included in the above requirement. + if !overlap.InRange(remoteMsg.FeeSatoshis) { + return nil, ErrFeeNotInOverlap + } + + // If the above checks pass, the funder must reply with + // the same fee_satoshis. + // + // BOLT#02: + // - if it is the funder: + // - ... + // - otherwise: + // - MUST reply with the same fee_satoshis. + _, err := c.proposeCloseSigned(remoteMsg.FeeSatoshis) + if err != nil { + return nil, err + } + + // Return nil values to indicate we are done with + // negotiation. + return nil, nil + } + + // If we are the fundee and we have already sent a ClosingSigned + // to the funder, we should not be calling this function. + // + // BOLT#02: + // - if it is the funder: + // - ... + // - otherwise: + // - MUST reply with the same fee_satoshis. + // + // Since the funder must reply with the same fee_satoshis, the + // calling function should not call into this negotiation + // function. + if c.lastFeeProposal != 0 { + return nil, ErrFeeRangeViolation + } + + // If we are the fundee and there is no overlap between their + // fee_range and our yet-to-be-sent fee_range, send a warning. + // + // BOLT#02: + // - if there is no overlap between that and its own fee_range + // - SHOULD send a warning. + // + // NOTE: The above SHOULD will probably be changed to MUST. + if overlap == nil { + warning := lnwire.NewWarning() + warning.ChanID = remoteMsg.ChannelID + warning.Data = lnwire.WarningData("ClosingSigned: no " + + "fee_range overlap") + return warning, nil + } + + // If we've reached this point, then we have to propose a fee + // in the overlap. + // + // BOLT#02: + // - otherwise (it is not the funder) + // - ... + // - otherwise + // - MUST propose a fee_satoshis in the overlap between + // received and (about-to-be) sent fee_range. + // + // If our ideal fee is in the overlap, use that. If it's not in + // the overlap, use the upper bound of the overlap. + var feeProposal btcutil.Amount + if overlap.InRange(c.idealFeeSat) { + feeProposal = c.idealFeeSat + } else { + feeProposal = overlap.MaxFeeSats + } + + closeSigned, err := c.proposeCloseSigned(feeProposal) + if err != nil { + return nil, err + } + + // If the feeProposal is not equal to the remote's FeeSatoshis, + // negotiation isn't done. + if feeProposal != remoteMsg.FeeSatoshis { + return closeSigned, nil + } + + // Otherwise, negotiation is done. + return nil, nil + } + + // Else, do the legacy negotiation. If rangeNegotiation is already set, + // fail outright. + if c.rangeNegotiation { + return nil, ErrCloseTypeChanged + } + + // Set legacyNegotiation to true. + c.legacyNegotiation = true + + // We'll now attempt to ratchet towards a fee deemed acceptable by both + // parties, factoring in our ideal fee rate, and the last proposed fee + // by both sides. + feeProposal := calcCompromiseFee( + c.chanPoint, c.idealFeeSat, c.lastFeeProposal, + remoteMsg.FeeSatoshis, + ) + if isFunder && feeProposal > c.maxFee { + return nil, fmt.Errorf("%w: %v > %v", ErrProposalExeceedsMaxFee, + feeProposal, c.maxFee) + } + + // With our new fee proposal calculated, we'll craft a new close signed + // signature to send to the other party so we can continue the fee + // negotiation process. + closeSigned, err := c.proposeCloseSigned(feeProposal) + if err != nil { + return nil, err + } + + // If the compromise fee doesn't match what the peer proposed, then + // we'll return this latest close signed message so we can continue + // negotiation. + if feeProposal != remoteMsg.FeeSatoshis { + chancloserLog.Debugf("ChannelPoint(%v): close tx fee "+ + "disagreement, continuing negotiation", c.chanPoint) + return closeSigned, nil + } + + // We are done with negotiation. + return nil, nil +} + // feeInAcceptableRange returns true if the passed remote fee is deemed to be // in an "acceptable" range to our local fee. This is an attempt at a // compromise and to ensure that the fee negotiation has a stopping point. We diff --git a/lnwallet/chancloser/chancloser_test.go b/lnwallet/chancloser/chancloser_test.go index 046ccb5152..de1bac217c 100644 --- a/lnwallet/chancloser/chancloser_test.go +++ b/lnwallet/chancloser/chancloser_test.go @@ -11,6 +11,7 @@ import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" @@ -69,7 +70,7 @@ func TestMaybeMatchScript(t *testing.T) { name: "upfront shutdown set, script not ok", shutdownScript: p2wkh, upfrontScript: p2wsh, - expectedErr: ErrUpfrontShutdownScriptMismatch, + expectedErr: htlcswitch.ErrUpfrontShutdownScriptMismatch, }, { name: "nil shutdown and empty upfront", @@ -165,7 +166,24 @@ func (m *mockChannel) CreateCloseProposal(fee btcutil.Amount, localScript, remoteScript []byte, ) (input.Signature, *chainhash.Hash, btcutil.Amount, error) { - return nil, nil, 0, nil + s := &lnwire.Sig{ + // r value + 0x4e, 0x45, 0xe1, 0x69, 0x32, 0xb8, 0xaf, 0x51, + 0x49, 0x61, 0xa1, 0xd3, 0xa1, 0xa2, 0x5f, 0xdf, + 0x3f, 0x4f, 0x77, 0x32, 0xe9, 0xd6, 0x24, 0xc6, + 0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd, 0x41, + // s value + 0x18, 0x15, 0x22, 0xec, 0x8e, 0xca, 0x07, 0xde, + 0x48, 0x60, 0xa4, 0xac, 0xdd, 0x12, 0x90, 0x9d, + 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22, + 0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09, + } + ecdsaSig, err := s.ToSignature() + if err != nil { + return nil, nil, 0, err + } + + return ecdsaSig, nil, 0, nil } func (m *mockChannel) CompleteCooperativeClose(localSig, @@ -194,6 +212,111 @@ func (m *mockCoopFeeEstimator) EstimateFee(chanType channeldb.ChannelType, return m.targetFee } +type mockFeeRangeEstimator struct{} + +func (m *mockFeeRangeEstimator) EstimateFee(chanType channeldb.ChannelType, + localTxOut, remoteTxOut *wire.TxOut, + idealFeeRate chainfee.SatPerKWeight) btcutil.Amount { + + // Use a weight of 2000 even though coop closing will never produce + // such a large transaction. This is needed so that the ChanCloser can + // create a realistic fee-range instead of a 1sat fee-range if the + // mockCoopFeeEstimator were to be used. + return idealFeeRate.FeeForWeight(2000) +} + +type feeRangeHarness struct { + t *testing.T + funderCloser *ChanCloser + fundeeCloser *ChanCloser +} + +// newFeeRangeHarness returns a new testing harness for testing fee-range logic. +// It returns two ChanCloser pointers that are set to the closeFeeNegotiation +// state and are already in the clean state. +func newFeeRangeHarness(t *testing.T, funderRate, + fundeeRate, maxFee chainfee.SatPerKWeight) (*feeRangeHarness, + *lnwire.ClosingSigned) { + + broadcastStub := func(*wire.MsgTx, string) error { return nil } + + funderCfg := ChanCloseCfg{ + Channel: &mockChannel{ + initiator: true, + }, + FeeEstimator: &mockFeeRangeEstimator{}, + BroadcastTx: broadcastStub, + MaxFee: maxFee, + } + funderCloser := NewChanCloser( + funderCfg, nil, funderRate, 0, nil, true, false, + ) + + fundeeCfg := ChanCloseCfg{ + Channel: &mockChannel{}, + FeeEstimator: &mockFeeRangeEstimator{}, + BroadcastTx: broadcastStub, + } + fundeeCloser := NewChanCloser( + fundeeCfg, nil, fundeeRate, 0, nil, false, false, + ) + + // Set both sides' state to closeFeeNegotiation and call ChannelClean. + funderCloser.state = closeFeeNegotiation + fundeeCloser.state = closeFeeNegotiation + + msg, done, err := funderCloser.ChannelClean() + require.NoError(t, err) + require.False(t, done) + require.Equal(t, 1, len(msg)) + + funderClosing, ok := msg[0].(*lnwire.ClosingSigned) + require.True(t, ok) + + msg, done, err = fundeeCloser.ChannelClean() + require.NoError(t, err) + require.False(t, done) + require.Nil(t, msg) + + harness := &feeRangeHarness{ + t: t, + funderCloser: funderCloser, + fundeeCloser: fundeeCloser, + } + + return harness, funderClosing +} + +// processCloseMsg allows the harness to process a close message and assert +// various things like whether negotiation is complete or whether an error +// should be returned from the ProcessCloseMsg call. +func (f *feeRangeHarness) processCloseMsg(closeMsg lnwire.Message, + fundee, expectMsg, done bool, expectedErr error) lnwire.Message { + + closer := f.funderCloser + if fundee { + closer = f.fundeeCloser + } + + msg, finished, err := closer.ProcessCloseMsg(closeMsg, true) + require.Equal(f.t, finished, done) + + if expectedErr != nil { + require.ErrorIs(f.t, err, expectedErr) + } else { + require.NoError(f.t, err) + } + + if expectMsg { + require.NotNil(f.t, msg) + require.Equal(f.t, 1, len(msg)) + return msg[0] + } + + require.Nil(f.t, msg) + return nil +} + // TestMaxFeeClamp tests that if a max fee is specified, then it's used instead // of the default max fee multiplier. func TestMaxFeeClamp(t *testing.T) { @@ -244,7 +367,7 @@ func TestMaxFeeClamp(t *testing.T) { Channel: &channel, MaxFee: test.inputMaxFee, FeeEstimator: &SimpleCoopFeeEstimator{}, - }, nil, test.idealFee, 0, nil, false, + }, nil, test.idealFee, 0, nil, false, false, ) // We'll call initFeeBaseline early here since we need @@ -285,7 +408,7 @@ func TestMaxFeeBailOut(t *testing.T) { MaxFee: idealFee * 2, } chanCloser := NewChanCloser( - closeCfg, nil, idealFee, 0, nil, false, + closeCfg, nil, idealFee, 0, nil, false, false, ) // We'll now force the channel state into the @@ -295,6 +418,11 @@ func TestMaxFeeBailOut(t *testing.T) { chanCloser.state = closeFeeNegotiation chanCloser.lastFeeProposal = absoluteFee + // Put the ChanCloser into a clean state by calling + // ChannelClean. + _, _, err := chanCloser.ChannelClean() + require.NoError(t, err) + // Next, we'll make a ClosingSigned message that // proposes a fee that's above the specified max fee. // @@ -306,7 +434,7 @@ func TestMaxFeeBailOut(t *testing.T) { FeeSatoshis: absoluteFee * 2, } - _, _, err := chanCloser.ProcessCloseMsg(closeMsg) + _, _, err = chanCloser.ProcessCloseMsg(closeMsg, true) switch isInitiator { // If we're the initiator, then we expect an error at @@ -322,3 +450,289 @@ func TestMaxFeeBailOut(t *testing.T) { }) } } + +// TestFeeRangeFundeeAgreesIdeal tests fee_range negotiation can end early if +// the fundee immediately agrees with the funder's fee_satoshis. +func TestFeeRangeFundeeAgreesIdeal(t *testing.T) { + t.Parallel() + + idealFeeRate := chainfee.SatPerKWeight(500) + harness, funderClosing := newFeeRangeHarness( + t, idealFeeRate, idealFeeRate, chainfee.SatPerKWeight(0), + ) + + // The fundee will process the close message. + msg := harness.processCloseMsg(funderClosing, true, true, true, nil) + + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + + // Assert that we are done with negotiation. + require.Equal( + t, fundeeClosing.FeeSatoshis, funderClosing.FeeSatoshis, + ) + + msg = harness.processCloseMsg(fundeeClosing, false, true, true, nil) + + funderClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + require.Equal( + t, fundeeClosing.FeeSatoshis, funderClosing.FeeSatoshis, + ) +} + +// TestFeeRangeFundeeAgreesOverlap tests that negotiation completes in two +// messages if the overlap's MaxFeeSats for the fundee is equal to the fee +// proposed by the funder. +func TestFeeRangeFundeeAgreesOverlap(t *testing.T) { + t.Parallel() + + // We use a fundee feerate of 250 so that it is out of the overlap and + // the fundee chooses the maximum overlap value. This happens to be the + // funder's MaxFee so negotiation completes early. + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(250) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, funderFeeRate, + ) + + // The fundee will process the close message. + msg := harness.processCloseMsg(funderClosing, true, true, true, nil) + + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + + // Assert that we are done with negotiation and that fundeeClosing uses + // a feerate of 500 at a weight of 2000. + expectedFee := funderFeeRate.FeeForWeight(2000) + require.Equal( + t, fundeeClosing.FeeSatoshis, funderClosing.FeeSatoshis, + ) + require.Equal(t, expectedFee, fundeeClosing.FeeSatoshis) + require.Equal(t, expectedFee, funderClosing.FeeRange.MaxFeeSats) + + msg = harness.processCloseMsg(fundeeClosing, false, true, true, nil) + + funderClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + require.Equal(t, expectedFee, funderClosing.FeeSatoshis) +} + +// TestFeeRangeRegular tests fee_range negotiation completes in the happy path. +func TestFeeRangeRegular(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // The fundee will process the close message. + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + + // Assert that the fundee replies with the fundeeFeeRate for 2000WU. + expectedFee := fundeeFeeRate.FeeForWeight(2000) + require.Equal(t, expectedFee, fundeeClosing.FeeSatoshis) + + // The funder should consider negotiation done at this point. + msg = harness.processCloseMsg(fundeeClosing, false, true, true, nil) + + funderClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + require.Equal(t, expectedFee, funderClosing.FeeSatoshis) + + // Now the fundee should also consider negotiation done. + msg = harness.processCloseMsg(funderClosing, true, true, true, nil) + + fundeeClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + require.Equal(t, expectedFee, fundeeClosing.FeeSatoshis) +} + +// TestFeeRangeCloseTypeChangedLegacy verifies that negotiation fails if the +// type of negotiation is changed in the middle of negotiation from legacy to +// range-based. We only test the fundee logic. +func TestFeeRangeCloseTypeChangedLegacy(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // Give the fundee a ClosingSigned message without a FeeRange. + funderClosing.FeeRange = nil + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + // Give the funder the message so that we can obtain a message to give + // the fundee. + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + + // The funder is technically done, but this does not matter since we are + // not testing the funder. + msg = harness.processCloseMsg(fundeeClosing, false, true, true, nil) + + // We'll change the FeeSatoshis of funderClosing so that we can test the + // close type logic. + funderClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + require.NotNil(t, funderClosing.FeeRange) + funderClosing.FeeSatoshis = btcutil.Amount(501) + + // The fundee should fail with ErrCloseTypeChanged. + _ = harness.processCloseMsg( + funderClosing, true, false, false, ErrCloseTypeChanged, + ) +} + +// TestFeeRangeCloseTypeChangedRange verifies that negotiation fails if the type +// of negotiation is changed from range-based to legacy. We only test the fundee +// logic since it is not possible to test the funder as the funder would already +// be finished negotiating. +func TestFeeRangeCloseTypeChangedRange(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // Give the fundee the ClosingSigned message to start range-based + // negotiation. + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + + // Give the message to the funder so we can get another message to give + // to the fundee. + msg = harness.processCloseMsg(fundeeClosing, false, true, true, nil) + + // We'll change FeeSatoshis so we can trigger the close-type-changed + // logic. We'll also un-set FeeRange. + funderClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + funderClosing.FeeSatoshis = btcutil.Amount(501) + funderClosing.FeeRange = nil + + // The fundee should fail with ErrCloseTypeChanged + _ = harness.processCloseMsg( + funderClosing, true, false, false, ErrCloseTypeChanged, + ) +} + +// TestFeeRangeNoOverlapFunder tests that the funder will fail if they receive a +// ClosingSigned with no fee-range overlap. +func TestFeeRangeNoOverlapFunder(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // Hand off the message to the fundee. + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + // We'll change the FeeRange to trigger the no overlap error for the + // funder. + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + fundeeClosing.FeeRange = &lnwire.FeeRange{} + + _ = harness.processCloseMsg( + fundeeClosing, false, false, false, ErrNoRangeOverlap, + ) +} + +// TestFeeRangeNoOverlapFundee tests that the fundee will send a warning if it +// receives a FeeRange that has no overlap with its own yet-to-be-sent FeeRange. +func TestFeeRangeNoOverlapFundee(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // Modify funderClosing to have a FeeRange with no overlap. + funderClosing.FeeRange = &lnwire.FeeRange{} + + // Hand off the message to the fundee and assert that we get back a + // warning. + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + _, ok := msg.(*lnwire.Warning) + require.True(t, ok) +} + +// TestFeeNotInOverlap tests that ErrFeeNotInOverlap is returned when the fee +// that the fundee proposes is not in the range overlap. +func TestFeeNotInOverlap(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // Hand off the message to the fundee. + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + // We will change FeeSatoshis to zero so that it is out of the overlap. + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + fundeeClosing.FeeSatoshis = btcutil.Amount(0) + + // The funder should error with ErrFeeNotInOverlap. + _ = harness.processCloseMsg( + fundeeClosing, false, false, false, ErrFeeNotInOverlap, + ) +} + +// TestErrFeeRangeViolation tests that ErrFeeRangeViolation is returned if the +// funder does not follow the spec by agreeing with the fundee's FeeSatoshis. +func TestErrFeeRangeViolation(t *testing.T) { + t.Parallel() + + funderFeeRate := chainfee.SatPerKWeight(500) + fundeeFeeRate := chainfee.SatPerKWeight(800) + + harness, funderClosing := newFeeRangeHarness( + t, funderFeeRate, fundeeFeeRate, chainfee.SatPerKWeight(0), + ) + + // Hand off the message to the fundee. + msg := harness.processCloseMsg(funderClosing, true, true, false, nil) + + fundeeClosing, ok := msg.(*lnwire.ClosingSigned) + require.True(t, ok) + + msg = harness.processCloseMsg(fundeeClosing, false, true, true, nil) + + // We'll modify FeeSatoshis to not match what the fundee sent. This will + // trigger ErrFeeRangeViolation. + funderClosing, ok = msg.(*lnwire.ClosingSigned) + require.True(t, ok) + funderClosing.FeeSatoshis = btcutil.Amount(501) + + _ = harness.processCloseMsg( + funderClosing, true, false, false, ErrFeeRangeViolation, + ) +} diff --git a/lnwallet/channel.go b/lnwallet/channel.go index acdfdea057..b6a91f3d16 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -5654,7 +5654,7 @@ func (lc *LightningChannel) ShortChanID() lnwire.ShortChannelID { // LocalUpfrontShutdownScript returns the local upfront shutdown script for the // channel. If it was not set, an empty byte array is returned. func (lc *LightningChannel) LocalUpfrontShutdownScript() lnwire.DeliveryAddress { - return lc.channelState.LocalShutdownScript + return lc.channelState.GetLocalShutdownScript() } // RemoteUpfrontShutdownScript returns the remote upfront shutdown script for the diff --git a/lnwire/closing_signed.go b/lnwire/closing_signed.go index 8e11c86993..74cba4fe27 100644 --- a/lnwire/closing_signed.go +++ b/lnwire/closing_signed.go @@ -2,11 +2,116 @@ package lnwire import ( "bytes" + "fmt" "io" "github.com/btcsuite/btcd/btcutil" + "github.com/lightningnetwork/lnd/tlv" ) +const ( + // FeeRangeType is the type used to store the optional fee range field + // in the ClosingSigned message. + FeeRangeType tlv.Type = 1 + + // FeeRangeRecordSize is the amount of bytes the fee range record size + // occupies (two uint64s). + FeeRangeRecordSize uint64 = 16 +) + +// FeeRange is a TLV in the ClosingSigned message that allows the sender to +// specify the minimum and maximum fee it will accept for a coop closing +// transaction. This version of the coop closing flow will usually complete in +// 2 or 3 rounds, but may take more if each side's fee range doesn't overlap. +type FeeRange struct { + // MinFeeSats is the minimum fee that the sender will accept. + MinFeeSats btcutil.Amount + + // MaxFeeSats is the maximum fee that the sender will accept. + MaxFeeSats btcutil.Amount +} + +// InRange returns whether a fee is in the fee range. +func (f *FeeRange) InRange(fee btcutil.Amount) bool { + return f.MinFeeSats <= fee && fee <= f.MaxFeeSats +} + +// GetOverlap takes two FeeRanges and returns the overlapping FeeRange between +// the two. If there is no overlap, nil is returned. +func (f *FeeRange) GetOverlap(other *FeeRange) *FeeRange { + var ( + minOfUpper btcutil.Amount + maxOfLower btcutil.Amount + ) + + // Determine the maximum of the lower bounds. + if f.MinFeeSats >= other.MinFeeSats { + maxOfLower = f.MinFeeSats + } else { + maxOfLower = other.MinFeeSats + } + + // Determine the minimum of the upper bounds. + if f.MaxFeeSats <= other.MaxFeeSats { + minOfUpper = f.MaxFeeSats + } else { + minOfUpper = other.MaxFeeSats + } + + // If the maximum of the lower bounds is greater than the minimum of the + // upper bounds, then there is no overlap. + if maxOfLower > minOfUpper { + return nil + } + + // There is an overlap, so return the range. + return &FeeRange{ + MinFeeSats: maxOfLower, + MaxFeeSats: minOfUpper, + } +} + +// NewRecord returns a TLV record that can be used to optionally encode a fee +// range that allows the fee negotiation process to use less rounds. +func (f *FeeRange) Record() tlv.Record { + return tlv.MakeStaticRecord( + FeeRangeType, f, FeeRangeRecordSize, eFeeRange, dFeeRange, + ) +} + +// eFeeRange is used to encode the fee range struct as a TLV record. +func eFeeRange(w io.Writer, val interface{}, buf *[8]byte) error { + if feeRange, ok := val.(*FeeRange); ok { + err := tlv.EUint64T(w, uint64(feeRange.MinFeeSats), buf) + if err != nil { + return err + } + + return tlv.EUint64T(w, uint64(feeRange.MaxFeeSats), buf) + } + + return tlv.NewTypeForEncodingErr(val, "FeeRange") +} + +// dFeeRange is used to decode the fee range TLV record into a FeeRange struct. +func dFeeRange(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { + if feeRange, ok := val.(*FeeRange); ok && l == FeeRangeRecordSize { + var min, max uint64 + + if err := tlv.DUint64(r, &min, buf, 8); err != nil { + return err + } + if err := tlv.DUint64(r, &max, buf, 8); err != nil { + return err + } + + feeRange.MinFeeSats = btcutil.Amount(min) + feeRange.MaxFeeSats = btcutil.Amount(max) + return nil + } + return tlv.NewTypeForDecodingErr(val, "FeeRange", l, FeeRangeRecordSize) +} + // ClosingSigned is sent by both parties to a channel once the channel is clear // of HTLCs, and is primarily concerned with negotiating fees for the close // transaction. Each party provides a signature for a transaction with a fee @@ -29,6 +134,10 @@ type ClosingSigned struct { // Signature is for the proposed channel close transaction. Signature Sig + // FeeRange is an optional range that the sender can set to compress the + // number of rounds in the coop close process. + FeeRange *FeeRange + // ExtraData is the set of data that was appended to this message to // fill out the full maximum transport message size. These fields can // be used to specify optional data such as custom TLV fields. @@ -36,11 +145,12 @@ type ClosingSigned struct { } // NewClosingSigned creates a new empty ClosingSigned message. -func NewClosingSigned(cid ChannelID, fs btcutil.Amount, - sig Sig) *ClosingSigned { +func NewClosingSigned(cid ChannelID, fs btcutil.Amount, sig Sig, + fr *FeeRange) *ClosingSigned { return &ClosingSigned{ ChannelID: cid, + FeeRange: fr, FeeSatoshis: fs, Signature: sig, } @@ -55,9 +165,40 @@ var _ Message = (*ClosingSigned)(nil) // // This is part of the lnwire.Message interface. func (c *ClosingSigned) Decode(r io.Reader, pver uint32) error { - return ReadElements( + err := ReadElements( r, &c.ChannelID, &c.FeeSatoshis, &c.Signature, &c.ExtraData, ) + if err != nil { + return err + } + + // Attempt to parse out the FeeRange record. + var fr FeeRange + typeMap, err := c.ExtraData.ExtractRecords(&fr) + if err != nil { + return err + } + + // Only set the FeeRange record if the TLV type was included in the + // stream. + if val, ok := typeMap[FeeRangeType]; ok && val == nil { + // Check that the fee range is sane before setting it. + if fr.MinFeeSats > fr.MaxFeeSats { + return fmt.Errorf("MinFeeSats greater than " + + "MaxFeeSats") + } + + // Check that FeeSatoshis is in the FeeRange. + if c.FeeSatoshis < fr.MinFeeSats || + c.FeeSatoshis > fr.MaxFeeSats { + + return fmt.Errorf("FeeSatoshis is not in FeeRange") + } + + c.FeeRange = &fr + } + + return nil } // Encode serializes the target ClosingSigned into the passed io.Writer @@ -77,6 +218,15 @@ func (c *ClosingSigned) Encode(w *bytes.Buffer, pver uint32) error { return err } + // We'll only encode the FeeRange in a TLV segment if it exists. + if c.FeeRange != nil { + recordProducers := []tlv.RecordProducer{c.FeeRange} + err := EncodeMessageExtraData(&c.ExtraData, recordProducers...) + if err != nil { + return err + } + } + return WriteBytes(w, c.ExtraData) } diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index 44d6cfb9b3..71ed24a30b 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -601,6 +601,14 @@ func TestLightningWireProtocol(t *testing.T) { return } + // With a 50% chance, add the FeeRange TLV record. + if r.Intn(2) == 0 { + req.FeeRange = &FeeRange{ + MinFeeSats: btcutil.Amount(uint64(r.Int63())), + MaxFeeSats: btcutil.Amount(uint64(r.Int63())), + } + } + v[0] = reflect.ValueOf(req) }, MsgCommitSig: func(v []reflect.Value, r *rand.Rand) { diff --git a/peer/brontide.go b/peer/brontide.go index 018f81245d..4e727dd31b 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -442,6 +442,10 @@ type Brontide struct { // a particular channel are sent over. localCloseChanReqs chan *htlcswitch.ChanClose + // coopCloseReady is a channel that is used by a ChannelLink to notify + // the peer.Brontide that the ClosingSigned phase can begin. + coopCloseReady chan *wire.OutPoint + // linkFailures receives all reported channel failures from the switch, // and instructs the channelManager to clean remaining channel state. linkFailures chan linkFailureReport @@ -488,6 +492,7 @@ func NewBrontide(cfg Config) *Brontide { activeMsgStreams: make(map[lnwire.ChannelID]*msgStream), activeChanCloses: make(map[lnwire.ChannelID]*chancloser.ChanCloser), localCloseChanReqs: make(chan *htlcswitch.ChanClose), + coopCloseReady: make(chan *wire.OutPoint), linkFailures: make(chan linkFailureReport), chanCloseMsgs: make(chan *closeMsg), resentChanSyncMsg: make(map[lnwire.ChannelID]struct{}), @@ -941,6 +946,52 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint, towerClient = p.cfg.TowerClient } + deliveryAddr, err := p.genDeliveryScript() + if err != nil { + return err + } + + chanID := lnwire.NewChanIDFromOutPoint(chanPoint) + + // If we need to retransmit Shutdown, create a ChanCloser and give it + // the Shutdown message. + retransmitShutdown := lnChan.State().HasSentShutdown() + if retransmitShutdown { + feePerKw, err := p.cfg.FeeEstimator.EstimateFeePerKW( + p.cfg.CoopCloseTargetConfs, + ) + if err != nil { + return err + } + + // Since we've sent Shutdown, the delivery script is already + // persisted. We won't know if it's locally initiated or not. + // At this stage, this is fine so we set it to true. + chanCloser, err := p.createChanCloser( + lnChan, lnChan.LocalUpfrontShutdownScript(), feePerKw, + nil, true, false, + ) + if err != nil { + peerLog.Errorf("unable to create chan closer: %v", err) + return err + } + + p.activeChanCloses[chanID] = chanCloser + + shutdownMsg := lnwire.NewShutdown( + chanID, lnChan.LocalUpfrontShutdownScript(), + ) + + // Give the Shutdown message to the ChanCloser without sending + // it. The link will send it during retransmission. + _, _, err = chanCloser.ProcessCloseMsg(shutdownMsg, false) + if err != nil { + peerLog.Errorf("unable to process shutdown: %v", err) + delete(p.activeChanCloses, chanID) + return err + } + } + linkCfg := htlcswitch.ChannelLinkConfig{ Peer: p, DecodeHopIterators: p.cfg.Sphinx.DecodeHopIterators, @@ -978,13 +1029,16 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint, NotifyInactiveChannel: p.cfg.ChannelNotifier.NotifyInactiveChannelEvent, HtlcNotifier: p.cfg.HtlcNotifier, GetAliases: p.cfg.GetAliases, + DeliveryAddr: deliveryAddr, + NotifySendingShutdown: p.HandleLocalCloseChanReqs, + NotifyCoopReady: p.HandleCoopReady, + RetransmitShutdown: retransmitShutdown, } // Before adding our new link, purge the switch of any pending or live // links going by the same channel id. If one is found, we'll shut it // down to ensure that the mailboxes are only ever under the control of // one link. - chanID := lnwire.NewChanIDFromOutPoint(chanPoint) p.cfg.Switch.RemoveLink(chanID) // With the channel link created, we'll now notify the htlc switch so @@ -1530,6 +1584,18 @@ out: case <-p.quit: break out } + + // Send the Shutdown to the link. If ProcessCloseMsg + // later fails validating the Shutdown and this message + // is sent to the link, the coop close process won't + // happen but the link will be unaware. This is fine as + // the link should eventually send a Shutdown and stop + // when the channel has no htlc's or updates. It will + // notify us that coop close is ready, and gracefully + // exit. + targetChan = msg.ChannelID + isLinkUpdate = p.isActiveChannel(msg.ChannelID) + case *lnwire.ClosingSigned: select { case p.chanCloseMsgs <- &closeMsg{msg.ChannelID, msg}: @@ -2448,6 +2514,11 @@ out: case req := <-p.localCloseChanReqs: p.handleLocalCloseReq(req) + // We've just been notified that the channel identified by + // chanPoint is ready to begin the next phase of coop close. + case chanPoint := <-p.coopCloseReady: + p.beginClosingSigned(chanPoint) + // We've received a link failure from a link that was added to // the switch. This will initiate the teardown of the link, and // initiate any on-chain closures if necessary. @@ -2576,12 +2647,6 @@ func (p *Brontide) fetchActiveChanCloser(chanID lnwire.ChannelID) ( return nil, ErrChannelNotFound } - // Optimistically try a link shutdown, erroring out if it failed. - if err := p.tryLinkShutdown(chanID); err != nil { - p.log.Errorf("failed link shutdown: %v", err) - return nil, err - } - // We'll create a valid closing state machine in order to respond to // the initiated cooperative channel closure. First, we set the // delivery script that our funds will be paid out to. If an upfront @@ -2612,7 +2677,7 @@ func (p *Brontide) fetchActiveChanCloser(chanID lnwire.ChannelID) ( } chanCloser, err = p.createChanCloser( - channel, deliveryScript, feePerKw, nil, false, + channel, deliveryScript, feePerKw, nil, false, false, ) if err != nil { p.log.Errorf("unable to create chan closer: %v", err) @@ -2624,37 +2689,6 @@ func (p *Brontide) fetchActiveChanCloser(chanID lnwire.ChannelID) ( return chanCloser, nil } -// chooseDeliveryScript takes two optionally set shutdown scripts and returns -// a suitable script to close out to. This may be nil if neither script is -// set. If both scripts are set, this function will error if they do not match. -func chooseDeliveryScript(upfront, - requested lnwire.DeliveryAddress) (lnwire.DeliveryAddress, error) { - - // If no upfront shutdown script was provided, return the user - // requested address (which may be nil). - if len(upfront) == 0 { - return requested, nil - } - - // If an upfront shutdown script was provided, and the user did not request - // a custom shutdown script, return the upfront address. - if len(requested) == 0 { - return upfront, nil - } - - // If both an upfront shutdown script and a custom close script were - // provided, error if the user provided shutdown script does not match - // the upfront shutdown script (because closing out to a different script - // would violate upfront shutdown). - if !bytes.Equal(upfront, requested) { - return nil, chancloser.ErrUpfrontShutdownScriptMismatch - } - - // The user requested script matches the upfront shutdown script, so we - // can return it without error. - return upfront, nil -} - // restartCoopClose checks whether we need to restart the cooperative close // process for a given channel. func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( @@ -2664,9 +2698,8 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( // have a closing transaction, then the cooperative close process was // started but never finished. We'll re-create the chanCloser state // machine and resend Shutdown. BOLT#2 requires that we retransmit - // Shutdown exactly, but doing so would mean persisting the RPC - // provided close script. Instead use the LocalUpfrontShutdownScript - // or generate a script. + // Shutdown exactly, but for 0.15.0 nodes, we don't persist the RPC + // provided close script. 0.16.0 nodes do persist the closing script. c := lnChan.State() _, err := c.BroadcastedCooperative() if err != nil && err != channeldb.ErrNoCloseTx { @@ -2678,8 +2711,9 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( return nil, nil } - // As mentioned above, we don't re-create the delivery script. - deliveryScript := c.LocalShutdownScript + // As mentioned above, we may not re-create the delivery script for + // older nodes. + deliveryScript := c.GetLocalShutdownScript() if len(deliveryScript) == 0 { var err error deliveryScript, err = p.genDeliveryScript() @@ -2705,8 +2739,10 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( channeldb.ChanStatusLocalCloseInitiator, ) + // We set the CleanOnReceive flag so the ChanCloser immediately calls + // ChannelClean on receipt of the peer's Shutdown message. chanCloser, err := p.createChanCloser( - lnChan, deliveryScript, feePerKw, nil, locallyInitiated, + lnChan, deliveryScript, feePerKw, nil, locallyInitiated, true, ) if err != nil { p.log.Errorf("unable to create chan closer: %v", err) @@ -2719,10 +2755,12 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( chanID := lnwire.NewChanIDFromOutPoint(&c.FundingOutpoint) p.activeChanCloses[chanID] = chanCloser - // Create the Shutdown message. - shutdownMsg, err := chanCloser.ShutdownChan() + // Recreate the Shutdown message and give it to the ChanCloser. + shutdownMsg := lnwire.NewShutdown(chanID, deliveryScript) + + _, _, err = chanCloser.ProcessCloseMsg(shutdownMsg, false) if err != nil { - p.log.Errorf("unable to create shutdown message: %v", err) + peerLog.Errorf("unable to process shutdown message: %v", err) delete(p.activeChanCloses, chanID) return nil, err } @@ -2735,7 +2773,7 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( func (p *Brontide) createChanCloser(channel *lnwallet.LightningChannel, deliveryScript lnwire.DeliveryAddress, fee chainfee.SatPerKWeight, req *htlcswitch.ChanClose, - locallyInitiated bool) (*chancloser.ChanCloser, error) { + locallyInitiated, cleanOnRecv bool) (*chancloser.ChanCloser, error) { _, startingHeight, err := p.cfg.ChainIO.GetBestBlock() if err != nil { @@ -2771,6 +2809,7 @@ func (p *Brontide) createChanCloser(channel *lnwallet.LightningChannel, uint32(startingHeight), req, locallyInitiated, + cleanOnRecv, ) return chanCloser, nil @@ -2800,70 +2839,59 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) { // out this channel on-chain, so we execute the cooperative channel // closure workflow. case contractcourt.CloseRegular: - // First, we'll choose a delivery address that we'll use to send the - // funds to in the case of a successful negotiation. - - // An upfront shutdown and user provided script are both optional, - // but must be equal if both set (because we cannot serve a request - // to close out to a script which violates upfront shutdown). Get the - // appropriate address to close out to (which may be nil if neither - // are set) and error if they are both set and do not match. - deliveryScript, err := chooseDeliveryScript( - channel.LocalUpfrontShutdownScript(), req.DeliveryScript, - ) - if err != nil { - p.log.Errorf("cannot close channel %v: %v", req.ChanPoint, err) - req.Err <- err - return - } + var err error - // If neither an upfront address or a user set address was - // provided, generate a fresh script. - if len(deliveryScript) == 0 { - deliveryScript, err = p.genDeliveryScript() + chanCloser, ok := p.activeChanCloses[chanID] + if ok { + // If the ChanCloser exists, replace its local delivery + // script with the one received in the request. This is + // necessary as if: + // - the peer sends Shutdown first, peer.Brontide + // generates the script + // - the Shutdown that the link sends will use a + // different script. + chanCloser.SetLocalScript(req.DeliveryScript) + } else { + // If the ChanCloser doesn't exist in activeChanCloses, + // we'll create a fresh one. + chanCloser, err = p.createChanCloser( + channel, req.DeliveryScript, + req.TargetFeePerKw, req, true, false, + ) if err != nil { p.log.Errorf(err.Error()) req.Err <- err return } - } - // Optimistically try a link shutdown, erroring out if it - // failed. - if err := p.tryLinkShutdown(chanID); err != nil { - p.log.Errorf("failed link shutdown: %v", err) - req.Err <- err - return + p.activeChanCloses[chanID] = chanCloser } - chanCloser, err := p.createChanCloser( - channel, deliveryScript, req.TargetFeePerKw, req, true, - ) + // Recreate the Shutdown message from the DeliveryScript and + // the ChannelID. We'll only feed this to the ChanCloser. + shutdownMsg := lnwire.NewShutdown(chanID, req.DeliveryScript) + + // Next, we'll give the Shutdown message to the ChanCloser + // state machine. There won't be any messages to process since + // ClosingSigned will only begin once the channel clean + // callback is called by the link. + _, _, err = chanCloser.ProcessCloseMsg(shutdownMsg, false) if err != nil { - p.log.Errorf(err.Error()) - req.Err <- err - return - } + err = fmt.Errorf("unable to process close msg: %v", + err) + peerLog.Error(err) - p.activeChanCloses[chanID] = chanCloser + // As the negotiations failed, we'll reset the channel + // state to ensure we act to on-chain events as normal. + chanCloser.Channel().ResetState() - // Finally, we'll initiate the channel shutdown within the - // chanCloser, and send the shutdown message to the remote - // party to kick things off. - shutdownMsg, err := chanCloser.ShutdownChan() - if err != nil { - p.log.Errorf(err.Error()) - req.Err <- err + if chanCloser.CloseRequest() != nil { + chanCloser.CloseRequest().Err <- err + } delete(p.activeChanCloses, chanID) - - // As we were unable to shutdown the channel, we'll - // return it back to its normal state. - channel.ResetState() return } - p.queueMsg(shutdownMsg, nil) - // A type of CloseBreach indicates that the counterparty has breached // the channel therefore we need to clean up our local state. case contractcourt.CloseBreach: @@ -2874,6 +2902,60 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) { } } +// beginClosingSigned begins the next phase of coop close for the channel +// identified by chanPoint. This will stop the link, retrieve the associated +// ChanCloser if it exists, and call ChannelClean, which will advance the +// underlying coop close state. +func (p *Brontide) beginClosingSigned(chanPoint *wire.OutPoint) { + // First, stop the link. It can't process updates at this point, but + // this completely stops it and removes it from the Switch's internal + // maps. + chanID := lnwire.NewChanIDFromOutPoint(chanPoint) + p.cfg.Switch.RemoveLink(chanID) + + // Next, we'll retrieve the ChanCloser for this particular outpoint. If + // it doesn't exist, we'll return early. This can happen if an error + // occurred while processing the peer's Shutdown, but it was still sent + // to the link. + chanCloser, ok := p.activeChanCloses[chanID] + if !ok { + peerLog.Errorf("chan closer for ChanID(%v) does not exist, "+ + "stopping coop close", chanID) + return + } + + // Now we'll let the ChanCloser know that the ClosingSigned phase can + // start. If an error is returned here, we'll return. One way this can + // happen is if an error occurred while processing the peer's Shutdown, + // but it was still sent to the link. + msgs, closeFin, err := chanCloser.ChannelClean() + if err != nil { + peerLog.Errorf("channel clean call failed for ChanID(%v), "+ + "stopping coop close: %v", chanID, err) + + if chanCloser.CloseRequest() != nil { + chanCloser.CloseRequest().Err <- err + } + delete(p.activeChanCloses, chanID) + return + } + + // We'll send out the single ClosingSigned message. We're either + // sending the first ClosingSigned or responding to our peer's + // ClosingSigned if they've sent it already. + for _, msg := range msgs { + p.queueMsg(msg, nil) + } + + // Return if the closing negotiation is still ongoing. + if !closeFin { + return + } + + // Otherwise, we've agreed on a closing fee after two signatures. + p.finalizeChanClosure(chanCloser) +} + // linkFailureReport is sent to the channelManager whenever a link reports a // link failure, and is forced to exit. The report houses the necessary // information to clean up the channel state, send back the error message, and @@ -2957,35 +3039,6 @@ 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( @@ -3412,9 +3465,7 @@ func (p *Brontide) handleCloseMsg(msg *closeMsg) { // Next, we'll process the next message using the target state machine. // We'll either continue negotiation, or halt. - msgs, closeFin, err := chanCloser.ProcessCloseMsg( - msg.msg, - ) + msgs, closeFin, err := chanCloser.ProcessCloseMsg(msg.msg, true) if err != nil { err := fmt.Errorf("unable to process close msg: %v", err) p.log.Error(err) @@ -3450,15 +3501,43 @@ func (p *Brontide) handleCloseMsg(msg *closeMsg) { // HandleLocalCloseChanReqs accepts a *htlcswitch.ChanClose and passes it onto // the channelManager goroutine, which will shut down the link and possibly -// close the channel. -func (p *Brontide) HandleLocalCloseChanReqs(req *htlcswitch.ChanClose) { +// close the channel. It takes an additional quit chan as a precaution to avoid +// potential deadlocks. +func (p *Brontide) HandleLocalCloseChanReqs(req *htlcswitch.ChanClose, + quit chan struct{}) { + select { case p.localCloseChanReqs <- req: p.log.Info("Local close channel request delivered to " + "peer") case <-p.quit: - p.log.Info("Unable to deliver local close channel request " + - "to peer") + peerLog.Infof("Unable to deliver local close channel request "+ + "to peer %x", p.PubKey()) + + case <-quit: + peerLog.Infof("Unable to process local close channel request "+ + "to peer %x", p.PubKey()) + } +} + +// HandleCoopReady is called by a ChannelLink to let this subsystem know that +// the channel is ready to be cooperatively closed. We'll also stop the link. +// It takes an additional quit chan as a precaution to avoid potential +// deadlocks. +func (p *Brontide) HandleCoopReady(chanPoint *wire.OutPoint, + quit chan struct{}) { + + select { + case p.coopCloseReady <- chanPoint: + peerLog.Infof("channel %v is ready to begin next phase of "+ + "coop close", chanPoint) + case <-p.quit: + peerLog.Infof("unable to begin next phase of coop close for "+ + "channel %v", chanPoint) + + case <-quit: + peerLog.Infof("unable to begin signing phase of coop close "+ + "for channel %v", chanPoint) } } diff --git a/peer/brontide_test.go b/peer/brontide_test.go index 44e63267b0..b379f97c0f 100644 --- a/peer/brontide_test.go +++ b/peer/brontide_test.go @@ -15,16 +15,12 @@ import ( "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/lntest/mock" - "github.com/lightningnetwork/lnd/lnwallet/chancloser" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/pool" "github.com/stretchr/testify/require" ) var ( - // p2SHAddress is a valid pay to script hash address. - p2SHAddress = "2NBFNJTktNa7GZusGbDbGKRZTxdK9VVez3n" - // p2wshAddress is a valid pay to witness script hash address. p2wshAddress = "bc1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3qccfmv3" ) @@ -56,31 +52,34 @@ func TestPeerChannelClosureAcceptFeeResponder(t *testing.T) { dummyDeliveryScript := genScript(t, p2wshAddress) // 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. + // node in this shutdown procedure. alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: lnwire.NewShutdown(chanID, dummyDeliveryScript), } - var msg lnwire.Message - select { - case outMsg := <-alicePeer.outgoingQueue: - msg = outMsg.msg - case <-time.After(timeout): - t.Fatalf("did not receive shutdown message") - } - - shutdownMsg, ok := msg.(*lnwire.Shutdown) - if !ok { - t.Fatalf("expected Shutdown message, got %T", msg) + // Alice calls HandleLocalCloseChanReqs to notify us that the link has + // sent Shutdown. + aliceDelivery := genScript(t, p2wshAddress) + aliceClose := &htlcswitch.ChanClose{ + CloseType: contractcourt.CloseRegular, + ChanPoint: bobChan.ChannelPoint(), + TargetFeePerKw: 300, + DeliveryScript: aliceDelivery, + Updates: make(chan interface{}, 2), + Err: make(chan error, 1), } + aliceQuit := make(chan struct{}) + alicePeer.HandleLocalCloseChanReqs(aliceClose, aliceQuit) - respDeliveryScript := shutdownMsg.Address + // Alice calls HandleCoopReady which marks the coop close flow as + // ready. + alicePeer.HandleCoopReady(bobChan.ChannelPoint(), aliceQuit) // Alice will then send a ClosingSigned message, indicating her proposed // closing transaction fee. Alice sends the ClosingSigned message as she is // the initiator of the channel. + var msg lnwire.Message select { case outMsg := <-alicePeer.outgoingQueue: msg = outMsg.msg @@ -97,13 +96,15 @@ func TestPeerChannelClosureAcceptFeeResponder(t *testing.T) { // so she knows we agreed. aliceFee := respClosingSigned.FeeSatoshis bobSig, _, _, err := bobChan.CreateCloseProposal( - aliceFee, dummyDeliveryScript, respDeliveryScript, + aliceFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err := lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "error parsing signature") - closingSigned := lnwire.NewClosingSigned(chanID, aliceFee, parsedSig) + closingSigned := lnwire.NewClosingSigned( + chanID, aliceFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -158,33 +159,19 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { dummyDeliveryScript := genScript(t, p2wshAddress) // We make Alice send a shutdown request. + aliceDelivery := genScript(t, p2wshAddress) updateChan := make(chan interface{}, 1) errChan := make(chan error, 1) closeCommand := &htlcswitch.ChanClose{ CloseType: contractcourt.CloseRegular, ChanPoint: bobChan.ChannelPoint(), Updates: updateChan, + DeliveryScript: aliceDelivery, TargetFeePerKw: 12500, Err: errChan, } alicePeer.localCloseChanReqs <- closeCommand - // We can now pull a Shutdown message off of Alice's outgoingQueue. - var msg lnwire.Message - select { - case outMsg := <-alicePeer.outgoingQueue: - msg = outMsg.msg - case <-time.After(timeout): - t.Fatalf("did not receive shutdown request") - } - - shutdownMsg, ok := msg.(*lnwire.Shutdown) - if !ok { - t.Fatalf("expected Shutdown message, got %T", msg) - } - - aliceDeliveryScript := shutdownMsg.Address - // Bob will respond with his own Shutdown message. alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, @@ -192,7 +179,12 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { dummyDeliveryScript), } + // Alice calls HandleCoopReady and will send ClosingSigned. + aliceQuit := make(chan struct{}) + alicePeer.HandleCoopReady(bobChan.ChannelPoint(), aliceQuit) + // Alice will reply with a ClosingSigned here. + var msg lnwire.Message select { case outMsg := <-alicePeer.outgoingQueue: msg = outMsg.msg @@ -208,14 +200,15 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { // message. bobFee := closingSignedMsg.FeeSatoshis bobSig, _, _, err := bobChan.CreateCloseProposal( - bobFee, dummyDeliveryScript, aliceDeliveryScript, + bobFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "unable to create close proposal") parsedSig, err := lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "unable to parse signature") - closingSigned := lnwire.NewClosingSigned(shutdownMsg.ChannelID, - bobFee, parsedSig) + closingSigned := lnwire.NewClosingSigned( + chanID, bobFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -288,23 +281,24 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { dummyDeliveryScript), } - var msg lnwire.Message - select { - case outMsg := <-alicePeer.outgoingQueue: - msg = outMsg.msg - case <-time.After(timeout): - t.Fatalf("did not receive shutdown message") - } - - shutdownMsg, ok := msg.(*lnwire.Shutdown) - if !ok { - t.Fatalf("expected Shutdown message, got %T", msg) + // Alice calls HandleLocalCloseChanReqs to notify that she sent + // Shutdown. + aliceDelivery := genScript(t, p2wshAddress) + aliceClose := &htlcswitch.ChanClose{ + CloseType: contractcourt.CloseRegular, + ChanPoint: bobChan.ChannelPoint(), + TargetFeePerKw: 300, + DeliveryScript: aliceDelivery, + Updates: make(chan interface{}, 2), + Err: make(chan error, 1), } + aliceQuit := make(chan struct{}) + alicePeer.HandleLocalCloseChanReqs(aliceClose, aliceQuit) - aliceDeliveryScript := shutdownMsg.Address + // Alice will now call HandleCoopReady and send ClosingSigned. + alicePeer.HandleCoopReady(bobChan.ChannelPoint(), aliceQuit) - // As Alice is the channel initiator, she will send her ClosingSigned - // message. + var msg lnwire.Message select { case outMsg := <-alicePeer.outgoingQueue: msg = outMsg.msg @@ -321,13 +315,15 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { preferredRespFee := aliceClosingSigned.FeeSatoshis increasedFee := btcutil.Amount(float64(preferredRespFee) * 2.5) bobSig, _, _, err := bobChan.CreateCloseProposal( - increasedFee, dummyDeliveryScript, aliceDeliveryScript, + increasedFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err := lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "error parsing signature") - closingSigned := lnwire.NewClosingSigned(chanID, increasedFee, parsedSig) + closingSigned := lnwire.NewClosingSigned( + chanID, increasedFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -361,13 +357,15 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { // We try negotiating a 2.1x fee, which should also be rejected. increasedFee = btcutil.Amount(float64(preferredRespFee) * 2.1) bobSig, _, _, err = bobChan.CreateCloseProposal( - increasedFee, dummyDeliveryScript, aliceDeliveryScript, + increasedFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err = lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "error parsing signature") - closingSigned = lnwire.NewClosingSigned(chanID, increasedFee, parsedSig) + closingSigned = lnwire.NewClosingSigned( + chanID, increasedFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -403,13 +401,15 @@ func TestPeerChannelClosureFeeNegotiationsResponder(t *testing.T) { // Finally, Bob will accept the fee by echoing back the same fee that Alice // just sent over. bobSig, _, _, err = bobChan.CreateCloseProposal( - aliceFee, dummyDeliveryScript, aliceDeliveryScript, + aliceFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err = lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "error parsing signature") - closingSigned = lnwire.NewClosingSigned(chanID, aliceFee, parsedSig) + closingSigned = lnwire.NewClosingSigned( + chanID, aliceFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -465,32 +465,18 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { // We make the initiator send a shutdown request. updateChan := make(chan interface{}, 1) errChan := make(chan error, 1) + aliceDelivery := genScript(t, p2wshAddress) closeCommand := &htlcswitch.ChanClose{ CloseType: contractcourt.CloseRegular, ChanPoint: bobChan.ChannelPoint(), Updates: updateChan, + DeliveryScript: aliceDelivery, TargetFeePerKw: 12500, Err: errChan, } alicePeer.localCloseChanReqs <- closeCommand - // Alice should now send a Shutdown request to Bob. - var msg lnwire.Message - select { - case outMsg := <-alicePeer.outgoingQueue: - msg = outMsg.msg - case <-time.After(timeout): - t.Fatalf("did not receive shutdown request") - } - - shutdownMsg, ok := msg.(*lnwire.Shutdown) - if !ok { - t.Fatalf("expected Shutdown message, got %T", msg) - } - - aliceDeliveryScript := shutdownMsg.Address - // Bob will answer the Shutdown message with his own Shutdown. dummyDeliveryScript := genScript(t, p2wshAddress) respShutdown := lnwire.NewShutdown(chanID, dummyDeliveryScript) @@ -499,8 +485,13 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { msg: respShutdown, } + // Alice will now mark the channel clean and send ClosingSigned. + aliceQuit := make(chan struct{}) + alicePeer.HandleCoopReady(bobChan.ChannelPoint(), aliceQuit) + // Alice should now respond with a ClosingSigned message with her ideal // fee rate. + var msg lnwire.Message select { case outMsg := <-alicePeer.outgoingQueue: msg = outMsg.msg @@ -519,14 +510,16 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { lastSentFee := increasedFee bobSig, _, _, err := bobChan.CreateCloseProposal( - increasedFee, dummyDeliveryScript, aliceDeliveryScript, + increasedFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err := lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "unable to parse signature") - closingSigned := lnwire.NewClosingSigned(chanID, increasedFee, parsedSig) + closingSigned := lnwire.NewClosingSigned( + chanID, increasedFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -562,14 +555,16 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { lastSentFee = increasedFee bobSig, _, _, err = bobChan.CreateCloseProposal( - increasedFee, dummyDeliveryScript, aliceDeliveryScript, + increasedFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err = lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "error parsing signature") - closingSigned = lnwire.NewClosingSigned(chanID, increasedFee, parsedSig) + closingSigned = lnwire.NewClosingSigned( + chanID, increasedFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -602,13 +597,15 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { // Bob will now accept their fee by sending back a ClosingSigned message // with an identical fee. bobSig, _, _, err = bobChan.CreateCloseProposal( - aliceFee, dummyDeliveryScript, aliceDeliveryScript, + aliceFee, dummyDeliveryScript, aliceDelivery, ) require.NoError(t, err, "error creating close proposal") parsedSig, err = lnwire.NewSigFromSignature(bobSig) require.NoError(t, err, "error parsing signature") - closingSigned = lnwire.NewClosingSigned(chanID, aliceFee, parsedSig) + closingSigned = lnwire.NewClosingSigned( + chanID, aliceFee, parsedSig, nil, + ) alicePeer.chanCloseMsgs <- &closeMsg{ cid: chanID, msg: closingSigned, @@ -636,220 +633,6 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { notifier.ConfChan <- &chainntnfs.TxConfirmation{} } -// TestChooseDeliveryScript tests that chooseDeliveryScript correctly errors -// when upfront and user set scripts that do not match are provided, allows -// matching values and returns appropriate values in the case where one or none -// are set. -func TestChooseDeliveryScript(t *testing.T) { - // generate non-zero scripts for testing. - script1 := genScript(t, p2SHAddress) - script2 := genScript(t, p2wshAddress) - - tests := []struct { - name string - userScript lnwire.DeliveryAddress - shutdownScript lnwire.DeliveryAddress - expectedScript lnwire.DeliveryAddress - expectedError error - }{ - { - name: "Neither set", - userScript: nil, - shutdownScript: nil, - expectedScript: nil, - expectedError: nil, - }, - { - name: "Both set and equal", - userScript: script1, - shutdownScript: script1, - expectedScript: script1, - expectedError: nil, - }, - { - name: "Both set and not equal", - userScript: script1, - shutdownScript: script2, - expectedScript: nil, - expectedError: chancloser.ErrUpfrontShutdownScriptMismatch, - }, - { - name: "Only upfront script", - userScript: nil, - shutdownScript: script1, - expectedScript: script1, - expectedError: nil, - }, - { - name: "Only user script", - userScript: script2, - shutdownScript: nil, - expectedScript: script2, - expectedError: nil, - }, - } - - for _, test := range tests { - test := test - - t.Run(test.name, func(t *testing.T) { - script, err := chooseDeliveryScript( - test.shutdownScript, test.userScript, - ) - if err != test.expectedError { - t.Fatalf("Expected: %v, got: %v", test.expectedError, err) - } - - if !bytes.Equal(script, test.expectedScript) { - t.Fatalf("Expected: %x, got: %x", test.expectedScript, script) - } - }) - } -} - -// TestCustomShutdownScript tests that the delivery script of a shutdown -// message can be set to a specified address. It checks that setting a close -// script fails for channels which have an upfront shutdown script already set. -func TestCustomShutdownScript(t *testing.T) { - script := genScript(t, p2SHAddress) - - // setShutdown is a function which sets the upfront shutdown address for - // the local channel. - setShutdown := func(a, b *channeldb.OpenChannel) { - a.LocalShutdownScript = script - b.RemoteShutdownScript = script - } - - tests := []struct { - name string - - // update is a function used to set values on the channel set up for the - // test. It is used to set values for upfront shutdown addresses. - update func(a, b *channeldb.OpenChannel) - - // userCloseScript is the address specified by the user. - userCloseScript lnwire.DeliveryAddress - - // expectedScript is the address we expect to be set on the shutdown - // message. - expectedScript lnwire.DeliveryAddress - - // expectedError is the error we expect, if any. - expectedError error - }{ - { - name: "User set script", - update: noUpdate, - userCloseScript: script, - expectedScript: script, - }, - { - name: "No user set script", - update: noUpdate, - }, - { - name: "Shutdown set, no user script", - update: setShutdown, - expectedScript: script, - }, - { - name: "Shutdown set, user script matches", - update: setShutdown, - userCloseScript: script, - expectedScript: script, - }, - { - name: "Shutdown set, user script different", - update: setShutdown, - userCloseScript: []byte("different addr"), - expectedError: chancloser.ErrUpfrontShutdownScriptMismatch, - }, - } - - for _, test := range tests { - test := test - - t.Run(test.name, func(t *testing.T) { - notifier := &mock.ChainNotifier{ - SpendChan: make(chan *chainntnfs.SpendDetail), - EpochChan: make(chan *chainntnfs.BlockEpoch), - ConfChan: make(chan *chainntnfs.TxConfirmation), - } - broadcastTxChan := make(chan *wire.MsgTx) - - mockSwitch := &mockMessageSwitch{} - - // Open a channel. - alicePeer, bobChan, err := createTestPeer( - t, notifier, broadcastTxChan, test.update, - mockSwitch, - ) - if err != nil { - t.Fatalf("unable to create test channels: %v", err) - } - - 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) - closeCommand := htlcswitch.ChanClose{ - CloseType: contractcourt.CloseRegular, - ChanPoint: chanPoint, - Updates: updateChan, - TargetFeePerKw: 12500, - DeliveryScript: test.userCloseScript, - Err: errChan, - } - - // Send the close command for the correct channel and check that a - // shutdown message is sent. - alicePeer.localCloseChanReqs <- &closeCommand - - var msg lnwire.Message - select { - case outMsg := <-alicePeer.outgoingQueue: - msg = outMsg.msg - case <-time.After(timeout): - t.Fatalf("did not receive shutdown message") - case err := <-errChan: - // Fail if we do not expect an error. - if err != test.expectedError { - t.Fatalf("error closing channel: %v", err) - } - - // Terminate the test early if have received an error, no - // further action is expected. - return - } - - // Check that we have received a shutdown message. - shutdownMsg, ok := msg.(*lnwire.Shutdown) - if !ok { - t.Fatalf("expected shutdown message, got %T", msg) - } - - // If the test has not specified an expected address, do not check - // whether the shutdown address matches. This covers the case where - // we expect shutdown to a random address and cannot match it. - if len(test.expectedScript) == 0 { - return - } - - // Check that the Shutdown message includes the expected delivery - // script. - if !bytes.Equal(test.expectedScript, shutdownMsg.Address) { - t.Fatalf("expected delivery script: %x, got: %x", - test.expectedScript, shutdownMsg.Address) - } - }) - } -} - // TestStaticRemoteDowngrade tests that we downgrade our static remote feature // bit to optional if we have legacy channels with a peer. This ensures that // we can stay connected to peers that don't support the feature bit that we diff --git a/peer/test_utils.go b/peer/test_utils.go index 41f6d2c0f6..bf12671886 100644 --- a/peer/test_utils.go +++ b/peer/test_utils.go @@ -467,8 +467,12 @@ 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 } +// NotifyShouldShutdown currently returns nil. +func (m *mockUpdateHandler) NotifyShouldShutdown( + req *htlcswitch.ChanClose) error { + + return nil +} type mockMessageConn struct { t *testing.T diff --git a/rpcserver.go b/rpcserver.go index b75295612b..74796d09e4 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2513,14 +2513,6 @@ func (r *rpcServer) CloseChannel(in *lnrpc.CloseChannelRequest, rpcsLog.Debugf("Target sat/kw for closing transaction: %v", int64(feeRate)) - // Before we attempt the cooperative channel closure, we'll - // examine the channel to ensure that it doesn't have a - // lingering HTLC. - if len(channel.ActiveHtlcs()) != 0 { - return fmt.Errorf("cannot co-op close channel " + - "with active htlcs") - } - // Otherwise, the caller has requested a regular interactive // cooperative channel closure. So we'll forward the request to // the htlc switch which will handle the negotiation and @@ -4210,9 +4202,10 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel, channel.PushAmountSat = uint64(amt) } - if len(dbChannel.LocalShutdownScript) > 0 { + if len(dbChannel.GetLocalShutdownScript()) > 0 { _, addresses, _, err := txscript.ExtractPkScriptAddrs( - dbChannel.LocalShutdownScript, r.cfg.ActiveNetParams.Params, + dbChannel.GetLocalShutdownScript(), + r.cfg.ActiveNetParams.Params, ) if err != nil { return nil, err diff --git a/server.go b/server.go index 4fc40e76a6..d40e26e3e8 100644 --- a/server.go +++ b/server.go @@ -628,24 +628,10 @@ func newServer(cfg *Config, listenAddrs []net.Addr, } s.htlcSwitch, err = htlcswitch.New(htlcswitch.Config{ - DB: dbs.ChanStateDB, - FetchAllOpenChannels: s.chanStateDB.FetchAllOpenChannels, - FetchAllChannels: s.chanStateDB.FetchAllChannels, - FetchClosedChannels: s.chanStateDB.FetchClosedChannels, - LocalChannelClose: func(pubKey []byte, - request *htlcswitch.ChanClose) { - - peer, err := s.FindPeerByPubStr(string(pubKey)) - if err != nil { - srvrLog.Errorf("unable to close channel, peer"+ - " with %v id can't be found: %v", - pubKey, err, - ) - return - } - - peer.HandleLocalCloseChanReqs(request) - }, + DB: dbs.ChanStateDB, + FetchAllOpenChannels: s.chanStateDB.FetchAllOpenChannels, + FetchAllChannels: s.chanStateDB.FetchAllChannels, + FetchClosedChannels: s.chanStateDB.FetchClosedChannels, FwdingLog: dbs.ChanStateDB.ForwardingLog(), SwitchPackager: channeldb.NewSwitchPackager(), ExtractErrorEncrypter: s.sphinx.ExtractErrorEncrypter,