Repository navigation
Fix four B2B keeper gaps found in the live sandbox tests - #1429
Merged
Merged
Conversation
The keeper only quoted on Ethereum mainnet, so on the Sepolia sandbox no projection defer ran and below-floor or over-cap swaps reverted on chain and backed off instead of waiting with a partner-visible reason. Pin the QuoterV2 per chain id and keep the unquoted first-route fallback only for chains without one.
The marker starts the RECOVERY_DELAY clock. A failed attempt or a retry backoff skip sent nothing, so a deposit whose every attempt reverted kept batchOpenedAt at zero until the deadline job marked it recovering, and its refund landed a whole extra delay late.
A skip that waits on another execution leaves the poke to that execution's own sequence, so an implicit-nonce poke never races it. A backoff reached by the top-of-cycle pending check now falls through to the slot check, which pokes, instead of returning a cycle early.
applyFeePolicy is permissionless and the contract expects the keeper to finalize an announced increase once its 24 h timelock elapsed, but no keeper code called it, so increases stayed pending forever.
A timed-out apply left the increase looking pending, so every later cycle re-sent it, stacked keeper nonces and held the worker loop for another receipt timeout. The test also pins that the apply lands before the swap reads the fee policy.
Without it the fixture fell into the apply path and logged a spurious getBlock failure on every run.
A payment below MIN_SWAP_FLOOR never arms the forwarder's batch marker, so recover() cannot move it and the oldest-first planner held every later deposit of the account until the refund deadline. The planner now passes over it with a below_minimum waiting reason, and the stranded balance monitor flags a marked refund the contract cannot recover so the operator refunds it by hand.
A recovering deposit whose funds left the clone through an unrecorded path (permissionless swap/forwardAll) would have been flagged for a manual refund, paying the client twice. The alert now also names the memo that keeps a later keeper recover exactly-once.
The check compared only the stuck deposits' EURe with the clone balance, so EURe of a younger settling deposit or left by an earlier manual refund could stand in for funds that already left on the permissionless path, prompting a double refund.
✅ Deploy Preview for vrtx-dashboard canceled.
|
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This was referenced Oct 9, 2026
✅ Deploy Preview for vortexfi canceled.
|
This was referenced Oct 9, 2026
A hand refund of a payment stuck below MIN_SWAP_FLOOR left its EURe on the clone, bypassed the one-refund-per-client gate, and left no recovery record. Topping the clone up to one wei above the floor lets poke arm the marker, so the keeper's normal recover and refund pipeline refund the payer's own EURe after RECOVERY_DELAY; the top-up stays on the clone below the floor and the odd wei keeps it unattributed. The alert now names the clone's balance in decimal EURe, the exact top-up (decimal and raw) and the token, and the funds-left warning no longer claims that all funds left a partly swapped deposit.
A recover that is mined but whose execution row is still Pending has already taken its EURe, yet the ledger still counted it, so a sibling stuck deposit got "reconcile, do not refund" instead of the operator alert. The keeper confirms the row on its next cycle; waiting one monitoring pass gives the right verdict.
Updating each loaded deposit keeps the instance in step with its row by itself, so the bulk update and the separate set loop that mirrored it into memory collapse into one write per deposit, in the same transaction.
The pass already skips an account while any of its recovers is pending, so the marked-deposit filter's pending check can never match. What remains is the confirmed-recover filter the ledger sum uses, so both now derive from one list, and recoverOf takes the one status each caller asks for.
The resend bound, the public-pool skip and the once-per-reason logging are already documented on the constant, the skip map and the inline check, so the docstring keeps only the decisions: why the keeper applies, why it remembers the last apply, and why a failure never stops the cycle.
Both resend guards block its second cycle, so it fails only when both are gone. The private-transport test pins the remembered apply and the public-pool test pins the nonce gate, each on its own: disabling either guard fails one of them.
Both describes defined a byte-identical activeAccountWithDeposit, so one file-scope copy serves both and a fixture change cannot drift between them.
The depth-check cases already run once per quoted chain, so each now also asserts that both quote sizes went to that chain's pinned QuoterV2, which makes the two standalone quoter tests redundant.
The paragraph gave the one-cycle candidacy of an unattr: row twice, once in a parenthetical and again as the reason the orphan inflow's poke is not retried. One sentence now carries the rule and its consequence.
Each sub-floor refund text restated a point it had just made: the ADR amendment its no-automatic-refund rule, the whole-cent rule and the timing; the architecture bullet the auto-only pipeline and the leftover top-up; the spec its hand-refund rule, in a form that contradicted the alert and off modes. The sandbox minimum now points to the production paragraph instead of repeating it. No decision or mode scope changes.
Keeper invariant 2 already states that keeper writes go through the private transport and that the public nonce counts cannot see them, so the fee-apply invariant cites it instead of repeating it.
The monitor took MIN_SWAP_FLOOR, RECOVERY_DELAY and TRIGGER_DELAY from whichever account the unordered query listed first and applied them to every clone, so during a migration the REFUND NEEDS OPERATOR top-up and the REFUND OVERDUE bounds could come from another factory. A clone off the trusted factory, which the keeper refuses, also got a top-up it could never act on; it is now skipped with an error, as the executor and the config reconciliation do.
REFUND NEEDS OPERATOR prescribed a top-up even when another marked payment on the same clone was flagged for reconciling in the same pass. The top-up arms the marker for that payment too, whose recover cannot succeed, so the keeper could keep retrying it while the payment the alert promised to refund waited. The unswapped payments now get a warning that they wait for the reconcile, which does not claim their own funds left the clone.
The keeper sends no second recover while another refund of the client is open, but REFUND NEEDS OPERATOR still asked for the top-up. Sent then, it arms the marker at the floor (at least minSwapAmount), and if the open refund outlives TRIGGER_DELAY the permissionless swap and forwardAll can reach the payer's and Vortex's EURe before the keeper may recover. The alert now warns that the payment waits and returns with the amount once that refund is redeemed.
REFUND OVERDUE counted a deposit whose recover attempts had failed as one no recover had been sent for, and only hinted at marker re-timing or another stuck refund. A recover that keeps reverting may mean its funds left by a path the ledger does not record, where a hand refund pays twice, so the alert now says no recover has succeeded, names the latest error and asks to reconcile the ledger instead.
With MONERIUM_B2B_AUTO_RECOVERY=off the deadline pass neither marks nor reports anything, and a clone below MIN_SWAP_FLOOR arms no marker, so the stranded check never fires and REFUND NEEDS OPERATOR needs a marked deposit: a payment below the floor reached nobody. The stranded monitor now warns for such a payment past its window and says to mark it, then follow REFUND NEEDS OPERATOR.
Every other alert and doc names "the internal B2B runbook §x"; the DEPTH BELOW FLOOR line alone said "the B2B operations runbook", so a search for the runbook reference missed it.
Monitoring invariant 2 first called DEPTH BELOW FLOOR an error-level line outright and only qualified it at the end; the code and the architecture doc log it as a warning off mainnet, so the invariant now says so where it introduces the line.
The keeper remembers the last fee apply and the logged skip reason per forwarder in module memory, and every fee-apply test used the same forwarder, so an earlier test's apply or skip carried into the next: a second test of the same skip saw it already logged and failed. A fresh forwarder per test keeps them independent without a test-only export.
Every sub-floor refund now leaves an unattr: top-up row on the clone, and only the settlingDeposits filter keeps the keeper from flagging, swapping and forwarding that Vortex EURe; no executor test covered it. The tests pin the filter, the poke once the top-up lifts the clone to the floor, and the recover of exactly the payer's EURe.
Keeper invariant 5 said only provider-attributed deposits make an account a conversion candidate while its own parenthetical named the unattr: cycle and pending executions, and invariant 7 and the fee section still said the executor only runs for accounts with a settling deposit. Both predate the candidate change; they now name all three.
The off-mode report sat between REFUND OVERDUE and the sub-floor reconcile rule, so "a swapped deposit... only warns" read as if it described the off-mode warning.
The two wait lines said "this alert then returns with the top-up amount", but they are not the alert that returns; naming REFUND NEEDS OPERATOR tells the operator which line carries the amount to send.
In off mode a marked sibling (e.g. one whose recover is confirmed and waits for the manual redeem) silenced the report for every other payment past its window on the clone, and nothing else reports them. The report names only minted or converting rows and the other sub-floor lines only marked ones, so they never name the same deposit.
The operator had to find the refund that holds the top-up back by hand, and in alert and off mode the hold lifts only once that deposit is closed after its manual redeem, not when the redeem went out.
Dropping the blockNumber filter passed every test, although the admin recover route refuses an unindexed deposit, so the report would ask to mark one that cannot be marked.
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
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.
Why
The live sandbox edge-case tests of the B2B onramp turned up four keeper gaps. This PR fixes all four and replaces #1425, #1426, #1427 and #1428. A follow-up review of the combined branch found no blockers; its findings are addressed here too. Commits keep one concern each.
RECOVERY_DELAYclock started a cycle late, so refunds landed about twice as late as promised.applyFeePolicy()is permissionless and the contract expects the keeper to finalize a timelocked increase, but no keeper code called it.poke()refuses to arm belowMIN_SWAP_FLOOR, sorecover()never becomes eligible for it on its own. The keeper also kept serving that deposit first, blocking the account's later deposits, and nobody was told.What changes
chain.tspins QuoterV2 per chain: mainnet0x61fFE014bA17989E743c5F6cB21bF9697530B21e(unchanged) and Sepolia0xEd1f6473345F45b75F8179591dd5bA1888cf2FB3, Uniswap's official Sepolia QuoterV2. Pricing and the executable-depth monitor use it. Off mainnet, DEPTH BELOW FLOOR logs at warn with a re-centre hint, so a drifting sandbox pool is not an incident.runConversionExecutorpokes an unarmed marker after a failed attempt and on a backoff skip, at most once per cycle, under the send lock.applyFeePolicy()before pricing, at most once per cycle, as a best-effort step: a failed read or a revert only skips it. It skips the apply while any keeper transaction (of any account) is unmined in the public pool, and while its own last apply is younger than 15 minutes with no receipt, so a private-relay apply is not re-sent. Each skip reason is logged once.below_minimumon it (cleared, with a freshsince, once a lowered minimum makes it swappable).MIN_SWAP_FLOORwith no USDC, REFUND NEEDS OPERATOR prints the exact EURe top-up to send to the clone:MIN_SWAP_FLOORminus the clone balance plus 1 wei, one wei more if that is a whole number of cents, so the mint watcher can never match it to an order. The keeper's existing poke then arms the marker, and its normalrecover()and the refund pipeline refund the payer's own EURe afterRECOVERY_DELAY, with the full refund record. The top-up stays on the clone below the floor as one unattributed row, and the marker switches off by itself. OutsideMONERIUM_B2B_AUTO_RECOVERY=autothe alert points to the manual redeem from the refund wallet. ADR-0005 gets an amendment, and the partner docs state the EUR 1 minimum in the production section.RECOVERY_DELAYplus one hour with no recover sent (error pastTRIGGER_DELAY), for example when younger forwards keep re-timing the marker. RECOVER STUCK PENDING warns for a recover execution pending over an hour (error after four).refund.refundedAtis the deposit's refunded transition, so a refund closed by operations reports its close instead of null or the failure time.Note: the depth monitor now also runs on the sandbox. The internal B2B runbook (§2.7, §3) is updated separately for the top-up procedure and the new alert lines.
Compatibility
Additive:
below_minimumis a newDepositWaitingReasonvalue; the shared endpoint types, OpenAPI and wire-contract snapshot are updated, and the docs now say new reasons may be added. No existing value, route or payload shape changes.refundedAtkeeps its type; for automatic refunds it still marks the refund's completion, and refunds closed before this change shift by a few seconds in the read API (no webhook is re-sent).Docs
The architecture doc, security spec (including new audit checklist lines) and ADR-0005 cover the per-chain quoter, when the keeper arms the marker, the fee-apply step and its guard, the recover backoff, the pass-over and its ordering cost, the top-up refund procedure and the new alerts. API docs pages 07 and 14 and the OpenAPI descriptions are updated.
Tests
Each fix ships tests that fail without it, including: Sepolia quotes and a projection defer fires; failed attempts and backoff skips poke an unarmed marker; a due increase is applied once and not re-sent while a private apply is unmined; failing swaps do not delay an eligible recover;
below_minimumclears with a freshsince; the top-up amount stays off a whole cent across repeated top-ups; REFUND OVERDUE and RECOVER STUCK PENDING fire only when they should; a late payer fill does not re-date a refunded deposit. The full apps/api suite passes: 2380 pass, 54 skip, 0 fail (after the cleanup below removed three redundant tests and the second review round added its own). Typecheck, verify, wire-contract, the API docs check and the api build are clean.Cleanup
A vortex-cleanup pass over this PR's own additions removed 70 net lines with no behaviour change, each removal checked against the code and, for tests, by breaking the guarded behaviour and seeing a remaining test fail:
below_minimumclear now updates the loaded deposits directly instead of a bulk update plus an in-memory sync; the monitor's sub-floor branch drops a filter clause that could never match after its pending-recover skip and computes the unrecovered set once; the fee-apply docstring no longer repeats its constants' comments.automode); the partner page's sandbox minimum note now points to the production paragraph.Kept on purpose: the unattributed top-up row in the "top-up landed" monitor test (keeps the fixture faithful to the real state after a top-up), the second half of the sequenced-poke test (it pins the one-cycle poke bound), and the inline fixtures in the manager-events tests (a helper for two call sites would not pay off).
Second review round
A second vortex-review (no P0 to P2) led to these fixes, each with a regression test, checked again by an independent verifier:
TRIGGER_DELAYopen the clone to the permissionless swap.MIN_SWAP_FLOORfrom the trusted factory and the delays from each clone, and skips (with an error line) a clone that is not on the trusted factory, instead of applying the first account's values to every clone.MONERIUM_B2B_AUTO_RECOVERY=off, a new REFUND DUE BELOW FLOOR warning names unmarked payments below the floor past their window, which nothing else reported in that mode.Not changed on purpose: a confirmed swap still resets a failing recover's backoff, and a fee apply held by a stuck keeper transaction is still logged once (both minor).