-
Notifications
You must be signed in to change notification settings - Fork 163
Simulate EIP-1271 orders at creation #4355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 29 commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
6897fec
Add EIP-1271 shadow simulator scaffolding
squadgazzz 4d63688
Fix clippy and align SimulationFailed with tuple convention
squadgazzz 7eb008b
Thread optional EIP-1271 shadow simulator into OrderValidator
squadgazzz 3d074e4
Extract shadow sim timeout constant and rename HTTP error code
squadgazzz d68cb90
Integrate EIP-1271 shadow simulation in OrderValidator
squadgazzz f0af347
Add shadow sim mode and timeout to orderbook config
squadgazzz bd51ed8
Wire EIP-1271 shadow simulator through orderbook runtime
squadgazzz 5c6e8d2
Rename shadow-sim types to drop the shadow prefix
squadgazzz 557ae35
Clean up remaining 'shadow' references in docs and locals
squadgazzz 6e7ee7a
Nits
squadgazzz 1733fb2
Group EIP-1271 sim deps into Eip1271SimConfig on OrderValidator
squadgazzz af31a39
Rename Eip1271Simulator trait to Eip1271Simulating; promote SimConfig…
squadgazzz 65a0290
Collapse build_1271_validator args to take Option<Eip1271Simulator>
squadgazzz a37c7a0
Move DEFAULT_EIP1271_SIM_TIMEOUT into test module
squadgazzz 9c93de6
Simplify Eip1271Simulator doc comment
squadgazzz f5ddba5
Rename cheap→signature in sim metrics; merge sim_only_total into total
squadgazzz cebd99b
Rename duration_seconds histogram to simulation_time
squadgazzz 67a4d32
Drop owner from sim logs; OrderUid already encodes it
squadgazzz 2fbf9f0
Split sim disagreement log into explicit arms instead of an empty-str…
squadgazzz a781c44
Use Debug formatting (?) in sim logs to match project convention
squadgazzz d11442e
Expand sim abbreviation to simulation across types, fields, metrics, …
squadgazzz eca5c61
Simplify Eip1271Simulating doc and scope its mock to cfg(test) only
squadgazzz 5407c8b
Add Disabled mode for order-creation EIP-1271 simulation
squadgazzz eed5990
Disabled by default
squadgazzz 2785108
Align default test with Disabled as the default simulation mode
squadgazzz 7281ded
Redundant comment
squadgazzz 3525684
Assert simulator is never invoked for non-EIP-1271 orders
squadgazzz 191d7ff
Consolidate EIP-1271 signature/simulation quadrant tests into a matrix
squadgazzz e62217c
Use .label() on outcomes instead of hardcoded strings in sim logs
squadgazzz 4c25c46
Address Claude-review feedback on PR 4355
squadgazzz 250f5da
Convert order_simulator::Error via From impl for ? ergonomics
squadgazzz c51a522
Review comments
squadgazzz 7bd2592
Nit
squadgazzz ea11e37
Add SignatureCheck::new constructor
squadgazzz ebfd4a9
Pull timeout handling out of simulation_fut in run_eip1271_with_signa…
squadgazzz 06c478a
Merge branch 'main' into feat/orderbook-eip1271-shadow-sim
squadgazzz 490bcdc
Address jose review nits
squadgazzz fbe765c
Comment
squadgazzz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| use { | ||
| crate::order_simulator::{self, OrderSimulator}, | ||
| async_trait::async_trait, | ||
| model::order::Order, | ||
| shared::order_validation::{Eip1271Simulating, Eip1271SimulationError}, | ||
| std::sync::Arc, | ||
| }; | ||
|
|
||
| /// Adapter exposing `OrderSimulator` via the | ||
| /// `shared::order_validation::Eip1271Simulating` trait. | ||
| /// | ||
| /// This is a temporary shim. Once the `simulator` crate is refactored to own | ||
| /// `OrderSimulator`, `OrderValidator` can depend on it directly and this | ||
| /// adapter can be deleted. | ||
| pub struct OrderSimulatorAdapter { | ||
| inner: Arc<OrderSimulator>, | ||
| } | ||
|
|
||
| impl OrderSimulatorAdapter { | ||
| pub fn new(inner: Arc<OrderSimulator>) -> Self { | ||
| Self { inner } | ||
| } | ||
| } | ||
|
|
||
| #[async_trait] | ||
| impl Eip1271Simulating for OrderSimulatorAdapter { | ||
| async fn simulate(&self, order: &Order) -> Result<(), Eip1271SimulationError> { | ||
| let swap = self | ||
| .inner | ||
| .encode_order(order, Vec::new(), None) | ||
| .await | ||
| .map_err(map_simulator_err)?; | ||
| let result = self | ||
| .inner | ||
| .simulate_swap(swap, None) | ||
| .await | ||
| .map_err(map_simulator_err)?; | ||
| match result.error { | ||
| None => Ok(()), | ||
| Some(reason) => Err(Eip1271SimulationError::Reverted { | ||
| reason, | ||
| tenderly_url: result.tenderly_url, | ||
| }), | ||
| } | ||
|
squadgazzz marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| fn map_simulator_err(err: order_simulator::Error) -> Eip1271SimulationError { | ||
|
squadgazzz marked this conversation as resolved.
Outdated
|
||
| match err { | ||
| order_simulator::Error::Other(e) | order_simulator::Error::MalformedInput(e) => { | ||
| Eip1271SimulationError::Infra(e) | ||
| } | ||
| } | ||
| } | ||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be part of either the configs crate or the order simulator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is in configs, this is the enum you're commenting on. The copy in shared::order_validation is only there because OrderValidator lives in shared and shared can't depend on configs, so we mirror plus convert at the call site in run.rs. Once Martin's simulator refactor lands and OrderSimulator moves out of orderbook, we can probably collapse both into one enum.