fix(builder): handle the post op revert 0.6 entry point bug

crates/builder/src/bundle_proposer.rs

@@ -14,7 +14,9 @@
 use std::{
     cmp,
     collections::{HashMap, HashSet},
+    future::Future,
     mem,
+    pin::Pin,
     sync::Arc,
     time::Duration,
 };
@@ -36,7 +38,7 @@ use rundler_sim::{
 use rundler_types::{Entity, EntityType, GasFees, Timestamp, UserOperation, UserOpsPerAggregator};
 use rundler_utils::{emit::WithEntryPoint, math};
 use tokio::{sync::broadcast, try_join};
-use tracing::{error, info};
+use tracing::{error, info, warn};
 
 use crate::emit::{BuilderEvent, OpRejectionReason, SkipReason};
 
@@ -439,6 +441,8 @@ where
             context.get_bundle_gas_limit(self.settings.chain_id),
             BUNDLE_TRANSACTION_GAS_OVERHEAD_PERCENT,
         );
+
+        // call handle ops with the bundle to filter any rejected ops before sending
         let handle_ops_out = self
             .entry_point
             .call_handle_ops(
@@ -447,7 +451,7 @@ where
                 gas,
             )
             .await
-            .context("should estimate gas for proposed bundle")?;
+            .context("should call handle ops with candidate bundle")?;
         match handle_ops_out {
             HandleOpsOut::Success => Ok(Some(gas)),
             HandleOpsOut::FailedOp(index, message) => {
@@ -467,6 +471,11 @@ where
                     .await;
                 Ok(None)
             }
+            HandleOpsOut::PostOpRevert => {
+                warn!("PostOpShortRevert error during gas estimation due to bug in the 0.6 entry point contract. Removing the offending op from the bundle.");
+                self.process_post_op_revert(context, gas).await?;
+                Ok(None)
+            }
         }
     }
 
@@ -559,6 +568,130 @@ where
         Ok(())
     }
 
+    // This function is called when a postOpRevert error is encountered during gas estimation.
+    // This is due to a bug in the 0.6 entry point and needs to be handled else the bundle transaction will revert on chain.
+    //
+    // This function attempts to remove any offending user ops. If it can't identify the offending user ops, it will remove all user ops
+    // from the bundle and from the pool.
+    async fn process_post_op_revert(
+        &self,
+        context: &mut ProposalContext,
+        gas: U256,
+    ) -> anyhow::Result<()> {
+        let agg_groups = context.to_ops_per_aggregator();
+        let mut op_index = 0;
+        let mut futures: Vec<Pin<Box<dyn Future<Output = Vec<usize>> + Send>>> = vec![];
+
+        for agg_group in agg_groups {
+            // For non-aggregated ops, re-simulate each op individually
+            if agg_group.aggregator.is_zero() {
+                for op in agg_group.user_ops {
+                    futures.push(Box::pin(
+                        self.check_for_post_op_revert_single_op(op, gas, op_index),
+                    ));
+                    op_index += 1;
+                }
+            } else {
+                // For aggregated ops, re-simulate the group
+                let len = agg_group.user_ops.len();
+                futures.push(Box::pin(
+                    self.check_for_post_op_revert_agg_ops(agg_group, gas, op_index),
+                ));
+                op_index += len;
+            }
+        }
+
+        let results = future::join_all(futures).await;
+        let mut to_remove = results.into_iter().flatten().collect::<Vec<_>>();
+        if to_remove.is_empty() {
+            // if we can't identify the offending user ops, remove all user ops from the bundle and from the pool
+            to_remove.extend(0..op_index);
+        }
+
+        // iterate in reverse so that we can remove ops without affecting the index of the next op to remove
+        for index in to_remove.into_iter().rev() {
+            self.emit(BuilderEvent::rejected_op(
+                self.builder_index,
+                self.op_hash(context.get_op_at(index)?),
+                OpRejectionReason::FailedInBundle {
+                    message: Arc::new("post op reverted leading to entry point revert".to_owned()),
+                },
+            ));
+            self.reject_index(context, index).await;
+        }
+
+        Ok(())
+    }
+
+    async fn check_for_post_op_revert_single_op(
+        &self,
+        op: UserOperation,
+        gas: U256,
+        op_index: usize,
+    ) -> Vec<usize> {
+        let op_hash = self.op_hash(&op);
+        let bundle = vec![UserOpsPerAggregator {
+            aggregator: Address::zero(),
+            signature: Bytes::new(),
+            user_ops: vec![op],
+        }];
+        let ret = self
+            .entry_point
+            .call_handle_ops(bundle, self.settings.beneficiary, gas)
+            .await;
+        match ret {
+            Ok(out) => {
+                if let HandleOpsOut::PostOpRevert = out {
+                    warn!(
+                        "PostOpRevert error found, removing {:?} from bundle",
+                        op_hash
+                    );
+                    vec![op_index]
+                } else {
+                    vec![]
+                }
+            }
+            Err(e) => {
+                // If we get an error here, we can't be sure if the op is the offending op or not, so we remove it to be safe
+                error!("Failed to call handle ops: {e} during postOpRevert handling, removing op");
+                vec![op_index]
+            }
+        }
+    }
+
+    async fn check_for_post_op_revert_agg_ops(
+        &self,
+        group: UserOpsPerAggregator,
+        gas: U256,
+        start_index: usize,
+    ) -> Vec<usize> {
+        let len = group.user_ops.len();
+        let agg = group.aggregator;
+        let bundle = vec![group];
+        let ret = self
+            .entry_point
+            .call_handle_ops(bundle, self.settings.beneficiary, gas)
+            .await;
+        match ret {
+            Ok(out) => {
+                if let HandleOpsOut::PostOpRevert = out {
+                    warn!(
+                        "PostOpRevert error found, removing all ops with aggregator {:?}",
+                        agg
+                    );
+                    (start_index..start_index + len).collect()
+                } else {
+                    vec![]
+                }
+            }
+            Err(e) => {
+                // If we get an error here, we can't be sure if the op is the offending op or not, so we remove it to be safe
+                error!("Failed to call handle ops: {e} during postOpRevert handling, removing all ops from aggregator {:?}", agg);
+                (start_index..start_index + len).collect()
+            }
+        }
+    }
+
     fn limit_user_operations_for_simulation(
         &self,
         ops: Vec<PoolOperation>,
@@ -1375,6 +1508,136 @@ mod tests {
         assert_eq!(gas_limit, expected_gas_limit);
     }
 
+    #[tokio::test]
+    async fn test_post_op_revert() {
+        let op1 = op_with_sender(address(1));
+        let bundle = mock_make_bundle(
+            vec![MockOp {
+                op: op1.clone(),
+                simulation_result: Box::new(|| Ok(SimulationSuccess::default())),
+            }],
+            vec![],
+            vec![HandleOpsOut::PostOpRevert, HandleOpsOut::PostOpRevert],
+            vec![],
+            U256::zero(),
+            U256::zero(),
+        )
+        .await;
+
+        assert!(bundle.rejected_entities.is_empty());
+        assert_eq!(bundle.rejected_ops, vec![op1]);
+        assert!(bundle.ops_per_aggregator.is_empty());
+    }
+
+    #[tokio::test]
+    async fn test_post_op_revert_two() {
+        let op1 = op_with_sender(address(1));
+        let op2 = op_with_sender(address(2));
+        let bundle = mock_make_bundle(
+            vec![
+                MockOp {
+                    op: op1.clone(),
+                    simulation_result: Box::new(|| Ok(SimulationSuccess::default())),
+                },
+                MockOp {
+                    op: op2.clone(),
+                    simulation_result: Box::new(|| Ok(SimulationSuccess::default())),
+                },
+            ],
+            vec![],
+            vec![
+                HandleOpsOut::PostOpRevert,
+                HandleOpsOut::PostOpRevert,
+                HandleOpsOut::Success,
+                HandleOpsOut::Success,
+            ],
+            vec![],
+            U256::zero(),
+            U256::zero(),
+        )
+        .await;
+
+        assert!(bundle.rejected_entities.is_empty());
+        assert_eq!(bundle.rejected_ops, vec![op1]);
+        assert_eq!(
+            bundle.ops_per_aggregator,
+            vec![UserOpsPerAggregator {
+                user_ops: vec![op2],
+                ..Default::default()
+            }]
+        );
+    }
+
+    #[tokio::test]
+    async fn test_post_op_revert_agg() {
+        let unaggregated_op = op_with_sender(address(1));
+        let aggregated_op_a1 = op_with_sender(address(2));
+        let aggregated_op_a2 = op_with_sender(address(3));
+        let aggregator_a_address = address(10);
+        let op_a1_aggregated_sig = 11;
+        let op_a2_aggregated_sig = 12;
+        let aggregator_a_signature = 101;
+        let bundle = mock_make_bundle(
+            vec![
+                MockOp {
+                    op: unaggregated_op.clone(),
+                    simulation_result: Box::new(|| Ok(SimulationSuccess::default())),
+                },
+                MockOp {
+                    op: aggregated_op_a1.clone(),
+                    simulation_result: Box::new(move || {
+                        Ok(SimulationSuccess {
+                            aggregator: Some(AggregatorSimOut {
+                                address: aggregator_a_address,
+                                signature: bytes(op_a1_aggregated_sig),
+                            }),
+                            ..Default::default()
+                        })
+                    }),
+                },
+                MockOp {
+                    op: aggregated_op_a2.clone(),
+                    simulation_result: Box::new(move || {
+                        Ok(SimulationSuccess {
+                            aggregator: Some(AggregatorSimOut {
+                                address: aggregator_a_address,
+                                signature: bytes(op_a2_aggregated_sig),
+                            }),
+                            ..Default::default()
+                        })
+                    }),
+                },
+            ],
+            vec![MockAggregator {
+                address: aggregator_a_address,
+                signature: Box::new(move || Ok(Some(bytes(aggregator_a_signature)))),
+            }],
+            vec![
+                HandleOpsOut::PostOpRevert, // bundle
+                HandleOpsOut::Success,      // unaggregated check
+                HandleOpsOut::PostOpRevert, // aggregated check
+                HandleOpsOut::Success,      // after remove
+            ],
+            vec![],
+            U256::zero(),
+            U256::zero(),
+        )
+        .await;
+
+        assert!(bundle.rejected_entities.is_empty());
+        assert_eq!(
+            bundle.rejected_ops,
+            vec![aggregated_op_a2, aggregated_op_a1]
+        );
+        assert_eq!(
+            bundle.ops_per_aggregator,
+            vec![UserOpsPerAggregator {
+                user_ops: vec![unaggregated_op],
+                ..Default::default()
+            }]
+        );
+    }
+
     struct MockOp {
         op: UserOperation,
         simulation_result:

crates/provider/src/ethers/entry_point.rs

@@ -81,6 +81,13 @@ where
             if let Ok(failure) = SignatureValidationFailed::decode(revert_data) {
                 return Ok(HandleOpsOut::SignatureValidationFailed(failure.aggregator));
             }
+            // Special handling for a bug in the 0.6 entry point contract to detect the bug where
+            // the `returndatacopy` opcode reverts due to a postOp revert and the revert data is too short.
+            // See https://github.com/eth-infinitism/account-abstraction/pull/325 for more details.
+            // NOTE: this error message is copied directly from Geth and assumes it will not change.
+            if error.to_string().contains("return data out of bounds") {
+                return Ok(HandleOpsOut::PostOpRevert);
+            }
         }
         Err(error)?
     }

crates/provider/src/traits/entry_point.rs

@@ -30,6 +30,9 @@ pub enum HandleOpsOut {
     FailedOp(usize, String),
     /// Call failed due to a signature validation failure
     SignatureValidationFailed(Address),
+    /// Call failed due to a bug in the 0.6 entry point contract https://github.com/eth-infinitism/account-abstraction/pull/325.
+    /// Special handling is required to remove the offending operation from the bundle.
+    PostOpRevert,
 }
 
 /// Trait for interacting with an entry point contract.