From d4c23aafb3a11cd6836afbf1633f237fab0028a3 Mon Sep 17 00:00:00 2001 From: Shunsuke Watanabe Date: Tue, 5 Nov 2024 21:18:48 +0000 Subject: [PATCH] fix: EREP-015 rule Paymaster opsSeen decrement --- Cargo.lock | 1 + Cargo.toml | 1 + crates/builder/Cargo.toml | 1 + crates/builder/src/bundle_proposer.rs | 50 +++++++++++++++++++++++-- crates/pool/proto/op_pool/op_pool.proto | 1 + crates/pool/src/mempool/reputation.rs | 15 ++++++++ crates/pool/src/mempool/uo_pool.rs | 3 ++ crates/pool/src/server/remote/protos.rs | 1 + crates/types/src/entity.rs | 2 + crates/types/src/pool/error.rs | 2 +- 10 files changed, 72 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3ffb61fec..ac4f9237e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4582,6 +4582,7 @@ dependencies = [ "parse-display", "pin-project", "prost", + "regex", "reqwest", "rslock", "ruint", diff --git a/Cargo.toml b/Cargo.toml index e44885b73..1d7632ca3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,6 +77,7 @@ prost = "0.13.3" serde = "1.0.210" serde_json = "1.0.128" rand = "0.8.5" +regex = "1" reqwest = { version = "0.12.8", default-features = false, features = ["rustls-tls"] } thiserror = "1.0.64" tokio = { version = "1.39.3", default-features = false, features = ["rt", "sync", "time"] } diff --git a/crates/builder/Cargo.toml b/crates/builder/Cargo.toml index 1f0039f6c..b38dc9131 100644 --- a/crates/builder/Cargo.toml +++ b/crates/builder/Cargo.toml @@ -37,6 +37,7 @@ num-traits = "0.2.19" parse-display.workspace = true pin-project.workspace = true prost.workspace = true +regex.workspace = true reqwest = { workspace = true, default-features = false, features = ["json"] } rslock = "0.4.0" ruint = { version = "1.12.3", features = ["num-traits"] } diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index b43d33f96..6904c2301 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/crates/builder/src/bundle_proposer.rs @@ -27,6 +27,7 @@ use futures_util::TryFutureExt; use linked_hash_map::LinkedHashMap; #[cfg(test)] use mockall::automock; +use regex::Regex; use rundler_provider::{ BundleHandler, DAGasOracleSync, DAGasProvider, EntryPoint, EvmProvider, HandleOpsOut, ProvidersWithEntryPointT, SignatureAggregator, @@ -40,8 +41,8 @@ use rundler_types::{ da::DAGasBlockData, pool::{Pool, PoolOperation, SimulationViolation}, Entity, EntityInfo, EntityInfos, EntityType, EntityUpdate, EntityUpdateType, GasFees, - Timestamp, UserOperation, UserOperationVariant, UserOpsPerAggregator, BUNDLE_BYTE_OVERHEAD, - TIME_RANGE_BUFFER, USER_OP_OFFSET_WORD_SIZE, + Timestamp, UserOperation, UserOperationVariant, UserOpsPerAggregator, ValidationRevert, + BUNDLE_BYTE_OVERHEAD, TIME_RANGE_BUFFER, USER_OP_OFFSET_WORD_SIZE, }; use rundler_utils::{emit::WithEntryPoint, math}; use tokio::{sync::broadcast, try_join}; @@ -1371,8 +1372,26 @@ impl ProposalContext { violations: Vec, entity_infos: EntityInfos, ) { + // If a staked factory or sender is present, we attribute errors to them directly. + + // In accordance with [EREP-015]/[EREP-020]/[EREP-030], responsibility for failures lies with the staked factory or account. + // Paymasters should not be held accountable, so the paymaster's `opsSeen` count should be decremented accordingly. + let is_factory_staked = entity_infos.factory.map_or(false, |f| f.is_staked); + let is_sender_staked = entity_infos.sender.is_staked; + if is_factory_staked || is_sender_staked { + if let Some(paymaster) = entity_infos.paymaster { + self.entity_updates.insert( + paymaster.address(), + EntityUpdate { + entity: paymaster.entity, + update_type: EntityUpdateType::PaymasterAmendment, + }, + ); + } + } + // [EREP-020] When there is a staked factory any error in validation is attributed to it. - if entity_infos.factory.map_or(false, |f| f.is_staked) { + if is_factory_staked { let factory = entity_infos.factory.unwrap(); self.entity_updates.insert( factory.address(), @@ -1385,7 +1404,7 @@ impl ProposalContext { } // [EREP-030] When there is a staked sender (without a staked factory) any error in validation is attributed to it. - if entity_infos.sender.is_staked { + if is_sender_staked { self.entity_updates.insert( entity_infos.sender.address(), EntityUpdate { @@ -1398,6 +1417,7 @@ impl ProposalContext { // If not a staked factory or sender, attribute errors to each entity directly. // For a given op, there can only be a single update per entity so we don't double count the [UREP-030] throttle penalty. + let mut paymaster_ammendment_required = false; for violation in violations { match violation { SimulationViolation::UsedForbiddenOpcode(entity, _, _) => { @@ -1450,12 +1470,34 @@ impl ProposalContext { ); } } + SimulationViolation::ValidationRevert(ValidationRevert::Operation { + entry_point_reason, + .. + }) => { + // [EREP-015] If user operations error derived from factory or account, paymaster opsSeen amendment is required. + let re = Regex::new(r"^AA[21]").unwrap(); + if re.is_match(&entry_point_reason) { + paymaster_ammendment_required = true; + } + } SimulationViolation::OutOfGas(entity) => { self.add_entity_update(entity, entity_infos) } _ => continue, } } + + if paymaster_ammendment_required { + if let Some(paymaster) = entity_infos.paymaster { + self.entity_updates.insert( + paymaster.address(), + EntityUpdate { + entity: paymaster.entity, + update_type: EntityUpdateType::PaymasterAmendment, + }, + ); + }; + } } // Add an entity update for the entity diff --git a/crates/pool/proto/op_pool/op_pool.proto b/crates/pool/proto/op_pool/op_pool.proto index 3c07ef621..d30c1431c 100644 --- a/crates/pool/proto/op_pool/op_pool.proto +++ b/crates/pool/proto/op_pool/op_pool.proto @@ -111,6 +111,7 @@ enum EntityUpdateType { ENTITY_UPDATE_TYPE_UNSPECIFIED = 0; ENTITY_UPDATE_TYPE_UNSTAKED_INVALIDATION = 1; ENTITY_UPDATE_TYPE_STAKED_INVALIDATION = 2; + ENTITY_UPDATE_TYPE_PAYMASTER_AMENDMENT = 3; } // A tuple consisting of an entity and what kind of update to perform on it diff --git a/crates/pool/src/mempool/reputation.rs b/crates/pool/src/mempool/reputation.rs index ea1a8b3fe..f8777902b 100644 --- a/crates/pool/src/mempool/reputation.rs +++ b/crates/pool/src/mempool/reputation.rs @@ -120,6 +120,10 @@ impl AddressReputation { self.state.write().add_seen(address); } + pub(crate) fn remove_seen(&self, address: Address) { + self.state.write().remove_seen(address); + } + pub(crate) fn handle_urep_030_penalty(&self, address: Address) { self.state.write().handle_urep_030_penalty(address); } @@ -218,6 +222,11 @@ impl AddressReputationInner { count.ops_seen += 1; } + fn remove_seen(&mut self, address: Address) { + let count = self.counts.entry(address).or_default(); + count.ops_seen = count.ops_seen.saturating_sub(1); + } + fn handle_urep_030_penalty(&mut self, address: Address) { let count = self.counts.entry(address).or_default(); count.ops_seen += self.params.bundle_invalidation_ops_seen_unstaked_penalty; @@ -315,6 +324,12 @@ mod tests { let counts = reputation.counts.get(&addr).unwrap(); assert_eq!(counts.ops_seen, 1000); assert_eq!(counts.ops_included, 1000); + + reputation.remove_seen(addr); + reputation.remove_included(addr); + let counts = reputation.counts.get(&addr).unwrap(); + assert_eq!(counts.ops_seen, 999); + assert_eq!(counts.ops_included, 999); } #[test] diff --git a/crates/pool/src/mempool/uo_pool.rs b/crates/pool/src/mempool/uo_pool.rs index 2af5ef6a5..792a25adb 100644 --- a/crates/pool/src/mempool/uo_pool.rs +++ b/crates/pool/src/mempool/uo_pool.rs @@ -732,6 +732,9 @@ where EntityUpdateType::StakedInvalidation => { self.reputation.handle_srep_050_penalty(entity.address); } + EntityUpdateType::PaymasterAmendment => { + self.reputation.remove_seen(entity.address); + } } if self.reputation.status(entity.address) == ReputationStatus::Banned { diff --git a/crates/pool/src/server/remote/protos.rs b/crates/pool/src/server/remote/protos.rs index 7806b18f0..cddd602e1 100644 --- a/crates/pool/src/server/remote/protos.rs +++ b/crates/pool/src/server/remote/protos.rs @@ -264,6 +264,7 @@ impl From for EntityUpdateType { match update_type { RundlerEntityUpdateType::UnstakedInvalidation => EntityUpdateType::UnstakedInvalidation, RundlerEntityUpdateType::StakedInvalidation => EntityUpdateType::StakedInvalidation, + RundlerEntityUpdateType::PaymasterAmendment => EntityUpdateType::PaymasterAmendment, } } } diff --git a/crates/types/src/entity.rs b/crates/types/src/entity.rs index 9414925c3..6beb10e9d 100644 --- a/crates/types/src/entity.rs +++ b/crates/types/src/entity.rs @@ -134,6 +134,8 @@ pub enum EntityUpdateType { UnstakedInvalidation, /// SREP-050 StakedInvalidation, + /// EREP-015 + PaymasterAmendment, } impl TryFrom for EntityUpdateType { diff --git a/crates/types/src/pool/error.rs b/crates/types/src/pool/error.rs index dad9401ff..cdef5fcb4 100644 --- a/crates/types/src/pool/error.rs +++ b/crates/types/src/pool/error.rs @@ -213,7 +213,7 @@ pub enum SimulationViolation { /// Simulation reverted with an unintended reason #[display("reverted while simulating {0} validation")] UnintendedRevert(EntityType, Option
), - /// Validation revert (only used for unsafe sim) + /// Validation revert (used for v0_7 sim and unsafe sim) #[display("validation revert: {0}")] ValidationRevert(ValidationRevert), /// Simulation did not revert, a revert is always expected