From 7dcaf90ada65966cf8352e3155ef72f86e7a1333 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 | 11 +- .../src/stateful_transaction_validator.rs | 27 ++--- .../stateful_transaction_validator_test.rs | 32 ++--- crates/gateway/src/utils.rs | 109 ++++-------------- 5 files changed, 54 insertions(+), 127 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 3460323bec8..81848904474 100644 --- a/crates/gateway/src/gateway.rs +++ b/crates/gateway/src/gateway.rs @@ -134,20 +134,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(&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 1b8ae35ceda..72bddcb7da2 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::{external_tx_to_account_tx, get_sender_address}; +use crate::utils::{executable_transaction_to_account_tx, get_sender_address}; #[cfg(test)] #[path = "stateful_transaction_validator_test.rs"] @@ -69,22 +71,17 @@ impl StatefulTransactionValidator { // conversion is also relevant for the Mempool. pub fn run_validate( &self, - external_tx: &RpcTransaction, - optional_class_info: Option, + executable_tx: &ExecutableTransaction, mut validator: V, ) -> StatefulTransactionValidatorResult { - let account_tx = external_tx_to_account_tx( - external_tx, - optional_class_info, - &self.config.chain_info.chain_id, - )?; + let account_tx = executable_transaction_to_account_tx(executable_tx)?; 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(external_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() })?; @@ -122,15 +119,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 525285d7d88..07918ed1d6c 100644 --- a/crates/gateway/src/stateful_transaction_validator_test.rs +++ b/crates/gateway/src/stateful_transaction_validator_test.rs @@ -3,7 +3,6 @@ use blockifier::blockifier::stateful_validator::{ StatefulValidatorResult as BlockifierStatefulValidatorResult, }; use blockifier::context::BlockContext; -use blockifier::execution::contract_class::ClassInfo; use blockifier::test_utils::CairoVersion; use blockifier::transaction::errors::{TransactionFeeError, TransactionPreValidationError}; use mempool_test_utils::invoke_tx_args; @@ -36,6 +35,7 @@ use crate::stateful_transaction_validator::{ MockStatefulTransactionValidatorTrait, StatefulTransactionValidator, }; +use crate::utils::external_tx_to_executable_tx; pub const STATEFUL_VALIDATOR_FEE_ERROR: BlockifierStatefulValidatorError = BlockifierStatefulValidatorError::TransactionPreValidationError( @@ -82,17 +82,14 @@ fn test_stateful_tx_validator( #[case] expected_result: BlockifierStatefulValidatorResult, stateful_validator: StatefulTransactionValidator, ) { - let optional_class_info = match &external_tx { - RpcTransaction::Declare(declare_tx) => Some( - ClassInfo::try_from( - GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default()) - .process_declare_tx(declare_tx) - .unwrap(), - ) - .unwrap(), - ), - _ => None, - }; + let gateway_compiler = + GatewayCompiler::new_cairo_lang_compiler(SierraToCasmCompilationConfig::default()); + let executable_tx = external_tx_to_executable_tx( + &external_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| { @@ -103,7 +100,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(&external_tx, optional_class_info, mock_validator); + let result = stateful_validator.run_validate(&executable_tx, mock_validator); assert_eq!(result, expected_result_as_stateful_transaction_result); } @@ -164,6 +161,13 @@ fn test_skip_stateful_validation( #[case] should_skip_validate: bool, stateful_validator: StatefulTransactionValidator, ) { + let executable_tx = external_tx_to_executable_tx( + &external_tx, + &GatewayCompiler::new_cairo_lang_compiler(Default::default()), + &stateful_validator.config.chain_info.chain_id, + ) + .unwrap(); + let sender_address = external_tx.calculate_sender_address().unwrap(); let mut mock_validator = MockStatefulTransactionValidatorTrait::new(); mock_validator @@ -174,5 +178,5 @@ fn test_skip_stateful_validation( .expect_validate() .withf(move |_, skip_validate| *skip_validate == should_skip_validate) .returning(|_, _| Ok(())); - let _ = stateful_validator.run_validate(&external_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 d63c7644e10..07b9e199324 100644 --- a/crates/gateway/src/utils.rs +++ b/crates/gateway/src/utils.rs @@ -1,4 +1,3 @@ -use blockifier::execution::contract_class::ClassInfo; use blockifier::transaction::account_transaction::AccountTransaction; use blockifier::transaction::transactions::{ DeclareTransaction as BlockifierDeclareTransaction, @@ -29,7 +28,7 @@ use starknet_api::transaction::{ use tracing::error; use crate::compilation::GatewayCompiler; -use crate::errors::{GatewaySpecError, StatefulTransactionValidatorResult}; +use crate::errors::GatewaySpecError; // Consider splitting this function into multiple smaller functions. pub fn external_tx_to_executable_tx( @@ -123,100 +122,34 @@ pub fn external_tx_to_executable_tx( }) } -// TODO(Arni): Delete this function. -pub fn external_tx_to_account_tx( - external_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 external_tx { - RpcTransaction::Declare(RpcDeclareTransaction::V3(tx)) => { - let declare_tx = DeclareTransaction::V3(DeclareTransactionV3 { - class_hash: ClassHash::default(), /* FIXME(yael 15/4/24): call the starknet-api - * function once ready */ - resource_bounds: tx.resource_bounds.clone().into(), - tip: tx.tip, - signature: tx.signature.clone(), - nonce: tx.nonce, - compiled_class_hash: tx.compiled_class_hash, - sender_address: tx.sender_address, - nonce_data_availability_mode: tx.nonce_data_availability_mode, - fee_data_availability_mode: tx.fee_data_availability_mode, - paymaster_data: tx.paymaster_data.clone(), - account_deployment_data: tx.account_deployment_data.clone(), - }); - 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); +// TODO(Arni): Move this function into the blockifier - use a try into. This is exactly the code +// dedup we are working on. +pub fn executable_transaction_to_account_tx( + tx: &ExecutableTransaction, +) -> Result { + match tx { + ExecutableTransaction::Declare(declare_tx) => { + let declare_tx = + BlockifierDeclareTransaction::try_from(declare_tx.clone()).map_err(|error| { + error!("Failed to convert declare tx: {}", error); GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() } })?; Ok(AccountTransaction::Declare(declare_tx)) } - RpcTransaction::DeployAccount(RpcDeployAccountTransaction::V3(tx)) => { - let deploy_account_tx = DeployAccountTransaction::V3(DeployAccountTransactionV3 { - resource_bounds: tx.resource_bounds.clone().into(), - tip: tx.tip, - signature: tx.signature.clone(), - nonce: tx.nonce, - class_hash: tx.class_hash, - contract_address_salt: tx.contract_address_salt, - constructor_calldata: tx.constructor_calldata.clone(), - nonce_data_availability_mode: tx.nonce_data_availability_mode, - fee_data_availability_mode: tx.fee_data_availability_mode, - paymaster_data: tx.paymaster_data.clone(), - }); - 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() } - })?; + ExecutableTransaction::DeployAccount(ExecutableDeployAccountTransaction { + tx: deploy_account_tx, + tx_hash, + contract_address, + }) => { let deploy_account_tx = BlockifierDeployAccountTransaction::new( - deploy_account_tx, - tx_hash, - contract_address, + deploy_account_tx.clone(), + *tx_hash, + *contract_address, ); Ok(AccountTransaction::DeployAccount(deploy_account_tx)) } - RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx)) => { - let invoke_tx = starknet_api::transaction::InvokeTransaction::V3(InvokeTransactionV3 { - resource_bounds: tx.resource_bounds.clone().into(), - tip: tx.tip, - signature: tx.signature.clone(), - nonce: tx.nonce, - sender_address: tx.sender_address, - calldata: tx.calldata.clone(), - nonce_data_availability_mode: tx.nonce_data_availability_mode, - fee_data_availability_mode: tx.fee_data_availability_mode, - paymaster_data: tx.paymaster_data.clone(), - account_deployment_data: tx.account_deployment_data.clone(), - }); - 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); + ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx: invoke_tx, tx_hash }) => { + let invoke_tx = BlockifierInvokeTransaction::new(invoke_tx.clone(), *tx_hash); Ok(AccountTransaction::Invoke(invoke_tx)) } }