diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs index a310b272a..40e07d413 100644 --- a/crates/builder/src/bundle_proposer.rs +++ b/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,133 @@ 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> + 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::>(); + 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 + error!( + "Failed to identify offending user ops, removing all user ops from bundle and 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 { + 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 { + 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, @@ -1375,6 +1511,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: diff --git a/crates/provider/src/ethers/entry_point.rs b/crates/provider/src/ethers/entry_point.rs index 23b89d196..88e4a83e5 100644 --- a/crates/provider/src/ethers/entry_point.rs +++ b/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)? } diff --git a/crates/provider/src/traits/entry_point.rs b/crates/provider/src/traits/entry_point.rs index e18200ca8..f0b0c90f3 100644 --- a/crates/provider/src/traits/entry_point.rs +++ b/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.