Skip to content

Commit

Permalink
Fix EREP-015 rule Paymaster opsSeen decrement
Browse files Browse the repository at this point in the history
  • Loading branch information
shunsukew committed Nov 5, 2024
1 parent 6c3691a commit 58d178b
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 3 deletions.
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.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ parse-display = "0.9.0"
pin-project = "1.0.12"
prost = "0.12.0"
serde = "1.0.160"
regex = "1"
serde_json = "1.0.64"
rand = "0.8.5"
reqwest = { version = "0.11.18", default-features = false, features = ["rustls-tls"] }
Expand Down
1 change: 1 addition & 0 deletions crates/builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ metrics.workspace = true
pin-project.workspace = true
prost.workspace = true
parse-display.workspace = true
regex.workspace = true
reqwest.workspace = true
rslock = "0.3.0"
rusoto_core = { version = "0.48.0", default-features = false, features = ["rustls"] }
Expand Down
45 changes: 42 additions & 3 deletions crates/builder/src/bundle_proposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::{
sync::Arc,
};

use regex::Regex;
use anyhow::Context;
use async_trait::async_trait;
use ethers::types::{Address, BlockId, Bytes, H256, U256};
Expand All @@ -39,7 +40,7 @@ use rundler_types::{
chain::ChainSpec,
pool::{Pool, PoolOperation, SimulationViolation},
Entity, EntityInfo, EntityInfos, EntityType, EntityUpdate, EntityUpdateType, GasFees,
Timestamp, UserOperation, UserOperationVariant, UserOpsPerAggregator, BUNDLE_BYTE_OVERHEAD,
Timestamp, UserOperation, UserOperationVariant, UserOpsPerAggregator, ValidationRevert, BUNDLE_BYTE_OVERHEAD,
TIME_RANGE_BUFFER, USER_OP_OFFSET_WORD_SIZE,
};
use rundler_utils::{emit::WithEntryPoint, math};
Expand Down Expand Up @@ -1261,8 +1262,26 @@ impl<UO: UserOperation> ProposalContext<UO> {
violations: Vec<SimulationViolation>,
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 {
entity_infos.paymaster.map(|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(),
Expand All @@ -1275,7 +1294,7 @@ impl<UO: UserOperation> ProposalContext<UO> {
}

// [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 {
Expand All @@ -1288,6 +1307,7 @@ impl<UO: UserOperation> ProposalContext<UO> {

// 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, _, _) => {
Expand Down Expand Up @@ -1340,12 +1360,31 @@ impl<UO: UserOperation> ProposalContext<UO> {
);
}
}
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 {
entity_infos.paymaster.map(|paymaster| {
self.entity_updates.insert(
paymaster.address(),
EntityUpdate {
entity: paymaster.entity,
update_type: EntityUpdateType::PaymasterAmendment,
},
);
});
}
}

// Add an entity update for the entity
Expand Down
1 change: 1 addition & 0 deletions crates/pool/proto/op_pool/op_pool.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions crates/pool/src/mempool/reputation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand Down
3 changes: 3 additions & 0 deletions crates/pool/src/mempool/uo_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,9 @@ where
}
EntityUpdateType::StakedInvalidation => {
self.reputation.handle_srep_050_penalty(entity.address);
},
EntityUpdateType::PaymasterAmendment => {
self.reputation.remove_seen(entity.address);
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/pool/src/server/remote/protos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl From<RundlerEntityUpdateType> for EntityUpdateType {
match update_type {
RundlerEntityUpdateType::UnstakedInvalidation => EntityUpdateType::UnstakedInvalidation,
RundlerEntityUpdateType::StakedInvalidation => EntityUpdateType::StakedInvalidation,
RundlerEntityUpdateType::PaymasterAmendment => EntityUpdateType::PaymasterAmendment,
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/types/src/entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ pub enum EntityUpdateType {
UnstakedInvalidation,
/// SREP-050
StakedInvalidation,
/// EREP-015
PaymasterAmendment,
}

impl TryFrom<i32> for EntityUpdateType {
Expand Down
3 changes: 3 additions & 0 deletions rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[toolchain]
channel = "1.79"
components = ["rustfmt", "clippy"]

0 comments on commit 58d178b

Please sign in to comment.