Unilateral exit reworked for the Breez SDK 0.25 flow (with automatic exit-state backups) - #84
Merged
Merged
Conversation
sethforprivacy
force-pushed
the
rebase/pr-19-breez-0.25
branch
from
September 24, 2026 16:25
ba102c6 to
cb25c99
Compare
PrepareUnilateralExitAsync quotes which leaves are worth forcing on-chain and what the exit costs; UnilateralExitAsync quotes, lets the caller veto, and builds the signed transaction set in one call, because exit quotes go stale silently as the wallet's tree moves. Both are mapped against the Breez.Sdk.Spark 0.22.0 binding (verified by reflection), with funding shortfall and spent-outpoint conflicts surfaced as typed exceptions and unknown SDK enum variants failing loudly rather than mislabeling broadcast instructions. Nothing here broadcasts; the SDK signs, the caller carries.
UnilateralExitSettings carries the disclosure acknowledgement (enforced server-side, the Stable Balance pattern) and an optional esplora override for funding discovery. The feature is gated by the FLINT_EXPERIMENTAL_UNILATERAL_EXIT environment variable so it exists only on hosts that opted in, and the funding key derivation constant (account 4607060', "FLT") is pinned here with the reasoning: a hardened non-standard account can never collide with BTCPay's own hot-wallet BIP84 account when the seed is shared.
UnilateralExitRecord persists an exit across its multi-day life: the quote the operator funded against (immutable identity columns), the per-exit funding key index, and the signed transaction set. A partial unique index enforces one active exit per store at the database level - the in-memory single-flight is an optimization, not the invariant - and updates are compare-and-set on the expected status with the JSON blobs coalesced, so a stale abandon can never clobber a build's only copy of the signed transactions. Contract tests run against the production EF store on a real Postgres.
SparkUnilateralExitService holds every guard: the disclosure gate, fee-rate
bounds, destination validation (shared with the sweep path so the two can
never drift), one exit at a time, and the recoverable-exceeds-fee rule
re-checked against a fresh quote inside the build's veto. Each exit gets
its own P2WPKH funding key at m/84'/{coin}'/4607060'/0/{index} so two exits
can never sign trees over the same funding outpoint; funding is discovered
through an esplora endpoint (mempool.space by default on mainnet,
configurable) without touching key material on the read path; and the build
re-quotes and re-persists the requirement before selecting funding, so a
top-up meeting the displayed number is always sufficient. A signed set is
persisted non-cancellably: a closed browser tab must not be able to discard
the only copy. The provisioner now carries the section across seed changes
like every other settings block.
One page, driven by the record's state: disclosure, quote form, funding (largest single confirmed output judged against the requirement, since the fees are paid from one output and a sum that adds up does not fund an exit), and the built transaction set with per-package submitpackage lines and broadcast-ordering instructions. The signed hex and commands render only for CanModifyStoreSettings - broadcasting is a money-moving capability, so view-only roles see counts, not hex. The controller holds zero policy and no JSON: the service hands the page typed data. Every route answers NotFound when the environment gate is off.
The trust model, limitations, sweeping docs and README no longer claim the plugin has no unilateral-exit path anywhere; they now scope the truth: every automated flow remains a cooperative exit, and the one unilateral path is the experimental, environment-gated, manually broadcast flow on the Advanced page. The limitations entry states the four hard limits plainly - the plugin never broadcasts, the pinned SDK still needs the operators reachable, the CPFP funding is hand-supplied, and settlement waits out multi-day CSV timelocks - plus the explorer disclosure caveat. Sweep-engine, sweep-record and Greenfield comments are rescoped the same way; the API remains deliberately exit-free.
…tus model, backup The 0.25 exit API inverts the flow PR 19 was written against: prepare now takes the funding kind and returns the document the build consumes, transactions carry a status union rather than a flat confirmation enum, and a wallet can be checked against the chain and backed up. This reworks the seam, the record and service, the page and the suite onto it. Also fixes two 0.23->0.25 breakages that were not PR 19's: GetSparkStatus now takes a request, and CrossChainRoutePair.supportedSources became acceptedAssets.
check_unilateral_exit reads the chain and nothing else, so the leaf values it is handed are not an input to the verdict. Say so where the placeholder is built, and say why it is zero rather than a plausible number: a zero cannot be mistaken for a real leaf value by a later reader.
The Advanced page, the exit controller's banner and the CHANGELOG all told an operator that a stored backup is imported automatically on the next wallet start. Nothing did it: ExitStateBackup was written by SetExitStateBackupAsync and read only to render a 'Stored' badge, so the blob was data the plugin recorded and never used. The backup is the recovery path for a wallet whose own storage is lost while the Spark operators are gone, so an operator was told their exit data was secured and would find out otherwise only when they needed it. The import now runs on the warm-up path, fire-and-forget like the first sync and behind the same exception boundary, so it cannot hold up BTCPay's startup. It is ordered before the sync rather than after, because the sync needs the operators and the import is for when they are gone. Nothing about it can fail a connect: a wallet whose backup will not import still has a working Lightning wallet, and the failure is logged rather than raised. The blob itself is never logged on either path.
The backup is imported on connect, and a failure there is deliberately not fatal, which leaves the server log as the only place it is visible. Tell the operator that rather than letting them assume a stored backup was applied.
…ented The comment said the import ran before the first sync and the code ran it after. The ordering the comment describes is the correct one: the SDK collects a leaf's exit data as it learns about the leaf, so importing first means a leaf whose chain existed only in the backup is present before anything asks the operators about it. The old order spent a round trip confirming a leaf set the import might have expanded, in the situation where the operators are least likely to answer.
Restoring this project alone rewrote 86 transitive pins downward relative to main, because BTCPay's dependency graph resolves differently for a single project than for the solution. Those unrelated downgrades had ridden along under a commit message about a code comment. This branch should change exactly one thing in these files — the Breez.Sdk.Spark pin, which is what the work is about — so both are restored from main and only that pin is re-applied. The diff is now the SDK bump and nothing else.
The exit surface was verified against the SDK's contract, the seam, a fake and unit tests, and none of that broadcasts anything — because the SDK never does, and the plugin's design is that the operator pushes the transactions out by hand. So the one thing no test had done is the thing the feature is for: force a real balance on chain through the real statechain tree. This drives the plugin's own code — SparkExitFundingKey for the funding key, the seam's quote/build/check — and broadcasts the result exactly as the exit page instructs: fan-out and sweep alone, tree nodes as packages via submitpackage, mining between rounds so each CSV timelock matures. Progress is read from the SDK's verdict and per-transaction readiness, the same two things the page renders. Verified passing against the fixture, and the assertions were mutation-tested: making the WaitingForDependencies status map to Ready instead of Waiting fails it at build time, naming the transaction that would be broadcast before its parent confirmed. Three things the runs taught, each now encoded: - The readiness invariant is 'at least one step Ready, nothing Unverified, and no step with an unconfirmed dependency claiming Ready'. Asserting 'all Ready' failed against correct behaviour, because an exit is a chain. - What arrives is recoverable + unspent funding - total fee, not recoverable - fee. Measured 149,901 + 2,914 - 2,620 = 150,195, which looks wrong and is the documented arithmetic: the CPFP and fan-out fees come out of the funding output, not the recovered value. - A multi-leaf exit on a shared chain hit the SDK's Redo verdict when another wallet mined concurrently, so the test pins to one leaf. That is the documented single-leaf shape and it keeps the test honest about what it proves. The test is its own trait and its own collection. It cannot share the suite's wallet, because it exits the balance and would starve the other tests, and it cannot share the stack, because a second wallet funded from the same SSP while the suite ran produced a measured failure in the suite's Lightning send. local-regtest.yml runs it as a separate step, after the suite and after an explicit SSP top-up: an exit converts its wallet to on-chain Bitcoin, which the fixture's return leg cannot undo, so each run permanently costs the SSP a wallet's worth. Also regenerates both lock files with --force-evaluate. CI restores in locked mode and the previous commit's restore from main left them inconsistent.
…ocal one CI restores in locked mode and was failing with NU1004 on the plugin project: the lock had been generated against the btcpayserver working tree on this machine, which sits at v2.4.2 because v2.4.4 does not build with the local .NET 10.0.400 Razor toolchain, while CI checks out the commit the repo actually records (v2.4.4). Two different dependency graphs, one lock file, and locked mode refuses the mismatch. Restore does not compile anything, so the locks can be generated against the right submodule even where that submodule cannot be built: checked out v2.4.4, regenerated both with --force-evaluate, and confirmed the exact two commands ci.yml runs now pass in locked mode. The diff is large and that is expected rather than drift: bumping Breez.Sdk.Spark from 0.23.0 to 0.25.0 changes that package's own dependency set, so the whole resolved graph downstream of it moves with it.
Category=LocalRegtestExit is the one category that is not revenue-neutral: an exit turns its wallet into on-chain Bitcoin, which the return leg cannot undo, so each run costs the SSP a wallet's worth and repeated runs empty it. Also records why it is its own CI step rather than part of Category=LocalRegtest - it cannot share the wallet, and a measured run showed it cannot share the stack either.
Copy, at the operator's direction: - the fee-rate hint now says what the rate is for and that it must clear the mempool minimum, because one rate prices the whole chain over hours or days; - the destination hint says the funds end up there and that changing it later is costly; - the 'Quote an exit' sub-copy is gone entirely; - the empty-selection refusal is now 'No leaves are large enough to exit properly at the selected fee-rate...', which tells the operator what to do rather than describing what Spark decided. The fee rate itself now defaults to mempool.space's half-hour recommendation, fetched through the explorer client the exit already has, clamped to the service's bounds, cached five minutes, and never throwing — an unreachable API or a regtest chain with no mempool.space leaves the field at a floor of 2 sat/vB instead. The recommendation is only fetched when the quote form will actually render, and a record's own rate still wins over it. Also enables the exit gate on the e2e BTCPay stack, which is where the product is exercised against real operators and was the one place the exit could not be seen. The Advanced page's backup paragraph now answers the question it invited: a backup is not an alternative to exiting, it is what keeps exiting possible, because collecting that data needs the operators up and the moment you need an exit is the moment you can no longer collect it.
The exit is the one screen in this plugin where the wording is load-bearing — it is read by someone whose operators have stopped answering — and it is also the hardest screen to look at: the routes 404 unless the experimental gate is on, and the states that matter (a quote that refused, an exit waiting on funding) only exist after real money has moved. So render them directly instead. Seven facts execute the plugin's own compiled pages over the view models SparkController actually projects, with the real view components, and write standalone HTML carrying BTCPay's own stylesheets. That makes every state reviewable in a browser with no login, no funding and no gate, and re-reviewable after the next copy edit without re-deriving any of this. Its assertions are only 'HTML was produced and is non-empty' — it is a review tool, not a behavioural test, and it should be read that way. Delete it if the maintenance weight ever outweighs that.
The backup was the operator's job: press Export, keep the copy, remember to do it again after anything arrives. That is exactly the job an always-on server should be doing, and the moment it matters most is the moment nobody is there to do it — the data an exit is built from needs Spark's operators to be reachable to collect, so the moment you need an exit is the moment you can no longer collect it. The plugin was already being told when leaves appear and throwing it away: the SDK event listener maps NewDeposits and ClaimedDeposits and the consumer only logged ClaimedDeposits. Those, plus any inbound payment, now request a refresh; a two-minute debounce coalesces the bursts an event stream produces, and an hourly safety net covers the channel being bounded and the code already documenting events as unreliable in both directions. Nothing is rewritten when the exported bytes are unchanged, because each write is multi-megabyte. The backup is a secret that grows with the wallet and it now gets written repeatedly, so it no longer lives in the store's JSON settings column, which is read in full on every settings read. It is one owner-only file per store under the data directory, written temp-then-rename so a half-written backup can never be the thing that gets imported, and a store upgrading from the old plugin has its stored blob imported, moved to the file, and the setting cleared — in that order, so the only copy of an exit cannot be lost to the migration. The Advanced page stops asking the operator to do it and starts showing them how current the automation is: the time it was last written, and a Download that hands over the stored copy. That copy is on the same server as the wallet, and the copy says so — a backup that dies with the machine is not a backup, and the page should not imply otherwise. Exporting by hand still works and now also refreshes the stored copy, so the bytes in the operator's clipboard and the bytes on disk cannot disagree.
A fresh-context pass over 1f02f4d returned CONFIRMED with six findings. Four were real; two were correct as designed and one of those was the verifier being careful rather than the code being wrong. The one that mattered: adoption cleared the deprecated settings slot in the database but not in the service's settings cache, and both whole-settings writers rebuild their payload from that cache. So for a store upgrading from the old plugin, the first settings action after a successful adoption — acknowledging the disclosure, setting the explorer URL — would write the multi-megabyte blob straight back into the settings column the move existed to get it out of. No loss and no new exposure, but the cost came back permanently, since adoption is never consulted again once the file is non-empty. Clearing now empties the cached instance too, and the clear is a seam of its own rather than a SetAsync, because a clear must not reconcile the running wallet. Second: clearing a backup only deleted the file. For a store whose legacy value had not been adopted yet, the next connect adopted it back and imported exactly the leaves the operator had asked to discard, while the page told them a restart would import nothing. A clear now clears both locations. Third: only the rename was guarded, so a failure inside the write itself — disk full, an IO error part-way through several megabytes — left a partial copy of the secret beside the real backup with nothing to remove it. The whole write path is guarded now; the target file is still only ever reached by the rename. The download's cache control needed no code: this controller already carries [ResponseCache(NoStore = true)] for every action, so the advisory was the verifier being right to ask and wrong about the answer. That is now pinned by a test rather than by an argument, along with the download's bytes and its nothing-stored redirect. The remaining advisory is recorded, not fixed: the whole automation is behind FLINT_EXPERIMENTAL_UNILATERAL_EXIT and is inert on a default deployment, like the rest of the exit feature.
The automatic-backup commit's own message says NewDeposits and ClaimedDeposits both request a backup refresh, but only the claim was given a case: a NewDeposits envelope falls through the consumer's default arm and is logged at trace, so money detected on-chain before its claim requested nothing. The listener maps it; the consumer threw it away. NewDeposits is the claim's precursor, not a leaf yet, so today's exit state does not cover it — but the claim usually lands well inside the two-minute debounce, and a refresh requested here means the pass that runs after it exports the leaf even when the claim event itself is one of the drops the event channel is documented as producing. Logged at debug: the claim's own operator-level line follows it, and a precursor that duplicates it would double-log every deposit. The harness now carries the real scheduler, so the test reads the pending mark directly instead of waiting on a debug log line.
The scheduler's belief about what is stored now moves with every write through the store seam, not just the scheduled pass. A decorator (TrackedExitStateBackupStore) composes the file store with the scheduler in DI, so the manual export, the paste, the clear and the legacy adoption cannot leave the belief stale behind their writes, and no future fifth writer can forget the bookkeeping — at a call site, WriteAsync looks complete without it. Reads stay pure pass-throughs: the pass seeds its own belief from a ReadAsync it makes itself, and incidental reads must not re-seed it. A delete notes null unconditionally once it returns, so a cleared backup self-heals on the next due pass — the page already promises the plugin keeps the backup current on its own. An export that comes back empty with nothing pending now records an idle pass. The pass ran and learned nothing, and recording that is the only thing that stops a wallet with no exit state yet from being asked at the task's one-minute cadence forever; the safety net re-asks it on schedule. A request that IS pending records nothing, exactly as before: it was earned by a real event, an empty answer does not serve it, and the next pass asks again on the event's behalf (ExitStateBackupScheduler.MarkIdlePass). The download streams instead of holding the secret twice in memory: a string read re-encoded to a byte array was the whole multi-megabyte blob twice over to produce one pass-through copy (IExitStateBackupStore.OpenReadAsync, FileShare.Read because a concurrent temp-rename replace is safe on POSIX — the open handle keeps the old inode). The backup file itself is now owner-only, not just the directory holding it: the temp is SetUnixFileMode(0600) before the rename, so no umask window ever carries a readable copy of the wallet's exit data, independent of the directory's 0700. The download's type pin moved FileContentResult to FileStreamResult with every asserted fact kept; the two new OpenReadAsync tests pin the absent-vs-stored answers and the rename-while-open semantics against the real filesystem. A fresh-context verifier confirmed the full acceptance: decorator truth incl. the seeding-after-restart path, streamed bytes byte-for-byte, 0600 mode, both cadence behaviors, and the whole suite (1613 total, 1480 passed, 0 failed, 133 env-gated skips).
sethforprivacy
force-pushed
the
rebase/pr-19-breez-0.25
branch
from
September 24, 2026 16:44
cb25c99 to
afd183f
Compare
sethforprivacy
changed the base branch from
main
to
sfp/flint-usdc-usdt-btcpay-03e659
September 28, 2026 10:57
28 tasks done
sethforprivacy
marked this pull request as ready for review
September 28, 2026 13:35
sethforprivacy
changed the base branch from
sfp/flint-usdc-usdt-btcpay-03e659
to
main
September 28, 2026 13:36
This was referenced Sep 28, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft tracking branch: PR #19's unilateral exit rebased onto the Breez SDK 0.25.0 binding, plus the automatic exit-state backup work from the 2026-09-14..16 sessions and today's review follow-ups.
Intended to supersede/rebase PR #19 once the SDK 0.26.0 bump lands (in flight on another branch); topology decided then.
State:
Today's commits: