diff --git a/db/round_store.go b/db/round_store.go index cafaaf85c..7170df226 100644 --- a/db/round_store.go +++ b/db/round_store.go @@ -91,6 +91,9 @@ type RoundStore interface { ctx context.Context, arg sqlc.UpdateRoundStatusParams, ) error + RetireCheckpointedRound(ctx context.Context, + arg sqlc.RetireCheckpointedRoundParams) (int64, error) + FinalizeRound(ctx context.Context, arg sqlc.FinalizeRoundParams) error InsertRoundBoardingIntent(ctx context.Context, @@ -208,6 +211,9 @@ type RoundStore interface { UpdateBoardingIntentStatus(ctx context.Context, arg sqlc.UpdateBoardingIntentStatusParams) error + RevertAdoptedBoardingIntent(ctx context.Context, + arg sqlc.RevertAdoptedBoardingIntentParams) error + // ClearPendingIntentAnchorByOutpoint deletes the pending-intent // anchor row bound to one outpoint. Called from CommitState in the // same transaction that records the round adopting the anchored @@ -803,6 +809,80 @@ func (s *RoundPersistenceStore) FinalizeRound(ctx context.Context, }) } +// FailRound retires a checkpointed round whose fate is known to be dead, and +// returns the boarding intents it adopted to the live pool. +// +// This is the exact inverse of the checkpoint write in CommitState, and it +// runs in one transaction for the same reason that one does: the round row +// and the intent statuses are a single fact about where the deposit lives, +// and a crash between the two halves would leave the deposit accounted for in +// a round that no longer exists. +// +// Intents revert to 'confirmed' rather than 'failed'. Nothing on-chain +// failed: a dead round proves the commitment was never broadcast, so the +// boarding UTXO is exactly as it was before the round started. 'confirmed' +// restores that truth, which both returns the deposit to the boardable pool +// and makes it sweepable again (boardingIntentSweepable excludes 'adopted', +// so an intent left adopted by a dead round is never swept, with or without +// its CSV expiring). +// +// Retiring a round that is not checkpointed is a no-op rather than an error. +// The round row is what the re-admission queries join on: a 'confirmed' round +// is exactly what keeps its deposits out of the boardable and sweepable pools +// once they have become VTXOs. Stamping such a row 'failed' would hand those +// deposits back while their commitment sits on-chain, so retirement leads with +// the guarded write and only gives the deposits back if it actually consumed a +// checkpointed row. +func (s *RoundPersistenceStore) FailRound(ctx context.Context, + roundID round.RoundID) error { + + writeTxOpts := WriteTxOption() + nowUnix := s.clock.Now().Unix() + roundIDStr := roundID.String() + + return s.db.ExecTx(ctx, writeTxOpts, func(q RoundStore) error { + // Retire the round first, and let the row count decide + // whether there is anything to give back. Ordering the + // guarded write ahead of the hand-back is what makes the + // no-op total: a failure that races a confirmation touches + // neither half. + retired, err := q.RetireCheckpointedRound( + ctx, sqlc.RetireCheckpointedRoundParams{ + RoundID: roundIDStr, + LastUpdateTime: nowUnix, + }, + ) + if err != nil { + return fmt.Errorf("retire round: %w", err) + } + + if retired == 0 { + return nil + } + + intents, err := q.GetRoundBoardingIntents(ctx, roundIDStr) + if err != nil { + return fmt.Errorf("fetch round intents: %w", err) + } + + for _, intent := range intents { + err := q.RevertAdoptedBoardingIntent( + ctx, sqlc.RevertAdoptedBoardingIntentParams{ + OutpointHash: intent.OutpointHash, + OutpointIndex: intent.OutpointIndex, + LastUpdateTime: nowUnix, + }, + ) + if err != nil { + return fmt.Errorf("revert boarding intent: %w", + err) + } + } + + return nil + }) +} + // SaveVTXOs persists one or more VTXOs after a round confirms. Each VTXO // includes its extracted tree path for unilateral exit. func (s *RoundPersistenceStore) SaveVTXOs(ctx context.Context, diff --git a/db/round_store_test.go b/db/round_store_test.go index 48afbfd81..b23b7619a 100644 --- a/db/round_store_test.go +++ b/db/round_store_test.go @@ -2077,3 +2077,284 @@ func TestOrphanSweepKeepsFailedSendRecord(t *testing.T) { require.Len(t, remaining, 1) require.Equal(t, retry.ID, remaining[0].ID) } + +// newRoundAndBoardingStoresForTest builds a round store and a boarding wallet +// store over one database. FailRound spans both domains, since it retires the +// round row and returns the deposits that row adopted, so a test that reads +// the deposit back has to see the same rows the round store wrote. +func newRoundAndBoardingStoresForTest(t *testing.T) (*RoundPersistenceStore, + *BoardingWalletStore, *BaseDB) { + + t.Helper() + + db := NewTestDB(t) + testClock := clock.NewDefaultClock() + + roundDB := NewTransactionExecutor( + db.BaseDB, + func(tx *sql.Tx) RoundStore { + return db.WithTx(tx) + }, + btclog.Disabled, + ) + boardingDB := NewTransactionExecutor( + db.BaseDB, + func(tx *sql.Tx) BoardingStore { + return db.WithTx(tx) + }, + btclog.Disabled, + ) + intentDB := NewTransactionExecutor( + db.BaseDB, + func(tx *sql.Tx) PendingIntentStore { + return db.WithTx(tx) + }, + btclog.Disabled, + ) + + roundStore := NewRoundPersistenceStore( + roundDB, &chaincfg.RegressionNetParams, testClock, + ) + boardingStore := NewBoardingWalletStore( + boardingDB, intentDB, &chaincfg.RegressionNetParams, testClock, + ) + + return roundStore, boardingStore, db.BaseDB +} + +// TestRoundStoreFailRoundReleasesDeposit is the money half of +// wavelength#1051. Arming the reconcile clock un-parks the client FSM, but a +// dead round still has to give the deposit back, and until FailRound existed +// nothing did: the checkpoint row stayed in ListActiveRounds and the intent +// stayed adopted, which keeps it out of both the sweep and the boardable pool +// permanently, with or without its CSV expiring. +// +// The recovery machinery was already built. ListBoardingIntentsByStatus and +// ListBoardingIntentsBySweepableStatuses both re-admit a confirmed intent +// exactly when its linked round reads 'failed'; nothing ever wrote that +// status, so the path was unreachable. +func TestRoundStoreFailRoundReleasesDeposit(t *testing.T) { + t.Parallel() + + roundStore, boardingStore, db := newRoundAndBoardingStoresForTest(t) + ctx := t.Context() + + // A confirmed deposit, adopted by a round that reached the checkpoint + // and then died. + intent := createSweepStoreIntent(t, boardingStore) + deadRound := testRoundIDDB("dead-deposit") + insertRoundBoardingIntentForTest( + t, db, deadRound.String(), + "input_sig_sent", intent, + ) + require.NoError( + t, boardingStore.UpdateBoardingIntentStatus( + ctx, intent.Outpoint, wallet.BoardingStatusAdopted, + ), + ) + + // While the round is live the deposit is correctly invisible: it is + // committed to a round, so it is neither boardable nor sweepable. + assertDepositRecoverable(t, boardingStore, intent, false) + + require.NoError(t, roundStore.FailRound(ctx, deadRound)) + + // The round no longer re-hydrates on startup. + active, err := roundStore.ListActiveRounds(ctx) + require.NoError(t, err) + require.Empty(t, active, "dead round still reloads on every start") + + // And the deposit is back: boardable for a retry, sweepable for an + // on-chain exit, and no longer pinned against the board limit. + assertDepositRecoverable(t, boardingStore, intent, true) + + adopted, err := boardingStore.FetchBoardingIntentsByStatus( + ctx, wallet.BoardingStatusAdopted, + ) + require.NoError(t, err) + require.Empty( + t, adopted, "deposit still counted against the board limit "+ + "after its round died", + ) +} + +// assertDepositRecoverable checks both recovery routes for a deposit at once: +// whether it can be boarded again, and whether it can be swept back on-chain. +func assertDepositRecoverable(t *testing.T, store *BoardingWalletStore, + intent wallet.BoardingIntent, want bool) { + + t.Helper() + + ctx := t.Context() + + confirmed, err := store.FetchBoardingIntentsByStatus( + ctx, wallet.BoardingStatusConfirmed, + ) + require.NoError(t, err) + + sweepable, err := store.FetchBoardingIntentsBySweepableStatuses( + ctx, []wallet.BoardingStatus{ + wallet.BoardingStatusConfirmed, + wallet.BoardingStatusFailed, + wallet.BoardingStatusExpired, + }, + ) + require.NoError(t, err) + + require.Lenf( + t, confirmed, boolToLen(want), + "boardable = %v, want %v", len(confirmed) > 0, want, + ) + require.Lenf( + t, sweepable, boolToLen(want), + "sweepable = %v, want %v", len(sweepable) > 0, want, + ) + + if want { + require.Equal(t, intent.Outpoint, confirmed[0].Outpoint) + require.Equal(t, intent.Outpoint, sweepable[0].Outpoint) + } +} + +// boolToLen maps an expectation about presence onto the expected slice length. +func boolToLen(present bool) int { + if present { + return 1 + } + + return 0 +} + +// TestRoundStoreFailRoundLeavesSweptDepositAlone pins the guard on the +// revert: the SQL only moves an intent that is still 'adopted', so a deposit +// a sweep has already claimed is not dragged back into the boardable pool by +// a late round retirement. +func TestRoundStoreFailRoundLeavesSweptDepositAlone(t *testing.T) { + t.Parallel() + + roundStore, boardingStore, db := newRoundAndBoardingStoresForTest(t) + ctx := t.Context() + + intent := createSweepStoreIntent(t, boardingStore) + sweptRound := testRoundIDDB("already-swept") + insertRoundBoardingIntentForTest( + t, db, sweptRound.String(), + "input_sig_sent", intent, + ) + require.NoError( + t, boardingStore.UpdateBoardingIntentStatus( + ctx, intent.Outpoint, wallet.BoardingStatusSweepPending, + ), + ) + + require.NoError(t, roundStore.FailRound(ctx, sweptRound)) + + got, err := boardingStore.GetIntent(ctx, intent.Outpoint) + require.NoError(t, err) + require.Equal( + t, wallet.BoardingStatusSweepPending, got.Status, + "round retirement clobbered an in-flight sweep", + ) +} + +// TestRoundStoreFailRoundOnlyRetiresCheckpointedRound pins the guard on the +// round half of the retirement, the counterpart to the intent guard above. +// +// The round row is the authority the re-admission queries join on: an adopted +// deposit stays out of the boardable and sweepable pools precisely while its +// round reads something other than 'failed'. So stamping a round 'failed' +// unconditionally is not a bookkeeping detail, it is the whole gate. A late or +// duplicate failure for a round that has already confirmed would hand back +// deposits whose commitment is on-chain and whose UTXO is now a VTXO, leaving +// the client offering an already-spent outpoint for a fresh board. +// +// The FSM cannot produce that ordering today, so this is a structural guard +// rather than a fix for a live bug: retirement consumes exactly the rows +// ListActiveRounds keys on, and anything else is a total no-op. +func TestRoundStoreFailRoundOnlyRetiresCheckpointedRound(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + + // roundStatus is the status the round row carries when the + // late failure lands. + roundStatus string + + // wantStatus is the status it must carry afterwards. + wantStatus string + + // wantRetired is whether the retirement should have consumed + // the row, which is also whether the deposit should come + // back. + wantRetired bool + }{{ + name: "checkpointed round retires", + roundStatus: "input_sig_sent", + wantStatus: "failed", + wantRetired: true, + }, { + name: "confirmed round survives", + roundStatus: "confirmed", + wantStatus: "confirmed", + wantRetired: false, + }, { + name: "failed round is not restamped", + roundStatus: "failed", + wantStatus: "failed", + wantRetired: false, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + roundStore, boardingStore, db := + newRoundAndBoardingStoresForTest(t) + ctx := t.Context() + + intent := createSweepStoreIntent(t, boardingStore) + roundID := testRoundIDDB(tc.name) + insertRoundBoardingIntentForTest( + t, db, roundID.String(), tc.roundStatus, intent, + ) + require.NoError( + t, boardingStore.UpdateBoardingIntentStatus( + ctx, intent.Outpoint, + wallet.BoardingStatusAdopted, + ), + ) + + require.NoError(t, roundStore.FailRound(ctx, roundID)) + + // The round row moved only if it was checkpointed. + // insertRoundBoardingIntentForTest stamps + // last_update_time at 100, so an untouched row still + // reads 100 while a retired one carries the store + // clock: that separates "already failed" from + // "restamped failed", which the status alone cannot. + row, err := db.GetRound(ctx, roundID.String()) + require.NoError(t, err) + require.Equal(t, tc.wantStatus, row.Status) + + if tc.wantRetired { + require.Greater( + t, row.LastUpdateTime, int64(100), + "retirement did not stamp the row", + ) + } else { + require.EqualValues( + t, 100, row.LastUpdateTime, + "retirement rewrote a settled row", + ) + } + + // And the deposit follows the round row: back in both + // recovery pools on a real retirement, still committed + // to its round otherwise. + assertDepositRecoverable( + t, boardingStore, intent, tc.wantRetired, + ) + }) + } +} diff --git a/db/sqlc/querier.go b/db/sqlc/querier.go index 875610f5e..3db4af9bb 100644 --- a/db/sqlc/querier.go +++ b/db/sqlc/querier.go @@ -362,6 +362,19 @@ type Querier interface { // PullActivityEvents returns transition rows strictly after the cursor in // event_seq order, the resumable-subscribe replay primitive. PullActivityEvents(ctx context.Context, arg PullActivityEventsParams) ([]ActivityEvent, error) + // Stamps a checkpointed round 'failed'. Guarded on 'input_sig_sent', the same + // status ListActiveRounds keys on, so retirement can only ever consume a row + // that is still re-hydrating on startup. The row count lets the caller tell a + // real retirement from a late or duplicate failure for a round that has since + // finalized, and skip the deposit hand-back in the latter case: the round row + // is what the re-admission queries join on, so stamping a confirmed round + // 'failed' would make deposits that are already VTXOs boardable again. + RetireCheckpointedRound(ctx context.Context, arg RetireCheckpointedRoundParams) (int64, error) + // Returns a boarding intent adopted by a dead round to 'confirmed'. The + // commitment never broadcast, so the UTXO is exactly as it was before the + // round: re-boardable, and sweepable again. Guarded on 'adopted' so a later + // sweep that already moved the intent on is never clobbered. + RevertAdoptedBoardingIntent(ctx context.Context, arg RevertAdoptedBoardingIntentParams) error SumBoardingIntentAmountsByStatus(ctx context.Context, status string) (interface{}, error) SumUnspentVTXOAmounts(ctx context.Context) (interface{}, error) UpdateBoardingIntentStatus(ctx context.Context, arg UpdateBoardingIntentStatusParams) error diff --git a/db/sqlc/queries/round.sql b/db/sqlc/queries/round.sql index 13d197d64..79e1ac459 100644 --- a/db/sqlc/queries/round.sql +++ b/db/sqlc/queries/round.sql @@ -52,6 +52,27 @@ SET status = 'confirmed', last_update_time = $5 WHERE round_id = $1; +-- name: RetireCheckpointedRound :execrows +-- Stamps a checkpointed round 'failed'. Guarded on 'input_sig_sent', the same +-- status ListActiveRounds keys on, so retirement can only ever consume a row +-- that is still re-hydrating on startup. The row count lets the caller tell a +-- real retirement from a late or duplicate failure for a round that has since +-- finalized, and skip the deposit hand-back in the latter case: the round row +-- is what the re-admission queries join on, so stamping a confirmed round +-- 'failed' would make deposits that are already VTXOs boardable again. +UPDATE rounds +SET status = 'failed', last_update_time = $2 +WHERE round_id = $1 AND status = 'input_sig_sent'; + +-- name: RevertAdoptedBoardingIntent :exec +-- Returns a boarding intent adopted by a dead round to 'confirmed'. The +-- commitment never broadcast, so the UTXO is exactly as it was before the +-- round: re-boardable, and sweepable again. Guarded on 'adopted' so a later +-- sweep that already moved the intent on is never clobbered. +UPDATE boarding_intents +SET status = 'confirmed', last_update_time = $3 +WHERE outpoint_hash = $1 AND outpoint_index = $2 AND status = 'adopted'; + -- Round boarding intents queries. -- name: InsertRoundBoardingIntent :exec diff --git a/db/sqlc/round.sql.go b/db/sqlc/round.sql.go index d710d0030..4be7f6d9f 100644 --- a/db/sqlc/round.sql.go +++ b/db/sqlc/round.sql.go @@ -1147,6 +1147,53 @@ func (q *Queries) MarkVTXOSpent(ctx context.Context, arg MarkVTXOSpentParams) er return err } +const RetireCheckpointedRound = `-- name: RetireCheckpointedRound :execrows +UPDATE rounds +SET status = 'failed', last_update_time = $2 +WHERE round_id = $1 AND status = 'input_sig_sent' +` + +type RetireCheckpointedRoundParams struct { + RoundID string + LastUpdateTime int64 +} + +// Stamps a checkpointed round 'failed'. Guarded on 'input_sig_sent', the same +// status ListActiveRounds keys on, so retirement can only ever consume a row +// that is still re-hydrating on startup. The row count lets the caller tell a +// real retirement from a late or duplicate failure for a round that has since +// finalized, and skip the deposit hand-back in the latter case: the round row +// is what the re-admission queries join on, so stamping a confirmed round +// 'failed' would make deposits that are already VTXOs boardable again. +func (q *Queries) RetireCheckpointedRound(ctx context.Context, arg RetireCheckpointedRoundParams) (int64, error) { + result, err := q.db.ExecContext(ctx, RetireCheckpointedRound, arg.RoundID, arg.LastUpdateTime) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const RevertAdoptedBoardingIntent = `-- name: RevertAdoptedBoardingIntent :exec +UPDATE boarding_intents +SET status = 'confirmed', last_update_time = $3 +WHERE outpoint_hash = $1 AND outpoint_index = $2 AND status = 'adopted' +` + +type RevertAdoptedBoardingIntentParams struct { + OutpointHash []byte + OutpointIndex int32 + LastUpdateTime int64 +} + +// Returns a boarding intent adopted by a dead round to 'confirmed'. The +// commitment never broadcast, so the UTXO is exactly as it was before the +// round: re-boardable, and sweepable again. Guarded on 'adopted' so a later +// sweep that already moved the intent on is never clobbered. +func (q *Queries) RevertAdoptedBoardingIntent(ctx context.Context, arg RevertAdoptedBoardingIntentParams) error { + _, err := q.db.ExecContext(ctx, RevertAdoptedBoardingIntent, arg.OutpointHash, arg.OutpointIndex, arg.LastUpdateTime) + return err +} + const SumUnspentVTXOAmounts = `-- name: SumUnspentVTXOAmounts :one SELECT COALESCE(SUM(amount), 0) as total FROM vtxos WHERE spent = FALSE diff --git a/round/AGENTS.md b/round/AGENTS.md index 630d6bc8a..6c0997e3d 100644 --- a/round/AGENTS.md +++ b/round/AGENTS.md @@ -93,10 +93,16 @@ state transitions and validation rules live under [Invariants](#invariants). ### Misc - `TimeoutPhase` (`fsm_timeouts.go`) — `TimeoutPhaseForfeitCollection` - (forfeit-signature collection window) and `TimeoutPhaseRegistration` + (forfeit-signature collection window), `TimeoutPhaseRegistration` (IntentSentState admission window; on expiry the FSM fails the round recoverably and emits `ReleaseForfeitReservation` so forfeit-reserved - inputs are not stranded — wavelength#653). Timeout outbox messages + inputs are not stranded — wavelength#653), + `TimeoutPhaseRefreshRegistration` (quiet period that coalesces + expiry-driven refreshes before registering their assembling round), and + `TimeoutPhaseStatusReconcile` (InputSigSentState liveness clock; on expiry + the FSM probes the operator with `QueryRoundStatus` and re-arms with + exponential backoff capped at `statusReconcileMaxBackoffShift` — see the + reconcile invariant below). Timeout outbox messages (`StartTimeoutReq`/`CancelTimeoutReq`) key on `RoundKeyStr` so temp-keyed rounds (pre-admission) can be timed. - `MaxQuoteEntriesPerClient = 1024` (`from_proto.go`) — bounds quote @@ -114,6 +120,12 @@ state transitions and validation rules live under [Invariants](#invariants). selects `defaultRegistrationTimeout` (60 s); negative disables the timeout (round waits indefinitely). Bounds how long forfeit-reserved inputs sit stranded when the server never responds (wavelength#653). +- `RoundClientConfig.StatusReconcileTimeout` — how long `InputSigSentState` + waits with no confirmation, no resolved failure, and no status answer + before probing the operator with `QueryRoundStatus` (wavelength#844); it + doubles as the retry interval between probes. Zero selects + `defaultStatusReconcileTimeout` (90 s); negative disables the reconcile + entirely, leaving only the wavelength#823 startup sweep. - `computeClientOperatorFee(intents, ownedVTXOs) int64` — Σ(boarding inputs) + Σ(forfeited VTXOs) − Σ(owned output VTXOs) − Σ(cooperative leave outputs), clamped to zero. Carried on @@ -167,6 +179,31 @@ state transitions and validation rules live under [Invariants](#invariants). - Round state is checkpointed atomically after tree validation — a crash before checkpoint means the client has no record of sent signatures. +- **The status-reconcile clock is armed for the whole of + `InputSigSentState`** (wavelength#844, wavelength#1051). Every door into + that state arms it whenever `env.StatusReconcileTimeout > 0`, and every + exit disarms it. There are three doors: `forfeitCollectionOutbox` + (forfeit-bearing rounds), the `PartialSigsSentState` → + `InputSigSentState` transition (boarding-only rounds, which never enter + forfeit collection), and `recoverActiveRounds` on restart. Arming is + **not** gated on `len(Intents.Forfeits) > 0`: for a forfeit-bearing round + the probe gates the reservation release on an authoritative dead answer, + but for *any* round it is the sole liveness clock in the checkpointed + state — an operator that rolls the round back before broadcast produces + no confirmation and no failure, so an unarmed boarding-only round strands + its deposit until the CSV expires. +- **Reconcile outbox ordering.** `processOutbox` abandons the rest of the + outbox on the first failing `Tell`, and the FSM has already checkpointed + by dispatch time, so the arm (`StartTimeoutReq`) must **lead** the + fallible sends (`SubmitVTXOForfeitSigsToServer`, + `SubmitForfeitSigRequest`, `RegisterConfirmationRequest`) — a mid-flight + send error would otherwise commit the state with no clock. The disarm + (`CancelTimeoutReq`, via `reconcileDisarmEvents` / + `appendReconcileDisarm` in `fsm_timeouts.go`) must **trail** every + delivery on the exit paths — confirmation notifications, the forfeit + release, and the terminal job drop — since cleanup must never gate + delivery. A cancel that never lands only leaks a one-shot timer, which + fires into a terminal state and self-loops. - Primary FSM handles interactive phases (through `InputSigSent`); a dedicated FSM per round handles confirmation monitoring. - The round actor does **not** mark VTXOs as `PendingForfeit` — the diff --git a/round/CLAUDE.md b/round/CLAUDE.md index 630d6bc8a..6c0997e3d 100644 --- a/round/CLAUDE.md +++ b/round/CLAUDE.md @@ -93,10 +93,16 @@ state transitions and validation rules live under [Invariants](#invariants). ### Misc - `TimeoutPhase` (`fsm_timeouts.go`) — `TimeoutPhaseForfeitCollection` - (forfeit-signature collection window) and `TimeoutPhaseRegistration` + (forfeit-signature collection window), `TimeoutPhaseRegistration` (IntentSentState admission window; on expiry the FSM fails the round recoverably and emits `ReleaseForfeitReservation` so forfeit-reserved - inputs are not stranded — wavelength#653). Timeout outbox messages + inputs are not stranded — wavelength#653), + `TimeoutPhaseRefreshRegistration` (quiet period that coalesces + expiry-driven refreshes before registering their assembling round), and + `TimeoutPhaseStatusReconcile` (InputSigSentState liveness clock; on expiry + the FSM probes the operator with `QueryRoundStatus` and re-arms with + exponential backoff capped at `statusReconcileMaxBackoffShift` — see the + reconcile invariant below). Timeout outbox messages (`StartTimeoutReq`/`CancelTimeoutReq`) key on `RoundKeyStr` so temp-keyed rounds (pre-admission) can be timed. - `MaxQuoteEntriesPerClient = 1024` (`from_proto.go`) — bounds quote @@ -114,6 +120,12 @@ state transitions and validation rules live under [Invariants](#invariants). selects `defaultRegistrationTimeout` (60 s); negative disables the timeout (round waits indefinitely). Bounds how long forfeit-reserved inputs sit stranded when the server never responds (wavelength#653). +- `RoundClientConfig.StatusReconcileTimeout` — how long `InputSigSentState` + waits with no confirmation, no resolved failure, and no status answer + before probing the operator with `QueryRoundStatus` (wavelength#844); it + doubles as the retry interval between probes. Zero selects + `defaultStatusReconcileTimeout` (90 s); negative disables the reconcile + entirely, leaving only the wavelength#823 startup sweep. - `computeClientOperatorFee(intents, ownedVTXOs) int64` — Σ(boarding inputs) + Σ(forfeited VTXOs) − Σ(owned output VTXOs) − Σ(cooperative leave outputs), clamped to zero. Carried on @@ -167,6 +179,31 @@ state transitions and validation rules live under [Invariants](#invariants). - Round state is checkpointed atomically after tree validation — a crash before checkpoint means the client has no record of sent signatures. +- **The status-reconcile clock is armed for the whole of + `InputSigSentState`** (wavelength#844, wavelength#1051). Every door into + that state arms it whenever `env.StatusReconcileTimeout > 0`, and every + exit disarms it. There are three doors: `forfeitCollectionOutbox` + (forfeit-bearing rounds), the `PartialSigsSentState` → + `InputSigSentState` transition (boarding-only rounds, which never enter + forfeit collection), and `recoverActiveRounds` on restart. Arming is + **not** gated on `len(Intents.Forfeits) > 0`: for a forfeit-bearing round + the probe gates the reservation release on an authoritative dead answer, + but for *any* round it is the sole liveness clock in the checkpointed + state — an operator that rolls the round back before broadcast produces + no confirmation and no failure, so an unarmed boarding-only round strands + its deposit until the CSV expires. +- **Reconcile outbox ordering.** `processOutbox` abandons the rest of the + outbox on the first failing `Tell`, and the FSM has already checkpointed + by dispatch time, so the arm (`StartTimeoutReq`) must **lead** the + fallible sends (`SubmitVTXOForfeitSigsToServer`, + `SubmitForfeitSigRequest`, `RegisterConfirmationRequest`) — a mid-flight + send error would otherwise commit the state with no clock. The disarm + (`CancelTimeoutReq`, via `reconcileDisarmEvents` / + `appendReconcileDisarm` in `fsm_timeouts.go`) must **trail** every + delivery on the exit paths — confirmation notifications, the forfeit + release, and the terminal job drop — since cleanup must never gate + delivery. A cancel that never lands only leaks a one-shot timer, which + fires into a terminal state and self-loops. - Primary FSM handles interactive phases (through `InputSigSent`); a dedicated FSM per round handles confirmation monitoring. - The round actor does **not** mark VTXOs as `PendingForfeit` — the diff --git a/round/actor.go b/round/actor.go index 54608407e..6afc3e30b 100644 --- a/round/actor.go +++ b/round/actor.go @@ -1408,14 +1408,18 @@ func (a *RoundClientActor) Start(ctx context.Context) error { } // A reloaded round sits back in InputSigSentState with its - // forfeit signatures already out, so the wavelength#844 - // hazard window reopens across the restart. Re-arm the - // status-reconcile timeout for forfeit-bearing rounds so a - // round whose failure raced the crash still converges on a - // QueryRoundStatus probe rather than stranding. - if len(round.Intents.Forfeits) > 0 && - a.env.StatusReconcileTimeout > 0 { - + // signatures already out, so the wavelength#844 hazard window + // reopens across the restart. Re-arm the status-reconcile + // timeout so a round whose failure raced the crash still + // converges on a QueryRoundStatus probe rather than stranding. + // + // Boarding-only rounds re-arm too. They hold no forfeit + // reservations, but the probe is still their only exit from + // InputSigSentState once the operator has rolled the round + // back before broadcast: no commitment can ever confirm and no + // failure will ever be delivered, so without the clock the + // deposit strands until the CSV expires (wavelength#1051). + if a.env.StatusReconcileTimeout > 0 { if err := a.processOutbox(ctx, []ClientOutMsg{ &StartTimeoutReq{ RoundKey: RoundKeyStr( @@ -2246,6 +2250,41 @@ func (a *RoundClientActor) handleCancelRound(ctx context.Context, }) } +// retireFailedRound settles the durable side of a failed round: the +// checkpoint row moves out of 'input_sig_sent' and the boarding intents it +// adopted return to the live pool. +// +// It is deliberately unconditional across failure paths. A round that never +// reached the checkpoint has no row to retire and no adopted intents, so both +// halves are no-op UPDATEs; gating on "was this round checkpointed" would +// duplicate that knowledge in the actor for no gain. The intent revert is +// itself guarded on 'adopted' in SQL, so a sweep that already moved a deposit +// on is never clobbered. +// +// Failure to retire is logged rather than propagated. The round has already +// failed and the client has already been told; a store error here means the +// row is reclaimed on a later pass rather than that the failure is in doubt. +func (a *RoundClientActor) retireFailedRound(ctx context.Context, + roundID RoundID) { + + // Confirmation handling and failure delivery can both outlive the + // request that triggered them, so the write uses a detached context. + opCtx := context.WithoutCancel(ctx) + + if err := a.cfg.RoundStore.FailRound(opCtx, roundID); err != nil { + a.log.WarnS(ctx, "Failed to retire dead round", + err, + slog.String("round_id", roundID.String()), + ) + + return + } + + a.log.InfoS(ctx, "Retired dead round and released its deposits", + slog.String("round_id", roundID.String()), + ) +} + // onRoundComplete is called when a round finishes successfully. This removes // the round from active tracking and archives the round data. func (a *RoundClientActor) onRoundComplete(ctx context.Context, roundID RoundID, @@ -2750,6 +2789,16 @@ func (a *RoundClientActor) processOutbox(ctx context.Context, // operator can track the join-to-completion ratio. a.emitRoundCompleted(ctx, roundIDStr, "failed") + // Retire the durable side of the round. Reaping only + // drops the in-memory FSM, so without this the + // checkpoint row stays in ListActiveRounds and is + // re-hydrated on every start, and the deposits it + // adopted stay adopted: out of the sweep and pinned + // against the board limit for good. + m.RoundID.WhenSome(func(id RoundID) { + a.retireFailedRound(ctx, id) + }) + case *TerminalJobFailedNotification: // A terminal-for-job round failure (e.g. the operator // could not fund the commitment tx). The accompanying diff --git a/round/actor_test.go b/round/actor_test.go index 6f527f813..1b91ef564 100644 --- a/round/actor_test.go +++ b/round/actor_test.go @@ -2,6 +2,7 @@ package round import ( "context" + "errors" "fmt" "testing" "time" @@ -3773,3 +3774,58 @@ func TestHandleRegisterIntent(t *testing.T) { ) }) } + +// TestFailedRoundIsRetiredDurably pins the actor half of the deposit release. +// Reaping only drops the in-memory FSM, so without this call the checkpoint +// row stays in ListActiveRounds and is re-hydrated on every start, and the +// deposits the round adopted stay adopted: out of the sweep and pinned +// against the board limit for good. RoundFailedNotification is the single +// choke point every failure path passes through, so retirement hangs there +// rather than on the individual FSM exits. +func TestFailedRoundIsRetiredDurably(t *testing.T) { + t.Parallel() + + h := newActorTestHarness(t) + h.setupMockRoundStoreForStart() + require.NoError(t, h.start()) + + roundID := testRoundID("retire-on-failure") + h.roundStore.On("FailRound", mock.Anything, roundID).Return(nil) + + err := h.actor.processOutbox(h.ctx, []ClientOutMsg{ + &RoundFailedNotification{ + RoundID: fn.Some(roundID), + Reason: "round dead at operator", + Recoverable: true, + }, + }) + require.NoError(t, err) + + h.roundStore.AssertCalled(t, "FailRound", mock.Anything, roundID) +} + +// TestFailedRoundRetirementSurvivesStoreError pins that a store error on +// retirement does not propagate. The round has already failed and the client +// has already been told; a failed write here means the row is reclaimed on a +// later pass, not that the failure is in doubt, so it must not abort the rest +// of the outbox. +func TestFailedRoundRetirementSurvivesStoreError(t *testing.T) { + t.Parallel() + + h := newActorTestHarness(t) + h.setupMockRoundStoreForStart() + require.NoError(t, h.start()) + + roundID := testRoundID("retire-store-error") + h.roundStore.On( + "FailRound", mock.Anything, roundID, + ).Return(errors.New("db down")) + + err := h.actor.processOutbox(h.ctx, []ClientOutMsg{ + &RoundFailedNotification{ + RoundID: fn.Some(roundID), + Reason: "round dead at operator", + }, + }) + require.NoError(t, err, "a failed retirement aborted the outbox") +} diff --git a/round/checkpoint_arming_test.go b/round/checkpoint_arming_test.go new file mode 100644 index 000000000..09db5fffe --- /dev/null +++ b/round/checkpoint_arming_test.go @@ -0,0 +1,297 @@ +package round + +import ( + "testing" + "time" + + "github.com/btcsuite/btcd/psbt/v2" + "github.com/btcsuite/btcd/wire/v2" + "github.com/lightninglabs/wavelength/lib/tree" + "github.com/lightninglabs/wavelength/lib/types" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// newArmingCommitmentTx builds the smallest commitment packet the outbox +// helper will accept. The arming invariant does not care what the round is +// paying, only that the checkpoint was reached, so an empty unsigned tx is +// enough: TxHash works on it and confirmationWatchScript returns nil rather +// than panicking on the missing outputs. +func newArmingCommitmentTx() *psbt.Packet { + return &psbt.Packet{ + UnsignedTx: wire.NewMsgTx(2), + } +} + +// newArmingEnv builds an environment carrying the given reconcile timeout. +// OperatorTerms has to be non-nil: the outbox reads MinConfirmations off it +// for the confirmation registration, so the bare struct literal the other +// reconcile tests use would panic here. +func newArmingEnv(reconcile time.Duration) *ClientEnvironment { + base := reconcileEnv() + base.StatusReconcileTimeout = reconcile + base.OperatorTerms = &types.OperatorTerms{ + MinConfirmations: 1, + } + + return base +} + +// InputSigSentState is the checkpointed point of no return: the client's +// signatures are conceptually gone, the operator may broadcast at any moment, +// and no further event is guaranteed to arrive. The status-reconcile clock is +// the only thing that turns operator silence back into progress, so every +// door into that state has to arm it. Miss one and the rounds that walk +// through it strand until the CSV expires, which is wavelength#1051. +// +// There are two live doors plus the restart reload, and they are easy to +// treat as one: a refresh round with forfeits goes through forfeit +// collection, while a boarding-only round skips that entirely and checkpoints +// straight from PartialSigsSentState. The second door is the one that went +// unarmed, and no per-door test noticed because the boarding harness left +// StatusReconcileTimeout at zero, which skips the arming branch outright. +// +// So the tests below assert the invariant rather than the doors: whatever +// path reaches the checkpoint must emit the arm, and it must emit it ahead of +// the requests that can fail. + +// findStartReconcile returns the status-reconcile StartTimeoutReq in an +// outbox along with its index, so callers can assert both presence and +// ordering. +func findStartReconcile(msgs []ClientOutMsg) (*StartTimeoutReq, int) { + for i, msg := range msgs { + req, ok := msg.(*StartTimeoutReq) + if !ok { + continue + } + if req.Phase == TimeoutPhaseStatusReconcile { + return req, i + } + } + + return nil, -1 +} + +// assertArmsReconcileFirst is the shared invariant: the outbox arms the +// status-reconcile clock, and does so before anything that can fail on the +// way out. processOutbox stops at the first failed Tell, so an arm sitting +// behind the sig and registration requests is an arm a single mailbox hiccup +// can skip, leaving the state committed with no clock behind it. +func assertArmsReconcileFirst(t *testing.T, msgs []ClientOutMsg, + timeout time.Duration, roundID RoundID) { + + t.Helper() + + arm, idx := findStartReconcile(msgs) + require.NotNilf( + t, arm, "checkpoint outbox never arms the status-reconcile "+ + "clock: a round reaching InputSigSentState through "+ + "this door has no liveness timer, so operator "+ + "silence strands it until the CSV expires "+ + "(wavelength#1051)", + ) + require.Equal(t, timeout, arm.Duration) + require.Equal(t, RoundKeyStr(roundID.KeyString()), arm.RoundKey) + + // Nothing may precede the arm, a cancel of another phase included: the + // timeout actor rejects a cancel exactly as it would reject the arm, so + // leading with one is the same hazard wearing bookkeeping's clothes. + for i, msg := range msgs[:idx] { + t.Fatalf("checkpoint outbox arms the reconcile clock at index "+ + "%d, behind %T at index %d: a failed Tell on that "+ + "message commits the checkpoint with no clock armed", + idx, msg, i) + } +} + +// TestForfeitDoorArmsReconcileClock covers the forfeit-bearing door: a +// refresh round leaves through forfeit collection, so the arm rides in that +// outbox. +func TestForfeitDoorArmsReconcileClock(t *testing.T) { + t.Parallel() + + const timeout = time.Minute + + roundID := reconcileRoundID(0x11) + state := &ForfeitSignaturesCollectingState{ + RoundID: roundID, + CommitmentTx: newArmingCommitmentTx(), + } + env := newArmingEnv(timeout) + + forfeits := map[wire.OutPoint]*types.ForfeitTxSig{ + reconcileOutpoint(0x22): {}, + } + + t.Run("with boarding sigs", func(t *testing.T) { + t.Parallel() + + msgs := state.forfeitCollectionOutbox( + env, forfeits, []*types.BoardingInputSignature{{}}, + ) + assertArmsReconcileFirst(t, msgs, timeout, roundID) + }) + + t.Run("without boarding sigs", func(t *testing.T) { + t.Parallel() + + msgs := state.forfeitCollectionOutbox(env, forfeits, nil) + assertArmsReconcileFirst(t, msgs, timeout, roundID) + }) +} + +// TestBoardingDoorArmsReconcileClock covers the door that was missed: a +// boarding-only round carries no forfeit mappings, so it never enters forfeit +// collection and checkpoints straight out of PartialSigsSentState. Before the +// fix this outbox held only the sig and registration requests, and the +// deposit had no clock at all. +func TestBoardingDoorArmsReconcileClock(t *testing.T) { + t.Parallel() + + const timeout = time.Minute + + h := newRealSigningTestHarness(t) + + intent := h.newTestBoardingIntentWithTapscript() + vtxoReq := h.newTestVTXORequestForIntent(intent) + vtxtTree := h.newTestVTXOTreeForIntents([]types.VTXORequest{vtxoReq}) + + validSigs, err := h.generateValidTreeSignatures(vtxtTree) + require.NoError(t, err) + require.NotEmpty(t, validSigs) + + commitmentTx := h.newCommitmentTxForIntents( + []BoardingIntent{intent}, vtxtTree, + ) + + clientTrees := make(map[SignerKey]*tree.Tree) + clientTrees[NewSignerKey(vtxoReq.SigningKey.PubKey)] = vtxtTree + + roundID := testRoundIDTr("round-boarding-arm-001") + state := &PartialSigsSentState{ + RoundID: roundID, + CommitmentTx: commitmentTx, + VTXOTreePaths: map[int]*tree.Tree{ + 0: vtxtTree, + }, + Intents: Intents{ + Boarding: []BoardingIntent{ + intent, + }, + VTXOs: []types.VTXORequest{ + vtxoReq, + }, + }, + ClientTrees: clientTrees, + BoardingInputIndices: map[wire.OutPoint]int{ + intent.Outpoint: 0, + }, + Musig2Sessions: make(map[SignerKey]*tree.SignerSession), + } + + // The harness leaves the reconcile timeout at zero, which skips the + // arming branch entirely. That default is exactly why every existing + // boarding test read this outbox and saw nothing missing. + h.env.StatusReconcileTimeout = timeout + + h.setupMockWalletForBoardingSigning() + h.setupMockRoundStoreForCommit() + h.withState(state) + + transition, err := h.sendEvent(&OperatorSigned{ + RoundID: roundID, + AggSigs: validSigs, + }) + require.NoError(t, err) + require.NotNil(t, transition) + + // The door has to actually land on the checkpoint, otherwise the + // arming assertion below would pass over a round that never reached + // the hazard window at all. + assertStateType[*InputSigSentState](h.boardingTestHarness) + + var msgs []ClientOutMsg + transition.NewEvents.WhenSome(func(emitted ClientEmittedEvent) { + msgs = emitted.Outbox + }) + assertArmsReconcileFirst(t, msgs, timeout, roundID) +} + +// TestReconcileArmingRespectsDisabled pins the opt-out on both doors: a zero +// timeout means the operator has turned the reconcile off, and neither door +// may arm a timer behind its back. +func TestReconcileArmingRespectsDisabled(t *testing.T) { + t.Parallel() + + roundID := reconcileRoundID(0x33) + state := &ForfeitSignaturesCollectingState{ + RoundID: roundID, + CommitmentTx: newArmingCommitmentTx(), + } + env := newArmingEnv(0) + + msgs := state.forfeitCollectionOutbox( + env, map[wire.OutPoint]*types.ForfeitTxSig{ + reconcileOutpoint(0x44): {}, + }, []*types.BoardingInputSignature{{}}, + ) + + arm, _ := findStartReconcile(msgs) + require.Nil( + t, arm, + "reconcile disabled but the outbox armed the clock anyway", + ) +} + +// TestRestartDoorArmsReconcileClock covers the third door: a checkpointed +// round reloaded from the store on startup. The two live doors build their +// outbox from the transition that carries them into the state, but nothing +// re-derives that outbox across a restart, so recoverActiveRounds has to arm +// the clock itself. A boarding-only round is exactly the case that used to be +// gated out here, which mattered doubly: while the live door was unarmed, the +// restart path was the only thing that could still rescue such a round, and +// it was gated on the same forfeit count. +func TestRestartDoorArmsReconcileClock(t *testing.T) { + t.Parallel() + + h := newActorTestHarness(t) + + roundID := testRoundID("restart-door-arms") + round := h.newTestRound(roundID) + + walletIntent := h.newTestBoardingIntent() + intent, err := buildBoardingIntentFromWallet(walletIntent) + require.NoError(t, err) + + // A boarding-only checkpoint: intents carry no forfeits at all, so the + // old gate skipped this round on the way back up. + h.roundStore.On( + "ListActiveRounds", mock.Anything, + ).Return([]*Round{round}, nil) + h.roundStore.On( + "FetchState", mock.Anything, round.RoundID, + ).Return( + round, + &InputSigSentState{ + RoundID: roundID, + CommitmentTx: round.CommitmentTx.UnwrapOrFail(t), + Intents: Intents{ + Boarding: []BoardingIntent{intent}, + }, + }, + nil, + ) + + require.NoError(t, h.start()) + + h.timeoutActor.assertTimeoutScheduled( + t, + makeTimeoutID( + RoundKeyStr( + roundID.KeyString(), + ), + TimeoutPhaseStatusReconcile, + ), + defaultStatusReconcileTimeout, + ) +} diff --git a/round/fsm_timeouts.go b/round/fsm_timeouts.go index 7c7ed8d31..b876d72a5 100644 --- a/round/fsm_timeouts.go +++ b/round/fsm_timeouts.go @@ -1,5 +1,9 @@ package round +import ( + fn "github.com/lightningnetwork/lnd/fn/v2" +) + // TimeoutPhase identifies which FSM phase owns a timeout. type TimeoutPhase string @@ -22,11 +26,13 @@ const ( // TimeoutPhaseStatusReconcile is the timeout phase for // InputSigSentState's round-status reconcile (wavelength#844). It is - // armed when the forfeit signatures leave the box and re-armed on - // every reconcile probe, so both a received round failure and total - // operator silence (the lumos#618 crash door) eventually drive a - // QueryRoundStatus. The reservation is released only on an - // authoritative dead answer, never on the timeout alone. + // armed on entry to InputSigSentState — for every checkpointed round, + // including a boarding-only one that submits no forfeit signatures + // (wavelength#1051) — and re-armed on every reconcile probe, so both a + // received round failure and total operator silence (the lumos#618 + // crash door) eventually drive a QueryRoundStatus. The reservation is + // released only on an authoritative dead answer, never on the timeout + // alone. TimeoutPhaseStatusReconcile TimeoutPhase = "status-reconcile" ) @@ -76,11 +82,54 @@ func statusReconcileProbeOutbox(roundID RoundID, env *ClientEnvironment, } // cancelStatusReconcileTimeout builds the outbox message that disarms the -// status-reconcile timeout, for the paths that resolve the round's fate -// (confirmation, or an authoritative dead answer). +// status-reconcile timeout, for the paths that resolve the round's fate: a +// confirmation, an authoritative dead answer, or a failure the operator +// delivered directly (which carries the same authority a probe would return). func cancelStatusReconcileTimeout(roundID RoundID) ClientOutMsg { return &CancelTimeoutReq{ RoundKey: RoundKeyStr(roundID.KeyString()), Phase: TimeoutPhaseStatusReconcile, } } + +// reconcileDisarmEvents builds the emitted-event option that disarms the +// status-reconcile clock on an exit from InputSigSentState. The clock is armed +// for the whole of that state, so every exit disarms it; the gate mirrors the +// arm sites so no cancel is emitted for a timer that was never scheduled. +func reconcileDisarmEvents(roundID RoundID, + env *ClientEnvironment) fn.Option[ClientEmittedEvent] { + + if env.StatusReconcileTimeout <= 0 { + return fn.None[ClientEmittedEvent]() + } + + return fn.Some(ClientEmittedEvent{ + Outbox: []ClientOutMsg{ + cancelStatusReconcileTimeout(roundID), + }, + }) +} + +// appendReconcileDisarm adds the status-reconcile disarm to the end of a +// transition's outbox, for exits that have already queued messages of their +// own. Disarming is cleanup and must stay behind every delivery: processOutbox +// abandons the rest of the outbox on the first failing Tell, and a cancel is +// one of the few entries that can fail, so a cancel sitting ahead of a +// notification lets a saturated timeout actor suppress it. Trailing costs +// nothing, since a cancel that never lands only leaks a one-shot timer, which +// fires into a terminal state and self-loops. +func appendReconcileDisarm(transition *ClientStateTransition, roundID RoundID, + env *ClientEnvironment) *ClientStateTransition { + + if env.StatusReconcileTimeout <= 0 { + return transition + } + + emitted := transition.NewEvents.UnwrapOr(ClientEmittedEvent{}) + emitted.Outbox = append( + emitted.Outbox, cancelStatusReconcileTimeout(roundID), + ) + transition.NewEvents = fn.Some(emitted) + + return transition +} diff --git a/round/harness_test.go b/round/harness_test.go index a3c5906f2..ddfe05137 100644 --- a/round/harness_test.go +++ b/round/harness_test.go @@ -98,6 +98,12 @@ func (m *MockRoundStore) FinalizeRound(ctx context.Context, roundID RoundID, return args.Error(0) } +func (m *MockRoundStore) FailRound(ctx context.Context, roundID RoundID) error { + args := m.Called(ctx, roundID) + + return args.Error(0) +} + func (m *MockRoundStore) FailForfeitIntents(ctx context.Context, outpoints []wire.OutPoint, reason string, code RoundFailureCode) error { diff --git a/round/interfaces.go b/round/interfaces.go index ead4dcdc2..011df672c 100644 --- a/round/interfaces.go +++ b/round/interfaces.go @@ -447,6 +447,24 @@ type RoundStore interface { FinalizeRound(ctx context.Context, roundID RoundID, txid chainhash.Hash, confInfo ConfInfo) error + // FailRound retires a checkpointed round whose fate is known to be + // dead, and returns the boarding intents it adopted to the live pool. + // It is the terminal-failure counterpart to FinalizeRound: without it + // the round row stays in ListActiveRounds and is re-hydrated on every + // start, and its deposits stay adopted, which keeps them out of the + // sweep and pinned against the board limit forever. + // + // Intents revert to BoardingStatusConfirmed, not + // BoardingStatusFailed: a dead round proves the commitment never + // broadcast, so nothing on-chain failed and the UTXO is exactly as it + // was before the round. + // + // Retiring a round that is not checkpointed is a no-op, not an error. + // A failure that races a confirmation must not reopen the round that + // won, so the implementation is required to consume only a + // checkpointed row. + FailRound(ctx context.Context, roundID RoundID) error + // FailForfeitIntents terminally fails the pending send intents anchored // to the given forfeited VTXO outpoints, recording the reason and typed // failure code. It is the terminal-failure counterpart to the anchor diff --git a/round/status_reconcile_test.go b/round/status_reconcile_test.go index f2a98bbb9..b1676f58e 100644 --- a/round/status_reconcile_test.go +++ b/round/status_reconcile_test.go @@ -5,11 +5,14 @@ import ( "testing" "time" + "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/chainhash/v2" "github.com/btcsuite/btcd/wire/v2" "github.com/btcsuite/btclog/v2" "github.com/lightninglabs/wavelength/lib/types" "github.com/lightninglabs/wavelength/rpc/roundpb" + fn "github.com/lightningnetwork/lnd/fn/v2" + "github.com/lightningnetwork/lnd/keychain" "github.com/stretchr/testify/require" ) @@ -425,4 +428,422 @@ func TestDeadStatusTerminalCodeRetiresJob(t *testing.T) { require.Equal( t, RoundFailureInsufficientOperatorFunds, notify.FailureCode, ) + + // The disarm is cleanup and must trail the job drop. A rejected + // CancelTimeoutReq aborts the rest of the outbox, so a cancel sitting + // ahead of the notification would let a saturated timeout actor leave + // the pending intent in recoverable replay, which is exactly what + // retiring the job prevents. + cancelIdx := outboxIndexOf[*CancelTimeoutReq](outbox) + require.NotEqual(t, -1, cancelIdx, "dead answer left the clock armed") + + notifyIdx := outboxIndexOf[*TerminalJobFailedNotification](outbox) + require.Less( + t, notifyIdx, cancelIdx, "disarm precedes the job drop, so "+ + "a rejected cancel would suppress it", + ) +} + +// outboxIndexOf returns the position of the first outbox message of type T, +// or -1 when the outbox carries none. Ordering assertions need the position +// rather than mere presence: processOutbox abandons the rest of the outbox on +// the first failing Tell, so whether a message is dispatched before or after a +// fallible send decides what a mid-flight error can strand. +func outboxIndexOf[T ClientOutMsg](outbox []ClientOutMsg) int { + for i, msg := range outbox { + if _, ok := msg.(T); ok { + return i + } + } + + return -1 +} + +// TestBoardingOnlyReconcileTimeoutProbes pins the wavelength#1051 fix on the +// timeout handler. A boarding-only round holds no forfeit reservations, so the +// old forfeit-count gate self-looped its expiry as a defensive no-op. But the +// probe is that round's sole exit from InputSigSentState once the operator has +// rolled the round back before broadcast: no commitment can confirm and no +// failure will ever be delivered. The expiry must therefore probe and re-arm +// exactly as a forfeit-bearing round does. +func TestBoardingOnlyReconcileTimeoutProbes(t *testing.T) { + t.Parallel() + + roundID := reconcileRoundID(0xb1) + s := reconcileState(roundID, nil) + + tr, err := s.ProcessEvent( + context.Background(), &StatusReconcileTimedOut{ + RoundID: roundID, + }, + reconcileEnv(), + ) + require.NoError(t, err) + + next, ok := tr.NextState.(*InputSigSentState) + require.True(t, ok, "expected InputSigSentState, got %T", tr.NextState) + require.Equal(t, uint32(1), next.ReconcileProbes) + + outbox := tr.NewEvents.UnwrapOr(ClientEmittedEvent{}).Outbox + + probe, ok := findOutbox[*QueryRoundStatusOutbox](outbox) + require.True(t, ok, "boarding-only round did not re-probe") + require.Equal(t, roundID, probe.RoundID) + + timeoutReq, ok := findOutbox[*StartTimeoutReq](outbox) + require.True(t, ok, "boarding-only reconcile window not re-armed") + require.Equal(t, TimeoutPhaseStatusReconcile, timeoutReq.Phase) +} + +// TestBoardingOnlyDeliveredFailureDisarms pins the disarm half of the +// wavelength#1051 invariant: the clock is armed for the whole of +// InputSigSentState, so every exit disarms it. The delivered-failure shortcut +// still fails a boarding-only round immediately (it signed nothing away, so +// nothing can strand), but it must now cancel the timer it armed on the way +// in rather than leave a one-shot running against a terminal round. +func TestBoardingOnlyDeliveredFailureDisarms(t *testing.T) { + t.Parallel() + + roundID := reconcileRoundID(0xb2) + s := reconcileState(roundID, nil) + + failure := &BoardingFailed{ + Reason: "operator rolled the round back", + Recoverable: true, + } + + tr, err := s.ProcessEvent(context.Background(), failure, reconcileEnv()) + require.NoError(t, err) + + failed, ok := tr.NextState.(*ClientFailedState) + require.True(t, ok, "expected ClientFailedState, got %T", tr.NextState) + require.Equal(t, failure.Reason, failed.Reason) + + outbox := tr.NewEvents.UnwrapOr(ClientEmittedEvent{}).Outbox + + cancel, ok := findOutbox[*CancelTimeoutReq](outbox) + require.True(t, ok, "delivered failure left the reconcile clock armed") + require.Equal(t, TimeoutPhaseStatusReconcile, cancel.Phase) + require.Equal(t, RoundKeyStr(roundID.KeyString()), cancel.RoundKey) +} + +// TestDeliveredFailureNoDisarmWhenReconcileDisabled pins the other side of +// that gate: with the reconcile opted out no clock was ever armed, so the +// failure path must not emit a cancel for a timer that does not exist. +func TestDeliveredFailureNoDisarmWhenReconcileDisabled(t *testing.T) { + t.Parallel() + + s := reconcileState(reconcileRoundID(0xb3), nil) + + env := reconcileEnv() + env.StatusReconcileTimeout = 0 + + tr, err := s.ProcessEvent(context.Background(), &BoardingFailed{ + Reason: "round failed", + Recoverable: true, + }, env) + require.NoError(t, err) + + _, ok := tr.NextState.(*ClientFailedState) + require.True(t, ok, "expected ClientFailedState, got %T", tr.NextState) + + outbox := tr.NewEvents.UnwrapOr(ClientEmittedEvent{}).Outbox + _, cancelled := findOutbox[*CancelTimeoutReq](outbox) + require.False(t, cancelled, "cancelled a timer that was never armed") +} + +// TestReconcileArmedBeforeFallibleSends pins the ordering the liveness clock +// depends on. processOutbox abandons the rest of the outbox on the first +// failing Tell, and the FSM has already checkpointed into InputSigSentState by +// the time the outbox is dispatched. Arming after the server sends would +// therefore let a mid-flight send error leave a checkpointed round with no +// clock for the rest of the session, reopening the wavelength#1051 strand +// through a different door. +func TestReconcileArmedBeforeFallibleSends(t *testing.T) { + t.Parallel() + + h := newTestHarness(t) + h.env.StatusReconcileTimeout = time.Minute + + intent := h.newTestBoardingIntent() + state := h.newForfeitCollectingState( + testRoundIDTr("round-arm-order"), Intents{ + Boarding: []BoardingIntent{intent}, + }, + nil, + ) + + outbox := state.forfeitCollectionOutbox( + h.env, nil, []*types.BoardingInputSignature{{}}, + ) + + armIdx := outboxIndexOf[*StartTimeoutReq](outbox) + require.NotEqual(t, -1, armIdx, "reconcile clock never armed") + + forfeitSigsIdx := outboxIndexOf[*SubmitVTXOForfeitSigsToServer](outbox) + boardingSigsIdx := outboxIndexOf[*SubmitForfeitSigRequest](outbox) + confRegIdx := outboxIndexOf[*RegisterConfirmationRequest](outbox) + + fallible := map[string]int{ + "SubmitVTXOForfeitSigsToServer": forfeitSigsIdx, + "SubmitForfeitSigRequest": boardingSigsIdx, + "RegisterConfirmationRequest": confRegIdx, + } + + for name, idx := range fallible { + require.NotEqual(t, -1, idx, "%s missing from outbox", name) + require.Lessf( + t, armIdx, idx, "reconcile clock armed after %s, so "+ + "a failed send strands the checkpointed round", + name, + ) + } +} + +// TestConfirmationDisarmTrailsNotifications pins the mirror-image ordering on +// the way out. The confirmation resolves the round's fate, but the disarm is +// cleanup and must never gate delivery: dispatching the cancel first lets a +// saturated or down timeout actor short-circuit processOutbox before +// VTXOCreatedNotification and RoundCompletedNotification, withholding +// already-persisted VTXOs from the manager and leaving onRoundComplete +// unfinalized. A cancel that never lands only leaks a one-shot timer, which +// fires into a terminal state and self-loops. +func TestConfirmationDisarmTrailsNotifications(t *testing.T) { + t.Parallel() + + h := newTestHarness(t) + h.setupMockVTXOStoreForSave() + h.env.StatusReconcileTimeout = time.Minute + + intent := h.newTestBoardingIntent() + state := h.newInputSigSentState( + testRoundIDTr("round-conf-order"), []BoardingIntent{intent}, + ) + h.withState(state) + + _, err := h.sendEvent(&BoardingConfirmed{ + TxID: state.CommitmentTx.UnsignedTx.TxHash(), + BlockHeight: 101, + BlockHash: chainhash.Hash{0x01, 0x02}, + Confirmations: 6, + }) + require.NoError(t, err) + + cancelIdx := outboxIndexOf[*CancelTimeoutReq](h.outboxMessages) + require.NotEqual(t, -1, cancelIdx, "confirmation left the clock armed") + + doneIdx := outboxIndexOf[*RoundCompletedNotification](h.outboxMessages) + require.NotEqual(t, -1, doneIdx, "no RoundCompletedNotification") + + require.Less( + t, doneIdx, cancelIdx, "disarm precedes the terminal "+ + "notifications, so a rejected cancel would "+ + "withhold confirmed funds", + ) +} + +// TestDeadStatusRetiresTheRound pins the durable half of the dead answer. +// Failing the FSM in memory is only half the exit: retirement of the +// persisted round hangs on RoundFailedNotification in the actor, so a dead +// answer that emits no notification leaves the checkpoint row in +// ListActiveRounds and never releases the deposits it adopted: they stay out +// of the sweep and pinned against the board limit, the wavelength#1051 strand +// this reconcile exists to end. The notification must also precede the +// disarm, since a rejected cancel abandons the rest of the outbox. +func TestDeadStatusRetiresTheRound(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + forfeits []types.ForfeitRequest + }{{ + // The wavelength#1051 shape: a boarding-only round holds no + // reservations, so the release is a no-op and the + // notification is the only thing standing between the user + // and a deposit stranded until the CSV expires. + name: "boarding only", + }, { + name: "with forfeits", + forfeits: []types.ForfeitRequest{ + mkForfeit(reconcileOutpoint(0x07), 10_000), + }, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + roundID := reconcileRoundID(0xa8) + s := reconcileState(roundID, tc.forfeits) + + dead := &RoundStatusReported{ + RoundID: roundID, + Status: roundStatusDead, + Detail: "round unknown to operator", + } + + tr, err := s.ProcessEvent( + context.Background(), dead, reconcileEnv(), + ) + require.NoError(t, err) + + outbox := tr.NewEvents.UnwrapOr( + ClientEmittedEvent{}, + ).Outbox + + failed, ok := findOutbox[*RoundFailedNotification]( + outbox, + ) + require.True( + t, ok, "no RoundFailedNotification: the "+ + "round fails in memory but its "+ + "checkpoint row and adopted "+ + "deposits survive", + ) + require.Equal( + t, fn.Some(roundID), failed.RoundID, + "retirement keys on the round id", + ) + require.Equal( + t, "round unknown to operator", failed.Reason, + ) + require.True(t, failed.Recoverable) + + failIdx := outboxIndexOf[*RoundFailedNotification]( + outbox, + ) + cancelIdx := outboxIndexOf[*CancelTimeoutReq](outbox) + require.NotEqual( + t, -1, cancelIdx, + "dead answer left the clock armed", + ) + require.Less( + t, failIdx, cancelIdx, "a rejected cancel "+ + "ahead of the notification would "+ + "suppress the retirement", + ) + }) + } +} + +// TestDeliveredFailureRetiresBoardingOnlyRound covers the other checkpointed +// exit into ClientFailedState: a failure the operator delivered directly to a +// round with nothing reserved. It carries the same authority a probe would +// return, so it retires the durable round for the same reason the dead answer +// does. +func TestDeliveredFailureRetiresBoardingOnlyRound(t *testing.T) { + t.Parallel() + + roundID := reconcileRoundID(0xa9) + s := reconcileState(roundID, nil) + + tr, err := s.ProcessEvent( + context.Background(), + &BoardingFailed{ + RoundID: fn.Some(roundID), + Reason: "operator dropped the round", + Recoverable: true, + }, + reconcileEnv(), + ) + require.NoError(t, err) + + _, ok := tr.NextState.(*ClientFailedState) + require.True(t, ok, "expected ClientFailedState, got %T", tr.NextState) + + outbox := tr.NewEvents.UnwrapOr(ClientEmittedEvent{}).Outbox + + failed, ok := findOutbox[*RoundFailedNotification](outbox) + require.True( + t, ok, "no RoundFailedNotification: the deposit stays "+ + "adopted and out of the sweep", + ) + require.Equal(t, "operator dropped the round", failed.Reason) + + failIdx := outboxIndexOf[*RoundFailedNotification](outbox) + cancelIdx := outboxIndexOf[*CancelTimeoutReq](outbox) + require.NotEqual( + t, -1, cancelIdx, "delivered failure left the clock armed", + ) + require.Less( + t, failIdx, cancelIdx, "a rejected cancel ahead of the "+ + "notification would suppress the retirement", + ) +} + +// TestConfirmedThenFailedDoesNotRetireTheRound is the deliberate exception to +// the rule the two tests above pin, and the reason retirement cannot simply be +// hung off every path into ClientFailedState. +// +// A BoardingConfirmed that fails to build the client VTXOs still fails the +// round in memory, but its commitment is already on-chain. The adopted +// deposits have become VTXOs, and the round row is what keeps them out of the +// boardable and sweepable pools: RoundFailedNotification would retire that row +// and hand back deposits the client no longer owns as UTXOs. Local +// bookkeeping broke; the round itself succeeded. +// +// The exit still disarms the reconcile clock, because the confirmation +// resolved the round's fate and a one-shot left armed would probe a round that +// has settled terminally. +// +// So this test asserts an absence. Routing this exit through +// checkpointedFailureOutbox would look like a tidy unification of the +// ClientFailedState exits and would break silently, in a direction nothing +// else here catches. +func TestConfirmedThenFailedDoesNotRetireTheRound(t *testing.T) { + t.Parallel() + + ownerKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + roundID := reconcileRoundID(0xaa) + s := reconcileState(roundID, nil) + + // A locally-owned VTXO request whose policy template cannot be + // decoded. That is the cheapest way to fail buildClientVTXOs; the + // branch under test does not care which of its errors fired, only + // that the confirmation could not be turned into local VTXOs. + s.Intents.VTXOs = []types.VTXORequest{{ + OwnerKey: keychain.KeyDescriptor{ + PubKey: ownerKey.PubKey(), + }, + PolicyTemplate: []byte{ + 0xff, + }, + }} + + tr, err := s.ProcessEvent( + context.Background(), &BoardingConfirmed{ + BlockHeight: 200, + Confirmations: 1, + }, + reconcileEnv(), + ) + require.NoError(t, err, "the build error is carried into the state") + + failedState, ok := tr.NextState.(*ClientFailedState) + require.True( + t, ok, "expected ClientFailedState, got %T", tr.NextState, + ) + require.Equal(t, "failed to build client VTXOs", failedState.Reason) + + outbox := tr.NewEvents.UnwrapOr(ClientEmittedEvent{}).Outbox + + // The clock is disarmed: the confirmation settled the round. + cancel, ok := findOutbox[*CancelTimeoutReq](outbox) + require.True( + t, ok, "confirmed-then-failed exit left the reconcile "+ + "clock armed on a terminally settled round", + ) + require.Equal(t, TimeoutPhaseStatusReconcile, cancel.Phase) + + // And the round is NOT retired. This is the assertion the test exists + // for. + _, retired := findOutbox[*RoundFailedNotification](outbox) + require.False( + t, retired, "the confirmed-then-failed exit emitted a "+ + "RoundFailedNotification: retiring a round whose "+ + "commitment confirmed re-admits deposits that are "+ + "already VTXOs", + ) } diff --git a/round/transitions.go b/round/transitions.go index 14d2bc3f6..d7fb24e71 100644 --- a/round/transitions.go +++ b/round/transitions.go @@ -264,6 +264,30 @@ func withFailureCode(t *ClientStateTransition, return t } +// checkpointedFailureOutbox builds the failure notification for an exit from +// InputSigSentState into ClientFailedState. +// +// The exits out of that state assemble their own transitions rather than +// routing through failWithNotification or failureOutbox, so each one has to +// carry this message itself. It is not merely observability: retirement of the +// durable round record hangs on RoundFailedNotification in the actor, so a +// checkpointed round that fails without emitting it keeps its input_sig_sent +// row in ListActiveRounds, is re-hydrated on every start, and never releases +// the boarding intents it adopted: they stay out of the sweep and pinned +// against the board limit (wavelength#1051). +func checkpointedFailureOutbox(roundID RoundID, reason string, recoverable bool, + err error) []ClientOutMsg { + + return []ClientOutMsg{ + &RoundFailedNotification{ + RoundID: fn.Some(roundID), + Reason: reason, + Recoverable: recoverable, + OriginalError: err, + }, + } +} + // failureOutbox builds the common failure notification and rollback messages // for a round that failed before forfeit signatures were sent. func failureOutbox(reason string, err error, recoverable bool, @@ -2886,33 +2910,35 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( s.CommitmentTx.UnsignedTx, s.VTXOTreePaths, ) - outboxMsgs := []ClientOutMsg{ - &CancelTimeoutReq{ - RoundKey: RoundKeyStr(s.RoundID.KeyString()), - Phase: TimeoutPhaseForfeitCollection, - }, - &SubmitVTXOForfeitSigsToServer{ - RoundID: s.RoundID, - ForfeitTxs: forfeitTxs, - }, - &RegisterConfirmationRequest{ - CallerID: callerID, - Txid: &txid, - PkScript: pkScript, - TargetConfs: env.OperatorTerms.MinConfirmations, - HeightHint: env.StartHeight, - }, - } + var outboxMsgs []ClientOutMsg // The forfeit signatures leave the box on this transition, opening the // wavelength#844 hazard window: from here on, a round failure (or a // silent operator, the lumos#618 crash door) must resolve through a // status reconcile before the reservations can be released. Arm the - // reconcile timeout so total silence still converges on a probe. A - // boarding-only round has no reservations to reconcile, so it skips - // the timer entirely, matching the forfeit-count gate every consumer - // of the timeout applies. - if len(s.Intents.Forfeits) > 0 && env.StatusReconcileTimeout > 0 { + // reconcile timeout so total silence still converges on a probe. + // + // Every round arms the clock, not only forfeit-bearing ones, because + // the probe serves two distinct purposes. A forfeit-bearing round + // needs it to gate the release on an authoritative dead answer. Any + // round at all needs it as the sole liveness clock in this state: a + // boarding-only round that skips the timer has no exit when the + // operator dies before broadcasting, since no commitment can confirm + // and no failure ever arrives, so the deposit strands until the CSV + // expires (wavelength#1051). + // + // The arm leads every other entry deliberately, the cancel below + // included. processOutbox abandons the rest of the outbox on the first + // failing Tell, and the FSM has already checkpointed into + // InputSigSentState by the time the outbox is dispatched, so arming + // after anything fallible would let a mid-flight error reopen the very + // strand this timer closes: a checkpointed round with no clock for the + // rest of the session. A cancel is one of the entries that can fail -- + // a saturated timeout actor rejects it exactly as it would reject the + // arm -- so the forfeit-collection cancel is no safer to lead with than + // the sends are. Arming first is the safe direction, since a later + // failure merely means the timer fires and probes. + if env.StatusReconcileTimeout > 0 { outboxMsgs = append(outboxMsgs, &StartTimeoutReq{ RoundKey: RoundKeyStr(s.RoundID.KeyString()), Phase: TimeoutPhaseStatusReconcile, @@ -2920,18 +2946,34 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( }) } - if len(boardingInputSigs) == 0 { - return outboxMsgs + // Disarming the forfeit-collection clock trails the arm for the reason + // above. A cancel that never lands only leaks a one-shot timer, and a + // forfeit-collection timeout firing after this transition reaches + // InputSigSentState, which does not handle it, self-loops. + outboxMsgs = append(outboxMsgs, &CancelTimeoutReq{ + RoundKey: RoundKeyStr(s.RoundID.KeyString()), + Phase: TimeoutPhaseForfeitCollection, + }) + + outboxMsgs = append(outboxMsgs, &SubmitVTXOForfeitSigsToServer{ + RoundID: s.RoundID, + ForfeitTxs: forfeitTxs, + }) + + if len(boardingInputSigs) > 0 { + outboxMsgs = append(outboxMsgs, &SubmitForfeitSigRequest{ + RoundID: s.RoundID, + Signatures: boardingInputSigs, + }) } - return append( - outboxMsgs[:2], append([]ClientOutMsg{ - &SubmitForfeitSigRequest{ - RoundID: s.RoundID, - Signatures: boardingInputSigs, - }, - }, outboxMsgs[2:]...)..., - ) + return append(outboxMsgs, &RegisterConfirmationRequest{ + CallerID: callerID, + Txid: &txid, + PkScript: pkScript, + TargetConfs: env.OperatorTerms.MinConfirmations, + HeightHint: env.StartHeight, + }) } func (s *ForfeitSignaturesCollectingState) checkpointRound( @@ -3367,7 +3409,32 @@ func (s *PartialSigsSentState) processEvent(ctx context.Context, ), ) - outboxMsgs := []ClientOutMsg{ + // This is the boarding-only door into InputSigSentState: the + // round carried no forfeit mappings, so it never passed + // through forfeit collection and never saw the arming there. + // It still checkpoints at the same point of no return, which + // makes the reconcile clock its sole liveness timer. Without + // it, an operator that dies before broadcasting leaves a + // client that never restarts with no exit at all: no + // commitment can confirm, no failure ever arrives, and the + // deposit strands until the CSV expires (wavelength#1051). + // + // Arm before the requests go out, not after. processOutbox + // stops at the first failed Tell, so a mailbox hiccup on the + // sig or registration request would otherwise commit this + // state with no clock behind it, unarmed until a restart. + // Arming early is harmless: the probe answers in-flight or + // dead, and both are safe. + outboxMsgs := make([]ClientOutMsg, 0, 3) + if env.StatusReconcileTimeout > 0 { + outboxMsgs = append(outboxMsgs, &StartTimeoutReq{ + RoundKey: RoundKeyStr(s.RoundID.KeyString()), + Phase: TimeoutPhaseStatusReconcile, + Duration: env.StatusReconcileTimeout, + }) + } + outboxMsgs = append( + outboxMsgs, forfeitSigReq, &RegisterConfirmationRequest{ CallerID: callerID, @@ -3376,7 +3443,7 @@ func (s *PartialSigsSentState) processEvent(ctx context.Context, TargetConfs: env.OperatorTerms.MinConfirmations, HeightHint: env.StartHeight, }, - } + ) // Checkpoint the round state at the "point of no return". // After sending boarding input signatures, the server may @@ -4359,17 +4426,36 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, // With no forfeit reservations at stake (a boarding-only // round), nothing can strand and nothing was signed away, so - // the round fails immediately as before. + // the round fails immediately as before. A delivered failure + // carries the same authority a probe would return, so this + // round needs no reconcile; disarm the clock it armed on the + // way into this state. if len(s.Intents.Forfeits) == 0 || env.StatusReconcileTimeout <= 0 { - return &ClientStateTransition{ + + // The round is checkpointed, so failing it in memory is + // only half the exit: the notification is what retires + // the persisted row and hands the adopted deposit back + // to the sweep. The disarm trails it, since a cancel + // ahead of a delivery can suppress that delivery. + transition := &ClientStateTransition{ NextState: &ClientFailedState{ Reason: evt.Reason, Error: evt.Error, Recoverable: evt.Recoverable, FailureCode: evt.FailureCode, }, - }, nil + NewEvents: fn.Some(ClientEmittedEvent{ + Outbox: checkpointedFailureOutbox( + s.RoundID, evt.Reason, + evt.Recoverable, evt.Error, + ), + }), + } + + return appendReconcileDisarm( + transition, s.RoundID, env, + ), nil } // Forfeit signatures are already out, so the notification alone @@ -4403,10 +4489,9 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, // failure resolution, and no status answer. Probe (again) and // re-arm. The timeout alone never fails the round: with // forfeit signatures out, only an authoritative dead answer - // makes the release safe. With no forfeits at stake the - // timeout should not even be armed; self-loop defensively. - if len(s.Intents.Forfeits) == 0 || - env.StatusReconcileTimeout <= 0 { + // makes the release safe, and a boarding-only round needs that + // same answer to learn its deposit will never convert. + if env.StatusReconcileTimeout <= 0 { return selfLoop(s), nil } @@ -4496,6 +4581,12 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, slog.Int("forfeit_count", len(s.Intents.Forfeits)), ) + // The notification rides out with the release. Without it the + // FSM would fail in memory while the checkpoint row stayed + // active and the adopted deposit stayed out of the sweep, + // which is the strand this reconcile exists to end: the dead + // answer proves the round can never finalize, so the durable + // side has to be retired too, not just the in-memory FSM. transition := &ClientStateTransition{ NextState: &ClientFailedState{ Reason: failure.Reason, @@ -4504,9 +4595,10 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, FailureCode: failure.FailureCode, }, NewEvents: fn.Some(ClientEmittedEvent{ - Outbox: []ClientOutMsg{ - cancelStatusReconcileTimeout(s.RoundID), - }, + Outbox: checkpointedFailureOutbox( + s.RoundID, failure.Reason, + failure.Recoverable, failure.Error, + ), }), } @@ -4514,9 +4606,23 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, // the pre-signing states: the commitment can never confirm. // The wrapper also retires the originating job on a // terminal-for-job failure code. - return releaseForfeitsOnFailure( + released, err := releaseForfeitsOnFailure( transition, nil, fn.Some(s.RoundID), s.Intents.Forfeits, ) + if err != nil { + return released, err + } + + // Disarm only once the wrapper has laid down the release and + // the job drop, so the cancel is genuinely last. Seeding it + // into the transition above would bury it mid-outbox: the + // wrapper prepends the release and appends the job-drop + // notification, and since a rejected cancel aborts the rest of + // the outbox while the release is fire-and-forget, that + // ordering would let a saturated timeout actor strand the + // pending intent in recoverable replay -- the one thing the + // job drop exists to prevent. + return appendReconcileDisarm(released, s.RoundID, env), nil case *BoardingConfirmed: env.Log.InfoS(ctx, "Commitment transaction confirmed", @@ -4532,7 +4638,12 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, ) if err != nil { - // Error carried into failed state. + // Error carried into failed state. This is an exit + // from InputSigSentState like any other, so it disarms + // the reconcile clock too: the confirmation already + // resolved the round's fate, and leaving the one-shot + // armed would fire a probe at a round that has settled + // terminally. return &ClientStateTransition{ //nolint:nilerr NextState: &ClientFailedState{ Reason: "failed to build client " + @@ -4540,6 +4651,9 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, Error: err, Recoverable: false, }, + NewEvents: reconcileDisarmEvents( + s.RoundID, env, + ), }, nil } @@ -4603,16 +4717,7 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, outflows := roundLedgerOutflows(s.RoundID, s.Intents) // Build outbox messages starting with standard notifications. - // The confirmation resolves the round's fate, so any armed - // status-reconcile probe is disarmed first. outbox := make([]ClientOutMsg, 0, 3) - if len(s.Intents.Forfeits) > 0 && - env.StatusReconcileTimeout > 0 { - - outbox = append( - outbox, cancelStatusReconcileTimeout(s.RoundID), - ) - } if len(vtxos) > 0 || len(outflows) > 0 || operatorFee > 0 { outbox = append(outbox, &VTXOCreatedNotification{ VTXOs: vtxos, @@ -4642,6 +4747,22 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, }) } + // The confirmation resolves the round's fate, so any armed + // status-reconcile probe is disarmed here. The disarm trails + // the notifications above rather than leading them: since + // processOutbox abandons the rest of the outbox on the first + // failing Tell, a saturated or down timeout actor would + // otherwise withhold confirmed funds from the VTXO manager and + // leave onRoundComplete unfinalized, even though the VTXOs are + // already persisted. Cleanup must never gate delivery. A + // cancel that never lands only leaks a one-shot timer, which + // fires into a terminal state and self-loops. + if env.StatusReconcileTimeout > 0 { + outbox = append( + outbox, cancelStatusReconcileTimeout(s.RoundID), + ) + } + return &ClientStateTransition{ NextState: &ConfirmedState{ TxID: evt.TxID,