multi: unify all unroll triggers behind the VTXO manager - #909
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the unilateral exit flow by routing vHTLC recovery and fraud escalation through the VTXO manager's single admission gate, ensuring consistent state transitions and preventing recovery-only targets from being incorrectly relived as spendable liquidity. A potential nil pointer dereference was identified in the recovery coordinator service when checking the unroll status, which should be resolved by adding a defensive nil check.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if !status.Found { | ||
| return fmt.Errorf("unroll admission returned without visible " + | ||
| "status") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
If s.unroll.GetStatus returns nil, nil (which is a common pattern in Go to represent a missing or uninitialized status, especially in test mocks), checking !status.Found directly will cause a nil pointer dereference panic. Adding a nil check on status ensures robust defensive programming.
| if !status.Found { | |
| return fmt.Errorf("unroll admission returned without visible " + | |
| "status") | |
| return nil | |
| } | |
| if status == nil || !status.Found { | |
| return nil | |
| } |
There was a problem hiding this comment.
Fixed in 07b39cb. Good catch, and it applied to both GetStatus read sites, not just this one: the sibling reconcileLoaded path dereferenced the status the same way. Guarded both with status == nil || !status.Found and added a coordinator test that drives RestoreNonTerminal against a nil-status source to pin it (it panics without the guard, which is how I caught the second site).
![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: f34bfd5545
ℹ️ 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".
| // witness against a vHTLC taproot tree would never sweep. | ||
| ensureReq := &unroll.EnsureUnrollRequest{ | ||
| Outpoint: op, | ||
| Trigger: unroll.TriggerRestart, |
There was a problem hiding this comment.
When a fraud spend is forced through the VTXO manager, there is now a crash/drop window after the VTXO status is committed to UnilateralExit but before the best-effort chain-resolver Tell creates the registry row. On restart this orphan scan re-admits every such target as TriggerRestart, so a fraud-triggered target loses TriggerFraudSpend; the unroll FSM only applies the fraud checkpoint deferral/backstop logic for TriggerFraudSpend (see shouldSubmitReadyFrontier), so recipient fraud recovery can resume with the wrong behavior. Persist/recover the original fraud trigger or avoid routing fraud orphans through this generic restart admission.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed the window is real: routing fraud through the manager commits UnilateralExit before the best-effort chain-resolver Tell, so a crash in that gap leaves an orphan the restart scan re-admits as TriggerRestart, dropping TriggerFraudSpend.
Two things worth pinning down on impact before deciding the fix:
-
It's a lost timing optimization, not a fund/liveness bug. Under a non-fraud trigger
shouldSubmitReadyFrontierreturns true, so the FSM submits the ready checkpoint immediately instead of deferring it to the fraud backstop deadline. That's a premature but safe broadcast: the exit still completes and the recipient still recovers the output; what's lost is the deferral (hold the checkpoint until near the CSV deadline). There's no path where the earlier broadcast helps an attacker. -
This is a byproduct of fixing the larger bug. Before this PR a fraud target never left the live set at all (it stayed
Live), so it was never aUnilateralExitorphan-scan candidate. The window only exists because fraud now correctly goes out of the live set through the manager.
On the fix: a faithful "recover the trigger" needs new durable fraud state. Unlike the vHTLC recovery policy (which the orphan scan already recovers from the recovery store via recoveryExitPolicies), the fraud watcher keeps its watch set in memory and has no store the scan can consult, so preserving the trigger means persisting fraud-forced exits durably and joining them in the scan.
Given the impact is a lost deferral optimization in a narrow crash window with no fund risk, my inclination is to document this as a known limitation consistent with the async-admission trade-off rather than add a durable fraud-trigger store in this PR. I'm getting a second opinion on that call and will follow up here either way.
There was a problem hiding this comment.
Second opinion is in and concurs: keep it documented, don't fix in this PR. The key confirmations from tracing the FSM: losing TriggerFraudSpend degrades to the same immediate-checkpoint-submission that manual and critical-expiry (the deadline-sensitive path) already use; the CSV delay is relative to checkpoint confirmation so earlier confirmation only starts the sweep clock earlier (never misses a deadline); checkpoints are the recipient's own pre-signed exit lineage, so publishing them reveals nothing and enables no attacker spend. And the fraud watcher can't paper over it on restart — it rebuilds watches from live descriptors, and an orphaned target is UnilateralExit, so it's never re-watched.
Documented the degraded behavior and the faithful follow-up at the re-admission site in e22f731 (recoverOrphanedUnrollJobs): the minimal correct fix is to stamp the trigger onto the VTXO row in the same tx that flips it to UnilateralExit and read it back off the descriptors the scan already lists — atomic, no new store, but a schema change and cleanly separable.
| if err != nil { | ||
| res := fn.Err[ManagerResp]( | ||
| fmt.Errorf("load vtxo for force-unroll: %w", err), | ||
| ) |
There was a problem hiding this comment.
In production VTXOPersistenceStore.GetVTXO returns an error on a miss (wrapped sql.ErrNoRows, and the interface documents “Returns error if not found”), so this new absent-actor path never reaches the descriptor == nil branch. A manual/fraud force for an outpoint the wallet no longer tracks now surfaces an internal load vtxo for force-unroll error instead of the intended Accepted:false, Reason:"no such vtxo"; handle the not-found error here before returning a hard failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 314cc51, a bit more thoroughly than a local not-found check. Rather than teach the manager about sql.ErrNoRows, the store now returns a typed domain sentinel vtxo.ErrVTXONotFound on a miss (wrapping sql.ErrNoRows in the chain so the other call sites that still test for it keep working), and spawnForceUnrollActor matches the sentinel to return Accepted:false, Reason:"no such vtxo". That keeps the domain manager off a persistence-layer detail and makes the decline read correctly for an outpoint the wallet no longer tracks. Added a db-store test asserting the sentinel (and back-compat with sql.ErrNoRows) plus a manager test for the wrapped-error path.
1b2113e to
b8ec48f
Compare
|
Added the faithful end-to-end regression for this fix in swapdk-server#214 (draft, stacked on this PR). It drives an out-swap into refund-without-receiver recovery, restarts the client daemon, and runs a real |
|
@claude review this |
|
Claude finished @Roasbeef's task in 3m 14s —— View job Review: unify unroll triggers behind the VTXO managerI read the full diff (
Overall this is a clean, well-reasoned consolidation. The single-admission-gate topology is the right shape, the string-typed trigger/policy threading to avoid the My findings below, roughly in priority order. 🔴 1. Fraud trigger is lost in the orphan-recovery scan (agrees with Codex P1)This PR introduces a new crash window that didn't exist for fraud before. Previously fraud called On restart,
So a fraud target that hit the crash window resumes with the wrong checkpoint-deferral/backstop behavior. Note the PR already solved the exactly analogous problem for the exit policy by threading Options: (a) persist/recover the original trigger the way you now recover the exit policy (e.g. a trigger map fed into the orphan scan), or (b) keep fraud orphans out of the generic 🟡 2. Missing VTXO surfaces an internal error instead of a clean decline (agrees with Codex P2)In Suggest handling not-found before the hard-error return: descriptor, err := m.cfg.Store.GetVTXO(ctx, outpoint)
if errors.Is(err, sql.ErrNoRows) {
res := fn.Ok[ManagerResp](&ForceUnrollResponse{
Accepted: false, Reason:,
})
return nil, &res
}
if err != nil { ... }(If 🟢 3. Defensive nil-status check (agrees with Gemini, low priority)
Notes / things I checked that look correct
Given finding #1 opens a real (if narrow) correctness gap for fraud recovery that this PR itself introduces, I'd treat it as a blocker or an explicit follow-up before merge; #2 is a small but user-visible RPC regression worth a quick fix; #3 is optional. |
In this commit, we widen the ForceUnroll path so a forced unilateral exit can carry two pieces of metadata end-to-end: the trigger that started it (manual, critical expiry, fraud spend) and an optional exit-spend policy identity (a vHTLC refund names its own policy). This is the groundwork for routing fraud and vHTLC recovery through the VTXO manager's admission gate rather than letting them admit the unroll registry behind the manager's back. We add these as a string-typed UnrollTrigger enum and an fn.Option[ExitPolicy] on actormsg.ForceUnrollRequest, mirror them onto the vtxo ForceUnrollEvent and ExpiringNotification, and populate them in every FSM arm that escalates to UnilateralExitState. They ride string-typed on the vtxo/actormsg side on purpose: unroll already imports vtxo, so the real unroll.StartTrigger / unroll.ExitPolicyKind can't be referenced here without a cycle. The darepod chain-resolver bridge converts them back at the seam where both packages are in scope. We also make UnilateralExitState re-emit the ExpiringNotification on a duplicate ForceUnrollEvent instead of silently self-looping. A first admission can be lost before the registry records it (a crash between the status flip and the registry UpsertRecord), so an idempotent re-admission lets a restart re-drive the exit under the same trigger and policy. The registry dedups against a live record, so a redundant re-admit is harmless.
In this commit, we teach Manager.handleForceUnroll to re-materialize a VTXO actor from its persisted descriptor when there is no live actor for the outpoint, then drive the ForceUnrollEvent through it carrying the request's trigger and exit policy. This is what lets the manager own the exit for triggers whose target is not a normal live coin. The vHTLC recovery target is materialized directly in the store and never had a manager actor, and any exiting VTXO that a restart left out of the live-recovery set (UnilateralExit is excluded from ListLiveVTXOs) has no actor either. Rather than let those callers admit the unroll registry behind the manager's back, the manager spawns the actor from the descriptor and runs the same transition every other trigger uses. A missing descriptor reports "no such vtxo" and a terminal descriptor reports "already terminal" instead of spawning an actor that would immediately reap itself, so the caller can tell a real transition apart from a no-op.
In this commit, we point the recipient fraud watcher at the VTXO manager instead of the unroll registry. When a watched ancestor of a tracked OOR VTXO is seen spent on-chain, the watcher now Asks the manager to force the affected target into unilateral exit under TriggerFraudSpend, and the manager owns the state transition and starts the durable unroll job through its chain-resolver seam. Before this, fraud admitted the unroll registry directly, so the VTXO was never flipped to UnilateralExit: it stayed live in the store, leaked back into the live set on the next restart, and was invisible to the #400 orphan-recovery scan. Converging on the manager fixes both: the coin is persisted out of the live set the moment fraud fires, and the restart scan covers it. A declined transition (the coin is already terminal, or the wallet no longer tracks it) is logged rather than surfaced as a hard error: the fraud watch has done all it can, and failing would only wedge escalation for the other targets sharing the ancestor.
In this commit, we route vHTLC recovery through the VTXO manager and wire the chain-resolver bridge that converts a manager exit notification into a registry admission, completing the unification of all four unroll triggers (manual, critical expiry, fraud, vHTLC) behind the manager's single admission gate. The recovery coordinator now hands off to the manager's ForceExit seam instead of admitting the unroll registry itself, carrying the recovery row's exit policy so the registry records the refund policy rather than the standard timeout. The materializer persists the recovery target directly into VTXOStatusUnilateralExit rather than Spending: Spending is returned by ListLiveVTXOs (status < 3 OR status = 7), so a Spending target leaked back into the live set on restart and poisoned sweep-all, which offered the already-exiting coin as a forfeit and got the whole round rejected with "forfeit VTXO is not live: status is unrolled_by_client". This is the root cause of the reported wedge. Admission is asynchronous now, so the coordinator no longer reads the registry record back for a synchronous policy check on the happy path. It keeps a best-effort guard: a visible record under a different policy still fails the recovery closed, while a not-yet-visible record is left to the registry's own validation and the restart re-drive. On the darepod side, the chain-resolver bridge maps the threaded trigger and exit policy back into unroll.StartTrigger / unroll.ExitPolicyKind (an empty trigger stays critical expiry, matching the auto-expiry default). We also restore in-flight recovery jobs before the generic orphan scan: restore drives the policy-bearing admission, which the LazyChainResolver buffers and replays to the registry the instant it is wired, and the registry is first-writer-wins on exit policy, so the no-policy orphan scan must run after that replay or a refund target would silently exit under the standard timeout.
In this commit, we fix a fund-safety regression the unification introduced: a recoverable unroll failure would relive a vHTLC recovery target into the live coin set. Because the manager now owns an actor for the recovery target and persists it as VTXOStatusUnilateralExit (both new in this series), the darepo-client#602 recovery edge (ExitOutcomeRecoverable rolls a no-footprint failure back to LiveState) now fires for it: the recovery output, which is a swap-contract output and not spendable liquidity, would become a live wallet coin, inflating balance and re-poisoning sweep-all. Before the series it was safe because the target was Spending with no manager actor, so both the store-path guard and boot reconciliation skipped it. We thread the unroll job's exit policy onto ExitOutcomeNotification (and ExitOutcomeResolution for the boot-reconcile path), sourced from the registry record in notifyVTXOExit and from the persisted unilateral-exit job in resolveExitOutcome. recoverExitedVTXO then refuses to relive a target whose policy is a known non-standard (recovery-only) policy, holding it in UnilateralExit instead. A clean refund failure is the owning recovery subsystem's job to retry or terminal-fail; the manager must not resurrect the coin as spendable in the meantime.
In this commit, we close a first-writer-wins hole in the boot orphan scan. The registry keeps whichever exit policy admits a target first and ignores later requests' policies. recoverOrphanedUnrollJobs re-admits every unilateral-exit VTXO under TriggerRestart with no policy, and vHTLC recovery targets are now in that scan (they are UnilateralExit, not Spending). If RestoreNonTerminal failed for a target on a prior boot (a transient error leaves it exiting on disk with no registry record), the no-policy scan would permanently claim it as a standard timeout exit, and a standard witness against a vHTLC taproot tree never sweeps. We hand the scan the durable exit policy of every non-terminal recovery target, indexed by outpoint, so it re-admits a refund target under the right policy even when it is the one that creates the record. The restore-before-scan ordering stays as belt-and-suspenders. We also pin the hand-maintained actormsg exit-policy enum to the canonical vhtlcrecovery constants with a mirror test, since actormsg can't import vhtlcrecovery without a cycle and a silent drift would break the round-trip.
In this commit, we source the exit policy the registry hands to the VTXO manager on a terminal unroll from the child's UnrollTerminatedMsg rather than from the registry's in-memory pending record. The prior F1 fix read record.ExitPolicyKind out of r.pending in handleTerminated. That cache is legitimately evicted the moment a child's async terminal persist completes (handlePersistRecordResult drops the entry once the store write lands), so a recovery-only vHTLC target can reach handleTerminated with no cached record at all. When that happens the kind arrives empty, the manager's Valid() guard misses, and the target gets relived to live: the exact darepo-client#602 relive bug the F1 fix was meant to close, just through a narrower window (one flaky admission-refinement persist under load). The child already knows its policy authoritatively from its own durable state via exitPolicyKind(), so we stamp it onto the terminal message and prefer it in handleTerminated. The record stays accurate for persistence and the manager sees the real kind whether or not the cache survived.
b8ec48f to
75f7d0a
Compare
In this commit, we update the per-package agent docs for the packages the unified-unroll change touches so they describe the flow as it now works: every unilateral-exit trigger (manual, critical expiry, fraud, vHTLC recovery) goes through the VTXO manager's admission gate. The fraud watcher and the vHTLC recovery coordinator docs now describe forcing the exit through the VTXO manager (VTXOManagerRef / the ExitAdmitter ForceExit seam) rather than talking to the unroll registry directly. The lib/actormsg, vtxo, and unroll docs pick up the trigger and exit-policy fields that ride the ForceUnroll path, the manager spawning an absent actor to force-unroll it, the recovery-only hold-in-exit on a recoverable failure, and the child-stamped ExitPolicyKind on the terminal handoff. The darepod doc picks up the policy-carrying boot ordering, the expiring-to-unroll bridge, and the recovery materializer persisting unilateral-exit.
In this commit, we give VTXOStore.GetVTXO a domain-level miss sentinel
so callers stop reaching for the persistence-layer sql.ErrNoRows. The
store translates a row miss into vtxo.ErrVTXONotFound (keeping
sql.ErrNoRows in the error chain so the call sites that still test for
it keep working while they migrate), and the VTXO manager matches the
sentinel instead.
This fixes a real papercut on the force-unroll path: the manager's
spawn-from-descriptor step treated a GetVTXO miss as a nil descriptor,
but the store signals a miss with an error, not a nil. A manual or fraud
force for an outpoint the wallet no longer tracks therefore surfaced an
internal "load vtxo for force-unroll" error instead of the intended
declined ForceUnrollResponse{Accepted: false, Reason: "no such vtxo"}.
Matching the sentinel makes the decline read correctly, and it keeps the
manager off a database/sql detail it had no business knowing.
In this commit, we nil-check the unroll status the coordinator reads back after forcing a recovery exit. Both the post-force policy-conflict guard and the status-reconcile path dereferenced the GetStatus result directly, so a status source that returns a nil status with no error would panic rather than read as "no record yet". A nil-with-no-error is a legitimate shape now that admission is asynchronous through the VTXO manager: the registry record may not be visible yet. We treat it the same as a not-found record, leaving the recovery active for the registry's own validation and the restart re-drive instead of crashing the service.
Review round addressed@gemini-code-assist @chatgpt-codex-connector thanks for the passes — here's what landed in response, on top of a rebase onto the latest Fixed:
Documented, not fixed (P1: fraud trigger in orphan recovery) — Full reasoning is in the inline replies on each thread. PTAL when you get a chance. |
|
@claude thanks for the thorough review — mapping your three findings to what landed (all on top of a rebase onto latest 🟡 #2 — missing VTXO surfaces an internal error (your P2 / Codex P2): fixed in 🟢 #3 — defensive nil-status (your Gemini agreement): fixed in 🔴 #1 — fraud trigger lost in orphan recovery (your P1 / Codex P1): took the "explicit follow-up" path you offered, documented in Full per-thread reasoning is in the inline replies. If you still read #1 as a hard blocker over the "premature-but-safe broadcast" framing, say so and I'll reconsider the same-tx stamp here. |
|
Claude finished @Roasbeef's task in 1m 22s —— View job Re-review of the fix round
Read all three commits plus 🟡 #2 —
|
There was a problem hiding this comment.
Code Review
This pull request refactors the unilateral exit and vHTLC recovery paths to route all exits (manual, critical-expiry, fraud, and vHTLC recovery) through the VTXO manager's single admission gate. This ensures the manager owns the state transition and persists the target into unilateral exit before starting the durable unroll job, preventing recovery-only targets from being mistakenly relived as spendable wallet liquidity. Feedback on the changes highlights a potential nil pointer dereference in the unroll registry if the pending record has been evicted, as well as a missing nil check for the target materializer during service initialization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if policyKind == "" { | ||
| policyKind = record.ExitPolicyKind | ||
| } |
There was a problem hiding this comment.
If the pending record has been evicted (record is nil) and the incoming req.ExitPolicyKind is empty (e.g., for a standard timeout exit), executing policyKind = record.ExitPolicyKind will result in a nil pointer dereference and panic. We should defensively check if record is not nil before accessing its fields.
| if policyKind == "" { | |
| policyKind = record.ExitPolicyKind | |
| } | |
| if policyKind == "" && record != nil { | |
| policyKind = record.ExitPolicyKind | |
| } |
| if cfg.Exiter == nil { | ||
| return nil, fmt.Errorf("exit admitter is required") | ||
| } |
There was a problem hiding this comment.
The cfg.TargetMaterializer is not checked for nil in NewService. If it is nil, any subsequent method calls on s.targetMaterializer will result in a nil pointer dereference panic. We should defensively validate that cfg.TargetMaterializer is not nil during service initialization.
if cfg.Exiter == nil {
return nil, fmt.Errorf("exit admitter is required")
}
if cfg.TargetMaterializer == nil {
return nil, fmt.Errorf("target materializer is required")
}In this commit, we close out the final review round. We add a test that drives the status-reconcile path (GetRecoveryStatus, which joins the durable row with unroll status without going through escalation) against a nil status source, pinning the second half of the nil-status guard independently of the escalation path. We also document, at the orphan-recovery re-admission, that a fraud-forced exit orphaned in the crash gap between the VTXO status flip and the registry admission re-admits as TriggerRestart and so loses its fraud checkpoint deferral. The effect is a premature but safe checkpoint broadcast (earlier fees, same funds outcome, no missed deadline), and the comment names the same-transaction trigger-stamp as the faithful follow-up shape should we ever want to close the gap.
e22f731 to
afc7b33
Compare
|
@claude appreciate the re-review and the independent FSM trace — good to have the premature-but-safe framing confirmed from your own read rather than taking my word for it. Nice catch on the zero-CSV guard being bypassed on the re-admit leg too; agree it collapses to the restart default and opens no new hole, but worth naming. Filed the follow-up as #914 (the same-tx trigger-stamp fix) so it doesn't live only in the code comment, and linked the issue from the comment at the re-admission site in |
|
Claude finished @Roasbeef's task in 17s —— View job Wrapping upThanks — that closes the loop cleanly. Tracking the same-tx trigger-stamp as #914 is exactly the right home for it: the code comment at the re-admission site names the degraded behavior, and now the issue carries the faithful fix so it won't get lost. To recap where the three findings landed:
Nothing outstanding from my side. LGTM. 👍 |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
In this PR, we unify every unilateral-exit ("unroll") trigger behind the
VTXO manager's single admission gate, so a coin that is exiting is always
persisted to
VTXOStatusUnilateralExit(out of the live set) before theunroll registry admits the job. This fixes a permanent wedge where a
cooperative
send --onchain --sweep-all(or any cooperative consumption)kept failing at the operator with:
The root cause
The VTXO manager was designed as the single admission gate, but two of the
four unroll triggers went around it. Manual (
da exit/ RPC) andcritical-expiry flow manager → FSM →
UnilateralExitState→ chain resolver→ registry. The other two did not:
fraud/actor.go) Asked the unroll registry directly.vhtlcrecovery/coordinator) admitted the registrydirectly, and its materializer persisted the recovery target as
VTXOStatusSpending.Spendingis status7, andListLiveVTXOsreturnsstatus < 3 OR status = 7. So the vHTLC recovery target was never flipped out of the live set:on the next daemon restart the manager rehydrated it as a live actor, it
re-entered coin selection and sweep-all, and the operator rejected the whole
round because it already considered the coin
unrolled_by_client. Everyretry re-included it, and it survived restarts. Fraud had the same shape
(the coin stayed live), and both were invisible to the
#400restartorphan-recovery scan, which keys off
UnilateralExit.The change
We make the manager own the exit for all four triggers.
ForceUnrollRequestnow carries the trigger and an optional exit-policy identity, threaded
string-typed through
ForceUnrollEventandExpiringNotificationinto thechain-resolver bridge, which converts them back into
unroll.StartTrigger/unroll.ExitPolicyKindat the one seam where both packages are in scope(they ride string-typed because
unrollalready importsvtxo, so the realtypes can't be referenced without a cycle). The manager can now
handleForceUnrolla VTXO with no live actor by re-materializing it from thepersisted descriptor, which is what lets it own the vHTLC recovery target
(materialized store-level, never a normal coin). vHTLC recovery now hands off
to the manager's
ForceExitseam, and the materializer persists the targetas
UnilateralExit, so it is out of the live set and inside the restartscan where an exiting coin belongs.
We also re-sequence boot recovery so the policy-bearing vHTLC restore admits
before the generic no-policy orphan scan: the
LazyChainResolverbuffersthe restore's admissions and replays them when the resolver is wired, and the
registry is first-writer-wins on exit policy, so a refund target must land its
policy first.
Review-driven hardening
An adversarial review of the "manager now owns the recovery target" topology
surfaced two failure-leg bugs, both fixed here:
#602no-footprint rollback) wouldrelive the recovery-only target into the live coin set, turning a
swap-contract output into spendable balance. We thread the job's exit
policy onto
ExitOutcomeNotificationand hold recovery-only targets inexit instead of reliving them.
standard timeout policy if restore failed for it once. We hand the scan the
durable exit policy of every non-terminal recovery target so it re-admits
under the right policy regardless of interleaving.
See each commit message for the detailed reasoning w.r.t the incremental
changes.
Testing
Unit coverage for the metadata threading, the manager admission gate
(spawn-from-descriptor + absent/terminal guards), the idempotent
UnilateralExitStatere-admission, the chain-resolver mapper, the recoverycoordinator handoff, and both review fixes (recovery-only hold on recoverable
failure, policy-carrying orphan scan). The full end-to-end vHTLC-refund →
unroll → sweep-all regression is a follow-up in
swapdk-server, where a realswap can be driven.