From d7ba348dbc3ae170be599cbd4461f43512ef60b2 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Thu, 15 Aug 2024 17:11:35 +0300 Subject: [PATCH] refactor: refactor the stateful tx validator to get executable tx as input --- crates/gateway/src/config.rs | 2 + crates/gateway/src/gateway.rs | 12 +-- .../src/stateful_transaction_validator.rs | 31 ++++--- .../stateful_transaction_validator_test.rs | 23 +++++- crates/gateway/src/utils.rs | 81 +------------------ 5 files changed, 46 insertions(+), 103 deletions(-) diff --git a/crates/gateway/src/config.rs b/crates/gateway/src/config.rs index 87543e2da8a..a6e2f1920c7 100644 --- a/crates/gateway/src/config.rs +++ b/crates/gateway/src/config.rs @@ -169,6 +169,8 @@ pub struct StatefulTransactionValidatorConfig { pub max_nonce_for_validation_skip: Nonce, pub validate_max_n_steps: u32, pub max_recursion_depth: usize, + // TODO(Arni): Move this member out of the stateful transaction validator config. Move it into + // the gateway config. This is used during the transalation from external_tx to executable_tx. pub chain_info: ChainInfo, } diff --git a/crates/gateway/src/gateway.rs b/crates/gateway/src/gateway.rs index f474160fb98..a19c24ddd4f 100644 --- a/crates/gateway/src/gateway.rs +++ b/crates/gateway/src/gateway.rs @@ -137,21 +137,11 @@ fn process_tx( &gateway_compiler, &stateful_tx_validator.config.chain_info.chain_id, )?; - let optional_class_info = match executable_tx { - starknet_api::executable_transaction::Transaction::Declare(tx) => { - Some(tx.class_info.try_into().map_err(|e| { - error!("Failed to convert Starknet API ClassInfo to Blockifier ClassInfo: {:?}", e); - GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() } - })?) - } - _ => None, - }; let validator = stateful_tx_validator.instantiate_validator(state_reader_factory)?; // TODO(Yael 31/7/24): refactor after IntrnalTransaction is ready, delete validate_info and // compute all the info outside of run_validate. - let validate_info = - stateful_tx_validator.run_validate(©_of_rpc_tx, optional_class_info, validator)?; + let validate_info = stateful_tx_validator.run_validate(&executable_tx, validator)?; // TODO(Arni): Add the Sierra and the Casm to the mempool input. Ok(MempoolInput { diff --git a/crates/gateway/src/stateful_transaction_validator.rs b/crates/gateway/src/stateful_transaction_validator.rs index 8b174fdc6b9..7ee942bcd5e 100644 --- a/crates/gateway/src/stateful_transaction_validator.rs +++ b/crates/gateway/src/stateful_transaction_validator.rs @@ -5,14 +5,16 @@ use blockifier::blockifier::stateful_validator::{ }; use blockifier::bouncer::BouncerConfig; use blockifier::context::BlockContext; -use blockifier::execution::contract_class::ClassInfo; use blockifier::state::cached_state::CachedState; use blockifier::transaction::account_transaction::AccountTransaction; use blockifier::versioned_constants::VersionedConstants; #[cfg(test)] use mockall::automock; use starknet_api::core::{ContractAddress, Nonce}; -use starknet_api::rpc_transaction::{RpcInvokeTransaction, RpcTransaction}; +use starknet_api::executable_transaction::{ + InvokeTransaction as ExecutableInvokeTransaction, + Transaction as ExecutableTransaction, +}; use starknet_api::transaction::TransactionHash; use starknet_types_core::felt::Felt; use tracing::error; @@ -20,7 +22,7 @@ use tracing::error; use crate::config::StatefulTransactionValidatorConfig; use crate::errors::{GatewaySpecError, StatefulTransactionValidatorResult}; use crate::state_reader::{MempoolStateReader, StateReaderFactory}; -use crate::utils::{get_sender_address, rpc_tx_to_account_tx}; +use crate::utils::get_sender_address; #[cfg(test)] #[path = "stateful_transaction_validator_test.rs"] @@ -69,19 +71,24 @@ impl StatefulTransactionValidator { // conversion is also relevant for the Mempool. pub fn run_validate( &self, - rpc_tx: &RpcTransaction, - optional_class_info: Option, + executable_tx: &ExecutableTransaction, mut validator: V, ) -> StatefulTransactionValidatorResult { - let account_tx = - rpc_tx_to_account_tx(rpc_tx, optional_class_info, &self.config.chain_info.chain_id)?; + let account_tx = AccountTransaction::try_from( + // TODO(Arni): create a try_from for &ExecutableTransaction. + executable_tx.clone(), + ) + .map_err(|error| { + error!("Failed to convert executable transaction into account transaction: {}", error); + GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } + })?; let tx_hash = account_tx.tx_hash(); let sender_address = get_sender_address(&account_tx); let account_nonce = validator.get_nonce(sender_address).map_err(|e| { error!("Failed to get nonce for sender address {}: {}", sender_address, e); GatewaySpecError::UnexpectedError { data: "Internal server error.".to_owned() } })?; - let skip_validate = skip_stateful_validations(rpc_tx, account_nonce); + let skip_validate = skip_stateful_validations(executable_tx, account_nonce); validator .validate(account_tx, skip_validate) .map_err(|err| GatewaySpecError::ValidationFailure { data: err.to_string() })?; @@ -119,15 +126,15 @@ impl StatefulTransactionValidator { // 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) -> bool { +fn skip_stateful_validations(tx: &ExecutableTransaction, account_nonce: Nonce) -> bool { match tx { - RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx)) => { + ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx, .. }) => { // 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. The mempool also // verifies that the deploy_account transaction exists. - tx.nonce == Nonce(Felt::ONE) && account_nonce == Nonce(Felt::ZERO) + tx.nonce() == Nonce(Felt::ONE) && account_nonce == Nonce(Felt::ZERO) } - RpcTransaction::DeployAccount(_) | RpcTransaction::Declare(_) => false, + ExecutableTransaction::DeployAccount(_) | ExecutableTransaction::Declare(_) => false, } } diff --git a/crates/gateway/src/stateful_transaction_validator_test.rs b/crates/gateway/src/stateful_transaction_validator_test.rs index cc871e49334..19078167f6d 100644 --- a/crates/gateway/src/stateful_transaction_validator_test.rs +++ b/crates/gateway/src/stateful_transaction_validator_test.rs @@ -22,9 +22,11 @@ use starknet_api::core::{ContractAddress, Nonce, PatriciaKey}; use starknet_api::rpc_transaction::RpcTransaction; use starknet_api::transaction::TransactionHash; use starknet_api::{contract_address, felt, patricia_key}; +use starknet_sierra_compile::config::SierraToCasmCompilationConfig; use starknet_types_core::felt::Felt; use super::ValidateInfo; +use crate::compilation::GatewayCompiler; use crate::config::StatefulTransactionValidatorConfig; use crate::errors::GatewaySpecError; use crate::state_reader::{MockStateReaderFactory, StateReaderFactory}; @@ -33,6 +35,7 @@ use crate::stateful_transaction_validator::{ MockStatefulTransactionValidatorTrait, StatefulTransactionValidator, }; +use crate::utils::compile_to_casm_and_convert_rpc_to_executable_tx; pub const STATEFUL_VALIDATOR_FEE_ERROR: BlockifierStatefulValidatorError = BlockifierStatefulValidatorError::TransactionPreValidationError( @@ -80,6 +83,15 @@ fn test_stateful_tx_validator( #[case] expected_result: BlockifierStatefulValidatorResult, stateful_validator: StatefulTransactionValidator, ) { + let gateway_compiler = + GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default()); + let executable_tx = compile_to_casm_and_convert_rpc_to_executable_tx( + rpc_tx, + &gateway_compiler, + &stateful_validator.config.chain_info.chain_id, + ) + .unwrap(); + let expected_result_as_stateful_transaction_result = expected_result.as_ref().map(|validate_info| *validate_info).map_err(|blockifier_error| { GatewaySpecError::ValidationFailure { data: blockifier_error.to_string() } @@ -89,7 +101,7 @@ fn test_stateful_tx_validator( mock_validator.expect_validate().return_once(|_, _| expected_result.map(|_| ())); mock_validator.expect_get_nonce().returning(|_| Ok(Nonce(Felt::ZERO))); - let result = stateful_validator.run_validate(&rpc_tx, None, mock_validator); + let result = stateful_validator.run_validate(&executable_tx, mock_validator); assert_eq!(result, expected_result_as_stateful_transaction_result); } @@ -151,6 +163,13 @@ fn test_skip_stateful_validation( stateful_validator: StatefulTransactionValidator, ) { let sender_address = rpc_tx.calculate_sender_address().unwrap(); + let executable_tx = compile_to_casm_and_convert_rpc_to_executable_tx( + rpc_tx, + &GatewayCompiler::new_cairo_lang_compiler(Default::default()), + &stateful_validator.config.chain_info.chain_id, + ) + .unwrap(); + let mut mock_validator = MockStatefulTransactionValidatorTrait::new(); mock_validator .expect_get_nonce() @@ -160,5 +179,5 @@ fn test_skip_stateful_validation( .expect_validate() .withf(move |_, skip_validate| *skip_validate == should_skip_validate) .returning(|_, _| Ok(())); - let _ = stateful_validator.run_validate(&rpc_tx, None, mock_validator); + let _ = stateful_validator.run_validate(&executable_tx, mock_validator); } diff --git a/crates/gateway/src/utils.rs b/crates/gateway/src/utils.rs index 8de9e92bc03..e48bb8312cc 100644 --- a/crates/gateway/src/utils.rs +++ b/crates/gateway/src/utils.rs @@ -1,27 +1,16 @@ -use blockifier::execution::contract_class::ClassInfo; use blockifier::transaction::account_transaction::AccountTransaction; -use blockifier::transaction::transactions::{ - DeclareTransaction as BlockifierDeclareTransaction, - DeployAccountTransaction as BlockifierDeployAccountTransaction, - InvokeTransaction as BlockifierInvokeTransaction, -}; -use starknet_api::core::{calculate_contract_address, ChainId, ContractAddress}; +use starknet_api::core::{ChainId, ContractAddress}; use starknet_api::executable_transaction::{ DeployAccountTransaction as ExecutableDeployAccountTransaction, InvokeTransaction as ExecutableInvokeTransaction, Transaction as ExecutableTransaction, }; use starknet_api::rpc_transaction::RpcTransaction; -use starknet_api::transaction::{ - DeclareTransaction, - DeployAccountTransaction, - InvokeTransaction, - TransactionHasher, -}; +use starknet_api::transaction::DeclareTransaction; use tracing::error; use crate::compilation::GatewayCompiler; -use crate::errors::{GatewaySpecError, StatefulTransactionValidatorResult}; +use crate::errors::GatewaySpecError; /// Converts an RPC transaction to an executable transaction. /// This conversion is dependent on the chain ID. @@ -69,70 +58,6 @@ pub fn compile_to_casm_and_convert_rpc_to_executable_tx( }) } -// TODO(Arni): Remove this function. Replace with a function that take ownership of RpcTransaction. -pub fn rpc_tx_to_account_tx( - rpc_tx: &RpcTransaction, - // FIXME(yael 15/4/24): calculate class_info inside the function once compilation code is ready - optional_class_info: Option, - chain_id: &ChainId, -) -> StatefulTransactionValidatorResult { - match rpc_tx { - RpcTransaction::Declare(tx) => { - let declare_tx: DeclareTransaction = tx.clone().into(); - let tx_hash = declare_tx - .calculate_transaction_hash(chain_id, &declare_tx.version()) - .map_err(|e| { - error!("Failed to calculate tx hash: {}", e); - GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } - })?; - let class_info = - optional_class_info.expect("declare transaction should contain class info"); - let declare_tx = BlockifierDeclareTransaction::new(declare_tx, tx_hash, class_info) - .map_err(|e| { - error!("Failed to convert declare tx hash to blockifier tx type: {}", e); - GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } - })?; - Ok(AccountTransaction::Declare(declare_tx)) - } - RpcTransaction::DeployAccount(tx) => { - let deploy_account_tx: DeployAccountTransaction = tx.clone().into(); - let contract_address = calculate_contract_address( - deploy_account_tx.contract_address_salt(), - deploy_account_tx.class_hash(), - &deploy_account_tx.constructor_calldata(), - ContractAddress::default(), - ) - .map_err(|e| { - error!("Failed to calculate contract address: {}", e); - GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } - })?; - let tx_hash = deploy_account_tx - .calculate_transaction_hash(chain_id, &deploy_account_tx.version()) - .map_err(|e| { - error!("Failed to calculate tx hash: {}", e); - GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } - })?; - let deploy_account_tx = BlockifierDeployAccountTransaction::new( - deploy_account_tx, - tx_hash, - contract_address, - ); - Ok(AccountTransaction::DeployAccount(deploy_account_tx)) - } - RpcTransaction::Invoke(tx) => { - let invoke_tx: InvokeTransaction = tx.clone().into(); - let tx_hash = invoke_tx - .calculate_transaction_hash(chain_id, &invoke_tx.version()) - .map_err(|e| { - error!("Failed to calculate tx hash: {}", e); - GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } - })?; - let invoke_tx = BlockifierInvokeTransaction::new(invoke_tx, tx_hash); - Ok(AccountTransaction::Invoke(invoke_tx)) - } - } -} - // TODO(yael 9/5/54): Should be implemented as part of InternalTransaction in starknet-api pub fn get_sender_address(tx: &AccountTransaction) -> ContractAddress { match tx {