From 540d3c0fc77223ec051277ba92f2c6d902788208 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 24 Sep 2024 15:35:55 +0900 Subject: [PATCH] multi: switch to lock time from sequence for coop close v2 --- lnwallet/chancloser/rbf_coop_test.go | 27 +-------------------- lnwallet/chancloser/rbf_coop_transitions.go | 15 ++++-------- lnwire/closing_complete.go | 8 +++--- lnwire/lnwire_test.go | 2 +- peer/brontide.go | 7 ++++++ 5 files changed, 18 insertions(+), 41 deletions(-) diff --git a/lnwallet/chancloser/rbf_coop_test.go b/lnwallet/chancloser/rbf_coop_test.go index 1bfd574ba..5cf985e8d 100644 --- a/lnwallet/chancloser/rbf_coop_test.go +++ b/lnwallet/chancloser/rbf_coop_test.go @@ -1430,31 +1430,6 @@ func TestRbfCloseClosingNegotiationRemote(t *testing.T) { closeHarness.assertNoStateTransitions() }) - // If the remote party sends us a signature with a final sequence, then - // we'll error out as it can't be RBF'd - t.Run("recv_offer_final_sequence", func(t *testing.T) { - closeHarness := newCloser(t, &harnessCfg{ - initialState: fn.Some[ProtocolState](startingState), - }) - defer closeHarness.stopAndAssert() - - // We should fail as they sent a non final sequence. - closeHarness.expectFailure(ErrNonFinalSequence) - - // We'll send an offer with something beyond the max RBF value, - // this should fail. - feeOffer := &OfferReceivedEvent{ - SigMsg: lnwire.ClosingComplete{ - FeeSatoshis: absoluteFee, - Sequence: mempool.MaxRBFSequence + 1, - }, - } - closeHarness.chanCloser.SendEvent(feeOffer) - - // We shouldn't have transitioned to a new state. - closeHarness.assertNoStateTransitions() - }) - // If our balance, is dust, then the remote party should send a // signature that doesn't include our output. t.Run("recv_offer_err_closer_no_closee", func(t *testing.T) { @@ -1559,7 +1534,7 @@ func TestRbfCloseClosingNegotiationRemote(t *testing.T) { feeOffer := &OfferReceivedEvent{ SigMsg: lnwire.ClosingComplete{ FeeSatoshis: absoluteFee, - Sequence: sequence, + LockTime: 10, ClosingSigs: lnwire.ClosingSigs{ CloserAndClosee: newSigTlv[tlv.TlvType3]( //nolint:ll remoteWireSig, diff --git a/lnwallet/chancloser/rbf_coop_transitions.go b/lnwallet/chancloser/rbf_coop_transitions.go index b3a038f38..de7a09dd9 100644 --- a/lnwallet/chancloser/rbf_coop_transitions.go +++ b/lnwallet/chancloser/rbf_coop_transitions.go @@ -716,7 +716,9 @@ func (l *LocalCloseStart) ProcessEvent(event ProtocolEvent, env *Environment, Msgs: []lnwire.Message{&lnwire.ClosingComplete{ ChannelID: env.ChanID, FeeSatoshis: absoluteFee, - Sequence: mempool.MaxRBFSequence, + // TODO(roasbeef): thread thru proper height + // value + LockTime: mempool.MaxRBFSequence, ClosingSigs: closingSigs, }}, }} @@ -862,18 +864,11 @@ func (l *RemoteCloseStart) ProcessEvent(event ProtocolEvent, env *Environment, // To start, we'll perform some basic validation of the sig // message they've sent. We'll validate that the remote party // actually has enough fees to pay the closing fees. - switch { - case !l.RemoteCanPayFees(msg.SigMsg.FeeSatoshis): + if !l.RemoteCanPayFees(msg.SigMsg.FeeSatoshis) { return nil, fmt.Errorf("%w: %v vs %v", ErrRemoteCannotPay, msg.SigMsg.FeeSatoshis, l.RemoteBalance.ToSatoshis()) - - // The sequence they send can't be the max sequence, as that would - // prevent RBF. - case msg.SigMsg.Sequence > mempool.MaxRBFSequence: - return nil, fmt.Errorf("%w: %v", ErrNonFinalSequence, - msg.SigMsg.Sequence) } // With the basic sanity checks out of the way, we'll now @@ -908,7 +903,7 @@ func (l *RemoteCloseStart) ProcessEvent(event ProtocolEvent, env *Environment, } chanOpts := []lnwallet.ChanCloseOpt{ - lnwallet.WithCustomSequence(msg.SigMsg.Sequence), + lnwallet.WithCustomSequence(mempool.MaxRBFSequence), } chancloserLog.Infof("responding to close w/ local_addr=%x, "+ diff --git a/lnwire/closing_complete.go b/lnwire/closing_complete.go index d33abf672..c16760a4e 100644 --- a/lnwire/closing_complete.go +++ b/lnwire/closing_complete.go @@ -34,9 +34,9 @@ type ClosingComplete struct { // channel would like to propose for the close transaction. FeeSatoshis btcutil.Amount - // Sequence is the sequence number to be used in the input spending the + // LockTime is the locktime number to be used in the input spending the // funding transaction. - Sequence uint32 + LockTime uint32 // ClosingSigs houses the 3 possible signatures that can be sent. ClosingSigs @@ -79,7 +79,7 @@ func decodeClosingSigs(c *ClosingSigs, tlvRecords ExtraOpaqueData) error { // passed io.Reader. func (c *ClosingComplete) Decode(r io.Reader, _ uint32) error { // First, read out all the fields that are hard coded into the message. - err := ReadElements(r, &c.ChannelID, &c.FeeSatoshis, &c.Sequence) + err := ReadElements(r, &c.ChannelID, &c.FeeSatoshis, &c.LockTime) if err != nil { return err } @@ -129,7 +129,7 @@ func (c *ClosingComplete) Encode(w *bytes.Buffer, _ uint32) error { return err } - if err := WriteUint32(w, c.Sequence); err != nil { + if err := WriteUint32(w, c.LockTime); err != nil { return err } diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index 6bfbb465e..f42426ac5 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -1352,7 +1352,7 @@ func TestLightningWireProtocol(t *testing.T) { req := ClosingComplete{ ChannelID: ChannelID(c), FeeSatoshis: btcutil.Amount(r.Int63()), - Sequence: uint32(r.Int63()), + LockTime: uint32(r.Int63()), ClosingSigs: ClosingSigs{}, } diff --git a/peer/brontide.go b/peer/brontide.go index a41b5080c..f0399b4e8 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -2275,6 +2275,13 @@ func messageSummary(msg lnwire.Message) string { return fmt.Sprintf("chan_id=%v, script=%x", msg.ChannelID, msg.Address[:]) + case *lnwire.ClosingComplete: + return fmt.Sprintf("chan_id=%v, fee_sat=%v, locktime=%v", + msg.ChannelID, msg.FeeSatoshis, msg.LockTime) + + case *lnwire.ClosingSig: + return fmt.Sprintf("chan_id=%v", msg.ChannelID) + case *lnwire.ClosingSigned: return fmt.Sprintf("chan_id=%v, fee_sat=%v", msg.ChannelID, msg.FeeSatoshis)