Repository navigation
Conversation
Breaking API Changes (Intentional)Breaking API changes detected and declared in the PR title. semver-checks output |
kayibal
force-pushed
the
feat/fee-token-hooks
branch
from
September 30, 2026 22:56
2956e0d to
5058098
Compare
kayibal
commented
Sep 30, 2026
kayibal
force-pushed
the
feat/fee-token-hooks
branch
from
October 13, 2026 13:40
f06f814 to
030386d
Compare
WHY: A service that shadows /v1/quote can change what the solver sees, but it cannot change the quote that comes back. For a fee-on-transfer token, the route is solved for the amount that arrives, but the calldata must pull the full amount. The caller also cannot refuse a route that the solver found, because the status setter is private to the crate. HOW: - Add `OrderQuote::set_amount_in`, and make `set_amount_out` public. - Add `OrderQuote::retract(status)`. It removes the route, the transaction, the fee breakdown, the simulation result, the surplus and the price impact. It sets the output and the gas to zero, as the no-route placeholder of the router does. `amount_in` does not change, because it is the amount of the order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WHY: A route override gets `AppState`, but it cannot read the market that the quotes are solved on. The field is `pub(crate)` also when the `experimental` feature is on, and the builder gives no other access. An override that must know the token registry cannot get it. HOW: - Add `AppState::market_data()`, behind the `experimental` feature, the same as the field. The field does not move and the default build does not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WHY: A simulation reports only the amount the router call returned. It does not show what each account sent and received. For a token that takes a fee on transfer, that is the data a caller needs to find the tax: the pool gets less than the sender paid. `eth_simulateV1` returns the logs, but the simulator discards them. It also reports a revert and a simulation that did not run as the same failure, so a caller cannot tell a route that does not execute from a node that did not answer. HOW: - Add `TokenTransfer` (token, from, to, amount). - `SimulationResult::Success` gets `transfers`: the ERC-20 `Transfer` events of the call, in the sequence of emission. The simulator decodes them from the `eth_simulateV1` logs and skips all other events. - Add `SimulationResult::Reverted`. `Failure` is now only for a simulation that did not run. - The field has `#[serde(skip)]`, and the fynd-rpc-types conversion maps `Reverted` to the wire `Failure`. The API response does not change. - The winning-protocols trace log shows `reverted` for a revert. Before, it showed `failed`. ADDITIONAL NOTES: There is no new router API. A caller continues to use `simulate_quotes` and reads the transfers from `OrderQuote::simulation_result`. Breaking change: `SimulationResult` has the new variant `Reverted`, and `Success` has the new field `transfers`. A downstream `match` or pattern on `SimulationResult` must change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WHY: A caller that encodes one quote of a request with a different floor must build new `EncodingOptions`. `EncodingOptions::new` sets all other options to their defaults, so the caller loses the transfer type, the permit, the client fee and the watermark of the request. A fee-on-transfer output token needs this: the receiver gets less than the router pays out, so the floor must be lower than the amount the router expects. HOW: - Add `EncodingOptions::with_slippage`. It changes only the slippage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: Layout discovery could not fund a reflection token (KISHU and its copies), so its sells could not be simulated. Such a token loops over a list in `balanceOf`. The sentinel written into the length of that list made the call run out of gas, and discovery stopped on the error. Also, such a token stores its balance multiplied by a rate near 10^51 and divides in `balanceOf`, so no sentinel is ever read back from the correct slot. HOW: - Take an out-of-gas answer as "not this slot", as a revert already is. - When no view places the balance, write a large value into each traced slot that is a mapping for the holder. A slot that reads back smaller but not zero, and reads back twice as much for twice the value, is a scaled balance with rate `value / answer`. - Move the choice of the traced slots into `candidates`, so that the sentinel probe and the scaled probe use the same slots. - `TokenLayout` records the encoding of the balance, and `balance_word` gives the word that funds it: the balance itself, or the balance times the rate, capped below `U256::MAX`. ADDITIONAL NOTES: The scaled probe runs only when the sentinel probes found no slot, so a token that is resolved today makes no additional call. Discovery stays cached for each token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: A simulation kept only the ERC-20 `Transfer` events of the call. For a reflection token that is not enough: the token takes its fee without an event, and its `Transfer` shows only the net amount. The gross amount a pool sent is only in the pool's own `Swap` event, which the simulator discarded. A caller could not find the tax of these tokens from the simulation. HOW: - Replace `TokenTransfer` with `EventLog` (address, topics, data): one log as the node returned it, not decoded. - `SimulationResult::Success` carries `logs`, all the events of the call in emission order, in place of `transfers`. - Remove the `Transfer` decoder. The caller decodes the events it needs. ADDITIONAL NOTES: The field keeps `#[serde(skip)]`. The API response does not change. Breaking change: `TokenTransfer` is removed, and the field of `SimulationResult::Success` is now `logs`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: The simulator ran every quote on the latest block, but the solver priced the quote on the block in `OrderQuote::block`. When a block landed between the two, the pools had moved, and the simulated amount was different from the quote by that move. On Base, with a block every 2 seconds, this made quotes look a few basis points wrong in one round and exact in the next. A caller that compares a simulation with its quote could not tell that drift from a real error. HOW: - `SimulatedCall` gets `block`. A quote's simulation uses the number of the quote's block. - `eth_simulateV1` and the `debug_traceCall` that names a revert both run on that block, so the trace still sees the state of the simulation. - Add a test that a node asked for a different block does not answer. It uses `wiremock`, which is now a dev-dependency of fynd-core. ADDITIONAL NOTES: The block is given by number, not by hash, so a test quote with a placeholder hash still simulates. A node that does not have the block yet refuses with "header not found", and the simulation is a failure. On Ethereum this occurred 8 times in about 70 simulations: the Tycho feed was one block ahead of the node. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: A simulation runs on the block the quote was priced on. The Tycho feed can have that block before the RPC node does, and the node then refuses with "header not found". On Ethereum this happened 8 times in about 70 simulations. Each one was reported as a failed simulation, like a node that does not answer. A caller that counts failures against a token could disable it for a delay of the node. HOW: - On "header not found" (also "block not found" and "unknown block", for other clients), ask again after 500 ms, then after double the previous wait each time. Stop when the waits add up to two block times. Other errors are not asked again. - Keep all attempts and waits within the request timeout (3 s by default). Each attempt gets only the time that remains, and a wait that leaves no time for one more attempt ends the retries. A node that is behind thus cannot hold a quote longer than a node that does not answer. - Take the block time from the chain in `QuoteSimulator::new`. `with_block_time` sets it for a simulator built from a provider. - Add `SimulationResult::BlockUnavailable` for a block that did not come in time. It is not a failure of the route or of its tokens, and the caller can ignore it. On the wire it is still a `failure`. ADDITIONAL NOTES: With the default timeout the node gets about 2.5 s to catch up, which is less than two block times on Ethereum. A block that arrives later gives `BlockUnavailable`. Breaking change: `SimulationResult` has the new variant `BlockUnavailable`. A downstream exhaustive `match` on it does not compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: This branch adds two outcomes to `SimulationResult` (`Reverted`, `BlockUnavailable`) and the `logs` field to `Success`. The enum is public and exhaustive, so each added outcome breaks every downstream `match` on it, now and each time an outcome is added again. HOW: - Mark `SimulationResult` `#[non_exhaustive]`, as `QuoteStatus` already is. Downstream matches handle future outcomes with a fallback arm. - The wire conversion gets that arm and sends an unknown outcome as a `failure`. - The doc comment now says the simulation runs on the quote's block, not the latest one. ADDITIONAL NOTES: The variants are not marked `#[non_exhaustive]`, so a downstream crate can still build them, for example in tests. Matches inside fynd-core stay exhaustive, so a new outcome still has to be handled there. Breaking change: a downstream `match` on `SimulationResult` needs a fallback arm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: `set_amount_in` and `set_amount_out` are public for fee-on-transfer callers, but they changed only the one amount. After `set_amount_out`, `amount_out_net_gas` still showed the old output, so a taxed buy reported a net-of-gas output above what the receiver gets, and even above `amount_out`. After `set_amount_in` on an encoded quote, the calldata still pulled the old amount. HOW: - `set_amount_out` moves `amount_out_net_gas` by the same difference, because the gas cost does not change. A net output at zero stays at zero: it hides how far the gas cost exceeds the output, so it cannot be raised without a guess. - `set_amount_in` drops the transaction, the fee breakdown and the simulation result. A setter cannot encode again, and calldata for the old amount must not stay on the quote. ADDITIONAL NOTES: `set_amount_out` leaves the transaction and the fee breakdown as they are. They describe what the router enforces, and for a taxed buy the router must still expect the untaxed output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: `AppState::market_data()` returned the `MarketData` every solve reads, and with it `write()`. A route override that only needs to read the market, such as the token registry, could change the shared state or hold its write lock and stop every solve. HOW: - Add `MarketReader`, a cheap clone of `MarketData` that offers only `read()`. `MarketData::reader` makes one. - `AppState::market_reader()` replaces `market_data()` and returns that handle. - The test writes through the feed's own `MarketData` and reads it back through the handle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: The simulation metrics count every simulation as a client quote. A service that simulates quotes for itself, such as samples of a fee-on-transfer token, adds reverts and large deviations that clients never get. The dashboards of client quotes then show wrong values. HOW: - Add `SimulationPurpose` with `Quote` and `FeeTokenSample`, exported at the crate root. - Add `WorkerPoolRouter::simulate_quotes_for`, which takes a purpose. `simulate_quotes` calls it with `Quote`, so its callers do not change. - Give `quote_simulations_total`, `quote_simulation_deviation_bps` and the two gas histograms a `purpose` label, with the value `quote` or `fee_token_sample`. - Show only `purpose="quote"` in the Grafana panels of simulation outcomes and deviation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: A route override that keeps its own view of the market (for example, the tokens and pools a route must not use) could only read the whole market again to see a change. On a large market with a new pool in most blocks, that is a full scan on a request path, once per block. The feed already broadcasts which pools each update adds and removes, but only fynd's own components can subscribe. HOW: - Add `MarketEvents`, a handle on the feed's broadcast that can subscribe but cannot send. - `SolverParts::market_events` gives one out, and fynd-rpc keeps it in `AppState`. - `AppState::subscribe_market_events` (experimental) returns a new receiver. Events sent before the call do not arrive, so a caller reads the market after it subscribes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: The docs build failed: the doc comment of the public `TokenLayout::balance_word` linked to the private `MAX_SCALED_WORD`, and the build treats that as an error. HOW: - Give the cap in the text (just below 2^250) and remove the link. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: The doc comments of this branch told the history of the work: the tokens and nodes that showed a problem, and why other designs were not used. A reader needs to know what an item does now. The history made the comments long and hid that data. HOW: - Start each doc comment with one sentence that says what the item is or does. - Keep only the data that a caller needs: what changes, what stays the same, units, errors. - Remove token names, node names, chain names and measured counts. - Remove the doc comments of tests whose name says what they assert. ADDITIONAL NOTES: Only comments change. The code is the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
WHY: The doc comments of the new and changed functions were dense and did not name the arguments, the return value or the errors. A reviewer had to read the body to know what a function takes and gives back. HOW: - Start each doc comment with one plain sentence that says what the function does. - Add `# Arguments`, `# Returns` and `# Errors` sections, as other fynd-core functions do. - Write the steps of a probe or a retry as a list or as short sentences. ADDITIONAL NOTES: Only comments change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o
kayibal
force-pushed
the
feat/fee-token-hooks
branch
from
October 14, 2026 12:58
030386d to
b8bb685
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.
Adds the hooks that code built on fynd needs to quote fee-on-transfer tokens: override quote amounts, read the simulation's event logs, simulate on the quote's own block, and follow the market read-only. With these hooks, a caller can learn each token's tax from simulations and correct the quotes.
Quotes
OrderQuote::set_amount_inandset_amount_outare public.set_amount_outmovesamount_out_net_gasby the same difference. A net output already at zero stays at zero.set_amount_indrops the transaction, the fee breakdown and the simulation result, because they were built for the old amount.OrderQuote::retract(status)withdraws a solved quote. It keeps only the status and the input amount.EncodingOptions::with_slippagereplaces the slippage and keeps the other options.Simulation
SimulationResult::Successcarrieslogs: Vec<EventLog>, every event the call emitted, undecoded. The field is#[serde(skip)], so the API response does not change.SimulationResult::Revertedfor a call that ran and reverted.Failurenow means only that the simulation did not run.SimulationResult::BlockUnavailablefor a node that does not have the quote's block.failure.SimulationResultis#[non_exhaustive], likeQuoteStatus.quote.block().number()), not on the latest block. The revert trace uses the same block.quote_simulations_total,quote_simulation_deviation_bps, the gas histograms) carry apurposelabel:quoteorfee_token_sample.simulate_quotesrecordsquote. The newsimulate_quotes_fortakes aSimulationPurpose.purpose="quote".BalanceEncoding::Scaled).Market access (behind
experimental)AppState::market_reader()returnsMarketReader, which can read the market data but not write it.AppState::subscribe_market_events()returns a receiver of the feed'sMarketEvents.SolverParts::market_events()gives the subscribe-onlyMarketEventshandle it comes from.Breaking changes
SimulationResulthas new variants, a new field, and is now#[non_exhaustive]. Downstream matches need a fallback arm.set_amount_inandset_amount_outchange more than the one amount (see above).Testing
wiremock, which is now a dev-dependency of fynd-core.-D warnings. The three panic-abort tests (test_simulate_path_contains_simulation_panic,test_water_fill_contains_simulation_panic,test_get_amount_out_guarded_converts_panic_to_error) abort locally on macOS, onmaintoo; CI runs them.🤖 Generated with Claude Code
https://claude.ai/code/session_01KDriANMezML2o1C6T9vP2o