fix: EREP-015 rule Paymaster opsSeen decrement (#905)

Co-authored-by: Dan Coombs <[email protected]>

Author: shunsukew

crates/builder/src/bundle_proposer.rs

@@ -40,8 +40,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};
@@ -1280,6 +1280,7 @@ impl<UO: UserOperation> ProposalContext<UO> {
                 } else {
                     EntityUpdateType::UnstakedInvalidation
                 },
+                ..Default::default()
             },
         );
         ret
@@ -1371,33 +1372,48 @@ 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_staked_factory = entity_infos.factory.map_or(false, |f| f.is_staked);
+        let is_staked_sender = entity_infos.sender.is_staked;
+        if is_staked_factory || is_staked_sender {
+            if let Some(paymaster) = entity_infos.paymaster {
+                self.add_erep_015_paymaster_amendment(paymaster.address())
+            }
+        }
+
         // [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_staked_factory {
             let factory = entity_infos.factory.unwrap();
             self.entity_updates.insert(
                 factory.address(),
                 EntityUpdate {
                     entity: factory.entity,
                     update_type: EntityUpdateType::StakedInvalidation,
+                    value: None,
                 },
             );
             return;
         }
 
         // [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_staked_sender {
             self.entity_updates.insert(
                 entity_infos.sender.address(),
                 EntityUpdate {
                     entity: entity_infos.sender.entity,
                     update_type: EntityUpdateType::StakedInvalidation,
+                    value: None,
                 },
             );
             return;
         }
 
         // 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_amendment_required = false;
         for violation in violations {
             match violation {
                 SimulationViolation::UsedForbiddenOpcode(entity, _, _) => {
@@ -1450,19 +1466,34 @@ 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.
+                    paymaster_amendment_required |=
+                        matches!(&entry_point_reason[..3], "AA1" | "AA2");
+                }
                 SimulationViolation::OutOfGas(entity) => {
                     self.add_entity_update(entity, entity_infos)
                 }
                 _ => continue,
             }
         }
+
+        if paymaster_amendment_required {
+            if let Some(paymaster) = entity_infos.paymaster {
+                self.add_erep_015_paymaster_amendment(paymaster.address())
+            };
+        }
     }
 
     // Add an entity update for the entity
     fn add_entity_update(&mut self, entity: Entity, entity_infos: EntityInfos) {
         let entity_update = EntityUpdate {
             entity,
             update_type: ProposalContext::<UO>::get_entity_update_type(entity.kind, entity_infos),
+            value: None,
         };
         self.entity_updates.insert(entity.address, entity_update);
     }
@@ -1517,6 +1548,25 @@ impl<UO: UserOperation> ProposalContext<UO> {
             }
         }
     }
+
+    fn add_erep_015_paymaster_amendment(&mut self, address: Address) {
+        // Insert to entity_updates if entry is vacant, otherwise increment the value (precicely EntityUpdate.value)
+        self.entity_updates
+            .entry(address)
+            .and_modify(|e| {
+                if let Some(value) = &mut e.value {
+                    *value += 1;
+                }
+            })
+            .or_insert_with(|| EntityUpdate {
+                entity: Entity {
+                    kind: EntityType::Paymaster,
+                    address,
+                },
+                update_type: EntityUpdateType::PaymasterOpsSeenDecrement,
+                value: Some(1),
+            });
+    }
 }
 
 #[cfg(test)]
@@ -1991,10 +2041,12 @@ mod tests {
                 EntityUpdate {
                     entity: Entity::paymaster(address(1)),
                     update_type: EntityUpdateType::UnstakedInvalidation,
+                    ..Default::default()
                 },
                 EntityUpdate {
                     entity: Entity::factory(address(3)),
                     update_type: EntityUpdateType::UnstakedInvalidation,
+                    ..Default::default()
                 },
             ]
         );
@@ -2008,6 +2060,212 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn test_paymaster_amended_by_staked_factory_revert() {
+        let sender = address(1);
+        let staked_factory = address(2);
+        let paymaster = address(3);
+
+        let deposit = parse_units("1", "ether").unwrap().into();
+
+        let entity_infos = EntityInfos {
+            sender: EntityInfo::new(Entity::account(sender), false),
+            factory: Some(EntityInfo::new(Entity::factory(staked_factory), true)),
+            paymaster: Some(EntityInfo::new(Entity::paymaster(paymaster), false)),
+            aggregator: None,
+        };
+
+        // EREP-015: If a staked factory or sender is present, we attribute errors to them directly.
+        // Expect EntityUpdateType::PaymasterOpsSeenDecrement to be recorded.
+        let op = op_with_sender_factory_paymaster(sender, staked_factory, paymaster);
+        let bundle = mock_make_bundle(
+            vec![MockOp {
+                op: op.clone(),
+                simulation_result: Box::new(move || {
+                    Err(SimulationError {
+                        violation_error: ViolationError::Violations(vec![]),
+                        entity_infos: Some(entity_infos),
+                    })
+                }),
+            }],
+            vec![],
+            vec![],
+            vec![deposit],
+            0,
+            0,
+            false,
+            ExpectedStorage::default(),
+            false,
+        )
+        .await;
+
+        let mut actual_entity_updates = bundle.entity_updates;
+        let mut expected_entity_updates = vec![
+            EntityUpdate {
+                entity: Entity::factory(staked_factory),
+                update_type: EntityUpdateType::StakedInvalidation,
+                ..Default::default()
+            },
+            EntityUpdate {
+                entity: Entity::paymaster(paymaster),
+                update_type: EntityUpdateType::PaymasterOpsSeenDecrement,
+                value: Some(1),
+            },
+        ];
+
+        // we want to check that the entity updates are the same regardless of order
+        actual_entity_updates.sort_by(|a, b| a.entity.address.cmp(&b.entity.address));
+        expected_entity_updates.sort_by(|a, b| a.entity.address.cmp(&b.entity.address));
+
+        assert_eq!(actual_entity_updates, expected_entity_updates);
+    }
+
+    #[tokio::test]
+    async fn test_paymaster_amended_by_staked_sender_revert() {
+        let sender = address(1);
+        let paymaster = address(2);
+
+        let deposit = parse_units("1", "ether").unwrap().into();
+
+        let entity_infos = EntityInfos {
+            sender: EntityInfo::new(Entity::account(sender), true),
+            factory: None,
+            paymaster: Some(EntityInfo::new(Entity::paymaster(paymaster), false)),
+            aggregator: None,
+        };
+
+        // EREP-015: If not a staked factory or sender, attribute errors to each entity directly.
+        // Expect EntityUpdateType::PaymasterOpsSeenDecrement to be recorded.
+        let op = op_with_sender_paymaster(sender, paymaster);
+        let bundle = mock_make_bundle(
+            vec![MockOp {
+                op: op.clone(),
+                simulation_result: Box::new(move || {
+                    Err(SimulationError {
+                        violation_error: ViolationError::Violations(vec![]),
+                        entity_infos: Some(entity_infos),
+                    })
+                }),
+            }],
+            vec![],
+            vec![],
+            vec![deposit],
+            0,
+            0,
+            false,
+            ExpectedStorage::default(),
+            false,
+        )
+        .await;
+
+        let mut actual_entity_updates = bundle.entity_updates;
+        let mut expected_entity_updates = vec![
+            EntityUpdate {
+                entity: Entity::account(sender),
+                update_type: EntityUpdateType::StakedInvalidation,
+                ..Default::default()
+            },
+            EntityUpdate {
+                entity: Entity::paymaster(paymaster),
+                update_type: EntityUpdateType::PaymasterOpsSeenDecrement,
+                value: Some(1),
+            },
+        ];
+
+        // we want to check that the entity updates are the same regardless of order
+        actual_entity_updates.sort_by(|a, b| a.entity.address.cmp(&b.entity.address));
+        expected_entity_updates.sort_by(|a, b| a.entity.address.cmp(&b.entity.address));
+
+        assert_eq!(actual_entity_updates, expected_entity_updates);
+    }
+
+    #[tokio::test]
+    async fn test_paymaster_amended_by_factory_or_sender_revert() {
+        let sender_1 = address(1);
+        let sender_2 = address(2);
+        let factory = address(3);
+        let paymaster = address(4);
+
+        let deposit = parse_units("1", "ether").unwrap().into();
+
+        let entity_infos_1 = EntityInfos {
+            sender: EntityInfo::new(Entity::account(sender_1), false),
+            factory: Some(EntityInfo::new(Entity::factory(factory), false)),
+            paymaster: Some(EntityInfo::new(Entity::paymaster(paymaster), false)),
+            aggregator: None,
+        };
+
+        let entity_infos_2 = EntityInfos {
+            sender: EntityInfo::new(Entity::account(sender_2), false),
+            factory: Some(EntityInfo::new(Entity::factory(factory), false)),
+            paymaster: Some(EntityInfo::new(Entity::paymaster(paymaster), false)),
+            aggregator: None,
+        };
+
+        // EREP-015: If a staked factory or sender is present, we attribute errors to them directly.
+        // Expect EntityUpdateType::PaymasterOpsSeenDecrement to be recorded.
+        let op_1 = op_with_sender_factory_paymaster(sender_1, factory, paymaster);
+        let op_2 = op_with_sender_factory_paymaster(sender_2, factory, paymaster);
+        let bundle = mock_make_bundle(
+            vec![
+                MockOp {
+                    op: op_1.clone(),
+                    simulation_result: Box::new(move || {
+                        Err(SimulationError {
+                            violation_error: ViolationError::Violations(vec![
+                                SimulationViolation::ValidationRevert(
+                                    ValidationRevert::Operation {
+                                        entry_point_reason: "AA1x: factory related errors"
+                                            .to_string(),
+                                        inner_revert_reason: None,
+                                        inner_revert_data: Bytes::new(),
+                                    },
+                                ),
+                            ]),
+                            entity_infos: Some(entity_infos_1),
+                        })
+                    }),
+                },
+                MockOp {
+                    op: op_2.clone(),
+                    simulation_result: Box::new(move || {
+                        Err(SimulationError {
+                            violation_error: ViolationError::Violations(vec![
+                                SimulationViolation::ValidationRevert(
+                                    ValidationRevert::Operation {
+                                        entry_point_reason: "AA2x: sender related errors"
+                                            .to_string(),
+                                        inner_revert_reason: None,
+                                        inner_revert_data: Bytes::new(),
+                                    },
+                                ),
+                            ]),
+                            entity_infos: Some(entity_infos_2),
+                        })
+                    }),
+                },
+            ],
+            vec![],
+            vec![],
+            vec![deposit, deposit],
+            0,
+            0,
+            false,
+            ExpectedStorage::default(),
+            false,
+        )
+        .await;
+
+        let actual_entity_updates = bundle.entity_updates;
+        let expected_entity_updates = vec![EntityUpdate {
+            entity: Entity::paymaster(paymaster),
+            update_type: EntityUpdateType::PaymasterOpsSeenDecrement,
+            value: Some(2),
+        }];
+
+        assert_eq!(actual_entity_updates, expected_entity_updates);
+    }
+
     #[tokio::test]
     async fn test_bundle_gas_limit_simple() {
         // Limit is 10M
@@ -2596,6 +2854,20 @@ mod tests {
         }
     }
 
+    fn op_with_sender_factory_paymaster(
+        sender: Address,
+        factory: Address,
+        paymaster: Address,
+    ) -> UserOperation {
+        UserOperation {
+            sender,
+            init_code: factory.to_vec().into(),
+            paymaster_and_data: paymaster.to_vec().into(),
+            pre_verification_gas: DEFAULT_PVG,
+            ..Default::default()
+        }
+    }
+
     fn op_with_sender_and_fees(
         sender: Address,
         max_fee_per_gas: u128,

crates/pool/proto/op_pool/op_pool.proto

@@ -111,12 +111,14 @@ enum EntityUpdateType {
   ENTITY_UPDATE_TYPE_UNSPECIFIED = 0;
   ENTITY_UPDATE_TYPE_UNSTAKED_INVALIDATION = 1;
   ENTITY_UPDATE_TYPE_STAKED_INVALIDATION = 2;
+  ENTITY_UPDATE_TYPE_PAYMASTER_OPS_SEEN_DECREMENT = 3;
 }
 
 // A tuple consisting of an entity and what kind of update to perform on it
 message EntityUpdate {
   Entity entity = 1;
   EntityUpdateType update_type = 2;
+  uint64 value = 3;
 }
 
 // Defines a UserOperation persisted in a local mempool