From 42118a70e133b7df844316f7b948ac07ba5e0714 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 27 Jul 2026 16:52:08 -0700 Subject: [PATCH 1/9] round: arm the status-reconcile clock for every checkpointed round In this commit, we widen the status-reconcile timer so it covers boarding-only rounds, not just the forfeit-bearing ones. The timer was introduced for wavelength#844, where the hazard is releasing forfeit reservations before the round's fate is known, so every site gated itself on a non-empty forfeit set. That gate reads as an optimization, but it quietly leaves boarding-only rounds with no liveness clock at all. Once a round reaches InputSigSentState its only exits are a commitment confirmation, a delivered failure, or the reconcile probe. When the operator rolls the round back before broadcast, no commitment can ever confirm and there is no durable round left to fail the client with, so a boarding-only round parks in InputSigSentState forever: the deposit never converts, nothing surfaces to the user, and only the CSV timeout gets the funds back. The probe already handles this exact answer, since a dead status fails the round and a release over an empty forfeit set is a no-op. We arm the clock on both doors into the state, the live signing transition and the restart path that reloads a checkpointed round, and teach the timeout handler to probe rather than self-loop when the round carries no forfeits. The two exits that previously left the timer running for these rounds now cancel it: the delivered-failure shortcut and the confirmation path, which both drop their forfeit-count gate. That leaves a simpler invariant than the one it replaces, since the clock is armed for the whole of InputSigSentState and every exit disarms it. Because that clock is now the only thing keeping a checkpointed round from parking forever, the ordering around it matters. processOutbox abandons the rest of the outbox on the first failing Tell, so the arm moves ahead of the fallible server sends: arming last would let a mid-flight send error reopen the same strand through a different door. The disarms move the other way, behind every delivery: on the confirmation path behind the VTXO and round-completion notifications, and on the dead-answer path behind the forfeit release and the job drop. A cancel is one of the few outbox entries that can fail, so leading with one lets a saturated timeout actor withhold already-persisted funds from the manager, or strand a pending intent in recoverable replay that the job drop exists to retire. Cleanup must never gate delivery. The confirmation path's VTXO-build error exit disarms as well, so the every-exit half of the invariant holds on every branch rather than most of them. The new cases in status_reconcile_test.go pin both halves: a boarding-only round probes and re-arms on expiry and disarms on a delivered failure, and the two orderings above are asserted by position rather than by presence. Fixes #1051 --- round/actor.go | 20 ++-- round/fsm_timeouts.go | 63 ++++++++-- round/status_reconcile_test.go | 212 +++++++++++++++++++++++++++++++++ round/transitions.go | 134 +++++++++++++-------- 4 files changed, 367 insertions(+), 62 deletions(-) diff --git a/round/actor.go b/round/actor.go index 54608407e..3a6c0965d 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( 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/status_reconcile_test.go b/round/status_reconcile_test.go index f2a98bbb9..40619fec4 100644 --- a/round/status_reconcile_test.go +++ b/round/status_reconcile_test.go @@ -425,4 +425,216 @@ 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", + ) } diff --git a/round/transitions.go b/round/transitions.go index 14d2bc3f6..5ad45cff9 100644 --- a/round/transitions.go +++ b/round/transitions.go @@ -2891,28 +2891,32 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( 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, - }, } // 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 the fallible sends below deliberately. 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 last would let a mid-flight send + // error reopen the very strand this timer closes: a checkpointed round + // with no clock for the rest of the session. Arming first is the safe + // direction, since a later send 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 +2924,25 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( }) } - if len(boardingInputSigs) == 0 { - return outboxMsgs + 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( @@ -4359,7 +4370,10 @@ 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{ @@ -4369,6 +4383,9 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, Recoverable: evt.Recoverable, FailureCode: evt.FailureCode, }, + NewEvents: reconcileDisarmEvents( + s.RoundID, env, + ), }, nil } @@ -4403,10 +4420,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 } @@ -4503,20 +4519,29 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, Recoverable: failure.Recoverable, FailureCode: failure.FailureCode, }, - NewEvents: fn.Some(ClientEmittedEvent{ - Outbox: []ClientOutMsg{ - cancelStatusReconcileTimeout(s.RoundID), - }, - }), } // The release is safe here for the same reason it is safe in // 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 +4557,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 +4570,9 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, Error: err, Recoverable: false, }, + NewEvents: reconcileDisarmEvents( + s.RoundID, env, + ), }, nil } @@ -4603,16 +4636,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 +4666,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, From adcea022437aef37d8ade663c579cdcb8546678f Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 7 Aug 2026 16:49:03 -0700 Subject: [PATCH 2/9] round: arm the reconcile clock on the boarding-only door too In this commit, we close the last unarmed door into InputSigSentState. The parent commit arms two of the three: the forfeit-bearing transition through ForfeitSignaturesCollectingState, and the restart reload in Start. The third is a boarding-only round, which never enters forfeit collection at all. PartialSigsSentState takes the len(ForfeitMappings) == 0 branch on OperatorSigned, signs its boarding inputs, and checkpoints straight into InputSigSentState with an outbox holding only the sig and registration requests. That door is the one wavelength#1051 walks through. The round sits at the same point of no return, but with no reconcile clock the state has no liveness timer at all: no commitment can confirm, no failure ever arrives, and a client that never restarts leaves the deposit stranded until the CSV expires. Only the restart path recovered it, which is why the DST catch converged and this stayed invisible in review. The tests assert the invariant rather than the doors, since a fourth door added later should fail rather than quietly repeat this: whatever path reaches the checkpoint must arm, and must arm ahead of anything that can fail on the way out. They also cover the parent's ordering fix and the disabled opt-out. Worth recording why no existing test caught it. The boarding harness leaves StatusReconcileTimeout at zero, which skips the arming branch outright, so every boarding test that read this outbox saw exactly what it expected to see. --- round/AGENTS.md | 41 ++++- round/CLAUDE.md | 41 ++++- round/checkpoint_arming_test.go | 302 ++++++++++++++++++++++++++++++++ round/transitions.go | 29 ++- 4 files changed, 407 insertions(+), 6 deletions(-) create mode 100644 round/checkpoint_arming_test.go 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/checkpoint_arming_test.go b/round/checkpoint_arming_test.go new file mode 100644 index 000000000..db729472a --- /dev/null +++ b/round/checkpoint_arming_test.go @@ -0,0 +1,302 @@ +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) + + for i, msg := range msgs[:idx] { + switch msg.(type) { + // A cancel of some other phase is bookkeeping on a timer that + // is already running, so it may precede the arm. + case *CancelTimeoutReq: + continue + + default: + 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/transitions.go b/round/transitions.go index 5ad45cff9..e712bc555 100644 --- a/round/transitions.go +++ b/round/transitions.go @@ -3378,7 +3378,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, @@ -3387,7 +3412,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 From beaa823b24a9b062201442eb16d23581e180640b Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 7 Aug 2026 19:14:08 -0700 Subject: [PATCH 3/9] db: add query to revert a round-adopted boarding intent In this commit, we add the query that returns a boarding intent adopted by a dead round to the confirmed pool. It is the inverse of the adopt write CommitState performs at the checkpoint. The status reverts to confirmed rather than failed. A dead round proves the commitment was never broadcast, so nothing on-chain failed and the UTXO is exactly as it was before the round started. Confirmed is also the status both recovery routes key on, so restoring it gives the deposit back to the boardable pool and to the sweep at once. The update is guarded on the intent still being adopted so a sweep that has already claimed the deposit is never dragged back. --- db/sqlc/queries/round.sql | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/db/sqlc/queries/round.sql b/db/sqlc/queries/round.sql index 13d197d64..3a19f7236 100644 --- a/db/sqlc/queries/round.sql +++ b/db/sqlc/queries/round.sql @@ -52,6 +52,15 @@ SET status = 'confirmed', last_update_time = $5 WHERE round_id = $1; +-- 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 From 28a9d2929704836a195332105c62bab5d8a0f00c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 7 Aug 2026 19:14:08 -0700 Subject: [PATCH 4/9] db: regenerate sqlc stubs for the boarding intent revert --- db/sqlc/querier.go | 5 +++++ db/sqlc/round.sql.go | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/db/sqlc/querier.go b/db/sqlc/querier.go index 875610f5e..0862694f4 100644 --- a/db/sqlc/querier.go +++ b/db/sqlc/querier.go @@ -362,6 +362,11 @@ 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) + // 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/round.sql.go b/db/sqlc/round.sql.go index d710d0030..1d0adf263 100644 --- a/db/sqlc/round.sql.go +++ b/db/sqlc/round.sql.go @@ -1147,6 +1147,27 @@ func (q *Queries) MarkVTXOSpent(ctx context.Context, arg MarkVTXOSpentParams) er return err } +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 From 9caa903d479d260031c7517872bd5196f657338e Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 7 Aug 2026 19:14:22 -0700 Subject: [PATCH 5/9] round+db: retire dead rounds and give the deposit back In this commit, we close the other half of wavelength#1051. Arming the reconcile clock un-parks the client FSM, but a dead round still has to return the money, and nothing did: RoundStore had no counterpart to FinalizeRound, so a round entering ClientFailedState kept its checkpoint row. reapFailedRounds only drops the in-memory FSM. Two things stayed stuck as a result. The row sat in ListActiveRounds and was re-hydrated on every start, re-arming and re-probing a round already known to be dead. Worse, the boarding intents it adopted stayed adopted, and boardingIntentSweepable excludes adopted, so the deposit was neither boardable nor sweepable. Not before the CSV and not after it either, since nothing writes the expired status. The user saw coins pinned against the board limit with no way to reach them. The recovery machinery was already built and simply unreachable. ListBoardingIntentsByStatus and ListBoardingIntentsBySweepableStatuses both re-admit a confirmed intent exactly when its linked round reads failed, and round_statuses has carried a failed row since the schema landed, commented "Round failed, intents may need recovery". Nothing ever wrote it. So FailRound writes it, and reverts the round's adopted intents in the same transaction. One transaction because the row and the intent statuses are a single fact about where the deposit lives, and a crash between the halves would strand it in a round that no longer exists. We reuse the existing UpdateRoundStatus rather than add a query, since unlike FinalizeRound this is purely a status move. The actor calls it from the RoundFailedNotification handler, the one choke point every failure path already passes through, so no individual FSM exit has to remember. A round that never reached the checkpoint has no row and no adopted intents, making both halves no-op updates, so the call needs no guard. A store error is logged rather than propagated: the round has already failed and the client has already been told, so a failed write means the row is reclaimed on a later pass, not that the failure is in doubt. --- db/round_store.go | 58 +++++++++++++ db/round_store_test.go | 179 +++++++++++++++++++++++++++++++++++++++++ round/actor.go | 45 +++++++++++ round/actor_test.go | 56 +++++++++++++ round/harness_test.go | 6 ++ round/interfaces.go | 13 +++ 6 files changed, 357 insertions(+) diff --git a/db/round_store.go b/db/round_store.go index cafaaf85c..13852c3d8 100644 --- a/db/round_store.go +++ b/db/round_store.go @@ -208,6 +208,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 +806,61 @@ 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). +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 { + 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) + } + } + + // Reuse the generic status setter rather than adding a + // dedicated query: unlike FinalizeRound, which also records + // the txid and confirmation height, retiring a round is + // purely a status move. + return q.UpdateRoundStatus(ctx, sqlc.UpdateRoundStatusParams{ + RoundID: roundIDStr, + Status: "failed", + LastUpdateTime: nowUnix, + }) + }) +} + // 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..65dad73c0 100644 --- a/db/round_store_test.go +++ b/db/round_store_test.go @@ -2077,3 +2077,182 @@ 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", + ) +} diff --git a/round/actor.go b/round/actor.go index 3a6c0965d..6afc3e30b 100644 --- a/round/actor.go +++ b/round/actor.go @@ -2250,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, @@ -2754,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/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..282b6ccba 100644 --- a/round/interfaces.go +++ b/round/interfaces.go @@ -447,6 +447,19 @@ 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. + 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 From f767dcd62f2b4e014a5507e3725c8693663c714a Mon Sep 17 00:00:00 2001 From: lightninglabs-deploy <58193817+lightninglabs-deploy@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:18:35 +0000 Subject: [PATCH 6/9] round: retire the round on the checkpointed failure exits In this commit, we make the retirement added in the previous commit actually reachable. Retirement hangs on RoundFailedNotification in the actor, and the comment on TestFailedRoundIsRetiredDurably states the premise it relies on: that the notification is the single choke point every failure path passes through. That is true of the paths built by failWithNotification and failureOutbox, and false of the two exits out of InputSigSentState, which assemble their transitions by hand. The dead-answer branch is the one that matters. It builds a ClientFailedState directly and hands it to releaseForfeitsOnFailure, which prepends the rollback and, for a boarding-only round, returns early on an empty one. So the outbox that leaves the authoritative dead verdict is a lone CancelTimeoutReq: the FSM fails in memory while the checkpoint row stays in ListActiveRounds and the deposits it adopted stay adopted, out of the sweep and pinned against the board limit. That is the wavelength#1051 strand the reconcile exists to end, still open on the exact path the reconcile added. The delivered-failure exit for a round with nothing reserved has the same shape and the same consequence, so it emits the notification too. Both keep the disarm trailing, since a cancel ahead of a delivery lets a saturated timeout actor suppress that delivery. The BoardingConfirmed error path is deliberately left alone: its commitment confirmed, so retiring the round would re-admit intents whose deposit already became a VTXO. The tests drive the FSM rather than feeding the notification to processOutbox by hand, which is what let the gap through: all three subcases fail without the fix. --- round/status_reconcile_test.go | 130 +++++++++++++++++++++++++++++++++ round/transitions.go | 59 +++++++++++++-- 2 files changed, 184 insertions(+), 5 deletions(-) diff --git a/round/status_reconcile_test.go b/round/status_reconcile_test.go index 40619fec4..5f17670d3 100644 --- a/round/status_reconcile_test.go +++ b/round/status_reconcile_test.go @@ -10,6 +10,7 @@ import ( "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/stretchr/testify/require" ) @@ -638,3 +639,132 @@ func TestConfirmationDisarmTrailsNotifications(t *testing.T) { "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", + ) +} diff --git a/round/transitions.go b/round/transitions.go index e712bc555..590309067 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, @@ -4401,17 +4425,30 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, // 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, }, - NewEvents: reconcileDisarmEvents( - s.RoundID, env, - ), - }, 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 @@ -4537,6 +4574,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, @@ -4544,6 +4587,12 @@ func (s *InputSigSentState) ProcessEvent(ctx context.Context, event ClientEvent, Recoverable: failure.Recoverable, FailureCode: failure.FailureCode, }, + NewEvents: fn.Some(ClientEmittedEvent{ + Outbox: checkpointedFailureOutbox( + s.RoundID, failure.Reason, + failure.Recoverable, failure.Error, + ), + }), } // The release is safe here for the same reason it is safe in From 3eaa7fd386158484c7faaa76ec4cf57f8340ed07 Mon Sep 17 00:00:00 2001 From: lightninglabs-deploy <58193817+lightninglabs-deploy@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:18:53 +0000 Subject: [PATCH 7/9] round: arm the reconcile clock ahead of the forfeit cancel In this commit, we close the last gap in the arming invariant. The forfeit-collection door arms the clock ahead of the sig and registration requests, on the reasoning that processOutbox abandons the outbox at the first failing Tell and the FSM has already checkpointed by dispatch time. The cancel of the forfeit-collection timeout still led the outbox, though, and a cancel is a Tell to the timeout actor like any other: the comment on releaseForfeitsOnFailure names CancelTimeoutReq as an entry that can fail mid-flight and short-circuit the rest, and this PR's own disarm site orders around a saturated timeout actor rejecting one. A rejection there commits the checkpoint with no clock until a restart, which is the strand the arm exists to prevent. So the arm leads the whole outbox and the cancel trails it. Trailing costs nothing: a cancel that never lands leaks a one-shot timer, and a forfeit-collection timeout arriving after the transition reaches InputSigSentState, which has no case for it, self-loops. assertArmsReconcileFirst carried the exemption that hid this, treating a cancel as bookkeeping that may precede the arm. It no longer does, so the invariant the helper claims is the one it checks: both forfeit-door subcases fail without the reorder. --- round/checkpoint_arming_test.go | 19 +++++++----------- round/transitions.go | 35 ++++++++++++++++++++------------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/round/checkpoint_arming_test.go b/round/checkpoint_arming_test.go index db729472a..09db5fffe 100644 --- a/round/checkpoint_arming_test.go +++ b/round/checkpoint_arming_test.go @@ -93,19 +93,14 @@ func assertArmsReconcileFirst(t *testing.T, msgs []ClientOutMsg, 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] { - switch msg.(type) { - // A cancel of some other phase is bookkeeping on a timer that - // is already running, so it may precede the arm. - case *CancelTimeoutReq: - continue - - default: - 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) - } + 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) } } diff --git a/round/transitions.go b/round/transitions.go index 590309067..d7fb24e71 100644 --- a/round/transitions.go +++ b/round/transitions.go @@ -2910,12 +2910,7 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( s.CommitmentTx.UnsignedTx, s.VTXOTreePaths, ) - outboxMsgs := []ClientOutMsg{ - &CancelTimeoutReq{ - RoundKey: RoundKeyStr(s.RoundID.KeyString()), - Phase: TimeoutPhaseForfeitCollection, - }, - } + 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 @@ -2932,14 +2927,17 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( // and no failure ever arrives, so the deposit strands until the CSV // expires (wavelength#1051). // - // The arm leads the fallible sends below deliberately. 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 last would let a mid-flight send - // error reopen the very strand this timer closes: a checkpointed round - // with no clock for the rest of the session. Arming first is the safe - // direction, since a later send failure merely means the timer fires - // and probes. + // 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()), @@ -2948,6 +2946,15 @@ func (s *ForfeitSignaturesCollectingState) forfeitCollectionOutbox( }) } + // 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, From 3f749d868eaf85ac543941f0678ff65746a13bbb Mon Sep 17 00:00:00 2001 From: lightninglabs-deploy <58193817+lightninglabs-deploy@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:22:52 +0000 Subject: [PATCH 8/9] round+db: scope round retirement to checkpointed rows In this commit, we make FailRound structurally unable to retire a round that is not checkpointed, closing the gap between the two halves of the retirement. The intent half was already guarded on 'adopted', so a deposit some other path had claimed was never dragged back. The round half went through UpdateRoundStatus, which is keyed on round_id alone and would stamp 'failed' over any status at all. That asymmetry matters more than it looks, because the round row is not bookkeeping: it is the gate. Both re-admission queries decide whether an adopted deposit is boardable or sweepable by joining through round_boarding_intents to the round row and asking whether it reads 'failed'. A confirmed round is exactly what keeps a deposit out of those pools once its commitment is on-chain and the UTXO has become a VTXO. Retiring such a round would therefore not merely mislabel a row, it would offer an already-spent outpoint back for a fresh board. Nothing produces that ordering today, since the FSM cannot fire RoundFailedNotification for a round it has already seen confirm. This is a structural guard, not a fix for a live bug, and the point is that a future caller cannot reintroduce the hazard by accident. We add a new query rather than tightening UpdateRoundStatus, which is a generic setter exposed on both sqlc.Querier and db.RoundStore. Silently narrowing it would turn every other status move into a conditional no-op under a name that promises otherwise, which is a worse trap than the one being closed. RetireCheckpointedRound is guarded on 'input_sig_sent', the exact status ListActiveRounds keys on, and returns its row count. FailRound leads with it and gives the deposits back only if it actually consumed a row, so a failure that races a confirmation touches neither half. The sqlc stubs were regenerated by hand because Docker is unavailable in this environment; CI's sqlc-check is the authority on them. Co-Authored-By: Claude Opus 5 --- db/round_store.go | 40 +++++++++++---- db/round_store_test.go | 102 ++++++++++++++++++++++++++++++++++++++ db/sqlc/querier.go | 8 +++ db/sqlc/queries/round.sql | 12 +++++ db/sqlc/round.sql.go | 26 ++++++++++ round/interfaces.go | 5 ++ 6 files changed, 184 insertions(+), 9 deletions(-) diff --git a/db/round_store.go b/db/round_store.go index 13852c3d8..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, @@ -822,6 +825,14 @@ func (s *RoundPersistenceStore) FinalizeRound(ctx context.Context, // 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 { @@ -830,6 +841,25 @@ func (s *RoundPersistenceStore) FailRound(ctx context.Context, 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) @@ -849,15 +879,7 @@ func (s *RoundPersistenceStore) FailRound(ctx context.Context, } } - // Reuse the generic status setter rather than adding a - // dedicated query: unlike FinalizeRound, which also records - // the txid and confirmation height, retiring a round is - // purely a status move. - return q.UpdateRoundStatus(ctx, sqlc.UpdateRoundStatusParams{ - RoundID: roundIDStr, - Status: "failed", - LastUpdateTime: nowUnix, - }) + return nil }) } diff --git a/db/round_store_test.go b/db/round_store_test.go index 65dad73c0..b23b7619a 100644 --- a/db/round_store_test.go +++ b/db/round_store_test.go @@ -2256,3 +2256,105 @@ func TestRoundStoreFailRoundLeavesSweptDepositAlone(t *testing.T) { "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 0862694f4..3db4af9bb 100644 --- a/db/sqlc/querier.go +++ b/db/sqlc/querier.go @@ -362,6 +362,14 @@ 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 diff --git a/db/sqlc/queries/round.sql b/db/sqlc/queries/round.sql index 3a19f7236..79e1ac459 100644 --- a/db/sqlc/queries/round.sql +++ b/db/sqlc/queries/round.sql @@ -52,6 +52,18 @@ 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 diff --git a/db/sqlc/round.sql.go b/db/sqlc/round.sql.go index 1d0adf263..4be7f6d9f 100644 --- a/db/sqlc/round.sql.go +++ b/db/sqlc/round.sql.go @@ -1147,6 +1147,32 @@ 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 diff --git a/round/interfaces.go b/round/interfaces.go index 282b6ccba..011df672c 100644 --- a/round/interfaces.go +++ b/round/interfaces.go @@ -458,6 +458,11 @@ type RoundStore interface { // 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 From 84a4fdc6bc9781cb01d0867a9c49682babf13e04 Mon Sep 17 00:00:00 2001 From: lightninglabs-deploy <58193817+lightninglabs-deploy@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:22:52 +0000 Subject: [PATCH 9/9] round: pin the confirmed-then-failed exit as non-retiring In this commit, we add a test asserting an absence: the BoardingConfirmed -> buildClientVTXOs error exit in InputSigSentState must emit the reconcile disarm and must NOT emit a RoundFailedNotification. Retirement now hangs entirely on that notification, and the two exits either side of this one both emit it. That makes this branch look like an oversight, and routing it through checkpointedFailureOutbox looks like a tidy unification of the ClientFailedState exits. It is not. The commitment already confirmed on this path. The adopted deposits have become VTXOs, and the round row is precisely what keeps them out of the boardable and sweepable pools, since both re-admission queries join through it and re-admit only on 'failed'. Retiring the round would hand back deposits the client no longer holds as UTXOs and offer already-spent outpoints for a fresh board. What failed here is local bookkeeping; the round itself succeeded. The disarm is still correct and still asserted: the confirmation resolved the round's fate, so a one-shot left armed would fire a probe at a round that has settled terminally. Nothing pinned either half of that reasoning, so a refactor could have made the change silently. Both assertions were verified to fail against a mutated transition: routing the branch through checkpointedFailureOutbox trips the retirement check, and dropping reconcileDisarmEvents trips the disarm check. Co-Authored-By: Claude Opus 5 --- round/status_reconcile_test.go | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/round/status_reconcile_test.go b/round/status_reconcile_test.go index 5f17670d3..b1676f58e 100644 --- a/round/status_reconcile_test.go +++ b/round/status_reconcile_test.go @@ -5,12 +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" ) @@ -768,3 +770,80 @@ func TestDeliveredFailureRetiresBoardingOnlyRound(t *testing.T) { "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", + ) +}