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(pool): handle UREP-020 and track reputation of unstaked entities #522

Merged
merged 6 commits into from
Jan 29, 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions bin/rundler/src/cli/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ pub struct PoolArgs {
pub max_size_in_bytes: usize,

#[arg(
long = "pool.max_userops_per_sender",
name = "pool.max_userops_per_sender",
env = "POOL_MAX_USEROPS_PER_SENDER",
long = "pool.same_sender_mempool_count",
name = "pool.same_sender_mempool_count",
env = "SAME_SENDER_MEMPOOL_COUNT",
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
default_value = "4"
)]
pub max_userops_per_sender: usize,
pub same_sender_mempool_count: usize,

#[arg(
long = "pool.min_replacement_fee_increase_percentage",
Expand Down Expand Up @@ -114,7 +114,7 @@ pub struct PoolArgs {
long = "pool.throttled_entity_live_blocks",
name = "pool.throttled_entity_live_blocks",
env = "POOL_THROTTLED_ENTITY_LIVE_BLOCKS",
default_value = "4"
default_value = "10"
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
)]
pub throttled_entity_live_blocks: u64,
}
Expand Down Expand Up @@ -156,7 +156,7 @@ impl PoolArgs {
chain_id: common.chain_id,
// Currently use the same shard count as the number of builders
num_shards: common.num_builders,
max_userops_per_sender: self.max_userops_per_sender,
same_sender_mempool_count: self.same_sender_mempool_count,
min_replacement_fee_increase_percentage: self
.min_replacement_fee_increase_percentage,
max_size_of_pool_bytes: self.max_size_in_bytes,
Expand Down
2 changes: 1 addition & 1 deletion crates/pool/proto/op_pool/op_pool.proto
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ message PaymasterBalanceTooLow {

message MaxOperationsReachedError {
uint64 num_ops = 1;
bytes sender_address = 2;
bytes entity_address = 2;
}

message EntityThrottledError {
Expand Down
4 changes: 2 additions & 2 deletions crates/pool/src/mempool/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ pub enum MempoolError {
/// and the replacement operation has lower gas price.
#[error("Replacement operation underpriced. Existing priority fee: {0}. Existing fee: {1}")]
ReplacementUnderpriced(U256, U256),
/// Max operations reached for this sender
#[error("Max operations ({0}) reached for sender {1}")]
/// Max operations reached for unstaked sender [UREP-010] or unstaked non-sender entity [UREP-020]
#[error("Max operations ({0}) reached for entity {1}")]
MaxOperationsReached(usize, Address),
alex-miao marked this conversation as resolved.
Show resolved Hide resolved
/// Multiple roles violation
/// Spec rule: STO-040
Expand Down
79 changes: 48 additions & 31 deletions crates/pool/src/mempool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use ethers::types::{Address, H256, U256};
use mockall::automock;
use rundler_sim::{EntityInfos, MempoolConfig, PrecheckSettings, SimulationSettings};
use rundler_types::{Entity, EntityType, EntityUpdate, UserOperation, ValidTimeRange};
use strum::IntoEnumIterator;
use tonic::async_trait;
pub(crate) use uo_pool::UoPool;

Expand Down Expand Up @@ -119,7 +118,7 @@ pub struct PoolConfig {
/// Chain ID this pool targets
pub chain_id: u64,
/// The maximum number of operations an unstaked sender can have in the mempool
pub max_userops_per_sender: usize,
pub same_sender_mempool_count: usize,
/// The minimum fee bump required to replace an operation in the mempool
/// Applies to both priority fee and fee. Expressed as an integer percentage value
pub min_replacement_fee_increase_percentage: u64,
Expand Down Expand Up @@ -215,9 +214,11 @@ pub struct PaymasterMetadata {
impl PoolOperation {
/// Returns true if the operation contains the given entity.
pub fn contains_entity(&self, entity: &Entity) -> bool {
self.entity_address(entity.kind)
.map(|address| address == entity.address)
.unwrap_or(false)
if let Some(e) = self.entity_infos.get(entity.kind) {
e.address == entity.address
} else {
false
}
}

/// Returns true if the operation requires the given entity to stake.
Expand All @@ -239,20 +240,31 @@ impl PoolOperation {

/// Returns an iterator over all entities that are included in this operation.
pub fn entities(&'_ self) -> impl Iterator<Item = Entity> + '_ {
EntityType::iter().filter_map(|entity| {
self.entity_address(entity)
.map(|address| Entity::new(entity, address))
self.entity_infos
.entities()
.map(|(t, entity)| Entity::new(t, entity.address))
}

/// Returns an iterator over all entities that need stake in this operation. This can be a subset of entities that are staked in the operation.
pub fn entities_requiring_stake(&'_ self) -> impl Iterator<Item = Entity> + '_ {
self.entity_infos.entities().filter_map(|(t, entity)| {
if self.requires_stake(t) {
Entity::new(t, entity.address).into()
} else {
None
}
})
}

/// Returns an iterator over all staked entities that are included in this operation.
pub fn staked_entities(&'_ self) -> impl Iterator<Item = Entity> + '_ {
EntityType::iter()
.filter(|entity| self.requires_stake(*entity))
.filter_map(|entity| {
self.entity_address(entity)
.map(|address| Entity::new(entity, address))
})
/// Return all the unstaked entities that are used in this operation.
pub fn unstaked_entities(&'_ self) -> impl Iterator<Item = Entity> + '_ {
self.entity_infos.entities().filter_map(|(t, entity)| {
if entity.is_staked {
None
} else {
Entity::new(t, entity.address).into()
}
})
}

/// Compute the amount of heap memory the PoolOperation takes up.
Expand All @@ -261,19 +273,12 @@ impl PoolOperation {
+ self.uo.heap_size()
+ self.entities_needing_stake.len() * std::mem::size_of::<EntityType>()
}

fn entity_address(&self, entity: EntityType) -> Option<Address> {
match entity {
EntityType::Account => Some(self.uo.sender),
EntityType::Paymaster => self.uo.paymaster(),
EntityType::Factory => self.uo.factory(),
EntityType::Aggregator => self.aggregator,
}
}
}

#[cfg(test)]
mod tests {
use rundler_sim::EntityInfo;

use super::*;

#[test]
Expand All @@ -298,19 +303,31 @@ mod tests {
sim_block_number: 0,
entities_needing_stake: vec![EntityType::Account, EntityType::Aggregator],
account_is_staked: true,
entity_infos: EntityInfos::default(),
entity_infos: EntityInfos {
factory: Some(EntityInfo {
address: factory,
is_staked: false,
}),
sender: EntityInfo {
address: sender,
is_staked: false,
},
paymaster: Some(EntityInfo {
address: paymaster,
is_staked: false,
}),
aggregator: Some(EntityInfo {
address: aggregator,
is_staked: false,
}),
},
};

assert!(po.requires_stake(EntityType::Account));
assert!(!po.requires_stake(EntityType::Paymaster));
assert!(!po.requires_stake(EntityType::Factory));
assert!(po.requires_stake(EntityType::Aggregator));

assert_eq!(po.entity_address(EntityType::Account), Some(sender));
assert_eq!(po.entity_address(EntityType::Paymaster), Some(paymaster));
assert_eq!(po.entity_address(EntityType::Factory), Some(factory));
assert_eq!(po.entity_address(EntityType::Aggregator), Some(aggregator));

let entities = po.entities().collect::<Vec<_>>();
assert_eq!(entities.len(), 4);
for e in entities {
Expand Down
71 changes: 42 additions & 29 deletions crates/pool/src/mempool/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ use crate::chain::{DepositInfo, MinedOp};
pub(crate) struct PoolInnerConfig {
entry_point: Address,
chain_id: u64,
max_userops_per_sender: usize,
max_size_of_pool_bytes: usize,
min_replacement_fee_increase_percentage: u64,
throttled_entity_mempool_count: u64,
Expand All @@ -51,7 +50,6 @@ impl From<PoolConfig> for PoolInnerConfig {
Self {
entry_point: config.entry_point,
chain_id: config.chain_id,
max_userops_per_sender: config.max_userops_per_sender,
max_size_of_pool_bytes: config.max_size_of_pool_bytes,
min_replacement_fee_increase_percentage: config.min_replacement_fee_increase_percentage,
throttled_entity_mempool_count: config.throttled_entity_mempool_count,
Expand Down Expand Up @@ -437,16 +435,6 @@ impl PoolInner {
self.remove_operation_by_hash(hash);
}

// Check sender count in mempool. If sender has too many operations, must be staked
if self.address_count(&op.uo.sender) >= self.config.max_userops_per_sender
&& !op.account_is_staked
{
return Err(MempoolError::MaxOperationsReached(
self.config.max_userops_per_sender,
op.uo.sender,
));
}

// check or update paymaster balance
if let Some(paymaster_meta) = paymaster_meta {
self.paymaster_balances
Expand Down Expand Up @@ -613,6 +601,8 @@ impl PoolMetrics {

#[cfg(test)]
mod tests {
use rundler_sim::{EntityInfo, EntityInfos};

use super::*;

#[test]
Expand Down Expand Up @@ -796,6 +786,10 @@ mod tests {
];
for mut op in ops.into_iter() {
op.aggregator = Some(agg);
op.entity_infos.aggregator = Some(EntityInfo {
address: agg,
is_staked: false,
});
pool.add_operation(op.clone(), None).unwrap();
}
assert_eq!(pool.by_hash.len(), 3);
Expand All @@ -817,6 +811,10 @@ mod tests {
];
for mut op in ops.into_iter() {
op.uo.paymaster_and_data = paymaster.as_bytes().to_vec().into();
op.entity_infos.paymaster = Some(EntityInfo {
address: op.uo.paymaster().unwrap(),
is_staked: false,
});
pool.add_operation(op.clone(), None).unwrap();
}
assert_eq!(pool.by_hash.len(), 3);
Expand All @@ -827,20 +825,6 @@ mod tests {
assert!(pool.best.is_empty());
}

#[test]
fn too_many_ops() {
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
let args = conf();
let mut pool = PoolInner::new(args.clone());
let addr = Address::random();
for i in 0..args.max_userops_per_sender {
let op = create_op(addr, i, 1);
pool.add_operation(op, None).unwrap();
}

let op = create_op(addr, args.max_userops_per_sender, 1);
assert!(pool.add_operation(op, None).is_err());
}

#[test]
fn address_count() {
let mut pool = PoolInner::new(conf());
Expand All @@ -851,8 +835,20 @@ mod tests {

let mut op = create_op(sender, 0, 1);
op.uo.paymaster_and_data = paymaster.as_bytes().to_vec().into();
op.entity_infos.paymaster = Some(EntityInfo {
address: op.uo.paymaster().unwrap(),
is_staked: false,
});
op.uo.init_code = factory.as_bytes().to_vec().into();
op.entity_infos.factory = Some(EntityInfo {
address: op.uo.factory().unwrap(),
is_staked: false,
});
op.aggregator = Some(aggregator);
op.entity_infos.aggregator = Some(EntityInfo {
address: aggregator,
is_staked: false,
});

let count = 5;
let mut hashes = vec![];
Expand Down Expand Up @@ -901,11 +897,11 @@ mod tests {
pool.add_operation(op, None).unwrap();
}

let op = create_op(Address::random(), args.max_userops_per_sender, 1);
let op = create_op(Address::random(), 4, 1);
assert!(pool.add_operation(op, None).is_err());

// on equal gas, worst should remain because it came first
let op = create_op(Address::random(), args.max_userops_per_sender, 2);
let op = create_op(Address::random(), 4, 2);
let result = pool.add_operation(op, None);
assert!(result.is_ok(), "{:?}", result.err());
}
Expand Down Expand Up @@ -949,13 +945,21 @@ mod tests {
let mut po1 = create_op(sender, 0, 10);
po1.uo.max_priority_fee_per_gas = 10.into();
po1.uo.paymaster_and_data = paymaster1.as_bytes().to_vec().into();
po1.entity_infos.paymaster = Some(EntityInfo {
address: po1.uo.paymaster().unwrap(),
is_staked: false,
});
let _ = pool.add_operation(po1, None).unwrap();
assert_eq!(pool.address_count(&paymaster1), 1);

let paymaster2 = Address::random();
let mut po2 = create_op(sender, 0, 11);
po2.uo.max_priority_fee_per_gas = 11.into();
po2.uo.paymaster_and_data = paymaster2.as_bytes().to_vec().into();
po2.entity_infos.paymaster = Some(EntityInfo {
address: po2.uo.paymaster().unwrap(),
is_staked: false,
});
let _ = pool.add_operation(po2.clone(), None).unwrap();

assert_eq!(pool.address_count(&sender), 1);
Expand Down Expand Up @@ -1029,7 +1033,6 @@ mod tests {
PoolInnerConfig {
entry_point: Address::random(),
chain_id: 1,
max_userops_per_sender: 16,
min_replacement_fee_increase_percentage: 10,
max_size_of_pool_bytes: 20 * mem_size_of_ordered_pool_op(),
throttled_entity_mempool_count: 4,
Expand All @@ -1051,8 +1054,18 @@ mod tests {
sender,
nonce: nonce.into(),
max_fee_per_gas: max_fee_per_gas.into(),

..UserOperation::default()
},
entity_infos: EntityInfos {
factory: None,
sender: EntityInfo {
address: sender,
is_staked: false,
},
paymaster: None,
aggregator: None,
},
..PoolOperation::default()
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/pool/src/mempool/reputation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl AddressReputation {
// make sure we aren't dividing by 0
0
} else {
included * self.params.inclusion_rate_factor / seen + std::cmp::min(included, 10_000)
self.params.inclusion_rate_factor * included / seen + std::cmp::min(included, 10_000)
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
};

// return ops allowed, as defined by UREP-020
Expand Down
Loading
Loading