Skip to content

Commit

Permalink
feat(starknet_gateway): use has tx from address to skip validate
Browse files Browse the repository at this point in the history
  • Loading branch information
ArniStarkware committed Jan 13, 2025
1 parent 7af75d6 commit fabe1b6
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 32 deletions.
13 changes: 8 additions & 5 deletions crates/starknet_gateway/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,14 @@ impl From<StatelessTransactionValidatorError> for GatewaySpecError {

/// Converts a mempool client result to a gateway result. Some errors variants are unreachable in
/// Gateway context, and some are not considered errors from the gateway's perspective.
pub fn mempool_client_result_to_gw_spec_result(
value: MempoolClientResult<()>,
) -> GatewayResult<()> {
/// If the result is a mempool client error that is not considered an error from the gateway's
/// perspective, the function returns `value_on_repressed_error`.
pub fn mempool_client_result_to_gw_spec_result<T>(
value: MempoolClientResult<T>,
value_on_repressed_error: T,
) -> GatewayResult<T> {
let err = match value {
Ok(()) => return Ok(()),
Ok(value) => return Ok(value),
Err(err) => err,
};
match err {
Expand All @@ -104,7 +107,7 @@ pub fn mempool_client_result_to_gw_spec_result(
MempoolError::P2pPropagatorClientError { .. } => {
// Not an error from the gateway's perspective.
warn!("P2p propagator client error: {}", mempool_error);
Ok(())
Ok(value_on_repressed_error)
}
MempoolError::TransactionNotFound { .. } => {
// This error is not expected to happen within the gateway, only from other
Expand Down
12 changes: 8 additions & 4 deletions crates/starknet_gateway/src/gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ impl Gateway {
) -> GatewayResult<TransactionHash> {
info!("Processing tx");
let process_tx_param = ProcessTxParams::new(self, tx);
let add_tx_args = process_tx_param.process_tx()?;
let add_tx_args = process_tx_param.process_tx().await?;

let tx_hash = add_tx_args.tx.tx_hash();

let add_tx_args = AddTransactionArgsWrapper { args: add_tx_args, p2p_message_metadata };
mempool_client_result_to_gw_spec_result(self.mempool_client.add_tx(add_tx_args).await)?;
mempool_client_result_to_gw_spec_result(self.mempool_client.add_tx(add_tx_args).await, ())?;
// TODO: Also return `ContractAddress` for deploy and `ClassHash` for Declare.
Ok(tx_hash)
}
Expand All @@ -85,6 +85,7 @@ struct ProcessTxParams {
stateful_tx_validator: Arc<StatefulTransactionValidator>,
state_reader_factory: Arc<dyn StateReaderFactory>,
gateway_compiler: Arc<GatewayCompiler>,
mempool_client: SharedMempoolClient,
chain_info: ChainInfo,
tx: RpcTransaction,
}
Expand All @@ -96,12 +97,13 @@ impl ProcessTxParams {
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,
}
}

fn process_tx(self) -> GatewayResult<AddTransactionArgs> {
async fn process_tx(self) -> GatewayResult<AddTransactionArgs> {
// TODO(Arni, 1/5/2024): Perform congestion control.

// Perform stateless validations.
Expand Down Expand Up @@ -129,7 +131,9 @@ impl ProcessTxParams {
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)
.await?;

// TODO(Arni): Add the Sierra and the Casm to the mempool input.
Ok(AddTransactionArgs { tx: executable_tx, account_state: AccountState { address, nonce } })
Expand Down
43 changes: 28 additions & 15 deletions crates/starknet_gateway/src/stateful_transaction_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@ use blockifier::versioned_constants::VersionedConstants;
use mockall::automock;
use starknet_api::block::BlockInfo;
use starknet_api::core::Nonce;
use starknet_api::executable_transaction::{
AccountTransaction as ExecutableTransaction,
InvokeTransaction as ExecutableInvokeTransaction,
};
use starknet_api::executable_transaction::AccountTransaction as ExecutableTransaction;
use starknet_gateway_types::errors::GatewaySpecError;
use starknet_mempool_types::communication::SharedMempoolClient;
use starknet_types_core::felt::Felt;
use tracing::error;

use crate::config::StatefulTransactionValidatorConfig;
use crate::errors::StatefulTransactionValidatorResult;
use crate::errors::{mempool_client_result_to_gw_spec_result, StatefulTransactionValidatorResult};
use crate::state_reader::{MempoolStateReader, StateReaderFactory};

#[cfg(test)]
Expand Down Expand Up @@ -55,13 +53,15 @@ impl StatefulTransactionValidatorTrait for BlockifierStatefulValidator {
}

impl StatefulTransactionValidator {
pub fn run_validate<V: StatefulTransactionValidatorTrait>(
pub async fn run_validate<V: StatefulTransactionValidatorTrait>(
&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).await?;
let only_query = false;
let charge_fee = enforce_fee(executable_tx, only_query);
let execution_flags = ExecutionFlags { only_query, charge_fee, validate: !skip_validate };
Expand Down Expand Up @@ -104,16 +104,29 @@ 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 {
match 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)
async fn skip_stateful_validations(
tx: &ExecutableTransaction,
account_nonce: Nonce,
mempool_client: SharedMempoolClient,
) -> StatefulTransactionValidatorResult<bool> {
if let ExecutableTransaction::Invoke(tx) = 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.
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 mempool_client_result_to_gw_spec_result(
mempool_client.has_tx_from_address(tx.sender_address()).await,
// If the mempool returns a `P2pPropagatorClientError`, we do not skip the
// validation.
false,
);
}
ExecutableTransaction::DeployAccount(_) | ExecutableTransaction::Declare(_) => false,
}

Ok(false)
}

pub fn get_latest_block_info(
Expand Down
47 changes: 39 additions & 8 deletions crates/starknet_gateway/src/stateful_transaction_validator_test.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::sync::Arc;

use blockifier::blockifier::stateful_validator::{
StatefulValidatorError as BlockifierStatefulValidatorError,
StatefulValidatorResult as BlockifierStatefulValidatorResult,
Expand Down Expand Up @@ -25,6 +27,7 @@ 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};
Expand Down Expand Up @@ -61,7 +64,8 @@ fn stateful_validator() -> StatefulTransactionValidator {
create_executable_invoke_tx(CairoVersion::Cairo1(RunnableCairo1::Casm)),
Err(STATEFUL_VALIDATOR_FEE_ERROR)
)]
fn test_stateful_tx_validator(
#[tokio::test]
async fn test_stateful_tx_validator(
#[case] executable_tx: AccountTransaction,
#[case] expected_result: BlockifierStatefulValidatorResult<()>,
stateful_validator: StatefulTransactionValidator,
Expand All @@ -77,7 +81,16 @@ 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_has_tx_from_address().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)
.await;
assert_eq!(result, expected_result_as_stateful_transaction_result);
}

Expand Down Expand Up @@ -112,19 +125,22 @@ 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(
executable_invoke_tx(invoke_tx_args!(nonce: nonce!(0))),
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!(
Expand All @@ -133,11 +149,20 @@ fn test_instantiate_validator(stateful_validator: StatefulTransactionValidator)
)
),
nonce!(1),
true,
false
)]
fn test_skip_stateful_validation(
#[case::should_not_skip_validation_no_tx_in_mempool(
executable_invoke_tx(invoke_tx_args!(nonce: nonce!(1))),
nonce!(0),
false,
false
)]
#[tokio::test]
async fn test_skip_stateful_validation(
#[case] executable_tx: AccountTransaction,
#[case] sender_nonce: Nonce,
#[case] has_tx: bool,
#[case] should_skip_validate: bool,
stateful_validator: StatefulTransactionValidator,
) {
Expand All @@ -146,5 +171,11 @@ 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_has_tx_from_address().returning(move |_| Ok(has_tx));
let mempool_client = Arc::new(mock_mempool_client);

let _ = stateful_validator
.run_validate(&executable_tx, sender_nonce, mempool_client, mock_validator)
.await;
}

0 comments on commit fabe1b6

Please sign in to comment.