refactor: move createTask simulation before s3 upload (#1567)

Co-authored-by: Uriel Mihura <[email protected]>

Author: MarcosNicolau

batcher/aligned-batcher/src/lib.rs

@@ -1428,28 +1428,12 @@ impl Batcher {
         let batch_merkle_root_hex = hex::encode(batch_merkle_root);
         info!("Batch merkle root: 0x{}", batch_merkle_root_hex);
         let file_name = batch_merkle_root_hex.clone() + ".json";
-
-        info!("Uploading batch to S3...");
-        self.upload_batch_to_s3(batch_bytes, &file_name).await?;
-
-        if let Err(e) = self
-            .telemetry
-            .task_uploaded_to_s3(&batch_merkle_root_hex)
-            .await
-        {
-            warn!("Failed to send task status to telemetry: {:?}", e);
-        };
-        info!("Batch sent to S3 with name: {}", file_name);
-
-        info!("Uploading batch to contract");
         let batch_data_pointer: String = "".to_owned() + &self.download_endpoint + "/" + &file_name;
 
         let num_proofs_in_batch = leaves.len();
-
         let gas_per_proof = (CONSTANT_GAS_COST
             + ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF * num_proofs_in_batch as u128)
             / num_proofs_in_batch as u128;
-
         let fee_per_proof = U256::from(gas_per_proof) * gas_price;
         let fee_for_aggregator = (U256::from(AGGREGATOR_GAS_COST)
             * gas_price
@@ -1465,12 +1449,31 @@ impl Batcher {
             respond_to_task_fee_limit,
         );
 
-        let proof_submitters = finalized_batch.iter().map(|entry| entry.sender).collect();
+        let proof_submitters: Vec<Address> =
+            finalized_batch.iter().map(|entry| entry.sender).collect();
+
+        self.simulate_create_new_task(
+            *batch_merkle_root,
+            batch_data_pointer.clone(),
+            proof_submitters.clone(),
+            fee_params.clone(),
+        )
+        .await?;
 
         self.metrics
             .gas_price_used_on_latest_batch
             .set(gas_price.as_u64() as i64);
 
+        info!("Uploading batch to S3...");
+        self.upload_batch_to_s3(batch_bytes, &file_name).await?;
+        if let Err(e) = self
+            .telemetry
+            .task_uploaded_to_s3(&batch_merkle_root_hex)
+            .await
+        {
+            warn!("Failed to send task status to telemetry: {:?}", e);
+        };
+        info!("Batch sent to S3 with name: {}", file_name);
         if let Err(e) = self
             .telemetry
             .task_created(
@@ -1483,6 +1486,7 @@ impl Batcher {
             warn!("Failed to send task status to telemetry: {:?}", e);
         };
 
+        info!("Submitting batch to contract");
         match self
             .create_new_task(
                 *batch_merkle_root,
@@ -1520,27 +1524,6 @@ impl Batcher {
         proof_submitters: Vec<Address>,
         fee_params: CreateNewTaskFeeParams,
     ) -> Result<TransactionReceipt, BatcherError> {
-        // First, we simulate the tx
-        retry_function(
-            || {
-                simulate_create_new_task_retryable(
-                    batch_merkle_root,
-                    batch_data_pointer.clone(),
-                    proof_submitters.clone(),
-                    fee_params.clone(),
-                    &self.payment_service,
-                    &self.payment_service_fallback,
-                )
-            },
-            ETHEREUM_CALL_MIN_RETRY_DELAY,
-            ETHEREUM_CALL_BACKOFF_FACTOR,
-            ETHEREUM_CALL_MAX_RETRIES,
-            ETHEREUM_CALL_MAX_RETRY_DELAY,
-        )
-        .await
-        .map_err(|e| e.inner())?;
-
-        // Then, we send the real tx
         let result = retry_function(
             || {
                 create_new_task_retryable(
@@ -1579,6 +1562,37 @@ impl Batcher {
         }
     }
 
+    /// Simulates the `create_new_task` transaction by sending an `eth_call` to the RPC node.
+    /// This function does not mutate the state but verifies if it will revert under the given conditions.
+    async fn simulate_create_new_task(
+        &self,
+        batch_merkle_root: [u8; 32],
+        batch_data_pointer: String,
+        proof_submitters: Vec<Address>,
+        fee_params: CreateNewTaskFeeParams,
+    ) -> Result<(), BatcherError> {
+        retry_function(
+            || {
+                simulate_create_new_task_retryable(
+                    batch_merkle_root,
+                    batch_data_pointer.clone(),
+                    proof_submitters.clone(),
+                    fee_params.clone(),
+                    &self.payment_service,
+                    &self.payment_service_fallback,
+                )
+            },
+            ETHEREUM_CALL_MIN_RETRY_DELAY,
+            ETHEREUM_CALL_BACKOFF_FACTOR,
+            ETHEREUM_CALL_MAX_RETRIES,
+            ETHEREUM_CALL_MAX_RETRY_DELAY,
+        )
+        .await
+        .map_err(|e| e.inner())?;
+
+        Ok(())
+    }
+
     /// Sends a transaction to Ethereum with the same nonce as the previous one to override it.
     /// Retries on recoverable errors with exponential backoff.
     /// Bumps the fee if not included in 6 blocks, using `calculate_bumped_gas_price`.

batcher/aligned-batcher/src/retry/batcher_retryables.rs

@@ -197,7 +197,13 @@ pub async fn simulate_create_new_task_retryable(
         .gas_price(fee_params.gas_price);
     // sends an `eth_call` request to the node
     match simulation.call().await {
-        Ok(_) => Ok(()),
+        Ok(_) => {
+            info!(
+                "Simulation task for: 0x{} succeeded.",
+                hex::encode(batch_merkle_root)
+            );
+            Ok(())
+        }
         Err(ContractError::Revert(err)) => {
             // Since transaction was reverted, we don't want to retry with fallback.
             warn!("Simulated transaction reverted {:?}", err);