Skip to content
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: WIP start splitting 0.6 and 0.7 logic #634

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/unit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ jobs:
steps:
- name: Checkout sources
uses: actions/checkout@v3
with:
submodules: 'recursive'

- name: Install toolchain
uses: dtolnay/rust-toolchain@stable
Expand Down
20 changes: 14 additions & 6 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
[submodule "crates/types/contracts/lib/account-abstraction"]
path = crates/types/contracts/lib/account-abstraction
[submodule "crates/types/contracts/lib/account-abstraction-versions/v0_7"]
path = crates/types/contracts/lib/account-abstraction-versions/v0_7
url = https://github.com/eth-infinitism/account-abstraction
branch = v0.5
branch = releases/v0.7
[submodule "crates/types/contracts/lib/account-abstraction-versions/v0_6"]
path = crates/types/contracts/lib/account-abstraction-versions/v0_6
url = https://github.com/eth-infinitism/account-abstraction
branch = releases/v0.6
[submodule "crates/types/contracts/lib/forge-std"]
path = crates/types/contracts/lib/forge-std
url = https://github.com/foundry-rs/forge-std
branch = chore/v1.5.0
[submodule "crates/types/contracts/lib/openzeppelin-contracts"]
path = crates/types/contracts/lib/openzeppelin-contracts
[submodule "crates/types/contracts/lib/openzeppelin-contracts-versions/v5_0"]
path = crates/types/contracts/lib/openzeppelin-contracts-versions/v5_0
url = https://github.com/OpenZeppelin/openzeppelin-contracts
branch = release-v5.0
[submodule "crates/types/contracts/lib/openzeppelin-contracts-versions/v4_9"]
path = crates/types/contracts/lib/openzeppelin-contracts-versions/v4_9
url = https://github.com/OpenZeppelin/openzeppelin-contracts
branch = release-v4.8
branch = release-v4.9
[submodule "test/spec-tests/bundler-spec-tests"]
path = test/spec-tests/bundler-spec-tests
url = https://github.com/alchemyplatform/bundler-spec-tests.git
Expand Down
2 changes: 1 addition & 1 deletion bin/tools/src/bin/send_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async fn main() -> anyhow::Result<()> {
// simply call the nonce method multiple times
for i in 0..10 {
println!("Sending op {i}");
let op = clients.new_wallet_op(wallet.nonce(), 0.into()).await?;
let op = clients.new_wallet_op(wallet.get_nonce(), 0.into()).await?;
let call = entry_point.handle_ops(vec![op], bundler_client.address());
rundler_dev::await_mined_tx(call.send(), "send user operation").await?;
}
Expand Down
86 changes: 33 additions & 53 deletions crates/builder/src/bundle_proposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@ use mockall::automock;
use rundler_pool::{PoolOperation, PoolServer};
use rundler_provider::{EntryPoint, HandleOpsOut, Provider};
use rundler_sim::{
gas::{self, GasOverheads},
EntityInfo, EntityInfos, ExpectedStorage, FeeEstimator, PriorityFeeMode, SimulationError,
gas, EntityInfo, EntityInfos, ExpectedStorage, FeeEstimator, PriorityFeeMode, SimulationError,
SimulationResult, SimulationViolation, Simulator, ViolationError,
};
use rundler_types::{
chain::ChainSpec, Entity, EntityType, EntityUpdate, EntityUpdateType, GasFees, Timestamp,
UserOperation, UserOpsPerAggregator,
chain::ChainSpec, Entity, EntityType, EntityUpdate, EntityUpdateType, GasFees, GasOverheads,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: GasOverheads should move to chain config

Timestamp, UserOperation, UserOpsPerAggregator,
};
use rundler_utils::{emit::WithEntryPoint, math};
use tokio::{sync::broadcast, try_join};
Expand Down Expand Up @@ -99,7 +98,7 @@ where
builder_index: u64,
pool: C,
simulator: S,
entry_point: E,
entry_point: Arc<E>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: figure this out, we should be consistent here, we don't need these Arc as everything inside the entry point class should be clone

provider: Arc<P>,
settings: Settings,
fee_estimator: FeeEstimator<P>,
Expand Down Expand Up @@ -230,7 +229,7 @@ where
builder_index: u64,
pool: C,
simulator: S,
entry_point: E,
entry_point: Arc<E>,
provider: Arc<P>,
settings: Settings,
event_sender: broadcast::Sender<WithEntryPoint<BuilderEvent>>,
Expand Down Expand Up @@ -266,17 +265,17 @@ where
required_op_fees: GasFees,
) -> Option<(PoolOperation, Result<SimulationResult, SimulationError>)> {
// filter by fees
if op.uo.max_fee_per_gas < required_op_fees.max_fee_per_gas
|| op.uo.max_priority_fee_per_gas < required_op_fees.max_priority_fee_per_gas
if op.uo.max_fee_per_gas() < required_op_fees.max_fee_per_gas
|| op.uo.max_priority_fee_per_gas() < required_op_fees.max_priority_fee_per_gas
{
self.emit(BuilderEvent::skipped_op(
self.builder_index,
self.op_hash(&op.uo),
SkipReason::InsufficientFees {
required_fees: required_op_fees,
actual_fees: GasFees {
max_fee_per_gas: op.uo.max_fee_per_gas,
max_priority_fee_per_gas: op.uo.max_priority_fee_per_gas,
max_fee_per_gas: op.uo.max_fee_per_gas(),
max_priority_fee_per_gas: op.uo.max_priority_fee_per_gas(),
},
},
));
Expand All @@ -286,7 +285,7 @@ where
// Check if the pvg is enough
let required_pvg = gas::calc_required_pre_verification_gas(
&self.settings.chain_spec,
self.provider.clone(),
self.entry_point.clone(),
&op.uo,
base_fee,
)
Expand All @@ -305,18 +304,18 @@ where
})
.ok()?;

if op.uo.pre_verification_gas < required_pvg {
if op.uo.pre_verification_gas() < required_pvg {
self.emit(BuilderEvent::skipped_op(
self.builder_index,
self.op_hash(&op.uo),
SkipReason::InsufficientPreVerificationGas {
base_fee,
op_fees: GasFees {
max_fee_per_gas: op.uo.max_fee_per_gas,
max_priority_fee_per_gas: op.uo.max_priority_fee_per_gas,
max_fee_per_gas: op.uo.max_fee_per_gas(),
max_priority_fee_per_gas: op.uo.max_priority_fee_per_gas(),
},
required_pvg,
actual_pvg: op.uo.pre_verification_gas,
actual_pvg: op.uo.pre_verification_gas(),
},
));
return None;
Expand All @@ -325,7 +324,11 @@ where
// Simulate
let result = self
.simulator
.simulate_validation(op.uo.clone(), Some(block_hash), Some(op.expected_code_hash))
.simulate_validation(
op.uo.clone().into(),
Some(block_hash),
Some(op.expected_code_hash),
)
.await;
let result = match result {
Ok(success) => (op, Ok(success)),
Expand Down Expand Up @@ -360,7 +363,7 @@ where
) -> ProposalContext {
let all_sender_addresses: HashSet<Address> = ops_with_simulations
.iter()
.map(|(op, _)| op.uo.sender)
.map(|(op, _)| op.uo.sender())
.collect();
let mut context = ProposalContext::new();
let mut paymasters_to_reject = Vec::<EntityInfo>::new();
Expand Down Expand Up @@ -410,25 +413,19 @@ where
}

// Skip this op if the bundle does not have enough remaining gas to execute it.
let required_gas = get_gas_required_for_op(
&self.settings.chain_spec,
gas_spent,
ov,
&op,
simulation.requires_post_op,
);
let required_gas = get_gas_required_for_op(&self.settings.chain_spec, gas_spent, &op);
if required_gas > self.settings.max_bundle_gas.into() {
continue;
}

if let Some(&other_sender) = simulation
.accessed_addresses
.iter()
.find(|&address| *address != op.sender && all_sender_addresses.contains(address))
.find(|&address| *address != op.sender() && all_sender_addresses.contains(address))
{
// Exclude ops that access the sender of another op in the
// batch, but don't reject them (remove them from pool).
info!("Excluding op from {:?} because it accessed the address of another sender in the bundle.", op.sender);
info!("Excluding op from {:?} because it accessed the address of another sender in the bundle.", op.sender());
self.emit(BuilderEvent::skipped_op(
self.builder_index,
self.op_hash(&op),
Expand Down Expand Up @@ -610,7 +607,7 @@ where
.iter()
.map(|op_with_simulation| op_with_simulation.op.clone())
.collect();
let result = Arc::clone(&self.provider)
let result = Arc::clone(&self.entry_point)
.aggregate_signatures(aggregator, ops)
.await
.map_err(anyhow::Error::from);
Expand Down Expand Up @@ -842,7 +839,7 @@ where
}

fn op_hash(&self, op: &UserOperation) -> H256 {
op.op_hash(self.entry_point.address(), self.settings.chain_spec.id)
op.hash(self.entry_point.address(), self.settings.chain_spec.id)
}
}

Expand All @@ -855,8 +852,9 @@ struct OpWithSimulation {
impl OpWithSimulation {
fn op_with_replaced_sig(&self) -> UserOperation {
let mut op = self.op.clone();
if let Some(aggregator) = &self.simulation.aggregator {
op.signature = aggregator.signature.clone();
if self.simulation.aggregator.is_some() {
// if using an aggregator, clear out the user op signature
op.clear_signature();
}
op
}
Expand Down Expand Up @@ -1039,13 +1037,7 @@ impl ProposalContext {
let mut max_gas = U256::zero();
for op_with_sim in self.iter_ops_with_simulations() {
let op = &op_with_sim.op;
let required_gas = get_gas_required_for_op(
chain_spec,
gas_spent,
ov,
op,
op_with_sim.simulation.requires_post_op,
);
let required_gas = get_gas_required_for_op(chain_spec, gas_spent, op);
max_gas = cmp::max(max_gas, required_gas);
gas_spent += gas::user_operation_gas_limit(
chain_spec,
Expand Down Expand Up @@ -1226,24 +1218,12 @@ impl ProposalContext {
}
}

fn get_gas_required_for_op(
chain_spec: &ChainSpec,
gas_spent: U256,
ov: GasOverheads,
op: &UserOperation,
requires_post_op: bool,
) -> U256 {
let post_exec_req_gas = if requires_post_op {
cmp::max(op.verification_gas_limit, ov.bundle_transaction_gas_buffer)
} else {
ov.bundle_transaction_gas_buffer
};

fn get_gas_required_for_op(chain_spec: &ChainSpec, gas_spent: U256, op: &UserOperation) -> U256 {
gas_spent
+ gas::user_operation_pre_verification_gas_limit(chain_spec, op, false)
+ op.verification_gas_limit * 2
+ op.call_gas_limit
+ post_exec_req_gas
+ op.total_verification_gas_limit()
+ op.required_pre_execution_buffer()
+ op.call_gas_limit()
Comment on lines +1224 to +1226
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: verify this change

}

#[cfg(test)]
Expand Down
4 changes: 2 additions & 2 deletions crates/builder/src/bundle_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ where
.remove_ops(
self.entry_point.address(),
ops.iter()
.map(|op| op.op_hash(self.entry_point.address(), self.chain_spec.id))
.map(|op| op.hash(self.entry_point.address(), self.chain_spec.id))
.collect(),
)
.await
Expand All @@ -565,7 +565,7 @@ where
}

fn op_hash(&self, op: &UserOperation) -> H256 {
op.op_hash(self.entry_point.address(), self.chain_spec.id)
op.hash(self.entry_point.address(), self.chain_spec.id)
}
}

Expand Down
12 changes: 6 additions & 6 deletions crates/builder/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ use ethers_signers::Signer;
use futures::future;
use futures_util::TryFutureExt;
use rundler_pool::PoolServer;
use rundler_provider::{EntryPoint, EthersEntryPoint};
use rundler_provider::{EntryPoint, EthersEntryPointV0_6};
use rundler_sim::{
MempoolConfig, PriorityFeeMode, SimulateValidationTracerImpl, SimulationSettings, SimulatorImpl,
MempoolConfig, PriorityFeeMode, SimulateValidationTracerImpl, SimulationSettings, SimulatorV0_6,
};
use rundler_task::Task;
use rundler_types::chain::ChainSpec;
Expand Down Expand Up @@ -262,15 +262,15 @@ where
bundle_priority_fee_overhead_percent: self.args.bundle_priority_fee_overhead_percent,
};

let ep = EthersEntryPoint::new(
let ep = EthersEntryPointV0_6::new(
self.args.chain_spec.entry_point_address,
Arc::clone(&provider),
);
let simulate_validation_tracer =
SimulateValidationTracerImpl::new(Arc::clone(&provider), ep.clone());
let simulator = SimulatorImpl::new(
let simulator = SimulatorV0_6::new(
Arc::clone(&provider),
ep.address(),
Arc::new(ep.clone()),
simulate_validation_tracer,
self.args.sim_settings,
self.args.mempool_configs.clone(),
Expand Down Expand Up @@ -311,7 +311,7 @@ where
index,
self.pool.clone(),
simulator,
ep.clone(),
Arc::new(ep.clone()),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: bundle proposer currently will only handle v0.6 UOs. Figure out how to share logic

Arc::clone(&provider),
proposer_settings,
self.event_sender.clone(),
Expand Down
10 changes: 5 additions & 5 deletions crates/dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ use ethers::{
utils::{self, hex, keccak256},
};
use rundler_types::{
contracts::{
contracts::v0_6::{
entry_point::EntryPoint, simple_account::SimpleAccount,
simple_account_factory::SimpleAccountFactory, verifying_paymaster::VerifyingPaymaster,
},
UserOperation,
UserOperationV0_6 as UserOperation,
};

/// Chain ID used by Geth in --dev mode.
Expand Down Expand Up @@ -320,7 +320,7 @@ pub async fn deploy_dev_contracts(entry_point_bytecode: &str) -> anyhow::Result<
init_code,
..base_user_op()
};
let op_hash = op.op_hash(entry_point.address(), DEV_CHAIN_ID);
let op_hash = op.hash(entry_point.address(), DEV_CHAIN_ID);
let signature = wallet_owner_eoa
.sign_message(op_hash)
.await
Expand Down Expand Up @@ -426,7 +426,7 @@ impl DevClients {
paymaster_and_data.extend(paymaster_signature.to_vec());
op.paymaster_and_data = paymaster_and_data.into()
}
let op_hash = op.op_hash(self.entry_point.address(), DEV_CHAIN_ID);
let op_hash = op.hash(self.entry_point.address(), DEV_CHAIN_ID);
let signature = self
.wallet_owner_signer
.sign_message(op_hash)
Expand Down Expand Up @@ -470,7 +470,7 @@ impl DevClients {
.context("call executed by wallet should have to address")?;
let nonce = self
.wallet
.nonce()
.get_nonce()
.await
.context("should read nonce from wallet")?;
let call_data = Bytes::clone(
Expand Down
Loading
Loading