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.

Sign up for GitHub

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

fix(pool): fix pool candidates metrics to align with builder #943

Merged
merged 1 commit into from
Dec 19, 2024
Merged

fix(pool): fix pool candidates metrics to align with builder #943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 20 additions & 38 deletions crates/pool/src/mempool/pool.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading