-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: end to end entry point routing #649
Conversation
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.
Looks good, just added a couple comments to consolidate the code at a later date based on implementation
match ep.version { | ||
EntryPointVersion::V0_6 => { | ||
let (handles, actions) = self | ||
.create_builders_v0_6(ep, Arc::clone(&provider), ep_v0_6.clone()) |
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.
Because the provider is already cloned within the ep_v0_6 struct I wonder if we should add an inner function to provider so that we can remove the second arg and clone from within the create_builders_v0_6
function. Does not really change the logic but the less args the better IMO
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.
From an interface perspective I don't like this. Its possible in the future an entry point wouldn't have an inner provider, that is specific to the ethers implementation.
@@ -255,6 +250,92 @@ where | |||
Box::new(self) | |||
} | |||
|
|||
async fn create_builders_v0_6<C, E>( |
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.
We should add a todo here to merge these builder creator functions into one using generics based on the UO version once entrypoint v7 safe simulation is implemented
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.
Yeah I found the types a little difficult to work with to get a common function. Adding a TODO
crates/pool/src/task.rs
Outdated
} | ||
|
||
async fn create_mempool_v0_6<P: Provider + Middleware>( | ||
fn create_mempool_v0_6<P: Provider + Middleware>( |
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.
Same thing here, it looks possible but need to play around with the code for this feature to make sure
@@ -25,10 +25,20 @@ use crate::simulation::SimulationViolation; | |||
/// Typically read from a JSON file using the `Deserialize` trait. | |||
#[derive(Debug, Clone, Deserialize, Default)] | |||
pub struct MempoolConfig { | |||
/// Entry point address this mempool is associated with. | |||
#[serde(rename = "camelCase")] | |||
pub(crate) entry_point: Address, |
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.
Will we have to add multiple mempool configs per network based on entrypoint addresses now?
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.
Added a TODO
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Proposed Changes
UNSAFE
and a hardcoded gas estimator