fix(bots): treat a reverted send as economic, not a send failure - #188
Merged
haydenshively merged 4 commits intoSep 1, 2026
Merged
Conversation
haydenshively
changed the title
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Split bot-kit's tx-error.ts into tx-send.error.ts (TxSendError only) and revert.utils.ts (arrow-const revert utilities), per the utility-isolation and one-error-class-per-file rules, and mirror the rename in each bot's own revert formatter. Add revertSelector, which unwraps a TxSendError and returns the revert payload's 4-byte selector, so a revert no ABI in reach can decode is still attributable. Widen SubmitOutcome's send_failed arm with executionRevert and log both it and the selector on tx.submit_failed; callers can now tell the chain declining a plan from the send machinery breaking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Blue's liquidation incentive is static, so a shortfall on this block does predict the next one. Document at the site that blue keeps backoff on every rejected send — including an execution revert — and pin it with a test so nobody harmonizes it with midnight's ramp-driven exemption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An execution-reverted send is the chain's economic verdict on a ramping incentive, not a fact about the next block: post-maturity LIF ramps over an hour, so backing off on a min-out shortfall samples that ramp exponentially and skips the contested block where the position first becomes fundable. Splits the no-broadcast outcome three ways (sendRefused / sendReverted / sendRejected, summing to notSent) and, on the reverted arm, CLEARS the position's deferred backoff rather than merely not arming it — the set is keyed by position, so a lower-ranked sibling's quote or simulation failure has usually armed it already, and a chain-declined send is later evidence than either. An opted-in cooldown is deliberately left armed. backoff.clear stays gated on a real broadcast. Adds a per-position streak of consecutive execution-reverted sends, reported as send.revert_streak past 15 minutes with the revert count, the last 4-byte selector, and whether it stayed constant — the backstop that makes running with no retry throttle defensible when a persistent estimator-only revert would otherwise re-quote forever. The threshold is a duration because the incentive ramps on wall clock, so attempts-to-clear shrinks every time the bot gets faster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`pendingBackoff.delete(label)` only held when the reverted send was the position's last event of the tick. Unlike a broadcast, an execution revert never enters `submittedLabels`, so the next-ranked sibling still runs and re-arms the entry — silently restoring the suppression on the ordinary multi-collateral position. Exempt the label instead and skip it in the tick's `finally`, so the exemption is order-independent. `pendingCooldown` stays armed, as decided. Cover both reverse orderings (revert then quote failure, revert then simulation revert); the pre-fix source fails exactly those two. Also report `send.revert_streak` on the crossing alone. The path has no throttle by design, so an escalated streak was shipping a warn every tick — two on a tick where both siblings reverted — for as long as the position stayed stuck. Correct the state-race mechanism while here: the simulation and the send's gas estimate both run at `latest`. They diverge because they come from different clients over their own failover transport pairs, not because either sees pending state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
added a commit
that referenced
this pull request
Sep 1, 2026
A regression sweep over #184/#187/#188/#189 found four behaviors the stack changed without meaning to. None are new functionality; each restores what the bots did on main. Detection-only blue could broadcast. The no-venues gate moved to AFTER unwrap resolution and the swap-free branch lost its `steps.length > 0` guard, so a venue-less deployment returned `kind: 'swap'` — which the tick takes straight to simulate+submit. docker-compose defaults Robinhood (4663) to ALLOW_DETECTION_ONLY with a funded key, documented as skipping every routed liquidation. `swapFreeWithoutVenues` (default false) restores the immediate refusal; midnight opts in, because its loan-as-collateral slots need no route and ALLOW_BAD_DEBT_ONLY is a supported posture there. That same reorder downgraded a transient unwrapper RPC failure from `no_config` (skip) to `failed` (arms backoff), pushing a deliberately unarmed deployment into a suppression state machine it never entered. Refusing before the unwrap chain fixes both at once, and spends no reads doing it. A send REJECTION now arms backoff even when a sibling execution-reverted. Both sets are keyed by position, so `backoffExempt` was cancelling the backoff a broken nonce/funds/RPC send earned; the position then re-sent every block while the send machinery was still broken. Phase A.5 no longer resolves routes for suppressed positions. It ran ahead of the cooldown/backoff gates, so a backed-off position spent an uncached read per candidate per tick — breaking backoff's contract that it bounds API and RPC usage under a backlog, and putting that latency in front of the first send of a maturity burst. Each of the 8 new tests was verified to fail against the pre-fix source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively
force-pushed
the
fix/send-revert-classification
branch
from
September 1, 2026 03:52
b2c9b96 to
adbb90b
Compare
haydenshively
marked this pull request as draft
September 1, 2026 03:54
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2c9b96795
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
haydenshively
marked this pull request as ready for review
September 1, 2026 03:59
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adbb90b312
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
haydenshively
added a commit
that referenced
this pull request
Sep 1, 2026
Triaged all 20 Codex/Devin findings across #184, #187, #188, #189 and #192. Fixed: - the venue-less swap-free exception admitted unwrap-only plans, so an ALLOW_BAD_DEBT_ONLY deployment could broadcast an asset-moving liquidation - candidate ranking trusted stale and incomplete curves that quoting refuses; both now share one exported `curveIsTrusted` - a Pendle PT resolved its unwrap chain twice per tick, both hosted API calls; new `previewTokenOut` seam answers phase A.5 from cache - phase A.5 resolved routes serially, in discovery order - a sibling's `no_route` suppressed a position whose other candidate was `floor_unmet`, which is meant to retry every block as the LIF ramps - the wall-clock cooldown verdict could flip mid-tick, leaving a candidate quoted but unpriced - revert-streak state never expired, so a reused label reported false crossings - `tx.submit_failed` carried no candidate discriminator - docs: the swap-free "iff" guarantee, `selectorConstant`, two README rows Declined, with reasoning for the threads: narrowing the backoff exemption to post-maturity plans (a regression — the sets are per-position while the mode is per-candidate), `stopAfterWinner`, and the sub-1e-18 rate truncation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cashd
approved these changes
Sep 1, 2026
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
2 participants
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.
Closes BOTS-88 and delivers BOTS-89's measurement. Stacked on #184 → the venue cost curve PR — this PR's own diff is
feat/venue-cost-curve...fix/send-revert-classification.Why
The 2026-08-28 15:00 UTC maturity was the first with #181 and #182 in production. All 13 positions cleared, but clearing took 4–9 minutes each against a competitor's 123 seconds on 31 Jul.
Backoff suppressed 69% of planned candidates. Of the 167 that reached
simulate.ok, 153 returnedsent: false. Their decoded reasons:Error(return too low)×103,Execution reverted for an unknown reason.×47,Error(Return amount is not enough)×3.I traced the emitting contracts by searching deployed Base bytecode:
return too lowcomes from the AMM pool 0x routed through (0x71c2ed90…, Metric/Kipseli family), andReturn amount is not enoughfrom KyberSwap'sMetaAggregationRouterV2inside a LiFi route. The string is a property of whichever pool the aggregator picked, not of the venue — so a decoded-string allowlist is open-ended by construction, and 47 of 153 carry a selector our decoder doesn't know.Mean gap between consecutive send attempts on one position was 25–31 s against
BACKOFF_BASE_BLOCKS=2— the exponential ramp fully engaged. That is the 4–9 minutes. Post-maturity LIF ramps, so a min-out shortfall says nothing about the next block.What
packages/bot-kit/src/tx-error.tssplit intotx-send.error.ts(theTxSendErrorclass alone) andrevert.utils.ts(the revert utilities as arrow constants), with all importers and each bot's mirrored wrapper renamed. NewrevertSelectorreturns the 4-byte selector — what makes the 47 unclassified reverts attributable next maturity.SubmitOutcomewidened to{ sent: false; reason: 'send_failed'; executionRevert: boolean; selector?: Hex }, both also logged ontx.submit_failed.sendRefused/sendReverted/sendRejected, with a new identitynotSent === sendRefused + sendReverted + sendRejected. An execution-reverted send does not arm backoff.Review notes
pendingBackoffis position-level and shared across sibling candidates (feat(midnight-liquidation): support loan-as-collateral markets #184), andsubmittedLabelsonly blocks siblings after a successful send — so a lower-ranked sibling running after a reverted send re-armed backoff and silently undid the exemption. Caught in review; the two reverse orderings are now pinned by tests that fail against the pre-fix source.clearing_time / sweep_period— and the sweep period is exactly what this stack shrinks. A count would need recalibrating on every latency win and would fire on healthy positions as sweeps got faster.prepareTransactionRequestpasses noblockTagandexperimental_blockTagis set nowhere, so both run atlatest. They diverge because they are issued by different clients over separatefailovertransport pairs. Still a state race; the mechanism was wrong and is corrected in the code comments.What BOTS-89's measurement actually showed
Receipts for all 14 confirmed fills give quoted−realized of
p50 0.0 / mean 6.35 bps, and−0.5…+2.2 bpsfor the nine 0x fills. So there is no uniform realized-output bias and nothing to discount — but this rules the structural explanation out only for the 0x AMM subset that landed. The 153 rejected attempts are unobserved and the 47 unclassified reverts remain unproven. The TIB states it that way on purpose.Verification
pnpm test211 files / 2837 tests, 0 failed · lint 0 warnings · knip clean · typecheck across bot-kit, swaps, both liquidators, both reallocation bots and crossed-books · fork suite 3/3. Break-one-assertion on every new assertion, plus source mutation for the latch.🤖 Generated with Claude Code