Repository navigation
fix(tollwallet): bound the fee precheck's keyset fetch against wedged mints - #532
Amperstrand wants to merge 2 commits into
Conversation
… mints SwapFeeSats' keyset fetch rides the lib's retry ladder (30 s per attempt, up to five attempts, two chained endpoints), so a wedged mint parked the payment's fee precheck for minutes before any deadline applied — freezing the payment lane and defeating PurchaseSession's receive timeout (#525; goroutine evidence in the issue). The fetch now runs under a 3 s budget; on expiry SwapFeeSats errors and callers fall through to Receive's own classification, which the WalletPort contract already allows. The abandoned goroutine ends on the lib's own schedule into a buffered channel — nothing waits on it.
|
Verification run (worktree at head Battery. Mechanism walk. The bound is a wall-clock race, not a context cancellation: One blocking nit — wrong PR link in the CHANGELOG. The new entry under Minor, non-blocking: a customer retrying against a still-wedged mint stacks one abandoned fetch goroutine per attempt for up to the ladder duration (~minutes); the payment lane unfreezes regardless, so this is a fragility note, not a defect. Test hygiene is fine — the budget-var mutation is safe because the package's tests don't run in parallel. Everything else checks out: two commits, one logical change, no attribution footers, new paths covered by real-network-shape tests, changelog otherwise accurate (the ~33 s arithmetic matches 3 s budget + 30 s receive timeout). Disposition: request-changes — solely for the |
|
Comparison evidence pass (tested side by side with #533, both green, both fresh on |
|
Closing as superseded by #533 — agreed with the review's architectural read: the 3 s budget is a payment-lane SLA, and policy belongs at the lane's call site (the merchant.go call site — its single production SwapFeeSats caller), not inside the WalletPort adapter. #533 carries that shape with its own tests, verified green side-by-side with this branch. Two things worth carrying forward from this PR's review:
|
CheckTokenSpendable's mint call rode the gonuts client's full retry ladder, so a wedged mint (accepts the connection, never answers — #525's class) would park wallet recover for minutes. The call now runs under a 10s budget (goroutine-abandon idiom; the abandoned request terminates on the client's own schedule) and budget expiry surfaces as an error, which recover already classifies as unknown — never spent. Self-contained twin of the closed #532's keyset-fetch bound; naming (silentMint) matches so the helpers can unify if that idiom re-lands.
CheckTokenSpendable's mint call rode the gonuts client's full retry ladder, so a wedged mint (accepts the connection, never answers — #525's class) would park wallet recover for minutes. The call now runs under a 10s budget (goroutine-abandon idiom; the abandoned request terminates on the client's own schedule) and budget expiry surfaces as an error, which recover already classifies as unknown — never spent. Self-contained twin of the closed #532's keyset-fetch bound; naming (silentMint) matches so the helpers can unify if that idiom re-lands.
|
Technical review (evidence-gathering pass, not an approval). What it does. Bounds the fee precheck at the wallet layer: Verification on this head ( Reconciliation with the other #525 fixes — these are layers, not duplicates:
One nit, worth a one-line fixup: the changelog entry links #529 (the stage-1 epoch PR) instead of this PR — copy-paste slip in the PR link, the prose itself is accurate (including the honest ~33 s customer-visible timeout figure). Disposition: land (with the link fix, or the maintainer's squash-message edit covers it) — then #533 behind it. |
CheckTokenSpendable's mint call rode the gonuts client's full retry ladder, so a wedged mint (accepts the connection, never answers — #525's class) would park wallet recover for minutes. The call now runs under a 10s budget (goroutine-abandon idiom; the abandoned request terminates on the client's own schedule) and budget expiry surfaces as an error, which recover already classifies as unknown — never spent. Self-contained twin of the closed #532's keyset-fetch bound; naming (silentMint) matches so the helpers can unify if that idiom re-lands.
CheckTokenSpendable's mint call rode the gonuts client's full retry ladder, so a wedged mint (accepts the connection, never answers — #525's class) would park wallet recover for minutes. The call now runs under a 10s budget (goroutine-abandon idiom; the abandoned request terminates on the client's own schedule) and budget expiry surfaces as an error, which recover already classifies as unknown — never spent. Self-contained twin of the closed #532's keyset-fetch bound; naming (silentMint) matches so the helpers can unify if that idiom re-lands.
CheckTokenSpendable's mint call rode the gonuts client's full retry ladder, so a wedged mint (accepts the connection, never answers — #525's class) would park wallet recover for minutes. The call now runs under a 10s budget (goroutine-abandon idiom; the abandoned request terminates on the client's own schedule) and budget expiry surfaces as an error, which recover already classifies as unknown — never spent. Self-contained twin of the closed #532's keyset-fetch bound; naming (silentMint) matches so the helpers can unify if that idiom re-lands.
Closes #525.
What
SwapFeeSats' keyset fetch rode the wallet library's full retry ladder — 30 s per HTTP attempt, up to five attempts, two chained endpoints — so a wedged mint (accepts nothing, never answers; PRTA #95's class) parked the payment's fee precheck for minutes before any deadline applied: goroutine evidence in #525 shows both in-flight payments parked atPurchaseSession → SwapFeeSats → GetMintActiveKeyset, minutes deep.The fetch now runs under a 3 s budget (
mintKeysetFeesBounded); on expirySwapFeeSatserrors and callers fall through to Receive's own classification — exactly the fallback theWalletPortcontract already documents for this method. The abandoned fetch goroutine ends on the library's own schedule into a buffered channel; nothing waits on it.Verification
Unit (new, red/green): a real silent TCP listener (accepts, never answers) —
TestMintKeysetFeesBoundedAgainstWedgedMintreturns the budget error in 0.30 s;TestMintKeysetFeesUnboundedBlocksOnWedgedMintpins the original still blocked at 2 s (the regression witness).Integration (cloud-lab, frozen mint, this fix stacked on #511):
payment-processing-timeout, "may still complete. Check your usage before retrying"late Receive completed … amount=50 — granting the paid-for session; steps converted, gate opened, MAC authorizedThis is also the first live validation of #511's late-reconciliation claim (previously unit-proven only): the two fixes compose exactly as designed — the budget gets the request to the timeout, the timeout hands off to the reconciler.
make go-battery: 16 modules green.Follow-ups noted in #525
The strictly-better variant (serve the fee from the wallet's cached keyset —
InputFeePpkis already in bolt) needs a small exported accessor in gonuts-tollgate; this PR's bound makes the lane safe now without a dependency-bump cycle.