Skip to content

Commit

Permalink
fix(pool): fix pool candidates metrics to align with builder
Browse files Browse the repository at this point in the history
  • Loading branch information
dancoombs committed Dec 18, 2024
1 parent 7aee108 commit 2a45191
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 65 deletions.
35 changes: 14 additions & 21 deletions crates/pool/src/mempool/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// If not, see https://www.gnu.org/licenses/.

use std::{
cmp::{self, Ordering},
cmp::Ordering,
collections::{hash_map::Entry, BTreeSet, HashMap, HashSet},
sync::Arc,
time::{Duration, SystemTime, UNIX_EPOCH},
Expand All @@ -24,11 +24,12 @@ use metrics::{Gauge, Histogram};
use metrics_derive::Metrics;
use parking_lot::RwLock;
use rundler_provider::DAGasOracleSync;
use rundler_sim::FeeUpdate;
use rundler_types::{
chain::ChainSpec,
da::DAGasBlockData,
pool::{MempoolError, PoolOperation},
Entity, EntityType, GasFees, Timestamp, UserOperation, UserOperationId, UserOperationVariant,
Entity, EntityType, Timestamp, UserOperation, UserOperationId, UserOperationVariant,
};
use rundler_utils::{emit::WithEntryPoint, math};
use tokio::sync::broadcast;
Expand Down Expand Up @@ -233,16 +234,14 @@ where
block_number: u64,
block_timestamp: Timestamp,
block_da_data: Option<&DAGasBlockData>,
candidate_gas_fees: GasFees,
base_fee: u128,
gas_fees: FeeUpdate,
) {
let sys_block_time = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("time should be after epoch");

let block_delta_time = sys_block_time.saturating_sub(self.prev_sys_block_time);
let block_delta_height = block_number.saturating_sub(self.prev_block_number);
let candidate_gas_price = base_fee + candidate_gas_fees.max_priority_fee_per_gas;
let mut expired = Vec::new();
let mut num_candidates = 0;
let mut events = vec![];
Expand All @@ -266,7 +265,7 @@ where
let required_da_gas = da_gas_oracle.calc_da_gas_sync(
&op.po.da_gas_data,
block_da_data,
op.uo().gas_price(base_fee),
op.uo().gas_price(gas_fees.base_fee),
);

let required_pvg = op.uo().required_pre_verification_gas(
Expand Down Expand Up @@ -298,11 +297,9 @@ where
}
}

let uo_gas_price = cmp::min(
op.uo().max_fee_per_gas(),
op.uo().max_priority_fee_per_gas() + base_fee,
);
if candidate_gas_price > uo_gas_price {
if op.uo().max_fee_per_gas() < gas_fees.uo_fees.max_fee_per_gas
|| op.uo().max_priority_fee_per_gas() < gas_fees.uo_fees.max_priority_fee_per_gas
{
// don't mark as ineligible, but also not a candidate
continue;
}
Expand Down Expand Up @@ -1201,7 +1198,7 @@ mod tests {
po1.valid_time_range.valid_until = Timestamp::from(1);
let hash = pool.add_operation(po1.clone(), 0).unwrap();

pool.do_maintenance(0, Timestamp::from(2), None, GasFees::default(), 0);
pool.do_maintenance(0, Timestamp::from(2), None, FeeUpdate::default());
assert_eq!(None, pool.get_operation_by_hash(hash));
}

Expand All @@ -1221,7 +1218,7 @@ mod tests {
po3.valid_time_range.valid_until = 9.into();
let hash3 = pool.add_operation(po3.clone(), 0).unwrap();

pool.do_maintenance(0, Timestamp::from(10), None, GasFees::default(), 0);
pool.do_maintenance(0, Timestamp::from(10), None, FeeUpdate::default());

assert_eq!(None, pool.get_operation_by_hash(hash1));
assert!(pool.get_operation_by_hash(hash2).is_some());
Expand Down Expand Up @@ -1271,8 +1268,7 @@ mod tests {
0,
0.into(),
Some(&DAGasBlockData::default()),
GasFees::default(),
0,
FeeUpdate::default(),
);

assert_eq!(pool.best_operations().collect::<Vec<_>>().len(), 1); // UO is now eligible
Expand Down Expand Up @@ -1307,8 +1303,7 @@ mod tests {
0,
0.into(),
Some(&DAGasBlockData::default()),
GasFees::default(),
0,
FeeUpdate::default(),
);

assert_eq!(pool.best_operations().collect::<Vec<_>>().len(), 0);
Expand Down Expand Up @@ -1343,8 +1338,7 @@ mod tests {
0,
0.into(),
Some(&DAGasBlockData::default()),
GasFees::default(),
0,
FeeUpdate::default(),
);

assert_eq!(pool.best_operations().collect::<Vec<_>>().len(), 1);
Expand Down Expand Up @@ -1382,8 +1376,7 @@ mod tests {
0,
0.into(),
Some(&DAGasBlockData::default()),
GasFees::default(),
base_fee,
FeeUpdate::default(),
);

assert_eq!(pool.best_operations().collect::<Vec<_>>().len(), 0);
Expand Down
39 changes: 14 additions & 25 deletions crates/pool/src/mempool/uo_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ use parking_lot::RwLock;
use rundler_provider::{
DAGasOracleSync, EvmProvider, ProvidersWithEntryPointT, SimulationProvider, StateOverride,
};
use rundler_sim::{Prechecker, Simulator};
use rundler_sim::{FeeUpdate, Prechecker, Simulator};
use rundler_types::{
pool::{
MempoolError, PaymasterMetadata, PoolOperation, Reputation, ReputationStatus, StakeStatus,
},
Entity, EntityUpdate, EntityUpdateType, EntryPointVersion, GasFees, UserOperation,
UserOperationId, UserOperationVariant,
Entity, EntityUpdate, EntityUpdateType, EntryPointVersion, UserOperation, UserOperationId,
UserOperationVariant,
};
use rundler_utils::emit::WithEntryPoint;
use tokio::sync::broadcast;
Expand Down Expand Up @@ -67,8 +67,7 @@ struct UoPoolState<D> {
throttled_ops: HashSet<B256>,
block_number: u64,
block_hash: B256,
gas_fees: GasFees,
base_fee: u128,
gas_fees: FeeUpdate,
}

impl<UP, EP> UoPool<UP, EP>
Expand All @@ -95,8 +94,7 @@ where
throttled_ops: HashSet::new(),
block_number: 0,
block_hash: B256::ZERO,
gas_fees: GasFees::default(),
base_fee: 0,
gas_fees: FeeUpdate::default(),
}),
reputation,
paymaster,
Expand Down Expand Up @@ -347,23 +345,23 @@ where

// update required bundle fees and update metrics
match self.pool_providers.prechecker().update_fees().await {
Ok((bundle_fees, base_fee)) => {
let max_fee = match format_units(bundle_fees.max_fee_per_gas, "gwei") {
Ok(fees) => {
let max_fee = match format_units(fees.bundle_fees.max_fee_per_gas, "gwei") {
Ok(s) => s.parse::<f64>().unwrap_or_default(),
Err(_) => 0.0,
};
self.metrics.current_max_fee_gwei.set(max_fee);

let max_priority_fee =
match format_units(bundle_fees.max_priority_fee_per_gas, "gwei") {
match format_units(fees.bundle_fees.max_priority_fee_per_gas, "gwei") {
Ok(s) => s.parse::<f64>().unwrap_or_default(),
Err(_) => 0.0,
};
self.metrics
.current_max_priority_fee_gwei
.set(max_priority_fee);

let base_fee_f64 = match format_units(base_fee, "gwei") {
let base_fee_f64 = match format_units(fees.base_fee, "gwei") {
Ok(s) => s.parse::<f64>().unwrap_or_default(),
Err(_) => 0.0,
};
Expand All @@ -374,8 +372,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;
state.gas_fees = fees;
}
}
Err(e) => {
Expand Down Expand Up @@ -441,13 +438,11 @@ where

// pool maintenance
let gas_fees = state.gas_fees;
let base_fee = state.base_fee;
state.pool.do_maintenance(
update.latest_block_number,
update.latest_block_timestamp,
da_block_data.as_ref(),
gas_fees,
base_fee,
);
}
let maintenance_time = start.elapsed();
Expand Down Expand Up @@ -935,7 +930,7 @@ mod tests {
da::DAGasUOData,
pool::{PrecheckViolation, SimulationViolation},
v0_6::UserOperation,
EntityInfo, EntityInfos, EntityType, EntryPointVersion, GasFees,
EntityInfo, EntityInfos, EntityType, EntryPointVersion,
UserOperation as UserOperationTrait, ValidTimeRange,
};

Expand Down Expand Up @@ -1898,15 +1893,9 @@ mod tests {
args.allowlist.clone().unwrap_or_default(),
));

prechecker.expect_update_fees().returning(|| {
Ok((
GasFees {
max_fee_per_gas: 0,
max_priority_fee_per_gas: 0,
},
0,
))
});
prechecker
.expect_update_fees()
.returning(|| Ok(FeeUpdate::default()));

for op in ops {
prechecker.expect_check().returning(move |_, _| {
Expand Down
4 changes: 2 additions & 2 deletions crates/sim/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ mod precheck;
#[cfg(feature = "test-utils")]
pub use precheck::MockPrechecker;
pub use precheck::{
PrecheckError, PrecheckReturn, Prechecker, PrecheckerImpl, Settings as PrecheckSettings,
MIN_CALL_GAS_LIMIT,
FeeUpdate, PrecheckError, PrecheckReturn, Prechecker, PrecheckerImpl,
Settings as PrecheckSettings, MIN_CALL_GAS_LIMIT,
};

/// Simulation and violation checking
Expand Down
42 changes: 25 additions & 17 deletions crates/sim/src/precheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ pub struct PrecheckReturn {
pub required_pre_verification_gas: u128,
}

/// Updated fees from the fee estimator
#[derive(Copy, Clone, Debug, Default)]
pub struct FeeUpdate {
/// Bundle fees
pub bundle_fees: GasFees,
/// User operation fees
pub uo_fees: GasFees,
/// Current base fee
pub base_fee: u128,
}

/// Trait for checking if a user operation is valid before simulation
/// according to the spec rules.
#[cfg_attr(feature = "test-utils", automock(type UO = rundler_types::v0_6::UserOperation;))]
Expand All @@ -61,7 +72,7 @@ pub trait Prechecker: Send + Sync {
/// Update and return the bundle fees.
///
/// This MUST be called at block boundaries before checking any operations.
async fn update_fees(&self) -> anyhow::Result<(GasFees, u128)>;
async fn update_fees(&self) -> anyhow::Result<FeeUpdate>;
}

/// Precheck error
Expand Down Expand Up @@ -145,13 +156,7 @@ struct AsyncData {

#[derive(Copy, Clone, Debug)]
struct AsyncDataCache {
fees: Option<FeeCache>,
}

#[derive(Copy, Clone, Debug)]
struct FeeCache {
bundle_fees: GasFees,
base_fee: u128,
fees: Option<FeeUpdate>,
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -183,16 +188,19 @@ where
})
}

async fn update_fees(&self) -> anyhow::Result<(GasFees, u128)> {
async fn update_fees(&self) -> anyhow::Result<FeeUpdate> {
let (bundle_fees, base_fee) = self.fee_estimator.required_bundle_fees(None).await?;

let mut cache = self.cache.write().unwrap();
cache.fees = Some(FeeCache {
let uo_fees = self.fee_estimator.required_op_fees(bundle_fees);
let fee_update = FeeUpdate {
bundle_fees,
uo_fees,
base_fee,
});
};

let mut cache = self.cache.write().unwrap();
cache.fees = Some(fee_update);

Ok((bundle_fees, base_fee))
Ok(fee_update)
}
}

Expand Down Expand Up @@ -365,7 +373,7 @@ where
op: &UO,
block: BlockHashOrNumber,
) -> anyhow::Result<AsyncData> {
let (_, base_fee) = self.get_fees().await?;
let FeeUpdate { base_fee, .. } = self.get_fees().await?;

let (
factory_exists,
Expand Down Expand Up @@ -431,9 +439,9 @@ where
.context("precheck should get sender balance")
}

async fn get_fees(&self) -> anyhow::Result<(GasFees, u128)> {
async fn get_fees(&self) -> anyhow::Result<FeeUpdate> {
if let Some(fees) = self.cache.read().unwrap().fees {
return Ok((fees.bundle_fees, fees.base_fee));
return Ok(fees);
}
self.update_fees().await
}
Expand Down

0 comments on commit 2a45191

Please sign in to comment.