From 67f7a22431899286a5ad398d8fa11f2b5bc4a28e Mon Sep 17 00:00:00 2001 From: Yael Doweck Date: Tue, 2 Jul 2024 11:03:12 +0300 Subject: [PATCH] feat: skip validation of an invoke transaction after deploy account --- Cargo.lock | 2 +- crates/gateway/src/gateway.rs | 3 +- crates/gateway/src/state_reader_test_utils.rs | 9 +- .../src/stateful_transaction_validator.rs | 48 +++++++-- .../stateful_transaction_validator_test.rs | 101 ++++++++++++++---- .../test_utils/src/starknet_api_test_utils.rs | 1 + 6 files changed, 125 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7281470d..95b1c628 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -550,7 +550,7 @@ dependencies = [ [[package]] name = "blockifier" version = "0.7.0-dev.1" -source = "git+https://github.com/starkware-libs/blockifier.git?branch=main-mempool#cbfddc375cd5bde74170e9f72b7fe7e2d2a21555" +source = "git+https://github.com/starkware-libs/blockifier.git?branch=main-mempool#9fdde90eab6b2897c45f3cf35eb9b834beb42802" dependencies = [ "anyhow", "ark-ec", diff --git a/crates/gateway/src/gateway.rs b/crates/gateway/src/gateway.rs index a9bc173f..4d3bc47a 100644 --- a/crates/gateway/src/gateway.rs +++ b/crates/gateway/src/gateway.rs @@ -135,9 +135,8 @@ fn process_tx( _ => None, }; - // TODO(Yael, 19/5/2024): pass the relevant deploy_account_hash. let tx_hash = - stateful_tx_validator.run_validate(state_reader_factory, &tx, optional_class_info, None)?; + stateful_tx_validator.run_validate(state_reader_factory, &tx, optional_class_info)?; // TODO(Arni): Add the Sierra and the Casm to the mempool input. Ok(MempoolInput { diff --git a/crates/gateway/src/state_reader_test_utils.rs b/crates/gateway/src/state_reader_test_utils.rs index 5f3adb27..0513695d 100644 --- a/crates/gateway/src/state_reader_test_utils.rs +++ b/crates/gateway/src/state_reader_test_utils.rs @@ -1,5 +1,5 @@ use blockifier::blockifier::block::BlockInfo; -use blockifier::context::{BlockContext, ChainInfo}; +use blockifier::context::BlockContext; use blockifier::execution::contract_class::ContractClass; use blockifier::state::errors::StateError; use blockifier::state::state_api::{StateReader as BlockifierStateReader, StateResult}; @@ -7,7 +7,6 @@ use blockifier::test_utils::contracts::FeatureContract; use blockifier::test_utils::dict_state_reader::DictStateReader; use blockifier::test_utils::initial_test_state::{fund_account, test_state_reader}; use blockifier::test_utils::{CairoVersion, BALANCE}; -use blockifier::versioned_constants::VersionedConstants; use starknet_api::block::BlockNumber; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; use starknet_api::hash::StarkFelt; @@ -73,12 +72,8 @@ pub fn local_test_state_reader_factory( cairo_version: CairoVersion, zero_balance: bool, ) -> TestStateReaderFactory { + let block_context = BlockContext::create_for_testing(); let account_balance = if zero_balance { 0 } else { BALANCE }; - let block_context = BlockContext::new_unchecked( - &BlockInfo::create_for_testing(), - &ChainInfo::create_for_testing(), - &VersionedConstants::create_for_testing(), - ); let account_contract = FeatureContract::AccountWithoutValidations(cairo_version); let test_contract = FeatureContract::TestContract(cairo_version); diff --git a/crates/gateway/src/stateful_transaction_validator.rs b/crates/gateway/src/stateful_transaction_validator.rs index c45a2887..109966b7 100644 --- a/crates/gateway/src/stateful_transaction_validator.rs +++ b/crates/gateway/src/stateful_transaction_validator.rs @@ -5,13 +5,15 @@ use blockifier::context::BlockContext; use blockifier::execution::contract_class::ClassInfo; use blockifier::state::cached_state::CachedState; use blockifier::versioned_constants::VersionedConstants; -use starknet_api::rpc_transaction::RPCTransaction; +use starknet_api::core::Nonce; +use starknet_api::hash::StarkFelt; +use starknet_api::rpc_transaction::{RPCInvokeTransaction, RPCTransaction}; use starknet_api::transaction::TransactionHash; use crate::config::StatefulTransactionValidatorConfig; use crate::errors::{StatefulTransactionValidatorError, StatefulTransactionValidatorResult}; use crate::state_reader::{MempoolStateReader, StateReaderFactory}; -use crate::utils::{external_tx_to_account_tx, get_tx_hash}; +use crate::utils::{external_tx_to_account_tx, get_sender_address, get_tx_hash}; #[cfg(test)] #[path = "stateful_transaction_validator_test.rs"] @@ -27,12 +29,12 @@ impl StatefulTransactionValidator { state_reader_factory: &dyn StateReaderFactory, external_tx: &RPCTransaction, optional_class_info: Option, - deploy_account_tx_hash: Option, ) -> StatefulTransactionValidatorResult { // TODO(yael 6/5/2024): consider storing the block_info as part of the // StatefulTransactionValidator and update it only once a new block is created. let latest_block_info = get_latest_block_info(state_reader_factory)?; - let state_reader = state_reader_factory.get_state_reader(latest_block_info.block_number); + let latest_block_number = latest_block_info.block_number; + let state_reader = state_reader_factory.get_state_reader(latest_block_number); let state = CachedState::new(state_reader); let versioned_constants = VersionedConstants::latest_constants_with_overrides( self.config.validate_max_n_steps, @@ -52,23 +54,47 @@ impl StatefulTransactionValidator { &versioned_constants, ); - let mut validator = BlockifierStatefulValidator::create( - state, - block_context, - self.config.max_nonce_for_validation_skip, - BouncerConfig::max(), - ); + let mut validator = + BlockifierStatefulValidator::create(state, block_context, BouncerConfig::max()); let account_tx = external_tx_to_account_tx( external_tx, optional_class_info, &self.config.chain_info.chain_id, )?; let tx_hash = get_tx_hash(&account_tx); - validator.perform_validations(account_tx, deploy_account_tx_hash)?; + + let account_nonce = validator.get_nonce(get_sender_address(external_tx))?; + let skip_validate = skip_stateful_validations( + external_tx, + account_nonce, + self.config.max_nonce_for_validation_skip, + )?; + validator.perform_validations(account_tx, skip_validate)?; Ok(tx_hash) } } +// Check if validation of an invoke transaction should be skipped due to deploy_account not being +// proccessed yet. This feature is used to improve UX for users sending deploy_account + invoke at +// once. +fn skip_stateful_validations( + tx: &RPCTransaction, + account_nonce: Nonce, + max_nonce_for_validation_skip: Nonce, +) -> StatefulTransactionValidatorResult { + match tx { + RPCTransaction::Invoke(RPCInvokeTransaction::V3(tx)) => { + // check if the transaction nonce is bigger than 1, meaning it is post + // deploy_account, and the account nonce is zero, meaning the account was not deployed + // yet. The mempool also verifies that the deploy_account transaction exists. + Ok(tx.nonce >= Nonce(StarkFelt::ONE) + && tx.nonce <= max_nonce_for_validation_skip + && account_nonce == Nonce(StarkFelt::ZERO)) + } + RPCTransaction::DeployAccount(_) | RPCTransaction::Declare(_) => Ok(false), + } +} + pub fn get_latest_block_info( state_reader_factory: &dyn StateReaderFactory, ) -> StatefulTransactionValidatorResult { diff --git a/crates/gateway/src/stateful_transaction_validator_test.rs b/crates/gateway/src/stateful_transaction_validator_test.rs index d3821782..69e90d87 100644 --- a/crates/gateway/src/stateful_transaction_validator_test.rs +++ b/crates/gateway/src/stateful_transaction_validator_test.rs @@ -1,25 +1,44 @@ +use assert_matches::assert_matches; use blockifier::blockifier::stateful_validator::StatefulValidatorError; use blockifier::context::BlockContext; +use blockifier::test_utils::dict_state_reader::DictStateReader; use blockifier::test_utils::CairoVersion; use blockifier::transaction::errors::{TransactionFeeError, TransactionPreValidationError}; -use rstest::rstest; +use blockifier::transaction::test_utils::block_context; +use rstest::{fixture, rstest}; +use starknet_api::core::Nonce; use starknet_api::hash::StarkFelt; use starknet_api::rpc_transaction::RPCTransaction; use starknet_api::transaction::TransactionHash; +use test_utils::invoke_tx_args; use test_utils::starknet_api_test_utils::{ - declare_tx, deploy_account_tx, invoke_tx, VALID_L1_GAS_MAX_AMOUNT, + declare_tx, deploy_account_tx, external_invoke_tx, invoke_tx, VALID_L1_GAS_MAX_AMOUNT, VALID_L1_GAS_MAX_PRICE_PER_UNIT, }; +use test_utils::starknet_api_test_utils::TEST_SENDER_ADDRESS; +use starknet_api::core::ContractAddress; use crate::config::StatefulTransactionValidatorConfig; use crate::errors::{StatefulTransactionValidatorError, StatefulTransactionValidatorResult}; use crate::gateway::compile_contract_class; use crate::state_reader_test_utils::{ local_test_state_reader_factory, local_test_state_reader_factory_for_deploy_account, - TestStateReaderFactory, + TestStateReader, TestStateReaderFactory, }; use crate::stateful_transaction_validator::StatefulTransactionValidator; +#[fixture] +fn stateful_validator(block_context: BlockContext) -> StatefulTransactionValidator { + StatefulTransactionValidator { + config: StatefulTransactionValidatorConfig { + max_nonce_for_validation_skip: Default::default(), + validate_max_n_steps: block_context.versioned_constants().validate_max_n_steps, + max_recursion_depth: block_context.versioned_constants().max_recursion_depth, + chain_info: block_context.chain_info().clone().into(), + }, + } +} + #[rstest] #[case::valid_invoke_tx_cairo1( invoke_tx(CairoVersion::Cairo1), @@ -69,26 +88,72 @@ fn test_stateful_tx_validator( #[case] external_tx: RPCTransaction, #[case] state_reader_factory: TestStateReaderFactory, #[case] expected_result: StatefulTransactionValidatorResult, + stateful_validator: StatefulTransactionValidator, ) { - let block_context = &BlockContext::create_for_testing(); - let stateful_validator = StatefulTransactionValidator { - config: StatefulTransactionValidatorConfig { - max_nonce_for_validation_skip: Default::default(), - validate_max_n_steps: block_context.versioned_constants().validate_max_n_steps, - max_recursion_depth: block_context.versioned_constants().max_recursion_depth, - chain_info: block_context.chain_info().clone().into(), - }, - }; let optional_class_info = match &external_tx { RPCTransaction::Declare(declare_tx) => Some(compile_contract_class(declare_tx).unwrap()), _ => None, }; - let result = stateful_validator.run_validate( - &state_reader_factory, - &external_tx, - optional_class_info, - None, - ); + let result = + stateful_validator.run_validate(&state_reader_factory, &external_tx, optional_class_info); assert_eq!(format!("{:?}", result), format!("{:?}", expected_result)); } + +#[rstest] +#[case::should_skip_validation( + external_invoke_tx(invoke_tx_args!{nonce: Nonce(StarkFelt::ONE)}), + empty_state_reader_factory(), + true +)] +#[case::should_not_skip_validation_nonce_over_max_nonce_for_skip( + external_invoke_tx(invoke_tx_args!{nonce: Nonce(StarkFelt::TWO)}), + empty_state_reader_factory(), + false +)] +#[case::should_not_skip_validation_non_invoke( + deploy_account_tx(), + empty_state_reader_factory(), + false +)] +#[case::should_not_skip_validation_account_nonce_1( + external_invoke_tx(invoke_tx_args!{sender_address: ContractAddress::from(TEST_SENDER_ADDRESS), nonce: Nonce(StarkFelt::ONE)}), + state_reader_factory_account_nonce_1(ContractAddress::from(TEST_SENDER_ADDRESS)), + false +)] +fn test_skip_stateful_validation( + #[case] external_tx: RPCTransaction, + #[case] state_reader_factory: TestStateReaderFactory, + #[case] should_pass_validation: bool, + mut stateful_validator: StatefulTransactionValidator, +) { + // Enable the validator skip validation for nonce = 1. + stateful_validator.config.max_nonce_for_validation_skip = Nonce(StarkFelt::ONE); + + let result = stateful_validator.run_validate(&state_reader_factory, &external_tx, None); + if should_pass_validation { + assert_matches!(result, Ok(_)); + } else { + // To be sure that the validations were actually skipped, we check that the error came from + // the blockifier stateful validations, and not from the pre validations since those are + // executed also when skip_validate is true. + assert_matches!(result, Err(StatefulTransactionValidatorError::StatefulValidatorError(err)) + if !matches!(err, StatefulValidatorError::TransactionPreValidationError(_))); + } +} + +fn empty_state_reader_factory() -> TestStateReaderFactory { + let block_context = BlockContext::create_for_testing(); + TestStateReaderFactory { + state_reader: TestStateReader { + blockifier_state_reader: DictStateReader::default(), + block_info: block_context.block_info().clone(), + }, + } +} + +fn state_reader_factory_account_nonce_1(sender_address: ContractAddress) -> TestStateReaderFactory { + let mut state_reader_factory = empty_state_reader_factory(); + state_reader_factory.state_reader.blockifier_state_reader.address_to_nonce.insert(sender_address, Nonce(StarkFelt::ONE)); + state_reader_factory +} diff --git a/crates/test_utils/src/starknet_api_test_utils.rs b/crates/test_utils/src/starknet_api_test_utils.rs index d458a868..b264f8f4 100644 --- a/crates/test_utils/src/starknet_api_test_utils.rs +++ b/crates/test_utils/src/starknet_api_test_utils.rs @@ -28,6 +28,7 @@ use crate::{ pub const VALID_L1_GAS_MAX_AMOUNT: u64 = 203483; pub const VALID_L1_GAS_MAX_PRICE_PER_UNIT: u128 = 100000000000; +pub const TEST_SENDER_ADDRESS: u128 = 0x1000; // Utils. pub enum TransactionType {