feat(pool): handle UREP-020 and track reputation of unstaked entities (…

…#522)

bin/rundler/src/cli/pool.rs

@@ -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",
         default_value = "4"
     )]
-    pub max_userops_per_sender: usize,
+    pub same_sender_mempool_count: usize,
 
     #[arg(
         long = "pool.min_replacement_fee_increase_percentage",
@@ -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"
     )]
     pub throttled_entity_live_blocks: u64,
 }
@@ -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,

crates/pool/proto/op_pool/op_pool.proto

@@ -409,7 +409,7 @@ message PaymasterBalanceTooLow {
 
 message MaxOperationsReachedError {
   uint64 num_ops = 1;
-  bytes sender_address = 2;
+  bytes entity_address = 2;
 }
 
 message EntityThrottledError {

crates/pool/src/mempool/error.rs

@@ -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),
     /// Multiple roles violation
     /// Spec rule: STO-040

crates/pool/src/mempool/mod.rs

@@ -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;
 
@@ -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,
@@ -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.
@@ -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.
@@ -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]
@@ -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 {

crates/pool/src/mempool/pool.rs

@@ -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,
@@ -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,
@@ -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
@@ -613,6 +601,8 @@ impl PoolMetrics {
 
 #[cfg(test)]
 mod tests {
+    use rundler_sim::{EntityInfo, EntityInfos};
+
     use super::*;
 
     #[test]
@@ -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);
@@ -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);
@@ -827,20 +825,6 @@ mod tests {
         assert!(pool.best.is_empty());
     }
 
-    #[test]
-    fn too_many_ops() {
-        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());
@@ -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![];
@@ -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());
     }
@@ -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);
@@ -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,
@@ -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()
         }
     }

crates/pool/src/mempool/reputation.rs

@@ -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)
         };
 
         // return ops allowed, as defined by UREP-020