refactor(mempool): delete thin tx struct (#377)

starkware-libs · Aug 20, 2024 · c34b81b · c34b81b
1 parent 9343539
commit c34b81b
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 52 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<T> = Result<T, MempoolError>;
-
-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,
-        })
-    }
-}