Generate report from simulation#4538
Conversation
|
Claude finished @MartinquaXD's task in 2m 27s —— View job PR Review
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the simulation builder to track state overrides that failed to resolve and introduces a new simulation_report method to generate high-level simulation reports using a Geth call tracer. Specifically, build_final_state_overrides now returns both the resolved overrides and the failed requests, which are then stored in EthCallInputs. Additionally, EthCallInputs::simulate was updated to take a reference instead of consuming the struct. No critical issues were found.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
MartinquaXD
changed the title
| Some(value) => Either::Left(value), | ||
| None => { | ||
| tracing::debug!(?request, "failed to compute state override"); | ||
| Either::Right(request) |
There was a problem hiding this comment.
Why use Either instead of Result?
There was a problem hiding this comment.
In those cases using Result most of the time means a sub-error results in the whole thing failing which is not the case here. I felt using Either communicated the "best effort" idea here better.
I don't feel strongly about this so if this is more confusing than Result as the target audience I can change it to Result.
There was a problem hiding this comment.
I find Either to be mostly used for unifying two different types, here the semantics seem to be error driven so I'd err (pun intended) on Result's side
Dealers choice though
| if let Err(err) = | ||
| apply_account_override(&mut state_overrides, address, account_override) | ||
| { | ||
| tracing::warn!(?err, %address, "conflicting state overrides for address, skipping"); |
There was a problem hiding this comment.
This never lands in failed_state_overrides, right? For the buy-token override that means #4539's allow_failed_buy_token_transfer can't see it and the order gets rejected. It probably can't happen for the buy token in practice, just flagging.
3 participants
Description
Follow up to #4537. In order to understand whether our simulation or the order is at fault we also need to know what went wrong while building the simulation. The most likely issue with our simulation will be that we fail to compute the necessary state overrides for the buy token balance.
Changes
Instead of surfacing those issues as a hard error we simply return our best effort simulation inputs together with the state override errors. This is because a simulation can theoretically still work without the override and now that we can generate a simulation report for reverting simulations we can still get some important information (setup of wrappers, pre-hooks, signature check).
How to test
Rather than adding a unit test for this this logic together with the other PRs of this stack will be covered by an e2e tests asserting that orders which trigger the specific limitations of the simulation approach can still be placed.