Skip to content

Commit

Permalink
refactor: wrap executable tx in account/l1handler enum
Browse files Browse the repository at this point in the history
  • Loading branch information
yair-starkware committed Nov 7, 2024
1 parent 5a28eb1 commit 75fcd63
Show file tree
Hide file tree
Showing 20 changed files with 117 additions and 81 deletions.
5 changes: 2 additions & 3 deletions crates/batcher/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use blockifier::context::{BlockContext, ChainInfo};
use blockifier::state::cached_state::CommitmentStateDiff;
use blockifier::state::errors::StateError;
use blockifier::state::global_cache::GlobalContractCache;
use blockifier::transaction::account_transaction::AccountTransaction;
use blockifier::transaction::errors::TransactionExecutionError as BlockifierTransactionExecutionError;
use blockifier::transaction::objects::TransactionExecutionInfo;
use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction;
Expand Down Expand Up @@ -163,8 +162,8 @@ impl BlockBuilderTrait for BlockBuilder {

let mut executor_input_chunk = vec![];
for tx in &next_tx_chunk {
executor_input_chunk
.push(BlockifierTransaction::Account(AccountTransaction::try_from(tx)?));
// TODO(yair): Avoid this clone.
executor_input_chunk.push(BlockifierTransaction::try_from(tx.clone())?);
}
let results = self.executor.lock().await.add_txs_to_block(&executor_input_chunk);
trace!("Transaction execution results: {:?}", results);
Expand Down
6 changes: 3 additions & 3 deletions crates/batcher/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use blockifier::blockifier::transaction_executor::VisitedSegmentsMapping;
use blockifier::bouncer::BouncerWeights;
use blockifier::state::cached_state::CommitmentStateDiff;
use indexmap::IndexMap;
use starknet_api::executable_transaction::Transaction;
use starknet_api::executable_transaction::{AccountTransaction, Transaction};
use starknet_api::felt;
use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs};
use starknet_api::transaction::TransactionHash;
Expand All @@ -14,10 +14,10 @@ use crate::block_builder::BlockExecutionArtifacts;
pub fn test_txs(tx_hash_range: Range<usize>) -> Vec<Transaction> {
tx_hash_range
.map(|i| {
Transaction::Invoke(executable_invoke_tx(InvokeTxArgs {
Transaction::Account(AccountTransaction::Invoke(executable_invoke_tx(InvokeTxArgs {
tx_hash: TransactionHash(felt!(u128::try_from(i).unwrap())),
..Default::default()
}))
})))
})
.collect()
}
Expand Down
9 changes: 8 additions & 1 deletion crates/batcher/src/transaction_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ pub struct ProposeTransactionProvider {
impl TransactionProvider for ProposeTransactionProvider {
async fn get_txs(&mut self, n_txs: usize) -> Result<NextTxs, TransactionProviderError> {
// TODO: Get also L1 transactions.
Ok(NextTxs::Txs(self.mempool_client.get_txs(n_txs).await?))
Ok(NextTxs::Txs(
self.mempool_client
.get_txs(n_txs)
.await?
.into_iter()
.map(Transaction::Account)
.collect(),
))
}
}

Expand Down
40 changes: 20 additions & 20 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,46 +96,46 @@ macro_rules! implement_account_tx_inner_getters {
};
}

impl TryFrom<&starknet_api::executable_transaction::Transaction> for AccountTransaction {
impl TryFrom<&starknet_api::executable_transaction::AccountTransaction> for AccountTransaction {
type Error = TransactionExecutionError;

fn try_from(
value: &starknet_api::executable_transaction::Transaction,
value: &starknet_api::executable_transaction::AccountTransaction,
) -> Result<Self, Self::Error> {
match value {
starknet_api::executable_transaction::Transaction::Declare(declare_tx) => {
starknet_api::executable_transaction::AccountTransaction::Declare(declare_tx) => {
Ok(Self::Declare(declare_tx.clone().try_into()?))
}
starknet_api::executable_transaction::Transaction::DeployAccount(deploy_account_tx) => {
Ok(Self::DeployAccount(DeployAccountTransaction {
tx: deploy_account_tx.clone(),
only_query: false,
}))
}
starknet_api::executable_transaction::Transaction::Invoke(invoke_tx) => {
starknet_api::executable_transaction::AccountTransaction::DeployAccount(
deploy_account_tx,
) => Ok(Self::DeployAccount(DeployAccountTransaction {
tx: deploy_account_tx.clone(),
only_query: false,
})),
starknet_api::executable_transaction::AccountTransaction::Invoke(invoke_tx) => {
Ok(Self::Invoke(InvokeTransaction { tx: invoke_tx.clone(), only_query: false }))
}
}
}
}

impl TryFrom<starknet_api::executable_transaction::Transaction> for AccountTransaction {
impl TryFrom<starknet_api::executable_transaction::AccountTransaction> for AccountTransaction {
type Error = TransactionExecutionError;

fn try_from(
executable_transaction: starknet_api::executable_transaction::Transaction,
executable_transaction: starknet_api::executable_transaction::AccountTransaction,
) -> Result<Self, Self::Error> {
match executable_transaction {
starknet_api::executable_transaction::Transaction::Declare(declare_tx) => {
starknet_api::executable_transaction::AccountTransaction::Declare(declare_tx) => {
Ok(Self::Declare(declare_tx.try_into()?))
}
starknet_api::executable_transaction::Transaction::DeployAccount(deploy_account_tx) => {
Ok(Self::DeployAccount(DeployAccountTransaction {
tx: deploy_account_tx,
only_query: false,
}))
}
starknet_api::executable_transaction::Transaction::Invoke(invoke_tx) => {
starknet_api::executable_transaction::AccountTransaction::DeployAccount(
deploy_account_tx,
) => Ok(Self::DeployAccount(DeployAccountTransaction {
tx: deploy_account_tx,
only_query: false,
})),
starknet_api::executable_transaction::AccountTransaction::Invoke(invoke_tx) => {
Ok(Self::Invoke(InvokeTransaction { tx: invoke_tx, only_query: false }))
}
}
Expand Down
16 changes: 16 additions & 0 deletions crates/blockifier/src/transaction/transaction_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ pub enum Transaction {
L1Handler(L1HandlerTransaction),
}

impl TryFrom<starknet_api::executable_transaction::Transaction> for Transaction {
type Error = super::errors::TransactionExecutionError;
fn try_from(
value: starknet_api::executable_transaction::Transaction,
) -> Result<Self, Self::Error> {
match value {
starknet_api::executable_transaction::Transaction::Account(tx) => {
Ok(Transaction::Account(tx.try_into()?))
}
starknet_api::executable_transaction::Transaction::L1Handler(tx) => {
Ok(Transaction::L1Handler(tx))
}
}
}
}

impl Transaction {
pub fn nonce(&self) -> Nonce {
match self {
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 @@ -3,7 +3,7 @@ use std::sync::Arc;

use blockifier::context::ChainInfo;
use papyrus_network_types::network_types::BroadcastedMessageMetadata;
use starknet_api::executable_transaction::Transaction;
use starknet_api::executable_transaction::AccountTransaction;
use starknet_api::rpc_transaction::RpcTransaction;
use starknet_api::transaction::TransactionHash;
use starknet_gateway_types::errors::GatewaySpecError;
Expand Down Expand Up @@ -127,7 +127,7 @@ fn process_tx(
compile_contract_and_build_executable_tx(tx, &gateway_compiler, &chain_info.chain_id)?;

// Perfom post compilation validations.
if let Transaction::Declare(executable_declare_tx) = &executable_tx {
if let AccountTransaction::Declare(executable_declare_tx) = &executable_tx {
if !executable_declare_tx.validate_compiled_class_hash() {
return Err(GatewaySpecError::CompiledClassHashMismatch);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/gateway/src/gateway_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use mockall::predicate::eq;
use papyrus_network_types::network_types::BroadcastedMessageMetadata;
use papyrus_test_utils::{get_rng, GetTestInstance};
use starknet_api::core::{ChainId, CompiledClassHash, ContractAddress};
use starknet_api::executable_transaction::{InvokeTransaction, Transaction};
use starknet_api::executable_transaction::{AccountTransaction, InvokeTransaction};
use starknet_api::rpc_transaction::{RpcDeclareTransaction, RpcTransaction};
use starknet_gateway_types::errors::GatewaySpecError;
use starknet_mempool_types::communication::{AddTransactionArgsWrapper, MockMempoolClient};
Expand Down Expand Up @@ -61,7 +61,7 @@ async fn test_add_tx() {
let (rpc_tx, address) = create_tx();
let rpc_invoke_tx =
assert_matches!(rpc_tx.clone(), RpcTransaction::Invoke(rpc_invoke_tx) => rpc_invoke_tx);
let executable_tx = Transaction::Invoke(
let executable_tx = AccountTransaction::Invoke(
InvokeTransaction::from_rpc_tx(rpc_invoke_tx, &ChainId::create_for_testing()).unwrap(),
);

Expand Down
2 changes: 1 addition & 1 deletion crates/gateway/src/stateful_transaction_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use blockifier::versioned_constants::VersionedConstants;
use mockall::automock;
use starknet_api::core::{ContractAddress, Nonce};
use starknet_api::executable_transaction::{
AccountTransaction as ExecutableTransaction,
InvokeTransaction as ExecutableInvokeTransaction,
Transaction as ExecutableTransaction,
};
use starknet_gateway_types::errors::GatewaySpecError;
use starknet_types_core::felt::Felt;
Expand Down
14 changes: 7 additions & 7 deletions crates/gateway/src/stateful_transaction_validator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use pretty_assertions::assert_eq;
use rstest::{fixture, rstest};
use starknet_api::block::GasPrice;
use starknet_api::core::Nonce;
use starknet_api::executable_transaction::Transaction;
use starknet_api::executable_transaction::AccountTransaction;
use starknet_api::execution_resources::GasAmount;
use starknet_api::test_utils::deploy_account::executable_deploy_account_tx;
use starknet_api::test_utils::invoke::executable_invoke_tx;
Expand Down Expand Up @@ -61,7 +61,7 @@ fn stateful_validator() -> StatefulTransactionValidator {
Err(STATEFUL_VALIDATOR_FEE_ERROR)
)]
fn test_stateful_tx_validator(
#[case] executable_tx: Transaction,
#[case] executable_tx: AccountTransaction,
#[case] expected_result: BlockifierStatefulValidatorResult<()>,
stateful_validator: StatefulTransactionValidator,
) {
Expand Down Expand Up @@ -108,23 +108,23 @@ fn test_instantiate_validator(stateful_validator: StatefulTransactionValidator)

#[rstest]
#[case::should_skip_validation(
Transaction::Invoke(executable_invoke_tx(invoke_tx_args!(nonce: nonce!(1)))),
AccountTransaction::Invoke(executable_invoke_tx(invoke_tx_args!(nonce: nonce!(1)))),
nonce!(0),
true
)]
#[case::should_not_skip_validation_nonce_over_max_nonce_for_skip(
Transaction::Invoke(executable_invoke_tx(invoke_tx_args!(nonce: nonce!(0)))),
AccountTransaction::Invoke(executable_invoke_tx(invoke_tx_args!(nonce: nonce!(0)))),
nonce!(0),
false
)]
#[case::should_not_skip_validation_non_invoke(
Transaction::DeployAccount(
AccountTransaction::DeployAccount(
executable_deploy_account_tx(deploy_account_tx_args!(), nonce!(0))
),
nonce!(0),
false)]
#[case::should_not_skip_validation_account_nonce_1(
Transaction::Invoke(executable_invoke_tx(
AccountTransaction::Invoke(executable_invoke_tx(
invoke_tx_args!(
nonce: nonce!(1),
sender_address: TEST_SENDER_ADDRESS.into()
Expand All @@ -134,7 +134,7 @@ fn test_instantiate_validator(stateful_validator: StatefulTransactionValidator)
false
)]
fn test_skip_stateful_validation(
#[case] executable_tx: Transaction,
#[case] executable_tx: AccountTransaction,
#[case] sender_nonce: Nonce,
#[case] should_skip_validate: bool,
stateful_validator: StatefulTransactionValidator,
Expand Down
2 changes: 1 addition & 1 deletion crates/gateway/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use starknet_api::core::ChainId;
use starknet_api::executable_transaction::{
AccountTransaction as ExecutableTransaction,
DeclareTransaction as ExecutableDeclareTransaction,
DeployAccountTransaction as ExecutableDeployAccountTransaction,
InvokeTransaction as ExecutableInvokeTransaction,
Transaction as ExecutableTransaction,
};
use starknet_api::rpc_transaction::{RpcDeclareTransaction, RpcTransaction};
use starknet_gateway_types::errors::GatewaySpecError;
Expand Down
14 changes: 7 additions & 7 deletions crates/mempool/src/communication.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use async_trait::async_trait;
use papyrus_network_types::network_types::BroadcastedMessageMetadata;
use starknet_api::executable_transaction::Transaction;
use starknet_api::executable_transaction::AccountTransaction;
use starknet_api::rpc_transaction::{
RpcDeployAccountTransaction,
RpcInvokeTransaction,
Expand Down Expand Up @@ -45,7 +45,7 @@ impl MempoolCommunicationWrapper {
async fn send_tx_to_p2p(
&self,
message_metadata: Option<BroadcastedMessageMetadata>,
tx: Transaction,
tx: AccountTransaction,
) -> MempoolResult<()> {
match message_metadata {
Some(message_metadata) => self
Expand All @@ -56,21 +56,21 @@ impl MempoolCommunicationWrapper {
None => {
let tx_hash = tx.tx_hash();
match tx {
Transaction::Invoke(invoke_tx) => self
AccountTransaction::Invoke(invoke_tx) => self
.mempool_p2p_propagator_client
.add_transaction(RpcTransaction::Invoke(RpcInvokeTransaction::V3(
invoke_tx.into(),
)))
.await
.map_err(|_| MempoolError::P2pPropagatorClientError { tx_hash })?,
Transaction::DeployAccount(deploy_account_tx) => self
AccountTransaction::DeployAccount(deploy_account_tx) => self
.mempool_p2p_propagator_client
.add_transaction(RpcTransaction::DeployAccount(
RpcDeployAccountTransaction::V3(deploy_account_tx.into()),
))
.await
.map_err(|_| MempoolError::P2pPropagatorClientError { tx_hash })?,
Transaction::Declare(_) => {}
AccountTransaction::Declare(_) => {}
}
Ok(())
}
Expand All @@ -82,7 +82,7 @@ impl MempoolCommunicationWrapper {
// TODO: Verify that only transactions that were added to the mempool are sent.
// TODO: handle declare correctly and remove this match.
match args_wrapper.args.tx {
Transaction::Declare(_) => Ok(()),
AccountTransaction::Declare(_) => Ok(()),
_ => self.send_tx_to_p2p(args_wrapper.p2p_message_metadata, args_wrapper.args.tx).await,
}
}
Expand All @@ -91,7 +91,7 @@ impl MempoolCommunicationWrapper {
self.mempool.commit_block(args)
}

fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<Transaction>> {
fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<AccountTransaction>> {
self.mempool.get_txs(n_txs)
}
}
Expand Down
12 changes: 6 additions & 6 deletions crates/mempool/src/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;

use starknet_api::block::GasPrice;
use starknet_api::core::{ContractAddress, Nonce};
use starknet_api::executable_transaction::Transaction;
use starknet_api::executable_transaction::AccountTransaction;
use starknet_api::transaction::{Tip, TransactionHash};
use starknet_mempool_types::errors::MempoolError;
use starknet_mempool_types::mempool_types::{
Expand Down Expand Up @@ -62,7 +62,7 @@ impl Mempool {
/// created.
// TODO: Consider renaming to `pop_txs` to be more consistent with the standard library.
#[tracing::instrument(skip(self), err)]
pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<Transaction>> {
pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<AccountTransaction>> {
let mut eligible_tx_references: Vec<TransactionReference> = Vec::with_capacity(n_txs);
let mut n_remaining_txs = n_txs;

Expand Down Expand Up @@ -252,7 +252,7 @@ impl Mempool {
}

#[tracing::instrument(level = "debug", skip(self, incoming_tx), err)]
fn handle_fee_escalation(&mut self, incoming_tx: &Transaction) -> MempoolResult<()> {
fn handle_fee_escalation(&mut self, incoming_tx: &AccountTransaction) -> MempoolResult<()> {
if !self.config.enable_fee_escalation {
return Ok(());
}
Expand Down Expand Up @@ -314,11 +314,11 @@ impl Mempool {
}

// TODO(Elin): move to a shared location with other next-gen node crates.
fn tip(tx: &Transaction) -> Tip {
fn tip(tx: &AccountTransaction) -> Tip {
tx.tip().expect("Expected a valid tip value.")
}

fn max_l2_gas_price(tx: &Transaction) -> GasPrice {
fn max_l2_gas_price(tx: &AccountTransaction) -> GasPrice {
tx.resource_bounds()
.expect("Expected a valid resource bounds value.")
.get_l2_bounds()
Expand All @@ -339,7 +339,7 @@ pub struct TransactionReference {
}

impl TransactionReference {
pub fn new(tx: &Transaction) -> Self {
pub fn new(tx: &AccountTransaction) -> Self {
TransactionReference {
address: tx.contract_address(),
nonce: tx.nonce(),
Expand Down
2 changes: 1 addition & 1 deletion crates/mempool/src/mempool_test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use pretty_assertions::assert_eq;
use rstest::{fixture, rstest};
use starknet_api::executable_transaction::Transaction;
use starknet_api::executable_transaction::AccountTransaction as Transaction;
use starknet_api::{contract_address, nonce};
use starknet_mempool_types::errors::MempoolError;
use starknet_mempool_types::mempool_types::AddTransactionArgs;
Expand Down
Loading

0 comments on commit 75fcd63

Please sign in to comment.