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: prep for da oracle with cache #834

Merged
merged 1 commit into from
Oct 9, 2024
Merged
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
8 changes: 5 additions & 3 deletions crates/builder/src/bundle_proposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ where
// (1) Filter out ops that don't pay enough to be included
let fee_futs = ops
.into_iter()
.map(|op| self.check_fees(op, base_fee, required_op_fees))
.map(|op| self.check_fees(op, block_hash, base_fee, required_op_fees))
.collect::<Vec<_>>();
let ops = future::join_all(fee_futs)
.await
Expand Down Expand Up @@ -345,6 +345,7 @@ where
async fn check_fees(
&self,
op: PoolOperation,
block_hash: B256,
base_fee: u128,
required_op_fees: GasFees,
) -> Option<PoolOperation> {
Expand Down Expand Up @@ -373,6 +374,7 @@ where
&self.settings.chain_spec,
&self.entry_point,
op.uo.as_ref(),
block_hash.into(),
base_fee,
)
.await
Expand Down Expand Up @@ -428,7 +430,7 @@ where
.simulator
.simulate_validation(
op.uo.clone().into(),
Some(block_hash),
block_hash,
Some(op.expected_code_hash),
)
.await;
Expand Down Expand Up @@ -2293,7 +2295,7 @@ mod tests {
simulator
.expect_simulate_validation()
.withf(move |_, &block_hash, &code_hash| {
block_hash == Some(current_block_hash) && code_hash == Some(expected_code_hash)
block_hash == current_block_hash && code_hash == Some(expected_code_hash)
})
.returning(move |op, _, _| simulations_by_op[&op.hash(entry_point_address, 0)]());
let mut entry_point = MockEntryPointV0_6::new();
Expand Down
4 changes: 2 additions & 2 deletions crates/builder/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ where
i + ep.bundle_builder_index_offset,
self.provider.clone(),
ep_v0_6.clone(),
UnsafeSimulator::new(self.provider.clone(), ep_v0_6.clone()),
UnsafeSimulator::new(ep_v0_6.clone()),
pk_iter,
)
.await?
Expand Down Expand Up @@ -273,7 +273,7 @@ where
i + ep.bundle_builder_index_offset,
self.provider.clone(),
ep_v0_7.clone(),
UnsafeSimulator::new(self.provider.clone(), ep_v0_7.clone()),
UnsafeSimulator::new(ep_v0_7.clone()),
pk_iter,
)
.await?
Expand Down
2 changes: 1 addition & 1 deletion crates/pool/src/mempool/paymaster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,9 +526,9 @@ mod tests {
valid_time_range: ValidTimeRange::all_time(),
expected_code_hash: B256::random(),
sim_block_hash: B256::random(),
sim_block_number: 0,
account_is_staked: true,
entity_infos: EntityInfos::default(),
sim_block_number: 0,
}
}

Expand Down
59 changes: 48 additions & 11 deletions crates/pool/src/mempool/uo_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{collections::HashSet, marker::PhantomData, sync::Arc};
use alloy_primitives::{utils::format_units, Address, B256, U256};
use itertools::Itertools;
use parking_lot::RwLock;
use rundler_provider::EntryPoint;
use rundler_provider::{EntryPoint, EvmProvider};
use rundler_sim::{Prechecker, Simulator};
use rundler_types::{
pool::{
Expand Down Expand Up @@ -44,12 +44,19 @@ use crate::{
/// Wrapper around a pool object that implements thread-safety
/// via a RwLock. Safe to call from multiple threads. Methods
/// block on write locks.
pub(crate) struct UoPool<UO: UserOperation, P: Prechecker, S: Simulator, E: EntryPoint> {
pub(crate) struct UoPool<
UO: UserOperation,
EP: EvmProvider,
P: Prechecker,
S: Simulator,
E: EntryPoint,
> {
config: PoolConfig,
state: RwLock<UoPoolState>,
paymaster: PaymasterTracker<E>,
reputation: Arc<AddressReputation>,
event_sender: broadcast::Sender<WithEntryPoint<OpPoolEvent>>,
provider: EP,
prechecker: P,
simulator: S,
_uo_type: PhantomData<UO>,
Expand All @@ -59,20 +66,23 @@ struct UoPoolState {
pool: PoolInner,
throttled_ops: HashSet<B256>,
block_number: u64,
block_hash: B256,
gas_fees: GasFees,
base_fee: u128,
}

impl<UO, P, S, E> UoPool<UO, P, S, E>
impl<UO, EP, P, S, E> UoPool<UO, EP, P, S, E>
where
UO: UserOperation,
EP: EvmProvider,
P: Prechecker<UO = UO>,
S: Simulator<UO = UO>,
E: EntryPoint,
{
pub(crate) fn new(
config: PoolConfig,
event_sender: broadcast::Sender<WithEntryPoint<OpPoolEvent>>,
provider: EP,
prechecker: P,
simulator: S,
paymaster: PaymasterTracker<E>,
Expand All @@ -83,12 +93,14 @@ where
pool: PoolInner::new(config.clone().into()),
throttled_ops: HashSet::new(),
block_number: 0,
block_hash: B256::ZERO,
gas_fees: GasFees::default(),
base_fee: 0,
}),
reputation,
paymaster,
event_sender,
provider,
prechecker,
simulator,
config,
Expand Down Expand Up @@ -137,9 +149,10 @@ where
}

#[async_trait]
impl<UO, P, S, E> Mempool for UoPool<UO, P, S, E>
impl<UO, EP, P, S, E> Mempool for UoPool<UO, EP, P, S, E>
where
UO: UserOperation + From<UserOperationVariant> + Into<UserOperationVariant>,
EP: EvmProvider,
P: Prechecker<UO = UO>,
S: Simulator<UO = UO>,
E: EntryPoint,
Expand Down Expand Up @@ -342,6 +355,7 @@ where
{
let mut state = self.state.write();
state.block_number = update.latest_block_number;
state.block_hash = update.latest_block_hash;
state.gas_fees = bundle_fees;
state.base_fee = base_fee;
}
Expand Down Expand Up @@ -419,6 +433,18 @@ where
);
}

// NOTE: We get the latest block from the provider here to avoid a race condition
// where the pool is still processing the previous block, but the user may have been
// notified of a new block.
//
// This doesn't clear all race conditions, as the pool may need to update its state before
// a UO can be valid, i.e. for replacement.
let (block_hash, block_number) = self
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This call is just moved up from the simulator into this function as the block is now needed by the prechecker as well.

Comment is there because I got burned by trying to use state.block_hash instead and ran into this race condition.

.provider
.get_latest_block_hash_and_number()
.await
.map_err(anyhow::Error::from)?;

// Check if op is already known or replacing another, and if so, ensure its fees are high enough
// do this before simulation to save resources
let replacement = self.state.read().pool.check_replacement(&op)?;
Expand All @@ -433,12 +459,14 @@ where

// Prechecks
let versioned_op = op.clone().into();
self.prechecker.check(&versioned_op).await?;
self.prechecker
.check(&versioned_op, block_hash.into())
.await?;

// Only let ops with successful simulations through
let sim_result = self
.simulator
.simulate_validation(versioned_op, None, None)
.simulate_validation(versioned_op, block_hash, None)
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
.await?;

// No aggregators supported for now
Expand All @@ -459,8 +487,8 @@ where
aggregator: None,
valid_time_range,
expected_code_hash: sim_result.code_hash,
sim_block_hash: sim_result.block_hash,
sim_block_number: sim_result.block_number.unwrap(), // simulation always returns a block number when called without a specified block_hash
sim_block_hash: block_hash,
sim_block_number: block_number,
account_is_staked: sim_result.account_is_staked,
entity_infos: sim_result.entity_infos,
};
Expand Down Expand Up @@ -733,7 +761,7 @@ mod tests {

use alloy_primitives::Bytes;
use mockall::Sequence;
use rundler_provider::{DepositInfo, MockEntryPointV0_6};
use rundler_provider::{DepositInfo, MockEntryPointV0_6, MockEvmProvider};
use rundler_sim::{
MockPrechecker, MockSimulator, PrecheckError, PrecheckSettings, SimulationError,
SimulationResult, SimulationSettings, ViolationError,
Expand Down Expand Up @@ -1517,6 +1545,7 @@ mod tests {
ops: Vec<OpWithErrors>,
) -> UoPool<
UserOperation,
impl EvmProvider,
impl Prechecker<UO = UserOperation>,
impl Simulator<UO = UserOperation>,
impl EntryPoint,
Expand All @@ -1530,6 +1559,7 @@ mod tests {
entrypoint: MockEntryPointV0_6,
) -> UoPool<
UserOperation,
impl EvmProvider,
impl Prechecker<UO = UserOperation>,
impl Simulator<UO = UserOperation>,
impl EntryPoint,
Expand All @@ -1555,6 +1585,11 @@ mod tests {
drop_min_num_blocks: 10,
};

let mut provider = MockEvmProvider::new();
provider
.expect_get_latest_block_hash_and_number()
.returning(|| Ok((B256::ZERO, 0)));

let mut simulator = MockSimulator::new();
let mut prechecker = MockPrechecker::new();

Expand Down Expand Up @@ -1585,7 +1620,7 @@ mod tests {
});

for op in ops {
prechecker.expect_check().returning(move |_| {
prechecker.expect_check().returning(move |_, _| {
if let Some(error) = &op.precheck_error {
Err(PrecheckError::Violations(vec![error.clone()]))
} else {
Expand All @@ -1603,7 +1638,6 @@ mod tests {
} else {
Ok(SimulationResult {
account_is_staked: op.staked,
block_number: Some(0),
valid_time_range: op.valid_time_range,
entity_infos: EntityInfos {
sender: EntityInfo {
Expand All @@ -1623,6 +1657,7 @@ mod tests {
UoPool::new(
args,
event_sender,
provider,
prechecker,
simulator,
paymaster,
Expand All @@ -1636,6 +1671,7 @@ mod tests {
) -> (
UoPool<
UserOperation,
impl EvmProvider,
impl Prechecker<UO = UserOperation>,
impl Simulator<UO = UserOperation>,
impl EntryPoint,
Expand All @@ -1655,6 +1691,7 @@ mod tests {
) -> (
UoPool<
UserOperation,
impl EvmProvider,
impl Prechecker<UO = UserOperation>,
impl Simulator<UO = UserOperation>,
impl EntryPoint,
Expand Down
5 changes: 3 additions & 2 deletions crates/pool/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ where
.context("entry point v0.6 not supplied")?;

if unsafe_mode {
let simulator = UnsafeSimulator::new(self.provider.clone(), ep.clone());
let simulator = UnsafeSimulator::new(ep.clone());
Self::create_mempool(
task_spawner,
chain_spec,
Expand Down Expand Up @@ -251,7 +251,7 @@ where
.context("entry point v0.7 not supplied")?;

if unsafe_mode {
let simulator = UnsafeSimulator::new(self.provider.clone(), ep.clone());
let simulator = UnsafeSimulator::new(ep.clone());
Self::create_mempool(
task_spawner,
chain_spec,
Expand Down Expand Up @@ -340,6 +340,7 @@ where
let uo_pool = UoPool::new(
pool_config.clone(),
event_sender,
provider,
prechecker,
simulator,
paymaster,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ use alloy_primitives::{Address, Bytes};
use alloy_provider::Provider as AlloyProvider;
use alloy_sol_types::sol;
use alloy_transport::Transport;
use NodeInterface::NodeInterfaceInstance;

use crate::ProviderResult;
use super::DAGasOracle;
use crate::{BlockHashOrNumber, ProviderResult};

// From https://github.com/OffchainLabs/nitro-contracts/blob/fbbcef09c95f69decabaced3da683f987902f3e2/src/node-interface/NodeInterface.sol#L112
sol! {
Expand All @@ -37,19 +39,41 @@ sol! {
}
}

pub(crate) async fn estimate_da_gas<AP: AlloyProvider<T>, T: Transport + Clone>(
provider: AP,
oracle_address: Address,
to_address: Address,
data: Bytes,
) -> ProviderResult<u128> {
let inst = NodeInterface::NodeInterfaceInstance::new(oracle_address, provider);
pub(super) struct ArbitrumNitroDAGasOracle<AP, T> {
node_interface: NodeInterfaceInstance<T, AP>,
}

// assume contract creation
let ret = inst
.gasEstimateL1Component(to_address, true, data)
.call()
.await?;
impl<AP, T> ArbitrumNitroDAGasOracle<AP, T>
where
AP: AlloyProvider<T>,
T: Transport + Clone,
{
pub(crate) fn new(oracle_address: Address, provider: AP) -> Self {
Self {
node_interface: NodeInterfaceInstance::new(oracle_address, provider),
}
}
}

Ok(ret.gasEstimateForL1 as u128)
#[async_trait::async_trait]
impl<AP, T> DAGasOracle for ArbitrumNitroDAGasOracle<AP, T>
where
AP: AlloyProvider<T>,
T: Transport + Clone,
{
async fn estimate_da_gas(
&self,
to_address: Address,
data: Bytes,
block: BlockHashOrNumber,
_gas_price: u128,
) -> ProviderResult<u128> {
let ret = self
.node_interface
.gasEstimateL1Component(to_address, true, data)
.block(block.into())
.call()
.await?;
Ok(ret.gasEstimateForL1 as u128)
}
}
Loading
Loading