Skip to content

multi: reclaim exits whose source batch was swept - #1065

Open
ellemouton wants to merge 4 commits into
mainfrom
agent/unroll-fail-swept-source
Open

multi: reclaim exits whose source batch was swept#1065
ellemouton wants to merge 4 commits into
mainfrom
agent/unroll-fail-swept-source

Conversation

@ellemouton

@ellemouton ellemouton commented Jul 29, 2026

Copy link
Copy Markdown
Member

Closes #1050.

Problem

A unilateral-exit job could sit in EXIT_JOB_STATUS_MATERIALIZING
forever after the Ark operator swept a source batch commitment output.
The confirmed sweep double-spends the recovery-tree root, so the exit can
never complete — but txconfirm never gives up on a no-mempool tx, so
the job never failed. The VTXO stayed EXIT / PENDING, its amount stayed
in pending_out_sat, and phase_detail kept implying progress. Nothing
watched the batch commitment output the tree root spends, so the conflict
was invisible to the client.

Fix

Detect (recovery + unroll) — Proof.RootExternalInputs() returns
the roots' external funding inputs (the batch commitment outpoints; one
per contributing commitment for OOR/fan-in VTXOs). The unroll actor arms
a spend watch on each, supplying the batch output's pkScript from the
descriptor ancestry for neutrino BIP-158 filter matching. A confirmed
foreign spend routes the exit job to a terminal ExitOutcomeConflicted;
our own root spending the same output reads as a benign parent
confirmation, so the watch is safe both ways. Arming is best-effort and
never blocks materialization.

Reclaim (unrollvtxo) — the operator can only sweep the batch
commitment output past batch expiry, so a swept source means the VTXO
is expired, not lost: its value is still recoverable through the
ordinary refresh path (#1000). So rather than retiring the coin to a
terminal Failed state, the VTXO manager routes it to the non-terminal
ExpiredState — quarantined from coin selection because its lineage
is dead, but reclaimed by the next block epoch through a cooperative
forfeit, exactly like a naturally-expired VTXO. The unroll job still
terminates as conflicted (the unilateral exit genuinely died); only the
coin's fate differs. A recovery-only target (a non-standard exit policy
such as a vHTLC refund) is held in exit instead, since a swap-contract
output must not be reclaimed as spendable wallet liquidity.

Why not terminal Failed?

An earlier revision of this PR retired the coin to Failed, on the
assumption that a swept source meant the funds were gone. That strands
recoverable value: post-#1000 the client reclaims expired VTXOs through
the normal refresh protocol with no sweep-confirmation gate
(TestSweepIntegrationReclaimWithoutObservingSweep), so a swept source
batch is precisely the condition that reclaim handles. Thanks to @sputn1ck
for catching this.

Testing

  • vtxo: TestUnilateralExitConflicts (routes to ExpiredState, emits no
    terminated notification), TestHandleExitOutcomeConflictedDrivesActorToExpired,
    and recovery-only / non-exiting guard tests on the manager fallback path.
  • recovery: RootExternalInputs returns/dedups/sorts root external inputs.
  • Full vtxo + waved + unroll suites green; make fmt-changed +
    make lint-changed-local clean.

🤖 Generated with Claude Code

@ellemouton

Copy link
Copy Markdown
Member Author

@claude please review this PR.

It closes #1050: a unilateral exit could sit in MATERIALIZING forever
after the operator swept the source batch commitment output (the confirmed
sweep double-spends our tree root, but txconfirm never gives up on a
no-mempool tx). The fix (a) arms a spend watch on the roots' external
funding inputs so the doomed job fails terminally, and (b) threads a new
vtxo.ExitOutcomeConflicted terminal outcome so the coin is retired out of
pending (FAILED) rather than left pending or relived as live.

This touches the VTXO-manager FSM and the unroll terminal/durability path,
so please scrutinise particularly:

  • The "don't relive" invariant. A source-batch conflict must land the
    VTXO in terminal Failed and must NEVER roll back to live (the operator
    has taken the output). Check handleTerminated / notifyVTXOExit
    precedence (conflicted vs recoverable) and the manager's
    conflictExitedVTXO (live-actor and no-actor store-fallback paths).

  • Restart durability. The conflict is persisted in two places — the
    unroll checkpoint (JobState.Conflicted, TLV record 23, omitted when
    false so old checkpoints stay byte-identical) and the registry DB status
    (FailedConflicted, append-only enum 8, excluded from the non-terminal
    query). Does the boot reconcile (resolveExitOutcome
    reconcileUnilateralExits) re-derive the same outcome, and is the
    crash-after-checkpoint-before-registry window handled?

  • Source-watch classification. A foreign spend of a source outpoint
    must hit case 4 (conflict), while our OWN root spending it must hit case
    2 (benign confirmation). Is that split correct in handleSpendObserved?
    And is the neutrino pkScript handling (drawn from desc.Ancestry batch
    outputs, best-effort/outpoint-only fallback) sound?

  • Detection completeness. Multi-input fan-in and OOR-descendant
    ancestries — does watching every root external input cover "any required
    parent branch was swept"?

@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude finished @ellemouton's task in 5m 23s —— View job


Review: unroll — fail and retire exits whose source batch was swept

I read the full diff plus the surrounding FSM, registry, manager-reconcile, and proof-assembler code. This is a careful, well-documented change and the four correctness pillars all hold up. Findings organized by the areas you flagged, then minor notes.


✅ 1. The "don't relive" invariant — correct

The precedence is right in all three places:

  • unroll/registry.go:970conflicted := Phase==Failed && Conflicted, and recoverable is explicitly && !conflicted. So a conflict is never recoverable even if it happens to have no on-chain footprint. This is the load-bearing case: a source sweep learned during materialization (before we broadcast anything) has HadOnChainFootprint == false, which without the Conflicted flag would have been misclassified as recoverable and relived. The flag correctly overrides. 👍
  • notifyVTXOExit (registry.go:1072) orders the Conflicted case before the !HadOnChainFootprint case, so the same override holds on the runtime path.
  • vtxo/manager.go:conflictExitedVTXO mirrors confirmExitedVTXO exactly (live-actor → ExitConflictedEvent; no-actor → direct VTXOStatusFailed write, guarded on Status == VTXOStatusUnilateralExit so a re-delivered conflict can't stomp a reissued coin). It has no recovery-only-target hold guard — which is correct: a swept vHTLC-refund source is still provably gone and must fail, not be held.
  • UnilateralExitStateExitConflictedEvent transitions to FailedState + VTXOStatusUpdate{Failed} + VTXOTerminatedNotification, and LiveState treats a stray ExitConflictedEvent as an idempotent no-op. Both correct.

✅ 2. Restart durability — correct, including the crash window

  • The checkpoint (JobState.Conflicted, TLV 23, emitted only when true) and DB status (FailedConflicted, enum 8, excluded from ListNonTerminal… via NOT IN (4,5,7,8)) are both threaded, and sameRegistryRecord includes ConflictedFailure (registry.go:1730) so the persistence dedup won't skip the conflicted write.
  • crash-after-checkpoint-before-registry is handled: on restart the record is still non-terminal, so the registry re-spawns the child; the child restores a terminal checkpoint with Conflicted=true, handleEvent hits inTerminalState() and calls notifyRegistryIfTerminal (actor.go:358), which now forwards Conflicted: job.Conflicted (actor.go:2350). terminalNotified is in-memory and resets per boot, so exactly one terminal handoff is driven through. The registry then marks FailedConflicted and notifies the manager. ✔
  • resolveExitOutcome (waved/server.go:4398) maps FailedConflicted → ExitOutcomeConflicted, so even if the runtime best-effort Tell to the manager is lost, the boot reconcile re-derives the same outcome from the durable DB status. ✔

✅ 3. Source-watch classification — correct

The case split in handleSpendObserved is sound because ordering does the work:

  • Our own root spending the batch output → its spending txid is a proof node → case 2 (TxConfirmedEvent, benign) fires before the source check ever runs.
  • A foreign sweep → spending txid not in the proof, ackProofOutputSpend returns false (the batch commitment tx is external, not a p.nodes member) → falls through to case 4, where sourceSpendWatches[spentOutpoint] matches and drives FailEvent{Conflict:true}.

I confirmed the batch commitment tx is not a proof node (that's exactly why RootExternalInputs classifies its output as external), so the two classifications can't collide. The three actor tests (TestSweptSourceFailsActor, TestOwnRootSpendOfSourceConfirms, TestSourceSpendWatchArmed) cover the split directly.

✅ 4. Detection completeness — correct

RootExternalInputs iterates every root in RootTxids() (layer 0) and collects every input not produced by an in-graph node, deduped and sorted. For multi-input fan-in each contributing commitment is a distinct root with its own external input; for OOR-descendant lineages the roots are the commitment outputs. Watching all of them covers "any required parent branch was swept." The recovery test asserts dedup/sort and exclusion of in-graph inputs.


Minor notes (non-blocking)

a) Neutrino pkScript matching depends on an index alignment that isn't asserted. sourcePkScripts keys the script map on the full tp.BatchOutpoint (hash + index), and RootExternalInputs returns full outpoints, so a hit requires the root's external-input index to equal tp.BatchOutpoint.Index. The proof assembler only validates the external input by .Hash (proof_assembler.go:246,255), not index. In practice the tree root spends exactly BatchOutpoint, so this holds — but if it ever didn't, neutrino would silently fall back to an outpoint-only watch that its BIP-158 filter can't match, i.e. the #1050 bug would remain unfixed for neutrino only. The lwwallet/Esplora path is unaffected. Two suggestions: (1) a one-line assertion/log if a source outpoint has no resolved pkScript under a neutrino backend, and (2) note that no test exercises the pkScript actually riding along — the fakeChainSourceRef matches by outpoint, so neutrino detection is effectively uncovered. Fix this →

b) TerminalVTXOObserver does not fire on the no-actor conflict/confirm paths. conflictExitedVTXO (and the pre-existing confirmExitedVTXO) write VTXOStatusFailed/Spent directly without emitting VTXOTerminatedNotification, so untrackFraudVTXO won't run for a fraud-triggered exit reconciled on restart with no live actor. This is pre-existing behavior that the conflict path faithfully mirrors, not a regression — flagging only in case fraud untracking on the restart path matters here.

c) Reorg after a confirmed conflict. A foreign spend drives a terminal FailedState; a deep reorg that later un-spends the batch output would leave the coin permanently Failed even though the exit became possible again. This is identical to the existing case-4 external-spend behavior (not introduced here), and deep reorgs are generally out of scope — noting for completeness only.

d) Nice touches worth calling out: the TLV-23 "omit when false → byte-identical old checkpoints" encoding, the append-only enum with the NOT IN query update kept in lockstep across .sql/.sql.go/querier.go, and the best-effort (never-blocks-materialization) arming with per-outpoint retry on the next ensureLoaded.


Verdict: The four invariants you asked me to scrutinise are all correctly handled, the durability crash-window is covered, and test coverage is strong across the FSM, registry, manager (both actor and no-actor paths), codec round-trip, and recovery accessor. My only substantive suggestion is note (a): give neutrino source-conflict detection some explicit coverage/guarding, since it silently degrades to outpoint-only if the batch-output index assumption ever breaks. Everything else is polish. I was unable to run go build/tests here (command approval not granted in this environment), so I relied on static analysis; the PR states all suites are green locally.
agent/unroll-fail-swept-source

@ellemouton
ellemouton force-pushed the agent/unroll-fail-swept-source branch from 00dbbb0 to c86a4e3 Compare July 29, 2026 23:28
@ellemouton

Copy link
Copy Markdown
Member Author

Thanks for the thorough review — addressed note (a) (the neutrino source-conflict coverage gap) in the latest push:

  • Visibility guard: ensureSourceSpendWatches now emits a DebugS breadcrumb when a source outpoint resolves no pkScript, so the outpoint-only fallback (which a neutrino BIP-158 filter can't match) is diagnosable from logs rather than silently degrading. lwwallet/Esplora are unaffected since they match by outpoint.
  • Test coverage: added TestSourceSpendWatchCarriesBatchPkScript, which gives the descriptor an ancestry fragment whose BatchOutpoint is the root's external funding input and asserts the RegisterSpendRequest is armed with that batch pkScript. The fakeChainSourceRef now records the per-registration pkScript (it previously only tracked outpoints, which is exactly why the ride-along was uncovered).

On the index-alignment observation: agreed it holds today because tree.Tree validates Root.Input == BatchOutpoint, so the root's external input is the full batch outpoint (hash+index). The new test pins the resolved-script path, and the breadcrumb catches any future divergence.

Notes (b) reorg / (c) TerminalVTXOObserver on the no-actor path are pre-existing behavior faithfully mirrored (not regressions), so leaving those as-is. Local unroll suite + lint-changed-local green.

@sputn1ck

Copy link
Copy Markdown
Member

after it fails, can it then do the recently added refresh flow?

@ellemouton

Copy link
Copy Markdown
Member Author

@sputn1ck - good catch. trying that now

@ellemouton
ellemouton force-pushed the agent/unroll-fail-swept-source branch from c86a4e3 to 2dcdeba Compare July 30, 2026 20:36
@ellemouton ellemouton changed the title unroll: fail and retire exits whose source batch was swept multi: reclaim exits whose source batch was swept Jul 30, 2026
@ellemouton

Copy link
Copy Markdown
Member Author

Good catch — you're right, and it changes the approach. I'd been treating a swept source batch as "funds gone," but the operator can only sweep that batch commitment output past batch expiry, so a swept source means the VTXO is expired, not lost — and #1000 already reclaims expired VTXOs through the normal refresh/forfeit flow with no sweep-confirmation gate (TestSweepIntegrationReclaimWithoutObservingSweep). Retiring the coin to Failed was stranding recoverable value.

Reworked accordingly (force-pushed, rebased onto current main so #1000 is present):

  • On a source-batch conflict the VTXO now routes to the non-terminal ExpiredState instead of Failed. It's quarantined from coin selection (its lineage is dead), the actor stays alive, and the next block epoch drives the ordinary cooperative reclaim — no new machinery, it reuses your expired-refresh path directly.
  • The unroll job still terminates as ExitOutcomeConflicted (the unilateral exit genuinely can't complete); only the coin's fate changed. So the detection half is untouched.
  • Recovery-only targets (e.g. a vHTLC refund) are held in exit rather than reclaimed, since a swap-contract output shouldn't become spendable wallet liquidity — same guard as the recoverable path.

PR description and the lead commit are retitled to match. Tests updated to assert the Expired routing + the recovery-only/non-exiting guards.

@ellemouton

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @ellemouton's task in 3m 34s —— View job


Re-review: unroll — reclaim exits whose source batch was swept

I focused this pass on the reworked reclaim path (Failed → non-terminal ExpiredState), since the detection half is unchanged from my earlier review and still holds. I re-read the manager FSM routing, the ExpiredState transition, statusToState, and the registry/waved outcome plumbing.

The rework is clean and the plumbing is faithful. One concern is worth a definitive answer before merge because it re-expresses the exact "don't relive" invariant you flagged — now routed through ExpiredState instead of a terminal state.


⚠️ Primary concern — can a swept-source conflict relive a dead-lineage coin via the ExpiredState block-epoch handler?

The new routing sends a conflicted exit to ExpiredState. But ExpiredState.ProcessEvent on the very next BlockEpochEvent re-evaluates expiry and, if it reads not expired, deliberately rolls the coin back to LiveState (vtxo/transitions.go:1488-1513, forfeitReleaseTransition). That branch keys entirely off CheckExpiry(vtxo, height), which compares currentHeight against the VTXO's own Descriptor.BatchExpiry (vtxo/expiry.go:152-156) — not the swept source's expiry.

So the reclaim is safe only if, at conflict time, currentHeight >= vtxo.BatchExpiry. The operator can only sweep source commitment C at height > C.expiry. That guarantees height >= vtxo.BatchExpiry only if vtxo.BatchExpiry <= C.expiry for every swept source C.

  • For a direct round VTXO, C is the VTXO's own batch, so this holds trivially. ✔
  • For an OOR / fan-in descendant, BatchExpiry is taken from a single candidate's GetBatchExpiryHeight() (oor/incoming_metadata_query.go:181, oor/incoming_vtxo.go:182). If a fan-in VTXO amalgamates inputs from commitments with differing expiries and the recorded BatchExpiry is the later one, then sweeping the earlier-expiring source gives C.expiry < height < vtxo.BatchExpiryCheckExpiry returns not expired → the next block epoch relives the coin to LiveState and re-admits it to coin selection on a provably-dead lineage.

That is precisely the invariant you asked me to protect ("must NEVER roll back to live — the operator has taken the output"), and it's new risk introduced by this rework: the old terminal-Failed approach was structurally immune because FailedState is terminal and has no rollback branch.

Please confirm that Descriptor.BatchExpiry for any fan-in / OOR VTXO is always <= min(expiry) across its contributing commitments (i.e. a descendant can never outlive any source it depends on). If that Ark invariant holds — which I believe it does for single-tree OOR, since the transfer inherits the shared output's expiry — then the rollback branch provably can't fire and the reclaim is correct. If a fan-in VTXO can ever record an expiry later than a contributing source, this reintroduces the relive bug and needs a guard (e.g. a Conflicted/dead-lineage flag on the state that suppresses the ExpiredState → LiveState rollback). A regression test that drives the conflicted VTXO through a BlockEpochEvent at C.expiry < height < BatchExpiry and asserts it stays quarantined would pin this down either way. Investigate →


✅ Manager routing (conflictExitedVTXO) — correct

Mirrors recoverExitedVTXO precisely:

  • Recovery-only guard short-circuits before any store access (manager.go:1220), so a vHTLC-refund source is held in UnilateralExit, never reclaimed as spendable liquidity. The two no-actor guard tests assert GetVTXO/UpdateVTXOStatus are never called. ✔
  • Live-actor path asks ExitConflictedEvent; the FSM emits VTXOStatusUpdate{Expired} via outbox and the actor stays alive. ✔
  • No-actor path spawns the actor before persisting the status flip (so a spawn failure leaves the coin in UnilateralExit on disk for boot to re-drive), and guards on Status == VTXOStatusUnilateralExit so a re-delivered conflict can't stomp a reissued/recovered coin. ✔ statusToState maps VTXOStatusExpired → ExpiredState (actor.go:815), so the respawned actor lands in the reclaim state and picks up the next epoch — identical to client: Recover expired VTXOs through normal refresh #1000's boot recovery.

✅ FSM transition & "don't relive" precedence — correct (modulo the concern above)

  • UnilateralExitState + ExitConflictedEventExpiredState{ObservedHeight: LastCheckedHeight}, emits VTXOStatusUpdate{Expired}, no VTXOTerminatedNotification (actor survives). TestUnilateralExitConflicts asserts exactly this, including assertOutboxLacks[*VTXOTerminatedNotification]. ✔
  • LiveState treats ExitConflictedEvent as an idempotent no-op alongside ExitFailedEvent/ExitConfirmedEvent — a stray/duplicate conflict can't retire a live coin. ✔

✅ Registry & durability — correct

  • handleTerminated: conflicted := Phase==Failed && Conflicted, and recoverable is explicitly && !conflicted (registry.go:970-977) — a conflict is never misclassified as recoverable. notifyVTXOExit orders the Conflicted case before the !HadOnChainFootprint case (registry.go:1072). sameRegistryRecord includes ConflictedFailure so the dedup won't skip the write. ✔
  • resolveExitOutcome maps FailedConflicted → ExitOutcomeConflicted carrying ExitPolicyKind (server.go:4431), so the durable DB status re-derives the same outcome on boot even if the runtime Tell is lost. The crash-after-checkpoint-before-registry window is covered as before. ✔
  • All three failure flavours surface as FAILED to clients (rpc_server.go); the enum stays append-only and excluded from the non-terminal query. ✔

✅ Detection half — unchanged, still sound

RootExternalInputs, the source-spend watch classification (own-root → case 2 benign; foreign sweep → case 4 conflict), and the neutrino pkScript ride-along (with the DebugS breadcrumb + TestSourceSpendWatchCarriesBatchPkScript you added last round) are untouched by the rework and remain correct.


Minor / non-blocking

a) Reason string is dropped on the live-actor reclaim. ExitConflictedEvent.Reason is threaded into the FSM, but the resulting ExpiredState carries no reason and the eventual reclaim forfeit has no audit trail linking it to the source-batch conflict. Not a correctness issue (the registry FailReason + FailedConflicted status hold the durable record), but a coin silently entering the refresh path with no local breadcrumb explaining why it expired early may be worth a one-line InfoS on the transition.

b) Pre-existing notes still stand (not introduced here): TerminalVTXOObserver doesn't fire on the no-actor path, and a deep reorg un-spending the batch output after a confirmed conflict isn't handled. Both mirror existing behavior.


Verdict: The reworked approach is a genuine improvement — reclaiming recoverable value instead of stranding it — and the manager/FSM/registry/durability plumbing is faithful and well-tested. The one thing I'd want confirmed before merge is the BatchExpiry <= min(source expiry) invariant for fan-in/OOR descendants, because the ExpiredState → LiveState rollback branch is a live path that could re-admit a dead-lineage coin if that invariant ever fails — which is exactly the "don't relive" guarantee this PR is meant to uphold. Everything else is polish.

I relied on static analysis (no build/test execution in this environment); the PR states the vtxo/waved/unroll suites are green locally.
agent/unroll-fail-swept-source

@ellemouton
ellemouton force-pushed the agent/unroll-fail-swept-source branch from 2dcdeba to 5613b36 Compare August 4, 2026 16:55
@ellemouton

Copy link
Copy Markdown
Member Author

Addressed the pre-merge concern (the ExpiredState → LiveState relive on a conflicted coin). Pushed.

Confirmed: the rollback branch is unreachable for a conflicted coin. The chain is:

  1. The operator can only spend a batch commitment output past that batch's expiry (the sweep leaf is absolute-timelocked to it).
  2. A VTXO's BatchExpiry is the most-restrictive expiry across all its contributing commitments — documented at oor.IncomingVTXOMetadata.BatchExpiry ("most-restrictive across all contributing rounds"), so no descendant can outlive any source it depends on.
  3. Therefore the confirmed foreign spend that triggers ExitConflictedEvent proves currentHeight ≥ swept-source expiry ≥ this VTXO's BatchExpiry. CheckExpiry reports Expired at that height and every height after it — so the ExpiredState block-epoch handler always takes the reclaim path, never the rollback, including after a restart (a respawn re-enters ExpiredState at a height still ≥ BatchExpiry).

On the trust boundary you rightly flagged: BatchExpiry here is operator-supplied and trusted verbatim, exactly as it is everywhere else in the client (the receive path never gets per-commitment expiry data to take a min over itself). The one place the client does synthesize a fan-in expiry locally — vHTLC recovery — enforces this same rule explicitly (waved/vhtlc_recovery_target.go, min(root.BatchExpiry), "a multi-input recovery target cannot outlive one of its contributing roots"). So relying on it here is consistent with the existing model rather than a new assumption.

Changes:

  • Documented the full invariant + why the rollback is unreachable, inline in the ExitConflictedEvent handler.
  • Added TestUnilateralExitConflictReclaimsWhenExpired: routes a conflict to ExpiredState, then drives a BlockEpochEvent at height ≥ BatchExpiry and asserts it reclaims (ForfeitRequestPendingForfeit) and never emits a Live status update.
  • Minor: added the InfoS breadcrumb on the conflict→reclaim transition carrying the Reason, so the early expiry has a local audit trail (the durable record was already in the registry FailReason/FailedConflicted).

vtxo suite + fmt-changed + lint-changed-local green.

@ellemouton

Copy link
Copy Markdown
Member Author

@claude please re-review — the pre-merge concern from the last round is addressed above.

@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @ellemouton's task in 1m 52s —— View job


Re-review: the ExpiredState → LiveState relive concern is resolved

Focused this pass on the one blocker from last round — whether a source-batch conflict routed to ExpiredState could be relived to LiveState on the next block epoch. I re-read the new ExitConflictedEvent handler, the ExpiredState block-epoch branch, CheckExpiry, the invariant sources, and the new test.

Verdict: addressed correctly. No remaining blockers.


✅ The rollback branch is provably unreachable for a conflicted coin

ExpiredState.ProcessEvent's BlockEpochEvent branch relives a coin only when CheckExpiry(vtxo, evt.Height) != ExpiryStatusExpired (vtxo/transitions.go:1515). CheckExpiry keys entirely off vtxo.BatchExpiry: blocksRemaining = BatchExpiry - currentHeight, and returns Expired the moment blocksRemaining <= 0 (vtxo/expiry.go:152-156). So the reclaim path is guaranteed whenever the observed height is >= BatchExpiry.

The chain of guarantees that this always holds for a conflict:

  1. The operator can only spend a batch commitment output past that batch's expiry (absolute-timelocked sweep leaf), so the confirmed foreign spend proves currentHeight >= swept-source.expiry.
  2. A VTXO's BatchExpiry is the most-restrictive expiry across every contributing commitment — I verified both anchors the code comment cites:
    • oor/incoming_vtxo.go:40-42: "most-restrictive across all contributing rounds."
    • waved/vhtlc_recovery_target.go:658-662: recoveryRootMeta takes the running min (root.BatchExpiry < batchExpiry), documented as "a multi-input recovery target cannot outlive one of its contributing roots."
  3. Therefore currentHeight >= swept-source.expiry >= BatchExpiry, and since block epochs only advance, every subsequent epoch — including a post-restart respawn re-entering ExpiredState at the current tip — is also >= BatchExpiry. The relive branch can never fire. ✔

This is the exact "must NEVER roll back to live" invariant, and it's now structurally guaranteed rather than incidentally true.

✅ The regression test pins it

TestUnilateralExitConflictReclaimsWhenExpired (vtxo/transitions_test.go:505) drives the conflict at height=1005 against the harness descriptor's BatchExpiry=1000, then sends a BlockEpochEvent and asserts:

  • lands in PendingForfeitState with a ForfeitRequest dispatched (reclaim), not LiveState;
  • no VTXOStatusUpdate{Live} is ever emitted (belt-and-suspenders loop over the outbox).

TestUnilateralExitConflicts separately pins the routing to ExpiredState + VTXOStatusUpdate{Expired} with no VTXOTerminatedNotification, so the actor survives to reclaim. Both directly exercise the guaranteed condition. ✔

✅ The InfoS breadcrumb + Reason threading

The conflict→reclaim transition now logs a Source-batch conflict: routing exit to expired reclaim line carrying the outpoint and evt.Reason (transitions.go:~1445), so the early expiry has a local audit trail. This closes minor note (a) from last round; the durable record was already in the registry FailReason/FailedConflicted. The inline comment documenting the full unreachability argument is excellent and will save the next reader the same investigation.


Everything else from prior rounds still holds

The detection half (RootExternalInputs, own-root→case 2 / foreign→case 4 classification, neutrino pkScript ride-along + TestSourceSpendWatchCarriesBatchPkScript), the manager routing (conflictExitedVTXO recovery-only guard + no-actor UnilateralExit-status guard), and the durability plumbing (FailedConflicted enum, TLV-23 checkpoint, resolveExitOutcome boot re-derivation) are unchanged and remain correct.

Pre-existing, non-blocking, faithfully mirrored (not regressions): TerminalVTXOObserver doesn't fire on the no-actor path, and a deep reorg un-spending the batch output after a confirmed conflict isn't handled.


LGTM. The reworked reclaim is a real improvement over terminal Failed — it recovers value the earlier revision would have stranded — and the one soundness concern is now both proven in a comment and pinned by a regression test.

Note: I relied on static analysis; go test ./vtxo/... wasn't approved in this environment. The PR reports the vtxo suite + fmt-changed + lint-changed-local green locally.
agent/unroll-fail-swept-source

RootExternalInputs returns the outpoints consumed by a proof's root
transactions that are not themselves produced by any node in the graph
-- the external funding inputs the whole recovery tree hangs off of. For
a round-direct VTXO this is the batch/commitment output the tree root
spends; for an OOR-chained or multi-input fan-in VTXO it is every
distinct commitment output rooting a local lineage fragment.

These are exactly the outpoints a competing party (an operator sweeping
an expired batch) can consume out from under an exit, so an upcoming
unroll change watches them to fail a doomed exit terminally instead of
materializing forever. The result is deduplicated and sorted so two
proofs built from the same node set yield identical output.
Add a distinct terminal job status for an exit defeated by a confirmed
conflicting spend -- the operator swept a source batch commitment output
the recovery tree depends on, so the exit can never complete
(wavelength#1050). It is separate from the plain Failed and the
FailedRecoverable statuses because boot-time reconciliation must treat
it differently: retire the target VTXO out of unilateral-exit (clearing
it from pending balance) rather than leave it pending forever, and --
unlike a recoverable failure -- never roll it back to live, since the
coin is provably gone.

The enum is append-only (value 8) so existing rows' numeric meaning
never shifts, and it is added to the non-terminal-jobs query's exclusion
list so a terminal conflict is not restored on restart.
make sqlc output for the non-terminal unilateral-exit job query, now
excluding the appended FailedConflicted status (8) alongside the other
terminal statuses.
A unilateral-exit job could sit in EXIT_JOB_STATUS_MATERIALIZING forever
after the Ark operator swept a source batch commitment output. The
confirmed sweep double-spends the recovery-tree root, so the exit can
never complete, but txconfirm never gives up on a no-mempool tx, so the
job never failed. The VTXO stayed EXIT / PENDING and its amount stayed
in pending_out_sat with phase_detail implying progress.

Detect the conflict by arming a spend watch on each root external
funding input (the batch commitment outpoints; one per contributing
commitment for OOR/fan-in VTXOs), supplying the batch pkScript from the
descriptor ancestry for neutrino filter matching. A confirmed foreign
spend routes the exit job to a terminal ExitOutcomeConflicted outcome;
our own root spending the same output reads as a benign parent
confirmation, so the watch is safe in both directions.

The operator can only sweep that batch output past batch expiry, so the
coin is expired, not lost: its value is still recoverable through the
ordinary refresh path (wavelength#1000). So instead of retiring the VTXO
to a terminal Failed state, the manager routes it to the non-terminal
ExpiredState -- quarantined from coin selection because its lineage is
dead, but reclaimed by the next block epoch through a cooperative
forfeit. The unroll job still terminates as conflicted; only the coin's
fate differs. A recovery-only target (a non-standard exit policy such as
a vHTLC refund) is held in exit instead, since a swap-contract output
must not be reclaimed as spendable wallet liquidity.
@ellemouton
ellemouton force-pushed the agent/unroll-fail-swept-source branch from 5613b36 to 2bccda4 Compare August 11, 2026 22:23
@levmi levmi added the P1 Priority 1 — high label Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Priority 1 — high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unroll: fail exits whose source batch was already swept

3 participants