From c1e9a0a61c68a822ce89dae40fc53e2e48c3534d Mon Sep 17 00:00:00 2001 From: Vid Kersic Date: Tue, 20 Feb 2024 12:15:49 +0100 Subject: [PATCH] chore: update bundler spec tests, update debug send bundle, aa51 --- .github/workflows/ci.yml | 2 +- README.md | 2 +- crates/bundler/src/bundler.rs | 6 +-- crates/grpc/src/bundler.rs | 28 +++++++++++-- crates/mempool/src/validate/mod.rs | 32 +++++++++------ crates/mempool/src/validate/simulation/mod.rs | 1 + .../simulation/verification_extra_gas.rs | 40 +++++++++++++++++++ crates/mempool/src/validate/validator.rs | 12 +++--- crates/primitives/src/constants.rs | 5 +++ crates/rpc/src/debug.rs | 1 - 10 files changed, 102 insertions(+), 27 deletions(-) create mode 100644 crates/mempool/src/validate/simulation/verification_extra_gas.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb74545f..6a45fdd2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,7 +99,7 @@ jobs: - uses: actions/checkout@v4 with: repository: eth-infinitism/bundler-spec-tests - ref: 'b9f192f39298e6586729d40f29e3098c92e5c0b9' + ref: '08cbbfcb9e37b84c0ef9e546975f88fa638cac61' submodules: true - uses: actions/checkout@v4 diff --git a/README.md b/README.md index 7bc4ba10..81f51289 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ make lint make test ``` -Official [bundler spec tests](https://github.com/eth-infinitism/bundler-spec-tests) developed by the [eth-infinitism](https://github.com/eth-infinitism/) team are also included in the repo's CI pipeline (commit: [b9f192f39298e6586729d40f29e3098c92e5c0b9](https://github.com/eth-infinitism/bundler-spec-tests/tree/b9f192f39298e6586729d40f29e3098c92e5c0b9)). You can find more information on how to run tests [here](https://github.com/eth-infinitism/bundler-spec-tests). Make sure your contribution doesn't break the tests! +Official [bundler spec tests](https://github.com/eth-infinitism/bundler-spec-tests) developed by the [eth-infinitism](https://github.com/eth-infinitism/) team are also included in the repo's CI pipeline (commit: [08cbbfcb9e37b84c0ef9e546975f88fa638cac61](https://github.com/eth-infinitism/bundler-spec-tests/tree/08cbbfcb9e37b84c0ef9e546975f88fa638cac61)). You can find more information on how to run tests [here](https://github.com/eth-infinitism/bundler-spec-tests). Make sure your contribution doesn't break the tests! ## Contact diff --git a/crates/bundler/src/bundler.rs b/crates/bundler/src/bundler.rs index e639fa3d..b750133b 100644 --- a/crates/bundler/src/bundler.rs +++ b/crates/bundler/src/bundler.rs @@ -143,10 +143,10 @@ where /// /// # Returns /// * `H256` - The hash - pub async fn send_bundle(&self, uos: &Vec) -> eyre::Result { + pub async fn send_bundle(&self, uos: &Vec) -> eyre::Result> { if uos.is_empty() { info!("Skipping creating a new bundle, no user operations"); - return Ok(H256::default()); + return Ok(None); }; info!( @@ -167,6 +167,6 @@ where self.beneficiary ); - Ok(hash) + Ok(Some(hash)) } } diff --git a/crates/grpc/src/bundler.rs b/crates/grpc/src/bundler.rs index 0c2a7e5b..f5c132fa 100644 --- a/crates/grpc/src/bundler.rs +++ b/crates/grpc/src/bundler.rs @@ -54,8 +54,8 @@ where Ok(uos) } - pub async fn send_bundles(&self) -> eyre::Result { - let mut tx_hashes: Vec = vec![]; + pub async fn send_bundles(&self) -> eyre::Result> { + let mut tx_hashes: Vec> = vec![]; for bundler in self.bundlers.iter() { let uos = @@ -67,7 +67,7 @@ where // FIXME: Because currently the bundler support multiple bundler and // we don't have a way to know which bundler is the one that is - Ok(tx_hashes.into_iter().next().expect("Must have at least one tx hash")) + Ok(tx_hashes.into_iter().next().expect("At least one bundler must be present")) } pub fn stop_bundling(&self) { @@ -158,7 +158,27 @@ where .send_bundles() .await .map_err(|e| tonic::Status::internal(format!("Send bundle now with error: {e:?}")))?; - Ok(Response::new(SendBundleNowResponse { res: Some(res.into()) })) + + if let Some(tx_hash) = res { + // wait for the tx to be mined + loop { + let tx_receipt = self + .bundlers + .first() + .expect("Must have at least one bundler") + .eth_client + .get_transaction_receipt(tx_hash) + .await; + if let Ok(tx_receipt) = tx_receipt { + if tx_receipt.is_some() { + break; + } + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + } + + Ok(Response::new(SendBundleNowResponse { res: Some(res.unwrap_or_default().into()) })) } } diff --git a/crates/mempool/src/validate/mod.rs b/crates/mempool/src/validate/mod.rs index a731b5e6..16360c97 100644 --- a/crates/mempool/src/validate/mod.rs +++ b/crates/mempool/src/validate/mod.rs @@ -119,7 +119,7 @@ macro_rules! sanity_check_impls { ( $( $name:ident )+ ) => { #[allow(non_snake_case)] #[async_trait::async_trait] - impl,)+ > SanityCheck for ($($name,)+) + impl,)+> SanityCheck for ($($name,)+) { async fn check_user_operation( &self, @@ -143,6 +143,7 @@ macro_rules! sanity_check_impls { } }; } + #[async_trait::async_trait] impl SanityCheck for () { async fn check_user_operation( @@ -164,7 +165,7 @@ impl SanityCheck for () { } } -// These macro enable people to chain sanity check implementations.: +// These macro enable people to chain sanity check implementations: // `(SanityCheck1, SanityCheck2, SanityCheck3, ...).check_user_operation(uo, mempool, reputation, // helper)`` SanityCheck1,2,3 could be any data type which implement SanityCheck trait. sanity_check_impls! { A } @@ -203,11 +204,12 @@ pub trait SimulationCheck: Send + Sync { helper: &mut SimulationHelper, ) -> Result<(), SimulationError>; } + macro_rules! simulation_check_impls { ( $( $name:ident )+ ) => { #[allow(non_snake_case)] #[async_trait::async_trait] - impl<$($name : SimulationCheck,)+ > SimulationCheck for ($($name,)+) + impl<$($name : SimulationCheck,)+> SimulationCheck for ($($name,)+) { fn check_user_operation( &self, @@ -223,15 +225,19 @@ macro_rules! simulation_check_impls { }; } -// These macro enable people to chain simulation check implementations.: +// These macro enable people to chain simulation check implementations: // `(SimulationCheck1, SimulationCheck2, SimulationCheck3, ...).check_user_operation(uo, helper)`` -// SimulationChekc1,2,3 could be any data type which implement SimulationCheck trait. -simulation_check_impls! {A} -simulation_check_impls! {A B} -simulation_check_impls! {A B C} -simulation_check_impls! {A B C D} -simulation_check_impls! {A B C D E} -simulation_check_impls! {A B C D E F} +// SimulationCheck1,2,3 could be any data type which implement SimulationCheck trait. +simulation_check_impls! { A } +simulation_check_impls! { A B } +simulation_check_impls! { A B C } +simulation_check_impls! { A B C D } +simulation_check_impls! { A B C D F } +simulation_check_impls! { A B C D F G } +simulation_check_impls! { A B C D F G I } +simulation_check_impls! { A B C D F G I J } +simulation_check_impls! { A B C D F G I J K } +simulation_check_impls! { A B C D F G I J K L } /// The [UserOperation](UserOperation) simulation trace check helper trait. pub struct SimulationTraceHelper<'a, M: Middleware + Send + Sync + 'static> { @@ -283,6 +289,7 @@ pub trait SimulationTraceCheck: Send + Sync { H: HashSetOp, R: ReputationEntryOp; } + macro_rules! simulation_trace_check_impls { ( $( $name:ident )+ ) => { #[allow(non_snake_case)] @@ -311,6 +318,7 @@ macro_rules! simulation_trace_check_impls { } }; } + #[async_trait::async_trait] impl SimulationTraceCheck for () { async fn check_user_operation( @@ -332,7 +340,7 @@ impl SimulationTraceCheck for () { } } -// These macro enable people to chain simulation check implementations.: +// These macro enable people to chain simulation check implementations: // `(SimulationTraceCheck1, SimulationTraceCheck2, SimulationTraceCheck3, // ...).check_user_operation(uo, mempool, reputeation helper)`` SimulationTraceCheck1,2,3 could be // any data type which implement SimulationTraceCheck trait. diff --git a/crates/mempool/src/validate/simulation/mod.rs b/crates/mempool/src/validate/simulation/mod.rs index d98845ea..9b42c468 100644 --- a/crates/mempool/src/validate/simulation/mod.rs +++ b/crates/mempool/src/validate/simulation/mod.rs @@ -2,3 +2,4 @@ //! timestamp via a `eth_call` to the Ethereum execution client. pub mod signature; pub mod timestamp; +pub mod verification_extra_gas; diff --git a/crates/mempool/src/validate/simulation/verification_extra_gas.rs b/crates/mempool/src/validate/simulation/verification_extra_gas.rs new file mode 100644 index 00000000..e0850a48 --- /dev/null +++ b/crates/mempool/src/validate/simulation/verification_extra_gas.rs @@ -0,0 +1,40 @@ +use crate::{ + validate::{SimulationCheck, SimulationHelper}, + SimulationError, +}; +use silius_contracts::entry_point::SimulateValidationResult; +use silius_primitives::{constants::validation::simulation::MIN_EXTRA_GAS, UserOperation}; + +#[derive(Clone)] +pub struct VerificationExtraGas; + +impl SimulationCheck for VerificationExtraGas { + /// The [check_user_operation] method implementation validates the needed extra gas. + /// + /// # Arguments + /// `uo` - Not used in this check + /// `helper` - The [SimulationHelper](crate::validate::SimulationHelper) + /// + /// # Returns + /// None if the check passes, otherwise a [SimulationError] error. + fn check_user_operation( + &self, + uo: &UserOperation, + helper: &mut SimulationHelper, + ) -> Result<(), SimulationError> { + let pre_op_gas = match helper.simulate_validation_result { + SimulateValidationResult::ValidationResult(res) => res.return_info.0, + SimulateValidationResult::ValidationResultWithAggregation(res) => res.return_info.0, + }; + + let extra_gas = uo.verification_gas_limit - (pre_op_gas - uo.pre_verification_gas); + + if extra_gas.as_u64() < MIN_EXTRA_GAS { + return Err(SimulationError::Validation { + inner: "Verification gas should have extra 2000 gas (has ${extra_gas})".into(), + }); + } + + Ok(()) + } +} diff --git a/crates/mempool/src/validate/validator.rs b/crates/mempool/src/validate/validator.rs index 549e62b9..f889675d 100644 --- a/crates/mempool/src/validate/validator.rs +++ b/crates/mempool/src/validate/validator.rs @@ -3,7 +3,9 @@ use super::{ call_gas::CallGas, entities::Entities, max_fee::MaxFee, paymaster::Paymaster, sender::Sender, unstaked_entities::UnstakedEntities, verification_gas::VerificationGas, }, - simulation::{signature::Signature, timestamp::Timestamp}, + simulation::{ + signature::Signature, timestamp::Timestamp, verification_extra_gas::VerificationExtraGas, + }, simulation_trace::{ call_stack::CallStack, code_hashes::CodeHashes, external_contracts::ExternalContracts, gas::Gas, opcodes::Opcodes, storage_access::StorageAccess, @@ -35,14 +37,14 @@ use tracing::debug; pub type StandardValidator = StandardUserOperationValidator< M, (Sender, VerificationGas, CallGas, MaxFee, Paymaster, Entities, UnstakedEntities), - (Signature, Timestamp), + (Signature, Timestamp, VerificationExtraGas), (Gas, Opcodes, ExternalContracts, StorageAccess, CallStack, CodeHashes), >; type UnsafeValidator = StandardUserOperationValidator< M, (Sender, VerificationGas, CallGas, MaxFee, Paymaster, Entities, UnstakedEntities), - (Signature, Timestamp), + (Signature, Timestamp, VerificationExtraGas), (), >; @@ -115,7 +117,7 @@ pub fn new_canonical( Entities, UnstakedEntities, ), - (Signature, Timestamp), + (Signature, Timestamp, VerificationExtraGas), (Gas, Opcodes, ExternalContracts, StorageAccess, CallStack, CodeHashes), ) } @@ -138,7 +140,7 @@ pub fn new_canonical_unsafe( Entities, UnstakedEntities, ), - (Signature, Timestamp), + (Signature, Timestamp, VerificationExtraGas), (), ) } diff --git a/crates/primitives/src/constants.rs b/crates/primitives/src/constants.rs index 1a5c37b8..2a1c4eae 100644 --- a/crates/primitives/src/constants.rs +++ b/crates/primitives/src/constants.rs @@ -56,6 +56,11 @@ pub mod validation { pub const THROTTLING_SLACK: u64 = 10; pub const BAN_SLACK: u64 = 50; } + + /// Simulation + pub mod simulation { + pub const MIN_EXTRA_GAS: u64 = 2000; + } } /// Flashbots relay endpoints diff --git a/crates/rpc/src/debug.rs b/crates/rpc/src/debug.rs index 98afa9c9..d9c3e6e6 100644 --- a/crates/rpc/src/debug.rs +++ b/crates/rpc/src/debug.rs @@ -230,7 +230,6 @@ impl DebugApiServer for DebugApiServerImpl { /// This is useful for testing or in situations where waiting for the next scheduled bundle is /// not desirable. /// - /// /// # Returns /// * `RpcResult` - The hash of the bundle that was sent. async fn send_bundle_now(&self) -> RpcResult {