From f737720877612813c52df7b7770053d9fafa42fd Mon Sep 17 00:00:00 2001 From: Arnon Hod Date: Wed, 15 Jan 2025 18:02:02 +0200 Subject: [PATCH] feat(starknet_gateway): use contains_tx_from to skip validate (#3294) --- crates/starknet_gateway/src/gateway.rs | 9 ++- .../src/stateful_transaction_validator.rs | 25 ++++++-- .../stateful_transaction_validator_test.rs | 63 ++++++++++++++----- 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/crates/starknet_gateway/src/gateway.rs b/crates/starknet_gateway/src/gateway.rs index 8bb153a4e7..af7788b0a5 100644 --- a/crates/starknet_gateway/src/gateway.rs +++ b/crates/starknet_gateway/src/gateway.rs @@ -93,6 +93,7 @@ struct ProcessTxBlockingTask { stateful_tx_validator: Arc, state_reader_factory: Arc, gateway_compiler: Arc, + mempool_client: SharedMempoolClient, chain_info: ChainInfo, tx: RpcTransaction, } @@ -104,6 +105,7 @@ impl ProcessTxBlockingTask { stateful_tx_validator: gateway.stateful_tx_validator.clone(), state_reader_factory: gateway.state_reader_factory.clone(), gateway_compiler: gateway.gateway_compiler.clone(), + mempool_client: gateway.mempool_client.clone(), chain_info: gateway.chain_info.clone(), tx, } @@ -137,7 +139,12 @@ impl ProcessTxBlockingTask { GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() } })?; - self.stateful_tx_validator.run_validate(&executable_tx, nonce, validator)?; + self.stateful_tx_validator.run_validate( + &executable_tx, + nonce, + self.mempool_client, + validator, + )?; // TODO(Arni): Add the Sierra and the Casm to the mempool input. Ok(AddTransactionArgs { tx: executable_tx, account_state: AccountState { address, nonce } }) diff --git a/crates/starknet_gateway/src/stateful_transaction_validator.rs b/crates/starknet_gateway/src/stateful_transaction_validator.rs index 1729d34220..c20568c5d9 100644 --- a/crates/starknet_gateway/src/stateful_transaction_validator.rs +++ b/crates/starknet_gateway/src/stateful_transaction_validator.rs @@ -8,6 +8,7 @@ use blockifier::state::cached_state::CachedState; use blockifier::transaction::account_transaction::{AccountTransaction, ExecutionFlags}; use blockifier::transaction::transactions::enforce_fee; use blockifier::versioned_constants::VersionedConstants; +use futures::executor::block_on; #[cfg(test)] use mockall::automock; use starknet_api::block::BlockInfo; @@ -17,6 +18,7 @@ use starknet_api::executable_transaction::{ InvokeTransaction as ExecutableInvokeTransaction, }; use starknet_gateway_types::errors::GatewaySpecError; +use starknet_mempool_types::communication::SharedMempoolClient; use starknet_types_core::felt::Felt; use tracing::error; @@ -59,9 +61,11 @@ impl StatefulTransactionValidator { &self, executable_tx: &ExecutableTransaction, account_nonce: Nonce, + mempool_client: SharedMempoolClient, mut validator: V, ) -> StatefulTransactionValidatorResult<()> { - let skip_validate = skip_stateful_validations(executable_tx, account_nonce); + let skip_validate = + skip_stateful_validations(executable_tx, account_nonce, mempool_client)?; let only_query = false; let charge_fee = enforce_fee(executable_tx, only_query); let execution_flags = ExecutionFlags { only_query, charge_fee, validate: !skip_validate }; @@ -104,15 +108,26 @@ impl StatefulTransactionValidator { /// Check if validation of an invoke transaction should be skipped due to deploy_account not being /// processed yet. This feature is used to improve UX for users sending deploy_account + invoke at /// once. -fn skip_stateful_validations(tx: &ExecutableTransaction, account_nonce: Nonce) -> bool { +fn skip_stateful_validations( + tx: &ExecutableTransaction, + account_nonce: Nonce, + mempool_client: SharedMempoolClient, +) -> StatefulTransactionValidatorResult { if let ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx, .. }) = tx { - // TODO(Arni): Add a verification that there is a deploy_account transaction in the mempool. // check if the transaction nonce is 1, meaning it is post deploy_account, and the // account nonce is zero, meaning the account was not deployed yet. - return tx.nonce() == Nonce(Felt::ONE) && account_nonce == Nonce(Felt::ZERO); + if tx.nonce() == Nonce(Felt::ONE) && account_nonce == Nonce(Felt::ZERO) { + // We verify that a deploy_account transaction exists for this account. It is sufficient + // to check if the account exists in the mempool since it means that either it has a + // deploy_account transaction or transactions with future nonces that passed + // validations. + return block_on(mempool_client.contains_tx_from(tx.sender_address())) + // TODO(Arni): consider using mempool_client_result_to_gw_spec_result for error handling. + .map_err(|err| GatewaySpecError::UnexpectedError { data: err.to_string() }); + } } - false + Ok(false) } pub fn get_latest_block_info( diff --git a/crates/starknet_gateway/src/stateful_transaction_validator_test.rs b/crates/starknet_gateway/src/stateful_transaction_validator_test.rs index 366ccd4431..61c6a66fd6 100644 --- a/crates/starknet_gateway/src/stateful_transaction_validator_test.rs +++ b/crates/starknet_gateway/src/stateful_transaction_validator_test.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use blockifier::blockifier::stateful_validator::{ StatefulValidatorError as BlockifierStatefulValidatorError, StatefulValidatorResult as BlockifierStatefulValidatorResult, @@ -18,13 +20,13 @@ use starknet_api::block::GasPrice; use starknet_api::core::Nonce; use starknet_api::executable_transaction::AccountTransaction; use starknet_api::execution_resources::GasAmount; -use starknet_api::test_utils::declare::TEST_SENDER_ADDRESS; use starknet_api::test_utils::deploy_account::executable_deploy_account_tx; use starknet_api::test_utils::invoke::executable_invoke_tx; use starknet_api::test_utils::NonceManager; use starknet_api::transaction::fields::Resource; use starknet_api::{deploy_account_tx_args, invoke_tx_args, nonce}; use starknet_gateway_types::errors::GatewaySpecError; +use starknet_mempool_types::communication::MockMempoolClient; use crate::config::StatefulTransactionValidatorConfig; use crate::state_reader::{MockStateReaderFactory, StateReaderFactory}; @@ -77,7 +79,19 @@ fn test_stateful_tx_validator( mock_validator.expect_validate().return_once(|_, _| expected_result.map(|_| ())); let account_nonce = nonce!(0); - let result = stateful_validator.run_validate(&executable_tx, account_nonce, mock_validator); + let mut mock_mempool_client = MockMempoolClient::new(); + mock_mempool_client.expect_contains_tx_from().returning(|_| { + // The mempool does not have any transactions from the sender. + Ok(false) + }); + let mempool_client = Arc::new(mock_mempool_client); + + let result = stateful_validator.run_validate( + &executable_tx, + account_nonce, + mempool_client, + mock_validator, + ); assert_eq!(result, expected_result_as_stateful_transaction_result); } @@ -112,32 +126,44 @@ fn test_instantiate_validator(stateful_validator: StatefulTransactionValidator) #[case::should_skip_validation( executable_invoke_tx(invoke_tx_args!(nonce: nonce!(1))), nonce!(0), + true, true )] -#[case::should_not_skip_validation_nonce_over_max_nonce_for_skip( +#[case::should_not_skip_validation_nonce_zero( executable_invoke_tx(invoke_tx_args!(nonce: nonce!(0))), nonce!(0), + true, + false +)] +#[case::should_not_skip_validation_nonce_over_one( + executable_invoke_tx(invoke_tx_args!(nonce: nonce!(2))), + nonce!(0), + true, false )] #[case::should_not_skip_validation_non_invoke( - executable_deploy_account_tx(deploy_account_tx_args!(), &mut NonceManager::default()) - , + executable_deploy_account_tx(deploy_account_tx_args!(), &mut NonceManager::default()), nonce!(0), - false) -] + true, + false + +)] #[case::should_not_skip_validation_account_nonce_1( - executable_invoke_tx( - invoke_tx_args!( - nonce: nonce!(1), - sender_address: TEST_SENDER_ADDRESS.into() - ) - ), + executable_invoke_tx(invoke_tx_args!(nonce: nonce!(1))), nonce!(1), + true, + false +)] +#[case::should_not_skip_validation_no_tx_in_mempool( + executable_invoke_tx(invoke_tx_args!(nonce: nonce!(1))), + nonce!(0), + false, false )] fn test_skip_stateful_validation( #[case] executable_tx: AccountTransaction, #[case] sender_nonce: Nonce, + #[case] contains_tx: bool, #[case] should_skip_validate: bool, stateful_validator: StatefulTransactionValidator, ) { @@ -146,5 +172,14 @@ fn test_skip_stateful_validation( .expect_validate() .withf(move |_, skip_validate| *skip_validate == should_skip_validate) .returning(|_, _| Ok(())); - let _ = stateful_validator.run_validate(&executable_tx, sender_nonce, mock_validator); + let mut mock_mempool_client = MockMempoolClient::new(); + mock_mempool_client.expect_contains_tx_from().returning(move |_| Ok(contains_tx)); + let mempool_client = Arc::new(mock_mempool_client); + + let _ = stateful_validator.run_validate( + &executable_tx, + sender_nonce, + mempool_client, + mock_validator, + ); }