Skip to content

Commit

Permalink
refactor: refactor the stateful tx validator to get executable tx as …
Browse files Browse the repository at this point in the history
…input
  • Loading branch information
ArniStarkware committed Aug 19, 2024
1 parent 74144ec commit 8fd60c6
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 45 deletions.
2 changes: 2 additions & 0 deletions crates/gateway/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
4 changes: 2 additions & 2 deletions crates/gateway/src/gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn process_tx(
// Perform stateless validations.
stateless_tx_validator.validate(&tx)?;

let _executable_tx = external_tx_to_executable_tx(
let executable_tx = external_tx_to_executable_tx(
&tx,
&gateway_compiler,
&stateful_tx_validator.config.chain_info.chain_id,
Expand All @@ -137,7 +137,7 @@ fn process_tx(
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, &gateway_compiler, 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 {
Expand Down
27 changes: 12 additions & 15 deletions crates/gateway/src/stateful_transaction_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@ 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;

use crate::compilation::GatewayCompiler;
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"]
Expand Down Expand Up @@ -67,22 +69,17 @@ impl StatefulTransactionValidatorTrait for BlockifierStatefulValidator {
impl StatefulTransactionValidator {
pub fn run_validate<V: StatefulTransactionValidatorTrait>(
&self,
external_tx: &RpcTransaction,
gateway_compiler: &GatewayCompiler,
executable_tx: &ExecutableTransaction,
mut validator: V,
) -> StatefulTransactionValidatorResult<ValidateInfo> {
let account_tx = external_tx_to_account_tx(
external_tx,
gateway_compiler,
&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() })?;
Expand Down Expand Up @@ -120,15 +117,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,
}
}

Expand Down
22 changes: 16 additions & 6 deletions crates/gateway/src/stateful_transaction_validator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,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(
Expand Down Expand Up @@ -83,6 +84,12 @@ fn test_stateful_tx_validator(
) {
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| {
Expand All @@ -93,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, &gateway_compiler, mock_validator);
let result = stateful_validator.run_validate(&executable_tx, mock_validator);
assert_eq!(result, expected_result_as_stateful_transaction_result);
}

Expand Down Expand Up @@ -154,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
Expand All @@ -164,9 +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,
&GatewayCompiler::new_cairo_lang_compiler(Default::default()),
mock_validator,
);
let _ = stateful_validator.run_validate(&executable_tx, mock_validator);
}
29 changes: 7 additions & 22 deletions crates/gateway/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use starknet_mempool_types::mempool_types::ThinTransaction;
use tracing::error;

use crate::compilation::GatewayCompiler;
use crate::errors::{GatewaySpecError, StatefulTransactionValidatorResult};
use crate::errors::GatewaySpecError;

pub fn external_tx_to_thin_tx(
external_tx: &RpcTransaction,
Expand Down Expand Up @@ -140,12 +140,12 @@ pub fn external_tx_to_executable_tx(
// 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,
tx: &ExecutableTransaction,
) -> Result<AccountTransaction, GatewaySpecError> {
match tx {
ExecutableTransaction::Declare(declare_tx) => {
let declare_tx =
BlockifierDeclareTransaction::try_from(declare_tx).map_err(|error| {
BlockifierDeclareTransaction::try_from(declare_tx.clone()).map_err(|error| {
error!("Failed to convert declare tx: {}", error);
GatewaySpecError::UnexpectedError { data: "Internal server error".to_owned() }
})?;
Expand All @@ -157,34 +157,19 @@ pub fn executable_transaction_to_account_tx(
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))
}
ExecutableTransaction::Invoke(ExecutableInvokeTransaction { tx: invoke_tx, tx_hash }) => {
let invoke_tx = BlockifierInvokeTransaction::new(invoke_tx, tx_hash);
let invoke_tx = BlockifierInvokeTransaction::new(invoke_tx.clone(), *tx_hash);
Ok(AccountTransaction::Invoke(invoke_tx))
}
}
}

// TODO(Arni): Remove this function.
pub fn external_tx_to_account_tx(
external_tx: &RpcTransaction,
gateway_compiler: &GatewayCompiler,
chain_id: &ChainId,
) -> StatefulTransactionValidatorResult<AccountTransaction> {
let executable_tx = external_tx_to_executable_tx(
external_tx,
// WARNING: this is bad design.
gateway_compiler,
chain_id,
)?;
executable_transaction_to_account_tx(executable_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 {
Expand Down

0 comments on commit 8fd60c6

Please sign in to comment.