Builder based API for simulator crate#4372
Conversation
|
Reminder: Please consider backward compatibility when modifying the API specification.
Caused by: |
|
What about accepting 422 instead of 400 for invalid app_data for the failing test local_node_http_validation? Since the outer JSON is valid and "not valid json" is a valid string value, this is really a validation error rather than a malformed request. We could simplify by: Changing the test to expect StatusCode::UNPROCESSABLE_ENTITY (422) instead of StatusCode::BAD_REQUEST (400) |
There was a problem hiding this comment.
Code Review
This pull request refactors the order simulation infrastructure by replacing the SwapSimulator and OrderSimulator with a more flexible SettlementSimulator and SimulationBuilder pattern. It also updates the OrderSimulationRequest DTO to include necessary fields for signature verification and introduces support for flashloan and hooks trampoline contracts. A critical compilation error was identified in the SettlementSimulator::new method where the Address constructor was used incorrectly on a type alias.
| let authenticator = Address(settlement.authenticator().call().await?.0); | ||
| let domain_separator = DomainSeparator(settlement.domainSeparator().call().await?.0); | ||
| let provider = settlement.provider().clone(); | ||
| let chain_id = provider.get_chain_id().await?; |
There was a problem hiding this comment.
The Address(...) constructor is redundant and will cause a compilation error because Address is a type alias for FixedBytes<20>, and type aliases cannot be used as tuple struct constructors in Rust. Additionally, DomainSeparator requires an explicit conversion from B256. These calls should remain sequential to avoid high load on the provider, consistent with the preference for sequential operations in debug contexts.
| let authenticator = Address(settlement.authenticator().call().await?.0); | |
| let domain_separator = DomainSeparator(settlement.domainSeparator().call().await?.0); | |
| let provider = settlement.provider().clone(); | |
| let chain_id = provider.get_chain_id().await?; | |
| let authenticator = settlement.authenticator().call().await?.0; | |
| let domain_separator = DomainSeparator(settlement.domainSeparator().call().await?.0.into()); | |
| let chain_id = provider.get_chain_id().await?; |
References
- For debug endpoints, prefer sequential database queries over parallel ones to avoid high database load, even if it results in slower performance.
Description
Already in a somewhat reasonable shape but still needs more polishing. Opening the PR to run all tests and perhaps already get a few comments of the shape of the API.
This new API for the
simulatorcrate approaches the problem from the other side. Instead of starting making it work first and foremost with the most complicated use case (i.e. trade verification) and building the simpler, more common use cases around that this API starts with the simple stuff and aims to be flexible enough to also support the more complicated things.Not only does this lead to a cleaner API (IMO) but also actual feature improvements. For example the trade verification logic replaces the user with a helper contract that we puppeteer to set up the necessary pre-conditions for quote verification. This approach is fundamentally incompatible with use cases where a pre-interaction deploys a smart contract that is actually the owner of the order (we did not have a way to know the state and bytecode of this helper contract to fall back to).
The new approach now allows us to easily opt-in to each "faking machinery" individually. So the simulation endpoint will only use balance overrides and allow listing a random address while the trade verification can pull out the big guns (many balance overrides, multiple fake contracts).
Additionally this PR adds the ability to correctly encode flashloans which was previously not supported by the
simulatorcrate.Changes
AnyoneAuthenticatorapproach with exactly overriding only the storage slot to allow list a specific address as a solverHow to test
updated existing e2e tests (populating the new fields in the custom order simulation request)