vnprc/hashpool
Watch
1
0
Fork
You've already forked hashpool
 0
Code Issues 16 Pull requests Projects Releases Packages Wiki Activity Actions

proxy wallet: sweep quotes across per-epoch currency units #23

Closed
vnprc-agent wants to merge 10 commits from agent/multi-unit-sweeper into agent/nutxx-pubkey-lookup
pull from: agent/multi-unit-sweeper
merge into: vnprc:agent/nutxx-pubkey-lookup
Conversation 6 Commits 10 Files changed 9 +1294 -130
vnprc-agent commented 2026-08-13 14:17:04 +01:00
Collaborator
Copy link

The proxy's quote sweeper now mints quotes in every per-epoch currency unit, not just the hash unit its wallet was built with.
Before this, after the first epoch rotation the sweeper fetched new-unit quotes from the mint, stored them, and silently never minted them.
Discovery unions two quote sources: the unit-agnostic pubkey lookup and one unit-blind read of the wallet DB's unissued quotes, so a failing lookup endpoint cannot hide DB backlog; units are never derived from mint info.
The sweeper builds a cheap per-unit wallet handle on first sight of a unit and runs the same sweep (status check, then P2PK-locked batch mint) once per unit; all handles share the one wallet database, seed, and mint metadata cache, so nothing changes on disk.
The faucet, web UI, and quote notification path keep using the single hash-unit wallet exactly as before.
Per unit and pass the whole pipeline is bounded: at most 200 quotes are selected (locally-mintable-looking first) and processed in chunks of 50 per batch call, so one bad quote id can wedge at most its own chunk; a rotation backlog drains over successive 15-second passes.
The reconcile log reports unissued counts per unit, which is the standing signal for fetched-but-never-minted divergence.
Proof: cargo check, test, and clippy are green with 12 unit tests on the discovery and selection logic; the end-to-end rotation witness still needs the mint-side epoch branch (#16) and runs on the dev stack after both land.
If it misbehaves, revert the two commits on this branch; the wallet DB schema is untouched, so rollback is safe.

Closes vnprc/hashpool#17. Refs vnprc/hashpool#13.

Risk

  • Per-unit wallet handles share one sqlite localstore and one metadata cache. Issue #17 verified the wallet schema and NUT-13 keyset counters are unit- or keyset-scoped, and units are swept sequentially within a pass, so there is no cross-unit contention; still, this is the first time multiple Wallet instances run over one store in this process.
  • Discovery no longer depends on the pubkey lookup alone: a lookup-endpoint failure (e.g. mint stops advertising a NUT-06 pubkey, or a NUT-XX regression) leaves every DB-known unit sweepable and visible in the reconcile log via the local-store union. Only quotes that never reached the DB at all (dropped SV2 notification while the lookup is also failing) wait for the lookup to recover.
  • Any unit with unissued quotes in the local store is batch-checked every pass while it has backlog, unpaid quotes included — the old single-unit behavior, generalized. Units holding permanently-unmintable quotes (e.g. expired unpaid) therefore stay in the scan and the log until settled or pruned; the cost is bounded (one sqlite read plus at most 4 status-check and 4 mint calls per unit per pass), and terminal-state eviction is deliberately left out of this PR.
  • The reconcile log format changed from Found N pending quotes to a per-unit summary line; anything scraping the old string would need updating.
  • Per-pass work per unit is bounded (200 selected, chunks of 50), so a very large backlog takes multiple passes to drain, logged each time it caps; quotes beyond the cap wait a pass even if paid, mitigated by selecting locally-mintable-looking quotes first so a stale head-of-line cannot starve them.

Validation

  • cargo check -p translator_sv2: clean.
  • cargo test -p translator_sv2: 32 passed, 0 failed (20 pre-existing, 12 new: unit-set derivation including the DB-backlog union, sweep planning covering ready-first ordering, cap and chunk boundaries and edge cases, per-unit unissued summary formatting).
  • cargo clippy -p translator_sv2 --all-targets: no new warnings. One pre-existing warning in untouched channel_manager.rs test code remains; a pre-existing useless-conversion warning in wallet.rs was removed by the rewrite.
  • rustfmt: both payment files are diff-free; in lib/mod.rs the edited region was formatted and two pre-existing drift spots elsewhere in the file were deliberately left alone to keep the diff minimal.
  • The union and the chunk planner are unit-tested as pure functions; the per-chunk network paths (batch status check, batch mint, chunk-failure isolation) are not exercised by tests here.
  • NOT validated here: the issue's witness — rotate mid-mining on the dev stack with #16's manual lever, watch the sweeper mint both old- and new-unit quotes, the per-unit reconcile log converge to zero unissued, and the faucet and web UI stay functional. That needs the mint-side branch and is the orchestrator's end-to-end step.
The proxy's quote sweeper now mints quotes in every per-epoch currency unit, not just the `hash` unit its wallet was built with. Before this, after the first epoch rotation the sweeper fetched new-unit quotes from the mint, stored them, and silently never minted them. Discovery unions two quote sources: the unit-agnostic pubkey lookup and one unit-blind read of the wallet DB's unissued quotes, so a failing lookup endpoint cannot hide DB backlog; units are never derived from mint info. The sweeper builds a cheap per-unit wallet handle on first sight of a unit and runs the same sweep (status check, then P2PK-locked batch mint) once per unit; all handles share the one wallet database, seed, and mint metadata cache, so nothing changes on disk. The faucet, web UI, and quote notification path keep using the single `hash`-unit wallet exactly as before. Per unit and pass the whole pipeline is bounded: at most 200 quotes are selected (locally-mintable-looking first) and processed in chunks of 50 per batch call, so one bad quote id can wedge at most its own chunk; a rotation backlog drains over successive 15-second passes. The reconcile log reports unissued counts per unit, which is the standing signal for fetched-but-never-minted divergence. Proof: cargo check, test, and clippy are green with 12 unit tests on the discovery and selection logic; the end-to-end rotation witness still needs the mint-side epoch branch (#16) and runs on the dev stack after both land. If it misbehaves, revert the two commits on this branch; the wallet DB schema is untouched, so rollback is safe. Closes vnprc/hashpool#17. Refs vnprc/hashpool#13. ## Risk - Per-unit wallet handles share one sqlite localstore and one metadata cache. Issue #17 verified the wallet schema and NUT-13 keyset counters are unit- or keyset-scoped, and units are swept sequentially within a pass, so there is no cross-unit contention; still, this is the first time multiple `Wallet` instances run over one store in this process. - Discovery no longer depends on the pubkey lookup alone: a lookup-endpoint failure (e.g. mint stops advertising a NUT-06 pubkey, or a NUT-XX regression) leaves every DB-known unit sweepable and visible in the reconcile log via the local-store union. Only quotes that never reached the DB at all (dropped SV2 notification while the lookup is also failing) wait for the lookup to recover. - Any unit with unissued quotes in the local store is batch-checked every pass while it has backlog, unpaid quotes included — the old single-unit behavior, generalized. Units holding permanently-unmintable quotes (e.g. expired unpaid) therefore stay in the scan and the log until settled or pruned; the cost is bounded (one sqlite read plus at most 4 status-check and 4 mint calls per unit per pass), and terminal-state eviction is deliberately left out of this PR. - The reconcile log format changed from `Found N pending quotes` to a per-unit summary line; anything scraping the old string would need updating. - Per-pass work per unit is bounded (200 selected, chunks of 50), so a very large backlog takes multiple passes to drain, logged each time it caps; quotes beyond the cap wait a pass even if paid, mitigated by selecting locally-mintable-looking quotes first so a stale head-of-line cannot starve them. ## Validation - `cargo check -p translator_sv2`: clean. - `cargo test -p translator_sv2`: 32 passed, 0 failed (20 pre-existing, 12 new: unit-set derivation including the DB-backlog union, sweep planning covering ready-first ordering, cap and chunk boundaries and edge cases, per-unit unissued summary formatting). - `cargo clippy -p translator_sv2 --all-targets`: no new warnings. One pre-existing warning in untouched `channel_manager.rs` test code remains; a pre-existing useless-conversion warning in `wallet.rs` was removed by the rewrite. - rustfmt: both payment files are diff-free; in `lib/mod.rs` the edited region was formatted and two pre-existing drift spots elsewhere in the file were deliberately left alone to keep the diff minimal. - The union and the chunk planner are unit-tested as pure functions; the per-chunk network paths (batch status check, batch mint, chunk-failure isolation) are not exercised by tests here. - NOT validated here: the issue's witness — rotate mid-mining on the dev stack with #16's manual lever, watch the sweeper mint both old- and new-unit quotes, the per-unit reconcile log converge to zero unissued, and the faucet and web UI stay functional. That needs the mint-side branch and is the orchestrator's end-to-end step. 
A CDK wallet only mints the unit it was constructed with, so after an
epoch rotation the sweeper fetched new-unit quotes but never minted
them. Split wallet construction into shared resources (one sqlite
localstore, one seed, one metadata cache) plus cheap per-unit wallet
handles built on demand. The sweeper derives the unit set from the
quotes returned by the pubkey lookup, lazily creates a handle per unit,
and runs the existing sweep body once per unit with the same P2PK
spending conditions. Mint batches are capped at 200 quotes per unit per
pass, handles are dropped once a unit has nothing left to mint, and the
reconcile log reports unissued counts per unit. The faucet and quote
notification handler keep using the base hash-unit wallet.
vnprc-agent commented 2026-08-13 19:16:05 +01:00
Author
Collaborator
Copy link

Adversarial review findings (read-only pass; fixes queued on this branch):

should-fix — unit discovery is hostage to the pubkey lookup. The sweep's unit set is fetched-quotes ∪ tracked-map ∪ base; the wallet DB is never enumerated. After a restart (tracked map empty), a persistent lookup failure that spares other endpoints (e.g. mint stops advertising a NUT-06 pubkey → MissingPubkey, or a NUT-XX regression — it's a fork-only extension) leaves foreign-unit unissued quotes in the DB unswept forever, while the base unit sweeps normally — and the per-unit reconcile log only covers units in the sweep set, so the advertised "fetched but never minted" divergence signal is blind to exactly this case. Transient failures are genuinely fine (quotes stay mintable server-side; next pass recovers). Fix: union in units from one unit-blind localstore.get_unissued_mint_quotes() call (zero network; the design doc forbids deriving from mint info, not from the local store) — this also makes the reconcile log cover every unit with DB backlog.

should-fix — the 200/unit cap bounds only batch_mint; stamping and batch_check_mint_quote_status stay unbounded. A large rotation backlog (the motivating case) produces one giant status-check POST; if that call fails (size limits, future mint-side batch cap), the unit retries the identical unbounded call every 15s with zero progress — and mint-side check_mint_quotes fails the whole batch on one unknown quote id, so a single pruned quote can block all other mintable value in its unit. Inherited shape, but this PR introduces the boundedness claim. Fix: cap/chunk the pending set before stamping and status-check; add per-unit backoff or poisoned-quote eviction.

nits — units with permanently-unmintable (e.g. expired-unpaid) quotes are tracked forever, scanning + batch-checking + logging every 15s across epochs (fold terminal-state eviction into the above); the tokio::sync::Mutex<HashMap> has exactly one holder ever — safe, verified no deadlock — but &mut owned by the loop would express the truth; WalletBuilder::build() writes set_ttl(Some(3600)) on the shared metadata cache per handle build (no-op today, silently resets any future custom TTL); two Risk bullets mischaracterize: "a lookup outage delays foreign-unit sweeping until the mint is reachable again" conflates endpoint-specific failure with mint unreachability, and "unpaid foreign-unit quotes are only re-checked once the mint reports them mintable" is false while a unit is tracked (they're batch-checked every pass — safe direction, still wrong).

Verified clean: eviction is provably safe under transient errors (drop only on a successful zero-row DB read); the metadata cache and localstore are genuinely shared (Arc identity, keyed by data pointer); faucet/notification wiring bitwise-equivalent; P2PK conditions and SplitTarget identical per-unit; all body validation claims reproduced exactly (29/0 tests, the clippy and fmt statements, 3-file diff, single revertable commit); zero file overlap with #24 so merge order is free — but note the merged system needs #24's cdk pin (3dea7086) for old-epoch quotes to stay checkable after processor retirement, and the e2e rotation witness (honestly disclosed as not-run) is what covers the combination.

Verdict: mergeable — no blockers; land the two should-fixes before #18 makes big backlogs routine.

Adversarial review findings (read-only pass; fixes queued on this branch): **should-fix — unit discovery is hostage to the pubkey lookup.** The sweep's unit set is fetched-quotes ∪ tracked-map ∪ base; the wallet DB is never enumerated. After a restart (tracked map empty), a *persistent* lookup failure that spares other endpoints (e.g. mint stops advertising a NUT-06 pubkey → `MissingPubkey`, or a NUT-XX regression — it's a fork-only extension) leaves foreign-unit unissued quotes in the DB unswept forever, while the base unit sweeps normally — and the per-unit reconcile log only covers units in the sweep set, so the advertised "fetched but never minted" divergence signal is blind to exactly this case. Transient failures are genuinely fine (quotes stay mintable server-side; next pass recovers). Fix: union in units from one unit-blind `localstore.get_unissued_mint_quotes()` call (zero network; the design doc forbids deriving from *mint info*, not from the local store) — this also makes the reconcile log cover every unit with DB backlog. **should-fix — the 200/unit cap bounds only `batch_mint`; stamping and `batch_check_mint_quote_status` stay unbounded.** A large rotation backlog (the motivating case) produces one giant status-check POST; if that call fails (size limits, future mint-side batch cap), the unit retries the identical unbounded call every 15s with zero progress — and mint-side `check_mint_quotes` fails the whole batch on one unknown quote id, so a single pruned quote can block all other mintable value in its unit. Inherited shape, but this PR introduces the boundedness claim. Fix: cap/chunk the pending set before stamping and status-check; add per-unit backoff or poisoned-quote eviction. **nits** — units with permanently-unmintable (e.g. expired-unpaid) quotes are tracked forever, scanning + batch-checking + logging every 15s across epochs (fold terminal-state eviction into the above); the `tokio::sync::Mutex<HashMap>` has exactly one holder ever — safe, verified no deadlock — but `&mut` owned by the loop would express the truth; `WalletBuilder::build()` writes `set_ttl(Some(3600))` on the *shared* metadata cache per handle build (no-op today, silently resets any future custom TTL); two Risk bullets mischaracterize: "a lookup outage delays foreign-unit sweeping until the mint is reachable again" conflates endpoint-specific failure with mint unreachability, and "unpaid foreign-unit quotes are only re-checked once the mint reports them mintable" is false while a unit is tracked (they're batch-checked every pass — safe direction, still wrong). Verified clean: eviction is provably safe under transient errors (drop only on a successful zero-row DB read); the metadata cache and localstore are genuinely shared (Arc identity, keyed by data pointer); faucet/notification wiring bitwise-equivalent; P2PK conditions and SplitTarget identical per-unit; all body validation claims reproduced exactly (29/0 tests, the clippy and fmt statements, 3-file diff, single revertable commit); zero file overlap with #24 so merge order is free — but note the merged system needs #24's cdk pin (`3dea7086`) for old-epoch quotes to stay checkable after processor retirement, and the e2e rotation witness (honestly disclosed as not-run) is what covers the combination. Verdict: mergeable — no blockers; land the two should-fixes before #18 makes big backlogs routine. 
Unit discovery was hostage to the pubkey lookup: after a restart, a
persistent lookup failure that spares other endpoints left foreign-unit
DB backlog unswept and invisible in the reconcile log. The sweep unit
set now also unions units from one unit-blind
localstore.get_unissued_mint_quotes() call (the unit filter in cdk is a
wallet-level retain, not SQL), so any unit with local backlog is swept
and logged regardless of lookup health.

The 200-per-unit cap previously bounded only batch_mint; stamping and
the batch status check ran over the whole backlog, and one giant status
POST could fail atomically every pass with zero progress. The cap now
bounds the whole per-unit pipeline: pending quotes are selected up
front (locally-mintable-looking first, so a stale head-of-line cannot
starve paid quotes behind it), capped at 200 per pass, and processed in
chunks of 50 through stamp, check, and mint. A failed chunk is logged
and skipped while the remaining chunks keep going, so a quote the mint
no longer knows wedges at most its own chunk.
vnprc-agent commented 2026-08-13 19:24:10 +01:00
Author
Collaborator
Copy link

Both should-fixes landed in c38658bf; PR body updated to match.

  1. Unit discovery no longer hostage to the pubkey lookup: the sweep unit set now also unions units from one unit-blind localstore.get_unissued_mint_quotes() call (verified at cba8469b the unit filter is a wallet-level retain, not SQL), so DB backlog is swept and shows in the per-unit reconcile log even when the lookup endpoint fails persistently.
  2. Whole pipeline bounded, not just batch_mint: up to 200 pending quotes are selected per unit per pass — locally-mintable-looking first, so a stale head-of-line cannot starve paid quotes behind the cap — then stamped, status-checked, and minted in chunks of 50; a failed chunk is logged and skipped while later chunks proceed, so an unknown quote id wedges at most its own chunk. No intra-pass backoff: the 15 s pass interval paces retries.

The two mischaracterized Risk bullets are corrected in the body. Tests now 32/0 for the package (12 sweeper units covering the union and the selection/chunk planner); clippy and fmt unchanged. Terminal-state eviction and the remaining nits are left as-is per scope.

Both should-fixes landed in c38658bf; PR body updated to match. 1. **Unit discovery no longer hostage to the pubkey lookup**: the sweep unit set now also unions units from one unit-blind `localstore.get_unissued_mint_quotes()` call (verified at cba8469b the unit filter is a wallet-level retain, not SQL), so DB backlog is swept and shows in the per-unit reconcile log even when the lookup endpoint fails persistently. 2. **Whole pipeline bounded, not just batch_mint**: up to 200 pending quotes are selected per unit per pass — locally-mintable-looking first, so a stale head-of-line cannot starve paid quotes behind the cap — then stamped, status-checked, and minted in chunks of 50; a failed chunk is logged and skipped while later chunks proceed, so an unknown quote id wedges at most its own chunk. No intra-pass backoff: the 15 s pass interval paces retries. The two mischaracterized Risk bullets are corrected in the body. Tests now 32/0 for the package (12 sweeper units covering the union and the selection/chunk planner); clippy and fmt unchanged. Terminal-state eviction and the remaining nits are left as-is per scope.
vnprc-agent commented 2026-10-07 04:47:09 +01:00
Author
Collaborator
Copy link

Re-pinned onto a reachable cdk revision and revalidated.

The cdk revision this branch inherited, cba8469b, is confirmed lost — it exists on no reachable remote and in no local object store (the full search is recorded on vnprc/hashpool#21). Merge commit 92bd651f brings in the updated base branch, which carries the replacement pin e5a85f810a91bd1d8882a4a60ed93fd60bb691c1 (vnprc/cdk nutxx-wallet-v2, workspace 0.17.0) together with master's devenv repair, without which devenv shell could not enter on this branch at all.

The merge was automatic, with no conflict. The sweeper source tree is byte-identical across it: git diff c38658bf..92bd651f -- roles/translator is empty, so nothing in this PR's actual subject matter changed.

Verified by the integrator independently of the worker that performed the merge:

  • roles/translator has zero diff across the merge, so the reviewed sweeper logic is untouched.
  • No occurrence of cba8469b remains in the tree; all 16 cdk entries in roles/Cargo.lock sit at e5a85f8, and the four pin sites agree.
  • MSRV lock pins intact by direct read: time 0.3.41, serde_with 3.17.0, darling 0.21.3, simple_asn1 0.6.3 against rustc 1.86.0.
  • Merge-duplication check, which plain test runs cannot catch: exactly one cdkCommit, one cdkRepo, and one [patch.crates-io] block per workspace. A merge that bound any of these twice would still have passed every check below.
  • cargo test -p translator_sv2 re-run by the integrator: 32 passed, 0 failed, matching this PR body's claim exactly (20 pre-existing plus the 12 payment::quote_sweeper::tests::* tests covering unit-set derivation, sweep planning, cap and chunk boundaries, and the per-unit unissued summary).
  • cargo check and cargo clippy -p translator_sv2 --all-targets clean; the one remaining warning is pre-existing test code at channel_manager.rs:900, not sweeper code.

No separate cold review pass was spent on this round, deliberately. The only change is a merge whose pin delta is identical to the one already cold-reviewed with no findings on vnprc/hashpool#21, over a source tree this merge provably does not touch; the merge-duplication trap that a reading might catch was instead checked mechanically above. The sweeper logic itself keeps the review pass and fix round it had in August. Per risk.md, passes stop where an available witness can execute the question instead.

Still not run, and unchanged from the original body: the end-to-end rotation witness — rotate mid-mining, watch the sweeper mint both old- and new-unit quotes, per-unit reconcile drain to zero. That needs the mint-side epoch branch and is the orchestrator's step once vnprc/hashpool#24 is buildable again.

Merge order: this PR's base is agent/nutxx-pubkey-lookup (vnprc/hashpool#21), so merge vnprc/hashpool#21 first, then this one.

Re-pinned onto a reachable cdk revision and revalidated. The cdk revision this branch inherited, `cba8469b`, is confirmed lost — it exists on no reachable remote and in no local object store (the full search is recorded on vnprc/hashpool#21). Merge commit `92bd651f` brings in the updated base branch, which carries the replacement pin `e5a85f810a91bd1d8882a4a60ed93fd60bb691c1` (`vnprc/cdk` `nutxx-wallet-v2`, workspace 0.17.0) together with master's devenv repair, without which `devenv shell` could not enter on this branch at all. The merge was automatic, with no conflict. The sweeper source tree is byte-identical across it: `git diff c38658bf..92bd651f -- roles/translator` is empty, so nothing in this PR's actual subject matter changed. Verified by the integrator independently of the worker that performed the merge: - `roles/translator` has zero diff across the merge, so the reviewed sweeper logic is untouched. - No occurrence of `cba8469b` remains in the tree; all 16 cdk entries in `roles/Cargo.lock` sit at `e5a85f8`, and the four pin sites agree. - MSRV lock pins intact by direct read: `time` 0.3.41, `serde_with` 3.17.0, `darling` 0.21.3, `simple_asn1` 0.6.3 against rustc 1.86.0. - Merge-duplication check, which plain test runs cannot catch: exactly one `cdkCommit`, one `cdkRepo`, and one `[patch.crates-io]` block per workspace. A merge that bound any of these twice would still have passed every check below. - `cargo test -p translator_sv2` re-run by the integrator: **32 passed, 0 failed**, matching this PR body's claim exactly (20 pre-existing plus the 12 `payment::quote_sweeper::tests::*` tests covering unit-set derivation, sweep planning, cap and chunk boundaries, and the per-unit unissued summary). - `cargo check` and `cargo clippy -p translator_sv2 --all-targets` clean; the one remaining warning is pre-existing test code at `channel_manager.rs:900`, not sweeper code. No separate cold review pass was spent on this round, deliberately. The only change is a merge whose pin delta is identical to the one already cold-reviewed with no findings on vnprc/hashpool#21, over a source tree this merge provably does not touch; the merge-duplication trap that a reading might catch was instead checked mechanically above. The sweeper logic itself keeps the review pass and fix round it had in August. Per `risk.md`, passes stop where an available witness can execute the question instead. Still not run, and unchanged from the original body: the end-to-end rotation witness — rotate mid-mining, watch the sweeper mint both old- and new-unit quotes, per-unit reconcile drain to zero. That needs the mint-side epoch branch and is the orchestrator's step once vnprc/hashpool#24 is buildable again. Merge order: this PR's base is `agent/nutxx-pubkey-lookup` (vnprc/hashpool#21), so merge vnprc/hashpool#21 first, then this one. 
Brings the cdk forge re-pin (github.com/vnprc/cdk -> ssh://git@forge.anarch.diy:2222/vnprc/cdk, rev e5a85f8 unchanged) plus the cargo fetch transport, justfile, and docs follow-ups.
vnprc-agent commented 2026-10-08 22:15:13 +01:00
Author
Collaborator
Copy link

Blocked by vnprc/hashpool#29, found by running this branch integrated with vnprc/hashpool#21 and #24 on a live dev stack.

Before any rotation this branch works: the sweeper minted ehash in the genesis epoch repeatedly, reaching a balance of 2752 across batches of 22 and 40 quotes. After just rotate-epoch it never mints again, in any unit, for the rest of the run:

reconciled 1 quote(s) (unit hash_<pool>_122: 1 unissued)
ERROR unit hash_<pool>_122: batch mint failed for a chunk of 1 quote(s): Unknown Keyset
ERROR unit hash_<pool>_122: batch mint failed ...: Quote already in use by another operation
ERROR unit hash_<pool>_122: batch mint failed for a chunk of 5 quote(s): Quote already in use by another operation

grep -c '_122 from' logs/proxy.log is 0 — not one quote in a post-rotation unit was ever minted, and the unissued count rose monotonically to 5 and stayed there. Every epoch after the first would lose all of its mining rewards.

Root cause of the first error is in this branch: WalletFactory::wallet_for_unit in roles/translator/src/lib/payment/wallet.rs is synchronous and makes no network calls, as its own doc comment states, so a new epoch's keyset is never fetched into the shared metadata cache that predates the epoch. The cheapness that makes per-unit handles free is what leaves them unable to mint a unit whose keyset is newer than the cache. The subsequent Quote already in use by another operation then appears to convert that transient failure into a permanent one.

Worth noting for the review record: all 12 of this branch's sweeper tests pass against the broken code, and the 32-test suite is green. The PR body honestly disclosed that the per-chunk network paths are not exercised by tests, and this defect lives exactly in that gap — unit-set derivation and chunk planning were tested as pure functions, while the mint call against an unseen keyset was not.

A fix is in progress. This PR should not merge until #29 is resolved and the rotation witness is re-run.

**Blocked by vnprc/hashpool#29**, found by running this branch integrated with vnprc/hashpool#21 and #24 on a live dev stack. Before any rotation this branch works: the sweeper minted ehash in the genesis epoch repeatedly, reaching a balance of 2752 across batches of 22 and 40 quotes. After `just rotate-epoch` it never mints again, in any unit, for the rest of the run: ``` reconciled 1 quote(s) (unit hash_<pool>_122: 1 unissued) ERROR unit hash_<pool>_122: batch mint failed for a chunk of 1 quote(s): Unknown Keyset ERROR unit hash_<pool>_122: batch mint failed ...: Quote already in use by another operation ERROR unit hash_<pool>_122: batch mint failed for a chunk of 5 quote(s): Quote already in use by another operation ``` `grep -c '_122 from' logs/proxy.log` is `0` — not one quote in a post-rotation unit was ever minted, and the unissued count rose monotonically to 5 and stayed there. Every epoch after the first would lose all of its mining rewards. Root cause of the first error is in this branch: `WalletFactory::wallet_for_unit` in `roles/translator/src/lib/payment/wallet.rs` is synchronous and makes no network calls, as its own doc comment states, so a new epoch's keyset is never fetched into the shared metadata cache that predates the epoch. The cheapness that makes per-unit handles free is what leaves them unable to mint a unit whose keyset is newer than the cache. The subsequent `Quote already in use by another operation` then appears to convert that transient failure into a permanent one. Worth noting for the review record: all 12 of this branch's sweeper tests pass against the broken code, and the 32-test suite is green. The PR body honestly disclosed that the per-chunk network paths are not exercised by tests, and this defect lives exactly in that gap — unit-set derivation and chunk planning were tested as pure functions, while the mint call against an unseen keyset was not. A fix is in progress. This PR should not merge until #29 is resolved and the rotation witness is re-run. 
After an epoch rotation the sweeper could never mint the new unit: the
shared mint metadata cache predates the new unit's keyset but still
looks "fresh" under KeysetLoadPolicy::CacheThenNetwork (populated,
within TTL), so every mint attempt failed with "Unknown Keyset"
forever. process_stored_quotes now forces one KeysetLoadPolicy::Refresh
the first time it builds a unit's wallet handle (ensure_keyset_cached),
cheap-probed with CacheOnly first so units sharing an already-fresh
cache don't pay for it again.

Investigated the second symptom (quotes permanently "already in use by
another operation" after the first Unknown Keyset): cdk's
MintSaga::prepare_batch reserves every quote in the batch before
looking up the active keyset, and unlike the single-quote mint path
has no wrapper that runs its registered compensation when that lookup
fails, so the reservation leaks with no saga record left behind to
recover from. That's cdk's own code (pinned dependency, out of scope
here). Added a unit test that reproduces the leak directly (Unknown
Keyset on the first batch_mint, QuoteAlreadyInUse on the second) and a
second test proving the keyset refresh clears the cache miss that
triggers it, so this specific wedge cannot occur once a unit's keyset
is refreshed before its first mint attempt. translator never calls
Wallet::recover_incomplete_sagas, so any other way a quote could end
up wedged (e.g. a genuine ambiguous network failure) still has no
recovery path here - a decision for the integrator, not fixed by this
change.
A failed batch mint leaks a quote reservation in the wallet's local
store: MintSaga::prepare_batch (cdk) reserves every quote and
registers a release-on-failure compensation, then looks up the active
keyset before anything runs that compensation. The single-quote path
wraps that lookup so an early failure still rolls back; prepare_batch
does not, so an early `?` (e.g. the unknown-keyset case already fixed
in hashpool#29, but any other failure in that window leaks the same
way) drops the compensation unrun. The reservation never clears, and
every later pass's batch_mint on that quote fails with "Quote already
in use by another operation" forever -- five quotes were observed
wedged this way on a live stack.

cdk exposes Wallet::recover_incomplete_sagas() for exactly this; the
translator never called it. Added recover_leaked_reservations(), run
once per unit's wallet handle at the start of every sweep pass (not
once for the whole store): recover_incomplete_sagas and its
cleanup_orphaned_quote_reservations sub-step both filter everything
they touch down to the handle's own (mint_url, unit), even though
every unit's handle shares one SQL-backed local store, so a call on
one unit's handle does nothing for another's leaked reservations.
Running it every 15s pass rather than only at startup is cheap: the
scan is local-store reads with no network I/O unless an incomplete
saga actually needs one, and that cost only shows up when there is
something to recover -- which is also what lets it clear a reservation
leaked earlier in the same run, not just a backlog inherited from a
previous process.

RecoveryReport does not count the orphaned-reservation branch this bug
hits (only the incomplete-saga branch), so recover_leaked_reservations
also counts this wallet's reserved quotes before and after the call
and logs when the count drops -- a silent recovery here is
indistinguishable from no leak at all.

New test leaked_reservation_recovers_and_mints_on_a_later_pass builds
on unknown_keyset_failure_wedges_the_quote_until_recovery_runs: it
drives a leaked reservation to the same wedged state, then shows
recovery clears it and the quote mints successfully on the next
attempt. Confirmed failing (quote stays reserved) with the recovery
call removed, and passing with it restored.
vnprc-agent commented 2026-10-08 23:56:21 +01:00
Author
Collaborator
Copy link

In plain English

Once the mint issues a separate kind of ecash for each mining epoch, the wallet has to be able to collect all of them. It could only collect the first kind. After the first epoch rotation it would notice the new rewards, write them down, and then never actually collect them — so every epoch after the first lost all its mining income.

This teaches the wallet to collect every kind of ecash it sees, not just the one it was built with, and to work through a backlog in bounded batches so one bad reward cannot block the rest.

Two real bugs this found, both now fixed here

Running the full stack turned up two faults that no test caught:

New epochs could not be collected at all. On first seeing a new kind of ecash, the wallet never asked the mint for the matching key, so every attempt failed. Rewards piled up uncollected forever. Fixed by asking the mint once, the first time a new kind appears.

A single failure wedged rewards permanently. When a collection attempt failed, it left the reward marked as "in progress" with nothing to ever clear it, so it could never be retried. Fixed by clearing those stuck markers on each pass, and saying so in the log when it happens.

Worth noting: all 32 tests on this branch passed while both bugs were live. They tested the planning logic, not the actual collection call. Only running the real thing found them.

Status

Verified on a running dev stack. Rewards are collected in every epoch, and a backlog of 55 previously stuck rewards drained in a single pass once the fixes were in.

Merge after vnprc/hashpool#21.

## In plain English Once the mint issues a separate kind of ecash for each mining epoch, the wallet has to be able to collect all of them. It could only collect the first kind. After the first epoch rotation it would notice the new rewards, write them down, and then never actually collect them — so every epoch after the first lost all its mining income. This teaches the wallet to collect every kind of ecash it sees, not just the one it was built with, and to work through a backlog in bounded batches so one bad reward cannot block the rest. ## Two real bugs this found, both now fixed here Running the full stack turned up two faults that no test caught: **New epochs could not be collected at all.** On first seeing a new kind of ecash, the wallet never asked the mint for the matching key, so every attempt failed. Rewards piled up uncollected forever. Fixed by asking the mint once, the first time a new kind appears. **A single failure wedged rewards permanently.** When a collection attempt failed, it left the reward marked as "in progress" with nothing to ever clear it, so it could never be retried. Fixed by clearing those stuck markers on each pass, and saying so in the log when it happens. Worth noting: all 32 tests on this branch passed while both bugs were live. They tested the planning logic, not the actual collection call. Only running the real thing found them. ## Status Verified on a running dev stack. Rewards are collected in every epoch, and a backlog of 55 previously stuck rewards drained in a single pass once the fixes were in. Merge after vnprc/hashpool#21. 
recover_leaked_reservations ran full saga recovery on every swept unit,
including the base unit the faucet drives with its own Send saga; two
drivers on one saga over shared proofs is a correctness hazard. Recovery
now skips the base unit, identified via the factory rather than a
hardcoded name.

ensure_keyset_cached logged a refresh failure and let the pass continue
into batch_mint anyway, recreating the leaked-reservation bug it exists
to prevent. It now reports success/failure and the caller skips the unit
for the pass on failure; the next pass retries from the local-store
backlog.

Also cuts test and scaffolding comments down to contract/trap per
allod/memory/writing.md.
recover_leaked_reservations skipped the base unit to avoid resuming the
faucet's Send saga, which also removed the only caller clearing a
base-unit reservation leaked by a failed mint -- it wedged forever.

Re-pin cdk to 2ac10545, which exposes cleanup_orphaned_quote_reservations:
it releases orphaned quote reservations without touching any saga, so
it's safe on the base wallet's shared handle. Recovery now runs
unconditionally for every swept unit, and uses the returned count
instead of querying mint quotes before and after.
vnprc-agent commented 2026-10-09 06:04:14 +01:00
Author
Collaborator
Copy link

Superseded by a squashed branch with the same tree. Closing and reopening because the forge refuses force-push on an open pull request branch; the replacement is byte-identical to the state verified here and carries the same base.

Superseded by a squashed branch with the same tree. Closing and reopening because the forge refuses force-push on an open pull request branch; the replacement is byte-identical to the state verified here and carries the same base.
vnprc-agent closed this pull request 2026-10-09 06:04:15 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
vnprc/hashpool!23
No description provided.