-
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: add drop local user operation endpoint #610
Conversation
Codecov ReportAttention:
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
So uo hash is not needed for this api? What's the difference between uo hash bs uo Id (sender + nonce) for serving uniqueness?
Also after canceling the uo with nonce K, should nonce still be a new nonce value? (K+1) or K can still be the nonce of the next uo?
|
||
/// Drops a user operation from the local mempool. | ||
/// | ||
/// Requirements: |
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.
Noted. So it costs gas for verification to cancel a UO. Why the 3rd requirement for this end point instead of just ignoring? Just curious
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 doesn't cost any gas on chain, but the UO must contain a high enough gas limit such that we can simulate it. This value is set by the user because its part of the UO that is signed.
if uo.pre_verification_gas != U256::zero() | ||
|| uo.call_gas_limit != U256::zero() | ||
|| uo.call_data.len() != 0 | ||
|| uo.max_fee_per_gas != U256::zero() |
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.
Do these requirements exist for simulate uo operation?
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.
I mean simulation verification
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.
The reason for these requirements is to ensure that the signed UO is not executable onchain. That is, it should have 0 gas fees associated with it. The most important one here is max_fee_per_gas == 0
, as that would cause the entrypoint to reject the UO. We could reduce this requirements set to just that.
Simulation verification when a normal UO is submitted doesn't have this requirement because the UO is meant to be included onchain.
There is no API for a sender to lookup pending UOs. So if they find that they are stuck, i.e. getting replacement underpriced consistently, they can issue a cancel just knowing their address and their nonce. No need to also lookup a hash.
K is still the nonce of the next UO, since nothing happened onchain. |
This is missing the 10 block delay that we discussed in the ticket to prevent spam, need to add that before merge. |
Also worth noting that this isn't a perfect drop mechanism. Its possible that the UO has already been bundled (or sent on the P2P network) before the drop. |
a727ed8
to
c635e0a
Compare
75f76c2
to
34dd83d
Compare
5f327b9
to
85504b4
Compare
34dd83d
to
f8c1c93
Compare
f8c1c93
to
6616322
Compare
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 a nit
@@ -46,21 +46,38 @@ where | |||
self.deref().address() | |||
} | |||
|
|||
async fn simulate_validation( | |||
async fn get_simulate_validation_call( | |||
&self, | |||
user_op: UserOperation, | |||
max_validation_gas: u64, | |||
) -> anyhow::Result<TypedTransaction> { | |||
let pvg = user_op.pre_verification_gas; |
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.
not a comment but having spaces between distinct code blocks is very handy for vim movements rip
6616322
to
1e72e7d
Compare
Closes #567
Proposed Changes
rundler_dropLocalUserOperation
rpc endpoint