refactor: move skip validation check to py_validator (#2013)

crates/blockifier/src/blockifier/stateful_validator.rs

@@ -1,9 +1,7 @@
 use std::sync::Arc;
 
 use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
-use starknet_api::core::Nonce;
-use starknet_api::transaction::TransactionHash;
-use starknet_types_core::felt::Felt;
+use starknet_api::core::{ContractAddress, Nonce};
 use thiserror::Error;
 
 use crate::blockifier::config::TransactionExecutorConfig;
@@ -19,7 +17,6 @@ use crate::state::errors::StateError;
 use crate::state::state_api::StateReader;
 use crate::transaction::account_transaction::AccountTransaction;
 use crate::transaction::errors::{TransactionExecutionError, TransactionPreValidationError};
-use crate::transaction::objects::TransactionInfo;
 use crate::transaction::transaction_execution::Transaction;
 use crate::transaction::transactions::ValidatableTransaction;
 
@@ -44,24 +41,19 @@ pub type StatefulValidatorResult<T> = Result<T, StatefulValidatorError>;
 /// Manages state related transaction validations for pre-execution flows.
 pub struct StatefulValidator<S: StateReader> {
     tx_executor: TransactionExecutor<S>,
-    max_nonce_for_validation_skip: Nonce,
 }
 
 impl<S: StateReader> StatefulValidator<S> {
-    pub fn create(
-        state: CachedState<S>,
-        block_context: BlockContext,
-        max_nonce_for_validation_skip: Nonce,
-    ) -> Self {
+    pub fn create(state: CachedState<S>, block_context: BlockContext) -> Self {
         let tx_executor =
             TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default());
-        Self { tx_executor, max_nonce_for_validation_skip }
+        Self { tx_executor }
     }
 
     pub fn perform_validations(
         &mut self,
         tx: AccountTransaction,
-        deploy_account_tx_hash: Option<TransactionHash>,
+        skip_validate: bool,
     ) -> StatefulValidatorResult<()> {
         // Deploy account transactions should be fully executed, since the constructor must run
         // before `__validate_deploy__`. The execution already includes all necessary validations,
@@ -71,14 +63,7 @@ impl<S: StateReader> StatefulValidator<S> {
             return Ok(());
         }
 
-        // First, we check if the transaction should be skipped due to the deploy account not being
-        // processed. It is done before the pre-validations checks because, in these checks, we
-        // change the state (more precisely, we increment the nonce).
         let tx_context = self.tx_executor.block_context.to_tx_context(&tx);
-        let skip_validate = self.skip_validate_due_to_unprocessed_deploy_account(
-            &tx_context.tx_info,
-            deploy_account_tx_hash,
-        )?;
         self.perform_pre_validation_stage(&tx, &tx_context)?;
 
         if skip_validate {
@@ -119,35 +104,6 @@ impl<S: StateReader> StatefulValidator<S> {
         Ok(())
     }
 
-    // Check if deploy account was submitted but not processed yet. If so, then skip
-    // `__validate__` method for subsequent transactions for a better user experience.
-    // (they will otherwise fail solely because the deploy account hasn't been processed yet).
-    fn skip_validate_due_to_unprocessed_deploy_account(
-        &mut self,
-        tx_info: &TransactionInfo,
-        deploy_account_tx_hash: Option<TransactionHash>,
-    ) -> StatefulValidatorResult<bool> {
-        let nonce = self
-            .tx_executor
-            .block_state
-            .as_ref()
-            .expect(BLOCK_STATE_ACCESS_ERR)
-            .get_nonce_at(tx_info.sender_address())?;
-        let tx_nonce = tx_info.nonce();
-
-        let deploy_account_not_processed =
-            deploy_account_tx_hash.is_some() && nonce == Nonce(Felt::ZERO);
-        let is_post_deploy_nonce = Nonce(Felt::ONE) <= tx_nonce;
-        let nonce_small_enough_to_qualify_for_validation_skip =
-            tx_nonce <= self.max_nonce_for_validation_skip;
-
-        let skip_validate = deploy_account_not_processed
-            && is_post_deploy_nonce
-            && nonce_small_enough_to_qualify_for_validation_skip;
-
-        Ok(skip_validate)
-    }
-
     fn validate(
         &mut self,
         tx: &AccountTransaction,
@@ -181,4 +137,16 @@ impl<S: StateReader> StatefulValidator<S> {
 
         Ok((validate_call_info, tx_receipt))
     }
+
+    pub fn get_nonce(
+        &mut self,
+        account_address: ContractAddress,
+    ) -> StatefulValidatorResult<Nonce> {
+        Ok(self
+            .tx_executor
+            .block_state
+            .as_ref()
+            .expect(BLOCK_STATE_ACCESS_ERR)
+            .get_nonce_at(account_address)?)
+    }
 }

crates/blockifier/src/blockifier/stateful_validator_test.rs

@@ -1,15 +1,16 @@
+use assert_matches::assert_matches;
 use rstest::rstest;
 use starknet_api::transaction::{Fee, TransactionVersion};
 
 use crate::blockifier::stateful_validator::StatefulValidator;
 use crate::context::BlockContext;
-use crate::nonce;
 use crate::test_utils::contracts::FeatureContract;
 use crate::test_utils::initial_test_state::{fund_account, test_state};
 use crate::test_utils::{CairoVersion, BALANCE};
 use crate::transaction::account_transaction::AccountTransaction;
 use crate::transaction::test_utils::{
-    block_context, create_account_tx_for_validate_test_nonce_0, FaultyAccountTxCreatorArgs, VALID,
+    block_context, create_account_tx_for_validate_test_nonce_0, FaultyAccountTxCreatorArgs,
+    INVALID, VALID,
 };
 use crate::transaction::transaction_types::TransactionType;
 
@@ -63,8 +64,31 @@ fn test_transaction_validator(
     }
 
     // Test the stateful validator.
-    let mut stateful_validator = StatefulValidator::create(state, block_context, nonce!(0_u32));
+    let mut stateful_validator = StatefulValidator::create(state, block_context);
 
-    let reuslt = stateful_validator.perform_validations(tx, None);
-    assert!(reuslt.is_ok(), "Validation failed: {:?}", reuslt.unwrap_err());
+    let result = stateful_validator.perform_validations(tx, false);
+    assert!(result.is_ok(), "Validation failed: {:?}", result.unwrap_err());
+}
+
+#[test]
+fn test_transaction_validator_skip_validate() {
+    let block_context = BlockContext::create_for_testing();
+    let faulty_account = FeatureContract::FaultyAccount(CairoVersion::Cairo1);
+    let state = test_state(&block_context.chain_info, BALANCE, &[(faulty_account, 1)]);
+
+    // Create a transaction that does not pass validations.
+    let tx = create_account_tx_for_validate_test_nonce_0(FaultyAccountTxCreatorArgs {
+        scenario: INVALID,
+        tx_type: TransactionType::InvokeFunction,
+        tx_version: TransactionVersion::THREE,
+        sender_address: faulty_account.get_instance_address(0),
+        class_hash: faulty_account.get_class_hash(),
+        max_fee: Fee(BALANCE),
+        ..Default::default()
+    });
+
+    let mut stateful_validator = StatefulValidator::create(state, block_context);
+    // The transaction validations should be skipped and the function should return Ok.
+    let result = stateful_validator.perform_validations(tx, true);
+    assert_matches!(result, Ok(()));
 }

crates/native_blockifier/src/py_validator.rs

@@ -1,11 +1,15 @@
-use blockifier::blockifier::stateful_validator::StatefulValidator;
+use blockifier::blockifier::stateful_validator::{StatefulValidator, StatefulValidatorResult};
 use blockifier::bouncer::BouncerConfig;
 use blockifier::context::BlockContext;
 use blockifier::state::cached_state::CachedState;
+use blockifier::transaction::account_transaction::AccountTransaction;
+use blockifier::transaction::objects::TransactionInfoCreator;
+use blockifier::transaction::transaction_types::TransactionType;
 use blockifier::versioned_constants::VersionedConstants;
 use pyo3::{pyclass, pymethods, PyAny};
 use starknet_api::core::Nonce;
 use starknet_api::transaction::TransactionHash;
+use starknet_types_core::felt::Felt;
 
 use crate::errors::NativeBlockifierResult;
 use crate::py_block_executor::PyGeneralConfig;
@@ -18,6 +22,7 @@ use crate::state_readers::py_state_reader::PyStateReader;
 #[pyclass]
 pub struct PyValidator {
     pub stateful_validator: StatefulValidator<PyStateReader>,
+    pub max_nonce_for_validation_skip: Nonce,
 }
 
 #[pymethods]
@@ -47,10 +52,9 @@ impl PyValidator {
 
         // Create the stateful validator.
         let max_nonce_for_validation_skip = Nonce(max_nonce_for_validation_skip.0);
-        let stateful_validator =
-            StatefulValidator::create(state, block_context, max_nonce_for_validation_skip);
+        let stateful_validator = StatefulValidator::create(state, block_context);
 
-        Ok(Self { stateful_validator })
+        Ok(Self { stateful_validator, max_nonce_for_validation_skip })
     }
 
     // Transaction Execution API.
@@ -64,8 +68,43 @@ impl PyValidator {
     ) -> NativeBlockifierResult<()> {
         let account_tx = py_account_tx(tx, optional_py_class_info).expect(PY_TX_PARSING_ERR);
         let deploy_account_tx_hash = deploy_account_tx_hash.map(|hash| TransactionHash(hash.0));
-        self.stateful_validator.perform_validations(account_tx, deploy_account_tx_hash)?;
+
+        // We check if the transaction should be skipped due to the deploy account not being
+        // processed.
+        let skip_validate = self
+            .skip_validate_due_to_unprocessed_deploy_account(&account_tx, deploy_account_tx_hash)?;
+        self.stateful_validator.perform_validations(account_tx, skip_validate)?;
 
         Ok(())
     }
 }
+
+impl PyValidator {
+    // Check if deploy account was submitted but not processed yet. If so, then skip
+    // `__validate__` method for subsequent transactions for a better user experience.
+    // (they will otherwise fail solely because the deploy account hasn't been processed yet).
+    pub fn skip_validate_due_to_unprocessed_deploy_account(
+        &mut self,
+        account_tx: &AccountTransaction,
+        deploy_account_tx_hash: Option<TransactionHash>,
+    ) -> StatefulValidatorResult<bool> {
+        if account_tx.tx_type() != TransactionType::InvokeFunction {
+            return Ok(false);
+        }
+        let tx_info = account_tx.create_tx_info();
+        let nonce = self.stateful_validator.get_nonce(tx_info.sender_address())?;
+
+        let deploy_account_not_processed =
+            deploy_account_tx_hash.is_some() && nonce == Nonce(Felt::ZERO);
+        let tx_nonce = tx_info.nonce();
+        let is_post_deploy_nonce = Nonce(Felt::ONE) <= tx_nonce;
+        let nonce_small_enough_to_qualify_for_validation_skip =
+            tx_nonce <= self.max_nonce_for_validation_skip;
+
+        let skip_validate = deploy_account_not_processed
+            && is_post_deploy_nonce
+            && nonce_small_enough_to_qualify_for_validation_skip;
+
+        Ok(skip_validate)
+    }
+}