Repository navigation
fix(boot): guard undrained onchain-receive rows; don't claim a legacy send row off one spend - #342
Conversation
… send row off one spend Two hazards that only bite rows written before a schema or corridor change, both on production data that predates the change. Boot guard (onchain-receive): #292 removed the onchain:BTC->arkade:BTC corridor with no boot check and no migration, so an upgrade past it silently strands any row still mid-swap in the now-dead receive_onchain_swap table. createServices now counts the live rows in that table across both DB layouts -- the consolidated swap file and the suffixed -onchain-receive file a split deployment no longer opens -- and refuses to start when any remain, naming the files. The operator drains them on a build that still serves the corridor, or sets ALLOW_UNDRAINED_ONCHAIN_RECEIVE=true to boot and abandon them deliberately. Legacy send recovery: the claiming-state recovery path booked claimed from a single provable spend when lockupLeftover reported nothing uncollected. For a row written before the lockup_outpoints column existed that check is vacuously empty -- it has no adopted set to compare against -- so a partially-claimed multi-output lockup would be closed claimed with the rest abandoned. It now skips the shortcut for a null-outpoints row and escalates to needs-review, the same posture a non-legacy partial claim already takes. Tests cover the guard across both layouts, the knob's exact-match read, and the legacy recovery row; the recovery test fails on the pre-fix code.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Review — @d4rp4t
Both fixes look correct after cross-checking the removed corridor's source.
Boot guard (onchainReceiveDrainGuard.ts)
Table name verified. const TABLE = 'receive_onchain_swap' matches the CREATE TABLE IF NOT EXISTS receive_onchain_swap statement in the store removed by #292. (The #292 PR description has a typo — onchain_receive_swap — but the DDL is receive_onchain_swap. The guard has the right name.)
LIVE_STATES list verified exhaustive. Cross-checked against the state machine removed in #292:
quoted { live: true, to: ['awaiting_confirmations', 'refused'] }
awaiting_confirmations { live: true, to: ['funding_arkade', 'refused', ...] }
funding_arkade { live: true, to: ['awaiting_claim', 'stuck'] }
awaiting_claim { live: true, to: ['claimed', 'refunding_arkade', 'stuck'] }
claimed { live: true, to: ['settled', 'stuck'] }
refunding_arkade { live: true, to: ['claimed', 'refunded', 'stuck'] }
Terminal states (settled, refunded, refused, stuck) are correctly excluded. claimed being in LIVE_STATES is intentional: in this corridor it is an intermediate state between verifying the preimage and confirming the onchain settlement — it needs driving to settled.
Split-file path correct. suffixed(swapDbPath, 'onchain-receive') exactly reproduces the removed layout entry (layout.onchainReceive = suffixed(swapDbPath, 'onchain-receive') in #292's diff). Both deployment topologies are covered.
Cleanup on guard trip (services.ts:517-520). swapFile.close() is called before the throw. At that point swapFile is the only open handle — resolveDbLayout is pure path computation, the split-file driver in findUndrainedOnchainReceive is opened/closed inside a try/finally, and no stores are opened until line 521. The error path is clean.
Escape hatch design. Env-only, exact-string 'true', not persisted in the config document — the boot error message tells operators how to invoke it, so it cannot be left silently enabled by accident. The read-only report path being left unguarded is the right call for observability.
Legacy claiming fix (onchainOrchestrator.ts:1135)
Direction is correct and conservative. For a legacy row (lockupOutpoints === null) the adopted-outpoint set is unknown, so lockupLeftover being vacuously empty is not proof the whole lockup was swept. The old code would transition to claimed off one provable spend and silently abandon any other funded outputs. The fix parks the row in stuck for review, which is the right call: money stays in the system until a human can assess it rather than the solver declaring victory.
The test correctly sets lockup_outpoints = NULL directly in the driver, confirms state === 'stuck', and checks that failureReason contains 'no claim txid' — matching the fail message at line 1141.
Config (config.ts)
allowUndrainedOnchainReceive: env.ALLOW_UNDRAINED_ONCHAIN_RECEIVE?.trim() === 'true' — the ?.trim() handles undefined correctly (evaluates to false). No cross-repo consumers of the Config interface were found in the other SDK and service clones.
Test coverage
All key cases covered: never-created table → 0; consolidated file counts only live rows; all-terminal → 0; live rows in the suffixed split file. The config tests confirm the exact-string semantics. The orchestrator test covers the new legacy-row park path. No gaps.
No blocking issues. LGTM.
Summary
Two guards for legacy rows a later schema change left the daemon unable to drive correctly.
Boot guard for undrained onchain-receive rows
#292 removed the onchain-receive corridor without a boot check, so an upgrade past it silently strands any row still mid-swap in the now-dead
receive_onchain_swaptable — the daemon no longer serves that corridor, so those swaps never progress and nobody is told.findUndrainedOnchainReceivecounts live rows (non-terminal states) in that table across both DB layouts: the consolidated swap file and the suffixed-onchain-receivefile a split deployment no longer opens.createServicesnow refuses to start when any are present, naming the files that hold them. The read-only report/watch path (openReportReaders) is deliberately left unguarded, so an operator can still inspect the stranded rows.The refusal is overridable with
ALLOW_UNDRAINED_ONCHAIN_RECEIVE=true, which boots anyway and leaves those rows undriven. This is an emergency hatch, not a persisted setting: it is env-only (not wired into the config document / console), discoverable from the boot error message, so it cannot be left silently enabled.onchain-send: don't book
claimedoff one spend for a legacy rowIn the onchain-send
claimingrecovery branch, a row predating thelockup_outpointscolumn (lockupOutpoints === null) has no adopted outpoint set, solockupLeftoveris vacuously empty. The old code read that emptiness as "fully claimed" and transitioned toclaimed— which, for a multi-output lockup the client funded, would abandon the outputs that one provable spend did not cover. Such a row now goes to review instead.Tests
test/db/onchainReceiveDrainGuard.test.ts(new): never-created table → 0; consolidated file counts only live rows; all-terminal → 0; live rows found in the suffixed split file.test/config.test.ts:ALLOW_UNDRAINED_ONCHAIN_RECEIVEadded to the config-key set; defaults false, true only on exact'true'.test/send/onchainOrchestrator.test.ts: a legacy row with no recorded outpoints parks (stuck) instead of claiming off one provable spend.All four gates green (typecheck, test, format:check, build).