fix(builder): fix builder state machine replacement tracking

alchemyplatform · Jun 25, 2024 · 6b8d23f · 6b8d23f
1 parent 3de6345
commit 6b8d23f
Show file tree

Hide file tree

Showing 7 changed files with 321 additions and 117 deletions.
diff --git a/bin/rundler/src/cli/builder.rs b/bin/rundler/src/cli/builder.rs
@@ -226,16 +226,25 @@ pub struct BuilderArgs {
     )]
     replacement_fee_percent_increase: u64,
 
-    /// Maximum number of times to increase gas fees when retrying a transaction
+    /// Maximum number of times to increase gas fees when retrying a cancellation transaction
     /// before giving up.
     #[arg(
-        long = "builder.max_fee_increases",
-        name = "builder.max_fee_increases",
-        env = "BUILDER_MAX_FEE_INCREASES",
-        // Seven increases of 10% is roughly 2x the initial fees.
-        default_value = "7"
+        long = "builder.max_cancellation_fee_increases",
+        name = "builder.max_cancellation_fee_increases",
+        env = "BUILDER_MAX_CANCELLATION_FEE_INCREASES",
+        default_value = "15"
     )]
-    max_fee_increases: u64,
+    max_cancellation_fee_increases: u64,
+
+    /// The maximum number of blocks to wait in a replacement underpriced state before issuing
+    /// a cancellation transaction.
+    #[arg(
+        long = "builder.max_replacement_underpriced_blocks",
+        name = "builder.max_replacement_underpriced_blocks",
+        env = "BUILDER_MAX_REPLACEMENT_UNDERPRICED_BLOCKS",
+        default_value = "20"
+    )]
+    max_replacement_underpriced_blocks: u64,
 
     /// The index offset to apply to the builder index
     #[arg(
@@ -350,7 +359,8 @@ impl BuilderArgs {
             sim_settings: common.try_into()?,
             max_blocks_to_wait_for_mine: self.max_blocks_to_wait_for_mine,
             replacement_fee_percent_increase: self.replacement_fee_percent_increase,
-            max_fee_increases: self.max_fee_increases,
+            max_cancellation_fee_increases: self.max_cancellation_fee_increases,
+            max_replacement_underpriced_blocks: self.max_replacement_underpriced_blocks,
             remote_address,
         })
     }

diff --git a/crates/builder/src/bundle_proposer.rs b/crates/builder/src/bundle_proposer.rs
@@ -104,18 +104,35 @@ pub(crate) trait BundleProposer: Send + Sync + 'static {
         &mut self,
         min_fees: Option<GasFees>,
         is_replacement: bool,
-    ) -> anyhow::Result<Bundle<Self::UO>>;
+    ) -> BundleProposerResult<Bundle<Self::UO>>;
 
     /// Gets the current gas fees
     ///
     /// If `min_fees` is `Some`, the proposer will ensure the gas fees returned are at least `min_fees`.
-    async fn estimate_gas_fees(&self, min_fees: Option<GasFees>)
-        -> anyhow::Result<(GasFees, U256)>;
+    async fn estimate_gas_fees(
+        &self,
+        min_fees: Option<GasFees>,
+    ) -> BundleProposerResult<(GasFees, U256)>;
 
     /// Notifies the proposer that a condition was not met during the last bundle proposal
     fn notify_condition_not_met(&mut self);
 }
 
+pub(crate) type BundleProposerResult<T> = std::result::Result<T, BundleProposerError>;
+
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum BundleProposerError {
+    #[error("No operations initially")]
+    NoOperationsInitially,
+    #[error("No operations after fee filtering")]
+    NoOperationsAfterFeeFilter,
+    #[error(transparent)]
+    ProviderError(#[from] rundler_provider::ProviderError),
+    /// All other errors
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
 #[derive(Debug)]
 pub(crate) struct BundleProposerImpl<UO, S, E, P, M> {
     builder_index: u64,
@@ -155,8 +172,11 @@ where
     async fn estimate_gas_fees(
         &self,
         required_fees: Option<GasFees>,
-    ) -> anyhow::Result<(GasFees, U256)> {
-        self.fee_estimator.required_bundle_fees(required_fees).await
+    ) -> BundleProposerResult<(GasFees, U256)> {
+        Ok(self
+            .fee_estimator
+            .required_bundle_fees(required_fees)
+            .await?)
     }
 
     fn notify_condition_not_met(&mut self) {
@@ -167,16 +187,16 @@ where
         &mut self,
         required_fees: Option<GasFees>,
         is_replacement: bool,
-    ) -> anyhow::Result<Bundle<UO>> {
+    ) -> BundleProposerResult<Bundle<UO>> {
         let (ops, (block_hash, _), (bundle_fees, base_fee)) = try_join!(
             self.get_ops_from_pool(),
             self.provider
                 .get_latest_block_hash_and_number()
-                .map_err(anyhow::Error::from),
+                .map_err(BundleProposerError::from),
             self.estimate_gas_fees(required_fees)
         )?;
         if ops.is_empty() {
-            return Ok(Bundle::default());
+            return Err(BundleProposerError::NoOperationsInitially);
         }
 
         tracing::debug!("Starting bundle proposal with {} ops", ops.len());
@@ -206,7 +226,7 @@ where
 
         tracing::debug!("Bundle proposal after fee limit had {} ops", ops.len());
         if ops.is_empty() {
-            return Ok(Bundle::default());
+            return Err(BundleProposerError::NoOperationsAfterFeeFilter);
         }
 
         // (2) Limit the amount of operations for simulation
@@ -686,7 +706,7 @@ where
     async fn estimate_gas_rejecting_failed_ops(
         &self,
         context: &mut ProposalContext<UO>,
-    ) -> anyhow::Result<Option<U256>> {
+    ) -> BundleProposerResult<Option<U256>> {
         // sum up the gas needed for all the ops in the bundle
         // and apply an overhead multiplier
         let gas = math::increase_by_percent(
@@ -731,7 +751,7 @@ where
         }
     }
 
-    async fn get_ops_from_pool(&self) -> anyhow::Result<Vec<PoolOperation>> {
+    async fn get_ops_from_pool(&self) -> BundleProposerResult<Vec<PoolOperation>> {
         // Use builder's index as the shard index to ensure that two builders don't
         // attempt to bundle the same operations.
         //
@@ -754,7 +774,7 @@ where
         &self,
         addresses: impl IntoIterator<Item = Address>,
         block_hash: H256,
-    ) -> anyhow::Result<HashMap<Address, U256>> {
+    ) -> BundleProposerResult<HashMap<Address, U256>> {
         let futures = addresses.into_iter().map(|address| async move {
             let deposit = self
                 .entry_point
@@ -1294,8 +1314,9 @@ impl<UO: UserOperation> ProposalContext<UO> {
                 }
                 SimulationViolation::UnintendedRevertWithMessage(entity_type, message, address) => {
                     match &message[..4] {
-                        // do not penalize an entity for invalid account nonces, which can occur without malicious intent from the sender
-                        "AA25" => {}
+                        // do not penalize an entity for invalid account nonces or already deployed senders,
+                        // which can occur without malicious intent from the sender or factory
+                        "AA10" | "AA25" => {}
                         _ => {
                             if let Some(entity_address) = address {
                                 self.add_entity_update(