Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: wrap executable tx in account/l1handler enum #1788

Merged
merged 1 commit into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading