feat(hindsight): decode and resolve reverted Relay swaps - #371
Open
kayibal wants to merge 2 commits into
Open
Conversation
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
from
July 29, 2026 17:00
036ce0c to
f2928ea
Compare
kayibal
force-pushed
the
feat/hindsight-positive-slippage
branch
from
July 30, 2026 12:26
41fde62 to
d0b5e69
Compare
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
2 times, most recently
from
July 30, 2026 13:38
235b207 to
2175525
Compare
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
from
July 31, 2026 12:21
2175525 to
2aff270
Compare
kayibal
force-pushed
the
feat/hindsight-calldata-decoding
branch
from
July 31, 2026 13:44
495a7c5 to
cc743ea
Compare
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
from
July 31, 2026 13:50
2aff270 to
134e666
Compare
kayibal
force-pushed
the
feat/hindsight-calldata-decoding
branch
from
July 31, 2026 14:31
cc743ea to
0fb420d
Compare
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
from
July 31, 2026 14:42
134e666 to
1792c69
Compare
kayibal
commented
Jul 31, 2026
Comment on lines
+27
to
+35
| /// A transaction that reverted on-chain, matched only by its entry point being a | ||
| /// revert-decoded venue. Every field is `Copy` (a reference, an `Address`, a `&'static str`), so | ||
| /// callers that only read it can dereference a borrowed match instead of taking ownership. | ||
| #[derive(Clone, Copy)] | ||
| pub(crate) struct MatchedRevertedTrade<'a> { | ||
| pub receipt: &'a AnyTransactionReceipt, | ||
| pub entry_point: Address, | ||
| pub venue: &'static str, | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Why do we need this? Couldn't we just add a reverted flag to the struct above? This struct basically is a duplication of the above struct.
Comment on lines
+166
to
+184
| /// A transaction that reverted on-chain, matched by entry point alone (a revert emits no logs) | ||
| /// and decoded from its trace: which venue and solver it was routed through, the trader's swap | ||
| /// terms when recoverable, and why it failed. | ||
| #[derive(Debug, Clone, serde::Serialize)] | ||
| pub(crate) struct DecodedRevert { | ||
| pub tx_hash: TxHash, | ||
| pub block_number: u64, | ||
| pub tx_index: u64, | ||
| pub venue: String, | ||
| /// The registry name of the solver frame's `to`, or the entry-point label when no solver | ||
| /// frame was found in the reverted trace at all. | ||
| pub solver: String, | ||
| /// The trader's swap terms recovered from the solver frame's calldata. `None` when no solver | ||
| /// frame was found, its `to` has no `swap_intent` support, or the calldata did not parse — | ||
| /// kept (not filtered out) so parser coverage is measurable against every reverted candidate. | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub intent: Option<SwapIntent>, | ||
| pub cause: RevertCause, | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Can't we model this with DecodedTrade? Just because the trade did not fill, it doesn't mean it wasn't a trade.
Comment on lines
+103
to
+108
| pub(crate) fn find_reverted_solver_frame<'a>( | ||
| frame: &'a CallFrame, | ||
| registry: &Registry, | ||
| ) -> Option<&'a CallFrame> { | ||
| find_solver_frame_impl(frame, registry, true) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Why do we need a separate path here? The function could simply ignore wether the transaction failed or not.
Comment on lines
+131
to
+137
| /// Fly's `InsufficientAmountOut()` custom error selector (`DexAggregator.sol`) — the marker for a | ||
| /// slippage-floor revert on Fly. | ||
| const FLY_INSUFFICIENT_AMOUNT_OUT: [u8; 4] = [0xe5, 0x29, 0x70, 0xaa]; | ||
| /// `KyberSwap`'s slippage-floor revert reason. | ||
| const KYBERSWAP_INSUFFICIENT_RETURN: &str = "Return amount is not enough"; | ||
| /// The geth call tracer's exact error string for a frame that ran out of gas. | ||
| const OUT_OF_GAS: &str = "out of gas"; |
Contributor
Author
There was a problem hiding this comment.
Solver specific data in general purpose module, this is a red flag.
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
3 times, most recently
from
August 3, 2026 17:26
4aa0fea to
10616a3
Compare
Extend the decode/resolve pipeline to capture reverted Relay swaps, which
settle nothing but still declare an on-chain floor worth measuring
against. A reverted trade is modeled as the same DecodedTrade a settled
one is, distinguished by a status field (TradeStatus::Settled or
Reverted { cause }) rather than a parallel type -- reviewer feedback on
the original design was that a revert is still a trade, just one that
did not fill, and the parallel MatchedRevertedTrade / DecodedRevert /
RevertComparison / RevertStateResult types and the separate
reverts-*.jsonl stream were shallow duplicates of the settled path.
matching::select handles both settled and reverted receipts in one
function, returning one MatchedSolverTrade with a reverted flag (settled
matched by tx.to/solver logs plus vetoes; reverted matched by entry point
alone, gated by REVERT_DECODED_VENUES -- a revert emits no logs, so
that's the only signal). trace::find_solver_frame is one function with a
strict-then-tolerant two-phase walk: prefer the frame that settled the
swap, and when nothing settled, fall back to the frame that tried --
this also means a settled trade whose router tried one solver (reverted)
before succeeding via another is never mis-attributed to the abandoned
attempt. classify_revert_cause asks solvers:: whether any registered
impl claims a frame as a slippage floor (SolverKnowledge::
is_slippage_floor, e.g. Fly's InsufficientAmountOut() selector,
KyberSwap's "Return amount is not enough" reason, 0x's
TooMuchSlippage(address,uint256,uint256) selector) rather than checking
solver-specific constants itself. When neither the tracer nor any
registered marker decodes a reason, the deepest reverted frame's raw
output selector is appended to the generic message (e.g. "execution
reverted (0x12345678)") instead of collapsing into an unclassifiable
bare string -- several RPC providers never populate a decoded reason at
all.
decoder::decode_block returns one Vec<DecodedTrade> with mixed status;
decode_reverted derives the venue from entry_point (like the settled
path), sets sender from the transaction (there is no netted flow to draw
a different tracked party from), and reuses solvers::attribution::
attribute for the solver label, since the unified find_solver_frame now
falls back correctly for reverted transactions. Settled-only fields
(amount_out, settled_gas, venue_fee_*, sandwich) are Option; token_in/
token_out/amount_in are Option too, None only when a reverted trade's
solver calldata didn't parse -- the trade is still recorded so parser
coverage stays measurable.
resolve_block_range takes the one trade list: trades with unknown terms
are recorded but not solved, settled and decodable-reverted trades go
through the same top/back wave. RangeComparison gains status (+cause),
sender (from the decoder, for segmenting venue-filler fills like Relay's
rotating filler without an on-chain lookup), and a fillable/margin_bps
floor judgment per state, computed against min_amount_out for both
settled and reverted trades when a floor is known -- a settled trade
cleared it by construction, but the margin is still informative.
Everything writes to the one comparisons-*.jsonl stream (RotatingWriter
is no longer prefix-parameterized) under a tx_hash key shared by both
statuses (renamed from settled_tx, a naming leftover from when reverts
were a separate stream). Telemetry keeps the external metric names
(hindsight_reverted_swaps_total, hindsight_revert_fillable_total,
hindsight_revert_margin_bps), now driven off status instead of separate
types. report:: filters status == "settled" explicitly before
aggregating savings/win-rate, rather than relying on absent fields.
0x's markers were verified against live Base monitor data and 0x's
published source (github.com/0xProject/0x-settler,
src/core/SettlerErrors.sol): TooMuchSlippage(address,uint256,uint256)'s
selector (0x97a6f3b9) appears bubbling through both registered 0x
addresses (Settler, AllowanceHolder) in real reverted traces, full
ABI-encoded as (token, expected, actual). A second string,
"return too low", is not 0x's own error -- it is a liquidity source (a
Kipseli PropAMM pool) 0x's Settler routes through -- but 0x is the only
address our registry recognizes anywhere in those traces, so it is
covered by 0x's is_slippage_floor rather than left unclassified.
Manual verification (pre-commit hook not run -- cargo nextest run
--workspace SIGABRTs on an unrelated fynd-core sim_guard test on this
machine, a known conda/cc PATH issue breaking panic unwinding):
- cargo check --workspace --all-features: pass
- cargo clippy --locked --workspace --all-targets --all-features
-- -D warnings: pass
- cargo nextest run -p hindsight --all-features: 316 passed, 1 skipped
- cargo +nightly fmt --all -- --check: pass
- RUSTDOCFLAGS="-D warnings" cargo doc for hindsight, fynd-core,
fynd-rpc-types, fynd-rpc, fynd-client: pass
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Update tools/CLAUDE.md and tools/hindsight/CLAUDE.md/README.md for the unified decode/resolve model: matching.rs and trace.rs describe one function each for settled+reverted (no select_reverted / find_reverted_solver_frame split), the solvers/ row covers is_slippage_floor (now including zeroex.rs's TooMuchSlippage selector), the resolve engine and report tables describe the single comparisons-*.jsonl stream (tx_hash key, sender field) and the status-filtered report reader, the Revert model section describes DecodedTrade's status field, the shared StateResult floor judgment, and the raw-selector fallback for an undecoded revert reason, and the Key types list drops DecodedRevert/RevertComparison/RevertStateResult in favor of DecodedTrade/RangeComparison carrying status. Docs-only change; no code checks apply. Pre-commit hook not run (see prior commit: cargo nextest run --workspace SIGABRTs on an unrelated fynd-core test on this machine). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kayibal
force-pushed
the
feat/hindsight-relay-reverts
branch
from
August 3, 2026 17:32
10616a3 to
29c9ec7
Compare
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.
What
Stacked on #388 (base branch
feat/hindsight-calldata-decoding). Now that the calldata-extractionlayer (
SwapIntent, per-solver parsers,RelayCalldata) has been split out into #388, this PR'sscope is the revert measurement pipeline only: the hindsight monitor also captures Relay swaps
that reverted on-chain and measures whether a Fynd quote would have cleared the trader's floor.
For each reverted Relay swap it decodes the intended trade from the transaction's trace —
reverted transactions emit no logs, but their call frames retain full calldata, reusing the same
SwapIntentrecovery the settled path uses — then solves it at top-of-block (N-1) and fresh atback-of-block (N) and records whether each quote is at or above
min_amount_out. Purpose:quantify how many reverts a sequencer that fills against interim block state would avoid, and by
what margin.
A reverted trade is modeled as the same
DecodedTradea settled one is, distinguished by astatusfield (settledorrevertedwith a cause) rather than a parallel set of types — arevert is still a trade, just one that did not fill.
Changes
decoder/matching.rs):matching::selecthandles both settled andreverted receipts in one function, returning one
MatchedSolverTradewith arevertedflag.Status-0 receipts whose
tx.tois a Relay entry point are traced in the same bounded wave assettled trades (venue allowlist, currently
relayonly).decoder/trace.rs): onetrace::find_solver_frame— prefer the frame that settled the swap, and when nothing settled,fall back to the frame that tried. This also protects a settled trade whose router tried one
solver (reverted) before succeeding via another: it is never mis-attributed to the abandoned
attempt.
classify_revert_causeasks each registeredSolverKnowledgeimpl whether itrecognizes a frame as a slippage floor (
is_slippage_floor— Fly'sInsufficientAmountOut()selector, KyberSwap's "Return amount is not enough" reason) rather than checking solver-specific
constants itself;
trace.rskeeps only the generic subtree walk, out-of-gas, anddeepest-revert-reason logic.
RevertCauseisslippage_floor,out_of_gas, orother.Undecodable reverts are kept with unknown terms so parser coverage is measurable.
decode_blockreturns oneVec<DecodedTrade>with mixed status.resolve/):resolve_block_rangetakes the one trade list. Trades withunknown terms are recorded but not solved; settled and decodable-reverted trades go through the
same top-of-block/back-of-block wave.
RangeComparisongainsstatus(+ cause) and afillable/margin_bpsjudgment per state, computed againstmin_amount_outfor both settledand reverted trades whenever a floor is known — a settled trade cleared it by construction, but
the margin is still informative.
comparisons-YYYY-MM-DD.jsonlstream(
RotatingWriteris no longer prefix-parameterized); a record'sstatusfield tells a settledtrade from a reverted one. The
reportsubcommand filters tostatus == "settled"explicitlybefore aggregating savings/win-rate, rather than relying on absent fields.
hindsight_reverted_swaps_total{venue,chain,cause,decoded},hindsight_revert_fillable_total{venue,chain,state,fillable}, andhindsight_revert_margin_bps{state,…}(signed histogram) — unchanged metric names, now drivenoff
statusinstead of a separate revert type.Notes
slippage_floorreverts are avoidable byfresher state, but the avoidance rate stays computable against either denominator offline.
to the deepest frame's error — the informative reason can sit on an outer frame.
Testing
against four real Base revert traces (Fly slippage, Kyber slippage, out-of-gas, transfer
failure), the unified resolve wave, JSONL records (settled and reverted), report status
filtering, telemetry rendering.
cargo check --workspace --all-features,cargo clippy --locked --workspace --all-targets --all-features -- -D warnings,cargo +nightly fmt --check, andRUSTDOCFLAGS="-D warnings" cargo doc --no-deps -p hindsight -p fynd-core -p fynd-rpc-types -p fynd-rpc -p fynd-clientallclean.
Pre-commit hook not used for the commits on the rebuilt branch:
cargo nextest run --workspace --bin fyndSIGABRTs on this machine on an unrelatedfynd-coretest (aconda/ccPATH issuebreaking Rust panic unwinding, environmental). Verified manually instead; see above.
🤖 Generated with Claude Code