refactor: increment nonce only after passing tx validation

crates/blockifier/src/blockifier/stateful_validator.rs

@@ -66,17 +66,28 @@ impl<S: StateReader> StatefulValidator<S> {
         let tx_context = self.tx_executor.block_context.to_tx_context(&tx);
         self.perform_pre_validation_stage(&tx, &tx_context)?;
 
-        if skip_validate {
-            return Ok(());
+        if !skip_validate {
+            // `__validate__` call.
+            let versioned_constants = &tx_context.block_context.versioned_constants();
+            let (_optional_call_info, actual_cost) =
+                self.validate(&tx, versioned_constants.tx_initial_gas())?;
+
+            // Post validations.
+            PostValidationReport::verify(&tx_context, &actual_cost)?;
         }
 
-        // `__validate__` call.
-        let versioned_constants = &tx_context.block_context.versioned_constants();
-        let (_optional_call_info, actual_cost) =
-            self.validate(&tx, versioned_constants.tx_initial_gas())?;
-
-        // Post validations.
-        PostValidationReport::verify(&tx_context, &actual_cost)?;
+        // See similar comment in `run_revertible` for context.
+        //
+        // From what I've seen there is not suitable method that is used by both the validator and
+        // the normal transaction flow where the nonce increment logic can be placed. So
+        // this is manually placed here.
+        //
+        // TODO: find a better place to put this without needing this duplication.
+        self.tx_executor
+            .block_state
+            .as_mut()
+            .expect(BLOCK_STATE_ACCESS_ERR)
+            .increment_nonce(tx_context.tx_info.sender_address())?;
 
         Ok(())
     }

crates/blockifier/src/transaction/account_transaction.rs

@@ -230,6 +230,12 @@ impl AccountTransaction {
         Ok(())
     }
 
+    /// This method no longer increments the nonce. Refer to individual functions to see
+    /// in which stage the nonce is incremented.
+    ///
+    /// Nonce incremental logics manually placed in
+    /// [`AccountTransaction::run_revertible`], [`AccountTransaction::run_non_revertible`],
+    /// [`StatefulValidator::perform_validations`](crate::blockifier::stateful_validator::StatefulValidator::perform_validations)
     fn handle_nonce(
         state: &mut dyn State,
         tx_info: &TransactionInfo,
@@ -248,7 +254,8 @@ impl AccountTransaction {
             account_nonce <= incoming_tx_nonce
         };
         if valid_nonce {
-            return Ok(state.increment_nonce(address)?);
+            // return Ok(state.increment_nonce(address)?);
+            return Ok(());
         }
         Err(TransactionPreValidationError::InvalidNonce {
             address,
@@ -422,7 +429,13 @@ impl AccountTransaction {
         let mut resources = ExecutionResources::default();
         let validate_call_info: Option<CallInfo>;
         let execute_call_info: Option<CallInfo>;
+
         if matches!(self, Self::DeployAccount(_)) {
+            // See similar comment in `run_revertible` for context.
+            //
+            // Not sure for this case if incrementing the nonce at this stage is correct.
+            state.increment_nonce(tx_context.tx_info.sender_address())?;
+
             // Handle `DeployAccount` transactions separately, due to different order of things.
             // Also, the execution context required form the `DeployAccount` execute phase is
             // validation context.
@@ -449,6 +462,10 @@ impl AccountTransaction {
                 validate,
                 charge_fee,
             )?;
+
+            // See similar comment in `run_revertible` for context.
+            state.increment_nonce(tx_context.tx_info.sender_address())?;
+
             execute_call_info =
                 self.run_execute(state, &mut resources, &mut execution_context, remaining_gas)?;
         }
@@ -495,6 +512,18 @@ impl AccountTransaction {
             charge_fee,
         )?;
 
+        // Increment the sender nonce only after the tx has passed validation.
+        //
+        // NOTE:
+        //
+        // Before this, the nonce is incremented in `AccountTransaction::handle_nonce` which is
+        // called at the initial stage of transaction processing (ie in
+        // ExecutableTransaction::execute_raw of AccountTransaction), which is even before the
+        // account validation logic is being run. Which is weird because the nonce would get
+        // incremented even if the transaction failed validation. And this is not what I observed
+        // from mainnet/testnet. So, I moved the nonce incrementation here.
+        state.increment_nonce(tx_context.tx_info.sender_address())?;
+
         let n_allotted_execution_steps = execution_context.subtract_validation_and_overhead_steps(
             &validate_call_info,
             &self.tx_type(),

crates/blockifier/src/transaction/transactions_test.rs

@@ -1015,6 +1015,15 @@ fn test_invalid_nonce(
         .perform_pre_validation_stage(&mut transactional_state, &valid_tx_context, false, false)
         .unwrap();
 
+    // See comments on changing where nonce is incremented in `ExecutableTransaction::execute_raw`
+    // of AccountTransaction.
+    //
+    // Basically, before this, the nonce is incremented in `perform_pre_validation_stage` but that
+    // method is called even before the account validation stage is executed. So, now we have to
+    // increment the sender's account nonce manually here.
+    let account = valid_tx_context.tx_info.sender_address();
+    transactional_state.increment_nonce(account).expect("failed to increment nonce");
+
     // Negative flow: account nonce = 1, incoming tx nonce = 0.
     let invalid_nonce = nonce!(0_u8);
     let invalid_tx =