From 901b5aaaaf5de35492c07b14776b021d5dc2bbab Mon Sep 17 00:00:00 2001 From: mohammad-starkware <130282237+MohammadNassar1@users.noreply.github.com> Date: Tue, 20 Aug 2024 11:58:55 +0300 Subject: [PATCH] refactor(mempool): delete thin tx struct (#377) --- Cargo.lock | 1 + crates/gateway/src/gateway_test.rs | 9 ++-- crates/mempool/Cargo.toml | 1 + crates/mempool/src/mempool_test.rs | 16 +++---- .../src/starknet_api_test_utils.rs | 28 +++++++++++++ crates/mempool_types/src/mempool_types.rs | 42 +------------------ 6 files changed, 45 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eda062ceca..83f15ffbee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8934,6 +8934,7 @@ dependencies = [ "async-trait", "derive_more", "itertools 0.10.5", + "mempool_test_utils", "pretty_assertions", "rstest", "starknet-types-core", diff --git a/crates/gateway/src/gateway_test.rs b/crates/gateway/src/gateway_test.rs index 4ea07c3656..4204f0b3bf 100644 --- a/crates/gateway/src/gateway_test.rs +++ b/crates/gateway/src/gateway_test.rs @@ -6,13 +6,13 @@ use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use blockifier::context::ChainInfo; use blockifier::test_utils::CairoVersion; -use mempool_test_utils::starknet_api_test_utils::invoke_tx; +use mempool_test_utils::starknet_api_test_utils::{create_executable_tx, invoke_tx}; use mockall::predicate::eq; use starknet_api::core::ContractAddress; use starknet_api::rpc_transaction::RpcTransaction; use starknet_api::transaction::TransactionHash; use starknet_mempool_types::communication::MockMempoolClient; -use starknet_mempool_types::mempool_types::{Account, AccountState, MempoolInput, ThinTransaction}; +use starknet_mempool_types::mempool_types::{Account, AccountState, MempoolInput}; use starknet_sierra_compile::config::SierraToCasmCompilationConfig; use crate::compilation::GatewayCompiler; @@ -65,8 +65,9 @@ async fn test_add_tx() { .expect_add_tx() .once() .with(eq(MempoolInput { - tx: (&ThinTransaction { sender_address, tx_hash, tip: *tx.tip(), nonce: *tx.nonce() }) - .into(), + // TODO(Arni): Use external_to_executable_tx instead of `create_executable_tx`. Consider + // creating a `convertor for testing` that does not do the compilation. + tx: create_executable_tx(sender_address, tx_hash, *tx.tip(), *tx.nonce()), account: Account { sender_address, state: AccountState { nonce: *tx.nonce() } }, })) .return_once(|_| Ok(())); diff --git a/crates/mempool/Cargo.toml b/crates/mempool/Cargo.toml index 07a94a8816..6a8da2fc99 100644 --- a/crates/mempool/Cargo.toml +++ b/crates/mempool/Cargo.toml @@ -19,6 +19,7 @@ tokio.workspace = true [dev-dependencies] assert_matches.workspace = true itertools.workspace = true +mempool_test_utils.workspace = true pretty_assertions.workspace = true rstest.workspace = true starknet-types-core.workspace = true diff --git a/crates/mempool/src/mempool_test.rs b/crates/mempool/src/mempool_test.rs index 733862ef5d..02fce85565 100644 --- a/crates/mempool/src/mempool_test.rs +++ b/crates/mempool/src/mempool_test.rs @@ -2,6 +2,7 @@ use std::cmp::Reverse; use std::collections::HashMap; use assert_matches::assert_matches; +use mempool_test_utils::starknet_api_test_utils::create_executable_tx; use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; use starknet_api::core::{ContractAddress, Nonce, PatriciaKey}; @@ -10,7 +11,7 @@ use starknet_api::hash::StarkHash; use starknet_api::transaction::{Tip, TransactionHash}; use starknet_api::{contract_address, felt, patricia_key}; use starknet_mempool_types::errors::MempoolError; -use starknet_mempool_types::mempool_types::{Account, AccountState, ThinTransaction}; +use starknet_mempool_types::mempool_types::{Account, AccountState}; use starknet_types_core::felt::Felt; use crate::mempool::{Mempool, MempoolInput, TransactionReference}; @@ -154,13 +155,14 @@ macro_rules! add_tx_input { let sender_address = contract_address!($sender_address); let account_nonce = Nonce(felt!($account_nonce)); let account = Account { sender_address, state: AccountState {nonce: account_nonce}}; - let tx = ThinTransaction { - tip: Tip($tip), - tx_hash: TransactionHash(StarkHash::from($tx_hash)), + + let tx = create_executable_tx( sender_address, - nonce: Nonce(felt!($tx_nonce)), - }; - MempoolInput { tx: (&tx).into(), account } + TransactionHash(StarkHash::from($tx_hash)), + Tip($tip), + Nonce(felt!($tx_nonce)), + ); + MempoolInput { tx, account } }}; (tx_hash: $tx_hash:expr, sender_address: $sender_address:expr, tx_nonce: $tx_nonce:expr, account_nonce: $account_nonce:expr) => { add_tx_input!(tip: 0, tx_hash: $tx_hash, sender_address: $sender_address, tx_nonce: $tx_nonce, account_nonce: $account_nonce) diff --git a/crates/mempool_test_utils/src/starknet_api_test_utils.rs b/crates/mempool_test_utils/src/starknet_api_test_utils.rs index 7948129d3c..9352c97b23 100644 --- a/crates/mempool_test_utils/src/starknet_api_test_utils.rs +++ b/crates/mempool_test_utils/src/starknet_api_test_utils.rs @@ -10,6 +10,7 @@ use blockifier::test_utils::{create_trivial_calldata, CairoVersion, NonceManager use serde_json::to_string_pretty; use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; use starknet_api::data_availability::DataAvailabilityMode; +use starknet_api::executable_transaction::{InvokeTransaction, Transaction}; use starknet_api::rpc_transaction::{ ContractClass, ResourceBoundsMapping, @@ -24,7 +25,9 @@ use starknet_api::transaction::{ ContractAddressSalt, PaymasterData, ResourceBounds, + ResourceBoundsMapping as ExecutableResourceBoundsMapping, Tip, + TransactionHash, TransactionSignature, TransactionVersion, }; @@ -538,3 +541,28 @@ pub fn external_tx_to_json(tx: &RpcTransaction) -> String { // Serialize back to pretty JSON string to_string_pretty(&tx_json).expect("Failed to serialize transaction") } + +pub fn create_executable_tx( + sender_address: ContractAddress, + tx_hash: TransactionHash, + tip: Tip, + nonce: Nonce, +) -> Transaction { + Transaction::Invoke(InvokeTransaction { + tx: starknet_api::transaction::InvokeTransaction::V3( + starknet_api::transaction::InvokeTransactionV3 { + sender_address, + tip, + nonce, + resource_bounds: ExecutableResourceBoundsMapping::default(), + signature: TransactionSignature::default(), + calldata: Calldata::default(), + nonce_data_availability_mode: DataAvailabilityMode::L1, + fee_data_availability_mode: DataAvailabilityMode::L1, + paymaster_data: PaymasterData::default(), + account_deployment_data: AccountDeploymentData::default(), + }, + ), + tx_hash, + }) +} diff --git a/crates/mempool_types/src/mempool_types.rs b/crates/mempool_types/src/mempool_types.rs index 6778573bb9..91f91a9fff 100644 --- a/crates/mempool_types/src/mempool_types.rs +++ b/crates/mempool_types/src/mempool_types.rs @@ -1,27 +1,9 @@ use serde::{Deserialize, Serialize}; use starknet_api::core::{ContractAddress, Nonce}; -use starknet_api::data_availability::DataAvailabilityMode; -use starknet_api::executable_transaction::{InvokeTransaction, Transaction}; -use starknet_api::transaction::{ - AccountDeploymentData, - Calldata, - PaymasterData, - ResourceBoundsMapping, - Tip, - TransactionHash, - TransactionSignature, -}; +use starknet_api::executable_transaction::Transaction; use crate::errors::MempoolError; -#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] -pub struct ThinTransaction { - pub sender_address: ContractAddress, - pub tx_hash: TransactionHash, - pub tip: Tip, - pub nonce: Nonce, -} - #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct AccountState { pub nonce: Nonce, @@ -42,25 +24,3 @@ pub struct MempoolInput { } pub type MempoolResult = Result; - -impl From<&ThinTransaction> for Transaction { - fn from(tx: &ThinTransaction) -> Self { - Transaction::Invoke(InvokeTransaction { - tx: starknet_api::transaction::InvokeTransaction::V3( - starknet_api::transaction::InvokeTransactionV3 { - sender_address: tx.sender_address, - tip: tx.tip, - nonce: tx.nonce, - resource_bounds: ResourceBoundsMapping::default(), - signature: TransactionSignature::default(), - calldata: Calldata::default(), - nonce_data_availability_mode: DataAvailabilityMode::L1, - fee_data_availability_mode: DataAvailabilityMode::L1, - paymaster_data: PaymasterData::default(), - account_deployment_data: AccountDeploymentData::default(), - }, - ), - tx_hash: tx.tx_hash, - }) - } -}