Skip to content

Commit

Permalink
fix(pool): fix pool candidates metrics to align with builder (#943)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dancoombs
dancoombs authored Dec 19, 2024
1 parent 7aee108 commit 4803826
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 82 deletions.
58 changes: 20 additions & 38 deletions crates/pool/src/mempool/pool.rs
View file
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 @@ -207,7 +208,6 @@ where
));

let hash = self.add_operation_internal(pool_op)?;
self.update_metrics();
Ok(hash)
}

Expand All @@ -233,16 +233,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 +264,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 +296,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 All @@ -324,6 +320,7 @@ where
self.metrics.num_candidates.set(num_candidates as f64);
self.prev_block_number = block_number;
self.prev_sys_block_time = sys_block_time;
self.update_metrics();
}

pub(crate) fn address_count(&self, address: &Address) -> usize {
Expand All @@ -343,9 +340,7 @@ where
}

pub(crate) fn remove_operation_by_hash(&mut self, hash: B256) -> Option<Arc<PoolOperation>> {
let ret = self.remove_operation_internal(hash, None);
self.update_metrics();
ret
self.remove_operation_internal(hash, None)
}

// STO-040
Expand Down Expand Up @@ -428,10 +423,7 @@ where
.uo()
.hash(mined_op.entry_point, self.config.chain_spec.id);

let ret = self.remove_operation_internal(hash, Some(block_number));

self.update_metrics();
ret
self.remove_operation_internal(hash, Some(block_number))
}

pub(crate) fn unmine_operation(&mut self, mined_op: &MinedOp) -> Option<Arc<PoolOperation>> {
Expand All @@ -440,10 +432,9 @@ where
self.mined_hashes_with_block_numbers
.remove(&(block_number, hash));

if let Err(error) = self.put_back_unmined_operation(op.clone()) {
if let Err(error) = self.add_operation_internal(op.clone()) {
info!("Could not put back unmined operation: {error}");
};
self.update_metrics();
Some(op.po.clone())
}

Expand Down Expand Up @@ -479,7 +470,6 @@ where
for &hash in &to_remove {
self.remove_operation_internal(hash, None);
}
self.update_metrics();
to_remove
}

Expand All @@ -495,7 +485,6 @@ where
for &hash in &to_remove {
self.remove_operation_internal(hash, None);
}
self.update_metrics();
to_remove
}

Expand All @@ -510,7 +499,6 @@ where
}
self.mined_hashes_with_block_numbers.remove(&(bn, hash));
}
self.update_metrics();
}

pub(crate) fn clear(&mut self) {
Expand Down Expand Up @@ -546,10 +534,6 @@ where
Ok(removed)
}

fn put_back_unmined_operation(&mut self, op: Arc<OrderedPoolOperation>) -> MempoolResult<B256> {
self.add_operation_internal(op)
}

fn add_operation_internal(
&mut self,
pool_op: Arc<OrderedPoolOperation>,
Expand Down Expand Up @@ -592,6 +576,7 @@ where
Err(MempoolError::DiscardedOnInsert)?;
}

self.update_metrics();
Ok(hash)
}

Expand Down Expand Up @@ -619,6 +604,7 @@ where
}

self.pool_size -= op.mem_size();
self.update_metrics();
Some(op.po.clone())
}

Expand Down Expand Up @@ -1201,7 +1187,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 +1207,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 +1257,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 +1292,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 +1327,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 +1365,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
View file
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
View file
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
Loading

0 comments on commit 4803826

Please sign in to comment.