round: arm the status-reconcile clock for every checkpointed round - #1052
round: arm the status-reconcile clock for every checkpointed round#1052Roasbeef wants to merge 9 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60e9fc5be3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if env.StatusReconcileTimeout <= 0 { | ||
| return selfLoop(s), nil |
There was a problem hiding this comment.
When this newly enabled boarding-only timeout receives a ROUND_STATUS_DEAD response, the existing branch only changes the in-memory FSM to ClientFailedState; the checkpoint row remains input_sig_sent and its boarding intents remain BoardingStatusAdopted. Consequently ListActiveRounds reloads the round after every restart, while the boarding sweep actor explicitly excludes adopted intents, so the deposit still cannot enter CSV recovery. The dead path needs to retire the persisted checkpoint and mark its boarding intents failed, not merely stop the in-memory reconciliation loop.
AGENTS.md reference: round/AGENTS.md:L167-L171
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex You are right on the facts, and this is the most important finding on the PR, but I am deliberately not fixing it here.
Verified both halves independently:
RoundStoreexposes onlyListActiveRoundsandFinalizeRound(round/interfaces.go), with no fail or delete path, so a round enteringClientFailedStateleaves its checkpoint row active and gets re-hydrated on every restart.reapFailedRoundsonly drops the in-memory FSM.- Nothing in production ever writes
BoardingStatusFailedorBoardingStatusExpired— grep turns up only the enum definitions and the sweep filter — andboardingIntentSweepablereturnsfalseforAdopted(wallet/boarding_sweep_actor.go), so the deposit does not enter early sweep recovery.
So the honest scope of this PR is narrower than its body claimed: it un-parks the FSM, but the money still does not come back before CSV. I have said so in the PR description.
Why it is not a fixup: the delivered-failure path has had this identical shape since well before this change, so it is inherited, not introduced. Closing it needs a new outbox message plus wallet/DB plumbing to revert Adopted and retire the persisted checkpoint — a separate change with its own tests, not something to bury in a fixup on this one. Flagging for a follow-up rather than filing unilaterally.
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
|
This PR's Go changes touch one documented package — diff --git a/round/AGENTS.md b/round/AGENTS.md
index 6c0997e3..24377625 100644
--- a/round/AGENTS.md
+++ b/round/AGENTS.md
@@ -185,12 +185,13 @@ state transitions and validation rules live under [Invariants](#invariants).
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
+ forfeit collection), and the `ListActiveRounds` resume loop in
+ `RoundClientActor.Start` 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
diff --git a/round/CLAUDE.md b/round/CLAUDE.md
index 6c0997e3..24377625 100644
--- a/round/CLAUDE.md
+++ b/round/CLAUDE.md
@@ -185,12 +185,13 @@ state transitions and validation rules live under [Invariants](#invariants).
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
+ forfeit collection), and the `ListActiveRounds` resume loop in
+ `RoundClientActor.Start` 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 checkpointedThe rest of the reconcile documentation this PR added checks out against the How to apply: save the diff above and
https://github.com/lightninglabs/wavelength/actions?query=branch%3Aboarding-checkpoint-liveness |
Roasbeef
added
the
backport-v0.1.x-branch
|
@claude review this |
|
Claude finished @Roasbeef's task in 5m 42s —— View job Review:
|
60e9fc5 to
c4986ec
Compare
|
Pushed a squashed update covering the review round (Codex, FixedCodex P1 — arm the reconcile timeout before the fallible sends. Confirmed and fixed. Codex P2 — cancel after the terminal notifications on the confirmation path. Confirmed and fixed, and it pairs with the above as one rule: the arm leads the fallible sends, the disarm trails them. Cleanup must never gate delivery. A cancel that never lands only leaks a one-shot timer that fires into a terminal state and self-loops; a cancel that runs first could withhold already-persisted VTXOs from the manager and leave Fable — the Fable — two stale comments in Tests. The PR previously carried none, which was the fair criticism. Five cases in Not fixed here — real, but pre-existing and wider than this PRCodex P1 — dead rounds are not retired durably, and boarding intents stay The consequence is that this PR un-parks the FSM but does not yet get the money back before CSV, which is narrower than the PR body claims. The delivered-failure path has had this same shape since before this change, so fixing it means a new outbox message plus wallet/DB plumbing to revert Fable — the post-admission span (
|
|
@claude I pushed a squashed update addressing your review and the Codex findings. Please re-review Actioned: the reconcile arm now leads the fallible sends in On your question — "does anything move a boarding intent out of Two things worth your attention on the re-review: whether the arm-leads/disarm-trails ordering rule is now applied consistently across every site in this state, and whether the new tests actually pin the behavior rather than restating the implementation. |
|
Claude finished @Roasbeef's task in 3m 54s —— View job Re-review of
|
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
43c2adf to
e9535bd
Compare
|
@claude Addressed the dead-answer ordering exception you found in the re-review. Verified it first: Took the local fix rather than making
So the rule is now uniform across all four exits: the arm leads the fallible sends, the disarm trails every delivery. |
|
Claude finished @Roasbeef's task in 2m 27s —— View job Re-review of the dead-answer ordering fixTraced the fix for the exception I flagged in the last round. It's correct and it closes the last asymmetry cleanly. The ordering now holds on the dead-answer door. The transition seeded into So the cancel is genuinely last. The rule is now uniform across all four exits (arm leads fallible sends, disarm trails every delivery):
The forfeit-bearing Choosing the local fix over global best-effort is the right call for this PR. Making The test pins behavior by position. VerdictThe dead-answer exception is resolved, and the arm-leads / disarm-trails invariant is now genuinely uniform across every I couldn't run |
e9535bd to
d70053e
Compare
|
Closed the last coverage gap from the re-review: It reloads a boarding-only checkpointed round through All three doors and all four exits now have direct coverage. |
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.
d70053e to
adcea02
Compare
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.
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.
|
@codex Following up on your P1 about dead rounds not being persisted: rather than leave it as a follow-up, we folded the fix into this PR so the whole story reviews as one change. Three new commits. Your diagnosis was right on both halves, and I verified each independently. The interesting part is that the recovery machinery was already built and simply unreachable. Two judgment calls worth flagging for review. Intents revert to
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9caa903d47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| m.RoundID.WhenSome(func(id RoundID) { | ||
| a.retireFailedRound(ctx, id) | ||
| }) |
There was a problem hiding this comment.
Fresh evidence in the updated code shows that the RoundStatusReported(DEAD) branch constructs ClientFailedState and emits only the forfeit rollback/job-drop and timeout cancellation; it never emits a RoundFailedNotification. Consequently this new handler is never reached for the dead-status path it is meant to fix, so FailRound is not called and the checkpoint remains input_sig_sent with its boarding deposits still adopted. Emit the retirement directly from that transition or include a RoundFailedNotification in its outbox.
AGENTS.md reference: round/AGENTS.md:L182-L194
Useful? React with 👍 / 👎.
| RoundKey: RoundKeyStr(s.RoundID.KeyString()), | ||
| Phase: TimeoutPhaseStatusReconcile, | ||
| Duration: env.StatusReconcileTimeout, | ||
| }) |
There was a problem hiding this comment.
Fresh evidence in the updated outbox shows that this arm is still appended after the leading CancelTimeoutReq at lines 2889-2894. If the timeout actor rejects that cancellation, processOutbox returns before scheduling this timer even though the FSM checkpoint has already committed InputSigSentState, leaving the round parked without reconciliation until a daemon restart. Put the reconcile arm before the cancellation as well, or make the old-timeout cancellation best-effort.
AGENTS.md reference: round/AGENTS.md:L195-L200
Useful? React with 👍 / 👎.
|
Pushed two commits addressing the open review threads on
The delivered-failure exit for a round with nothing reserved has the same shape and is fixed alongside it. The
Still open: @codex's older "Persist dead boarding rounds before leaving reconciliation" thread. That was the same defect from the other end, and On the red CI: infrastructure, not the code. All four genuine failures on run 31234528204 ( Verified locally: @codex please re-review the two commits above. |
|
To use Codex here, create a Codex account and connect to github. |
Review:
|
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.
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.
448b3ae to
3eaa7fd
Compare
Review:
|
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Review: arming the status-reconcile clock for every checkpointed roundI read through all 9 commits, the FSM exits in What's strong
Things worth confirming
Nothing here blocks; (1) and (2) are confirmations more than defects. Nicely |
2 participants
A boarding deposit is real money the user parked in an on-chain address to
convert into an off-chain VTXO, and partway through that conversion the
client hits a point of no return where it has signed and can only wait. If
the operator drops the round at exactly that moment, say its finalize write
fails so it rolls back before broadcasting, then nothing will ever tell the
client: no transaction exists to confirm, and no round record survives to
send a failure. The client had one remaining way to find out, a timer that
asks the operator "is this round dead?", but that timer was only armed for
rounds carrying forfeits, which a boarding-only round has none of. So the
client waits forever, the user sees a deposit that never became spendable
and no error explaining why, and the only way to get the coins back is to
wait out the CSV timelock and exit unilaterally. This PR arms that timer for
every round, so the client asks, gets told the round is dead, and fails
cleanly instead of hanging, and gives the deposit back.
In FSM terms,
InputSigSentStatehas exactly three exits: the commitmentconfirms, the operator delivers a failure, or the reconcile probe returns an
authoritative status. The probe's timer arrived with #844, whose hazard is
releasing forfeit reservations before the round's fate is known, so every
site that touched it gated on a non-empty forfeit set. Read on its own that
gate looks like an optimization, since a boarding-only round has no
reservations to reconcile. What it actually does is shut all three exits at
once. Widening it needs no new protocol surface and no new state, since a
dead status already fails the round and
releaseForfeitsOnFailureover anempty forfeit set is a no-op.
So we arm the clock on all three doors into the state: the
forfeit-collection transition, the
PartialSigsSentStatetransition aboarding-only round takes instead (the door #1051 actually walks through),
and the
recoverActiveRoundsreload on restart. All four exits disarm, andthe timeout handler probes rather than self-looping when the round carries
no forfeits. That leaves a simpler invariant than the one it replaces: the
clock is armed for the whole of
InputSigSentState, and every exit disarmsit.
Ordering is part of that contract.
processOutboxabandons the rest of theoutbox on the first failing
Tell, and the FSM has already checkpointed bydispatch time, so the arm leads the fallible sends; arming last would reopen
this same strand through a different door. The disarms trail every delivery
for the mirror-image reason, since cleanup must never gate confirmed funds
or a terminal job drop.
How it was found, and how it is verified
The phase 5 DST universal quiescence oracle, not a report. At the end of
every workload run the harness fast-forwards past every TTL, lets the
reconcilers fire, then asserts that nothing money-bearing is parked in a
state something was supposed to move.
[materialize, refreshOk, storeFault]passes every per-op assertion in the suite and is caught only by that
end-of-run check:
We pointed the harness at this branch with the suppressing classification
deleted, making the oracle unconditional: full suite green (90.9s), 500-seed
soak green. In-repo,
checkpoint_arming_test.goandstatus_reconcile_test.gocover every door and exit and assert the twoorderings by position rather than presence; every case fails against the
pre-fix code.
Giving the deposit back
Un-parking the FSM is not the same as getting the money back, so the second
half of this PR does that.
RoundStorehad no counterpart toFinalizeRound, so a failed round kept its checkpoint row: it stayed inListActiveRoundsand was re-hydrated on every start, and the boardingintents it adopted stayed adopted. Since
boardingIntentSweepableexcludesadopted, the deposit was neither boardable nor sweepable, not before theCSV and not after it either, because nothing writes the expired status. The
coins stayed pinned against the board limit with no way to reach them.
The recovery machinery turned out to be already built and simply
unreachable.
ListBoardingIntentsByStatusandListBoardingIntentsBySweepableStatusesboth re-admit a confirmed intentexactly when its linked round reads
failed, andround_statuseshascarried a
failedrow since the schema landed, commented "Round failed,intents may need recovery". Nothing ever wrote it.
So
FailRoundwrites it, and reverts the round's adopted intents in thesame transaction. Intents revert to
confirmedrather thanfailed, sincea dead round proves the commitment never broadcast: nothing on-chain failed,
and the UTXO is exactly as it was before the round. That restores both
recovery routes at once, and the revert is guarded on
adoptedin SQL so asweep already in flight is never clobbered. The actor calls it from the
RoundFailedNotificationhandler, the one choke point every failure pathalready passes through.
Fixes #1051