Skip to content

Commit

Permalink
chore: update bundler spec tests, update debug send bundle, aa51
Browse files Browse the repository at this point in the history
  • Loading branch information
Vid201 committed Feb 20, 2024
1 parent 560b796 commit 4f4a0fc
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions crates/bundler/src/bundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ where
///
/// # Returns
/// * `H256` - The hash
pub async fn send_bundle(&self, uos: &Vec<UserOperation>) -> eyre::Result<H256> {
pub async fn send_bundle(&self, uos: &Vec<UserOperation>) -> eyre::Result<Option<H256>> {
if uos.is_empty() {
info!("Skipping creating a new bundle, no user operations");
return Ok(H256::default());
return Ok(None);
};

info!(
Expand All @@ -167,6 +167,6 @@ where
self.beneficiary
);

Ok(hash)
Ok(Some(hash))
}
}
28 changes: 24 additions & 4 deletions crates/grpc/src/bundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ where
Ok(uos)
}

pub async fn send_bundles(&self) -> eyre::Result<H256> {
let mut tx_hashes: Vec<H256> = vec![];
pub async fn send_bundles(&self) -> eyre::Result<Option<H256>> {
let mut tx_hashes: Vec<Option<H256>> = vec![];

for bundler in self.bundlers.iter() {
let uos =
Expand All @@ -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) {
Expand Down Expand Up @@ -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()) }))
}
}

Expand Down
32 changes: 20 additions & 12 deletions crates/mempool/src/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ macro_rules! sanity_check_impls {
( $( $name:ident )+ ) => {
#[allow(non_snake_case)]
#[async_trait::async_trait]
impl<M: Middleware, $($name : SanityCheck<M>,)+ > SanityCheck<M> for ($($name,)+)
impl<M: Middleware, $($name : SanityCheck<M>,)+> SanityCheck<M> for ($($name,)+)
{
async fn check_user_operation<T, Y, X, Z, H, R>(
&self,
Expand All @@ -143,6 +143,7 @@ macro_rules! sanity_check_impls {
}
};
}

#[async_trait::async_trait]
impl<M: Middleware> SanityCheck<M> for () {
async fn check_user_operation<T, Y, X, Z, H, R>(
Expand All @@ -164,7 +165,7 @@ impl<M: Middleware> SanityCheck<M> 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 }
Expand Down Expand Up @@ -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,
Expand All @@ -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> {
Expand Down Expand Up @@ -283,6 +289,7 @@ pub trait SimulationTraceCheck<M: Middleware>: Send + Sync {
H: HashSetOp,
R: ReputationEntryOp;
}

macro_rules! simulation_trace_check_impls {
( $( $name:ident )+ ) => {
#[allow(non_snake_case)]
Expand Down Expand Up @@ -311,6 +318,7 @@ macro_rules! simulation_trace_check_impls {
}
};
}

#[async_trait::async_trait]
impl<M: Middleware> SimulationTraceCheck<M> for () {
async fn check_user_operation<T, Y, X, Z, H, R>(
Expand All @@ -332,7 +340,7 @@ impl<M: Middleware> SimulationTraceCheck<M> 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.
Expand Down
1 change: 1 addition & 0 deletions crates/mempool/src/validate/simulation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
40 changes: 40 additions & 0 deletions crates/mempool/src/validate/simulation/verification_extra_gas.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
}
12 changes: 7 additions & 5 deletions crates/mempool/src/validate/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -35,14 +37,14 @@ use tracing::debug;
pub type StandardValidator<M> = StandardUserOperationValidator<
M,
(Sender, VerificationGas, CallGas, MaxFee, Paymaster, Entities, UnstakedEntities),
(Signature, Timestamp),
(Signature, Timestamp, VerificationExtraGas),
(Gas, Opcodes, ExternalContracts, StorageAccess, CallStack, CodeHashes),
>;

type UnsafeValidator<M> = StandardUserOperationValidator<
M,
(Sender, VerificationGas, CallGas, MaxFee, Paymaster, Entities, UnstakedEntities),
(Signature, Timestamp),
(Signature, Timestamp, VerificationExtraGas),
(),
>;

Expand Down Expand Up @@ -115,7 +117,7 @@ pub fn new_canonical<M: Middleware + 'static>(
Entities,
UnstakedEntities,
),
(Signature, Timestamp),
(Signature, Timestamp, VerificationExtraGas),
(Gas, Opcodes, ExternalContracts, StorageAccess, CallStack, CodeHashes),
)
}
Expand All @@ -138,7 +140,7 @@ pub fn new_canonical_unsafe<M: Middleware + Clone + 'static>(
Entities,
UnstakedEntities,
),
(Signature, Timestamp),
(Signature, Timestamp, VerificationExtraGas),
(),
)
}
Expand Down
5 changes: 5 additions & 0 deletions crates/primitives/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion crates/rpc/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<H256>` - The hash of the bundle that was sent.
async fn send_bundle_now(&self) -> RpcResult<H256> {
Expand Down

0 comments on commit 4f4a0fc

Please sign in to comment.