lnwallet/chancloser: increase test coverage of state machine

lnwallet/chancloser/rbf_coop_test.go

@@ -157,6 +157,27 @@ func assertUnknownEventFail(t *testing.T, startingState ProtocolState) {
 	})
 }
 
+// assertSpendEventCloseFin asserts that the state machine transitions to the
+// CloseFin state when a spend event is received.
+func assertSpendEventCloseFin(t *testing.T, startingState ProtocolState) {
+	t.Helper()
+
+	// If a spend event is received, the state machine should transition to
+	// the CloseFin state.
+	t.Run("spend_event", func(t *testing.T) {
+		closeHarness := newCloser(t, &harnessCfg{
+			initialState: fn.Some(startingState),
+		})
+		defer closeHarness.stopAndAssert()
+
+		closeHarness.chanCloser.SendEvent(
+			context.Background(), &SpendEvent{},
+		)
+
+		closeHarness.assertStateTransitions(&CloseFin{})
+	})
+}
+
 type harnessCfg struct {
 	initialState fn.Option[ProtocolState]
 
@@ -862,7 +883,28 @@ func TestRbfChannelActiveTransitions(t *testing.T) {
 		closeHarness.waitForMsgSent()
 	})
 
-	// TODO(roasbeef): thaw height fail
+	// If the remote party attempts to close, and a thaw height is active,
+	// but not yet met, then we should fail.
+	t.Run("remote_initiated_thaw_height_close_fail", func(t *testing.T) {
+		closeHarness := newCloser(t, &harnessCfg{
+			localUpfrontAddr: fn.Some(localAddr),
+			thawHeight:       fn.Some(uint32(100000)),
+		})
+		defer closeHarness.stopAndAssert()
+
+		// Next, we'll emit the recv event, with the addr of the remote
+		// party.
+		closeHarness.chanCloser.SendEvent(
+			ctx, &ShutdownReceived{
+				ShutdownScript: remoteAddr,
+				BlockHeight:    1,
+			},
+		)
+
+		// We expect a failure as the block height is less than the
+		// start height.
+		closeHarness.expectFailure(ErrThawHeightNotReached)
+	})
 
 	// When we receive a shutdown, we should transition to the shutdown
 	// pending state, with the local+remote shutdown addrs known.
@@ -906,6 +948,9 @@ func TestRbfChannelActiveTransitions(t *testing.T) {
 
 	// Any other event should be ignored.
 	assertUnknownEventFail(t, &ChannelActive{})
+
+	// Sending a Spend event should transition to CloseFin.
+	assertSpendEventCloseFin(t, &ChannelActive{})
 }
 
 // TestRbfShutdownPendingTransitions tests the transitions of the RBF closer
@@ -1134,6 +1179,9 @@ func TestRbfShutdownPendingTransitions(t *testing.T) {
 
 	// Any other event should be ignored.
 	assertUnknownEventFail(t, startingState)
+
+	// Sending a Spend event should transition to CloseFin.
+	assertSpendEventCloseFin(t, startingState)
 }
 
 // TestRbfChannelFlushingTransitions tests the transitions of the RBF closer
@@ -1255,6 +1303,9 @@ func TestRbfChannelFlushingTransitions(t *testing.T) {
 
 	// Any other event should be ignored.
 	assertUnknownEventFail(t, startingState)
+
+	// Sending a Spend event should transition to CloseFin.
+	assertSpendEventCloseFin(t, startingState)
 }
 
 // TestRbfCloseClosingNegotiationLocal tests the local portion of the primary
@@ -1496,13 +1547,67 @@ func TestRbfCloseClosingNegotiationLocal(t *testing.T) {
 			&ClosingNegotiation{},
 		)
 	})
+
+	// Make sure that we'll go to the error state if we try to try a close
+	// that we can't pay for.
+	t.Run("send_offer_cannot_pay_for_fees", func(t *testing.T) {
+		firstState := &ClosingNegotiation{
+			PeerState: lntypes.Dual[AsymmetricPeerState]{
+				Local: &LocalCloseStart{
+					CloseChannelTerms: closeTerms,
+				},
+			},
+			CloseChannelTerms: closeTerms,
+		}
+
+		closeHarness := newCloser(t, &harnessCfg{
+			initialState:     fn.Some[ProtocolState](firstState),
+			localUpfrontAddr: fn.Some(localAddr),
+		})
+		defer closeHarness.stopAndAssert()
+
+		// We'll prep to return an absolute fee that's much higher than
+		// the amount we have in the channel.
+		closeHarness.expectFeeEstimate(btcutil.SatoshiPerBitcoin, 1)
+
+		rbfFeeBump := chainfee.FeePerKwFloor.FeePerVByte()
+		localOffer := &SendOfferEvent{
+			TargetFeeRate: rbfFeeBump,
+		}
+
+		// Next, we'll send in this event, which should fail as we can't
+		// actually pay for fees.
+		closeHarness.chanCloser.SendEvent(ctx, localOffer)
+
+		// We should transition to the CloseErr (within
+		// ClosingNegotiation) state.
+		closeHarness.assertStateTransitions(&ClosingNegotiation{})
+
+		// If we get the state, we should see the expected ErrState.
+		currentState := assertStateT[*ClosingNegotiation](closeHarness)
+
+		closeErrState, ok := currentState.PeerState.GetForParty(
+			lntypes.Local,
+		).(*CloseErr)
+		require.True(t, ok)
+		require.IsType(
+			t, &ErrStateCantPayForFee{}, closeErrState.ErrState,
+		)
+	})
+
+	// Any other event should be ignored.
+	assertUnknownEventFail(t, startingState)
+
+	// Sending a Spend event should transition to CloseFin.
+	assertSpendEventCloseFin(t, startingState)
 }
 
 // TestRbfCloseClosingNegotiationRemote tests that state machine is able to
 // handle RBF iterations to sign for the closing transaction of the remote
 // party.
 func TestRbfCloseClosingNegotiationRemote(t *testing.T) {
 	t.Parallel()
+
 	ctx := context.Background()
 
 	localBalance := lnwire.NewMSatFromSatoshis(40_000)
@@ -1533,7 +1638,6 @@ func TestRbfCloseClosingNegotiationRemote(t *testing.T) {
 	}
 
 	balanceAfterClose := remoteBalance.ToSatoshis() - absoluteFee
-
 	sequence := uint32(mempool.MaxRBFSequence)
 
 	// This case tests that if we receive a signature from the remote
@@ -1785,4 +1889,112 @@ func TestRbfCloseClosingNegotiationRemote(t *testing.T) {
 			false,
 		)
 	})
+
+	// Any other event should be ignored.
+	assertUnknownEventFail(t, startingState)
+
+	// Sending a Spend event should transition to CloseFin.
+	assertSpendEventCloseFin(t, startingState)
+}
+
+// TestRbfCloseErr tests that the state machine is able to properly restart
+// the state machine if we encounter an error.
+func TestRbfCloseErr(t *testing.T) {
+	localBalance := lnwire.NewMSatFromSatoshis(40_000)
+	remoteBalance := lnwire.NewMSatFromSatoshis(50_000)
+
+	closeTerms := &CloseChannelTerms{
+		ShutdownBalances: ShutdownBalances{
+			LocalBalance:  localBalance,
+			RemoteBalance: remoteBalance,
+		},
+		ShutdownScripts: ShutdownScripts{
+			LocalDeliveryScript:  localAddr,
+			RemoteDeliveryScript: remoteAddr,
+		},
+	}
+	startingState := &ClosingNegotiation{
+		PeerState: lntypes.Dual[AsymmetricPeerState]{
+			Local: &CloseErr{
+				CloseChannelTerms: closeTerms,
+			},
+		},
+		CloseChannelTerms: closeTerms,
+	}
+
+	absoluteFee := btcutil.Amount(10_100)
+	balanceAfterClose := localBalance.ToSatoshis() - absoluteFee
+
+	// From the error state, we should be able to kick off a new iteration
+	// for a local fee bump.
+	t.Run("send_offer_restart", func(t *testing.T) {
+		closeHarness := newCloser(t, &harnessCfg{
+			initialState: fn.Some[ProtocolState](startingState),
+		})
+		defer closeHarness.stopAndAssert()
+
+		rbfFeeBump := chainfee.FeePerKwFloor.FeePerVByte()
+		localOffer := &SendOfferEvent{
+			TargetFeeRate: rbfFeeBump,
+		}
+
+		// Now we expect that another full RBF iteration takes place (we
+		// initiate a new local sig).
+		closeHarness.assertSingleRbfIteration(
+			localOffer, balanceAfterClose, absoluteFee,
+			noDustExpect,
+		)
+
+		// We should terminate in the negotiation state.
+		closeHarness.assertStateTransitions(
+			&ClosingNegotiation{},
+		)
+	})
+
+	// From the error state, we should be able to handle the remote party
+	// kicking off a new iteration for a fee bump.
+	t.Run("recv ofer restart", func(t *testing.T) {
+		startingState := &ClosingNegotiation{
+			PeerState: lntypes.Dual[AsymmetricPeerState]{
+				Remote: &CloseErr{
+					CloseChannelTerms: closeTerms,
+					Party:             lntypes.Remote,
+				},
+			},
+			CloseChannelTerms: closeTerms,
+		}
+
+		closeHarness := newCloser(t, &harnessCfg{
+			initialState:     fn.Some[ProtocolState](startingState),
+			localUpfrontAddr: fn.Some(localAddr),
+		})
+		defer closeHarness.stopAndAssert()
+
+		feeOffer := &OfferReceivedEvent{
+			SigMsg: lnwire.ClosingComplete{
+				CloserScript: remoteAddr,
+				CloseeScript: localAddr,
+				FeeSatoshis:  absoluteFee,
+				LockTime:     1,
+				ClosingSigs: lnwire.ClosingSigs{
+					CloserAndClosee: newSigTlv[tlv.TlvType3]( //nolint:ll
+						remoteWireSig,
+					),
+				},
+			},
+		}
+
+		sequence := uint32(mempool.MaxRBFSequence)
+
+		// As we're already in the negotiation phase, we'll now trigger
+		// a new iteration by having the remote party send a new offer
+		// sig.
+		closeHarness.assertSingleRemoteRbfIteration(
+			feeOffer, balanceAfterClose, absoluteFee, sequence,
+			false,
+		)
+	})
+
+	// Sending a Spend event should transition to CloseFin.
+	assertSpendEventCloseFin(t, startingState)
 }

lnwallet/chancloser/rbf_coop_transitions.go

@@ -22,6 +22,12 @@ import (
 	"github.com/lightningnetwork/lnd/tlv"
 )
 
+var (
+	// ErrInvalidStateTransition is returned if the remote party tries to
+	// close, but the thaw height hasn't been matched yet.
+	ErrThawHeightNotReached = fmt.Errorf("thaw height not reached")
+)
+
 // sendShutdownEvents is a helper function that returns a set of daemon events
 // we need to emit when we decide that we should send a shutdown message. We'll
 // also mark the channel as borked as well, as at this point, we no longer want
@@ -107,11 +113,11 @@ func validateShutdown(chanThawHeight fn.Option[uint32],
 		// reject the shutdown message as we can't yet co-op close the
 		// channel.
 		if msg.BlockHeight < thawHeight {
-			return fmt.Errorf("initiator attempting to "+
+			return fmt.Errorf("%w: initiator attempting to "+
 				"co-op close frozen ChannelPoint(%v) "+
 				"(current_height=%v, thaw_height=%v)",
-				chanPoint, msg.BlockHeight,
-				thawHeight)
+				ErrThawHeightNotReached, chanPoint,
+				msg.BlockHeight, thawHeight)
 		}
 
 		return nil
@@ -693,18 +699,27 @@ func (c *ClosingNegotiation) ProcessEvent(event ProtocolEvent, env *Environment,
 		return nil, fmt.Errorf("event violates close terms: %w", err)
 	}
 
+	shouldRouteTo := func(party lntypes.ChannelParty) bool {
+		state := c.PeerState.GetForParty(party)
+		if state == nil {
+			return false
+		}
+
+		return state.ShouldRouteTo(event)
+	}
+
 	// If we get to this point, then we have an event that'll drive forward
 	// the negotiation process.  Based on the event, we'll figure out which
 	// state we'll be modifying.
 	switch {
-	case c.PeerState.GetForParty(lntypes.Local).ShouldRouteTo(event):
+	case shouldRouteTo(lntypes.Local):
 		chancloserLog.Infof("ChannelPoint(%v): routing %T to local "+
 			"chan state", env.ChanPoint, event)
 
 		// Drive forward the local state based on the next event.
 		return processNegotiateEvent(c, event, env, lntypes.Local)
 
-	case c.PeerState.GetForParty(lntypes.Remote).ShouldRouteTo(event):
+	case shouldRouteTo(lntypes.Remote):
 		chancloserLog.Infof("ChannelPoint(%v): routing %T to remote "+
 
 			"chan state", env.ChanPoint, event)