refactor: use InternalRpcTransaction in gateway and mempool

Author: noamsp-starkware

crates/starknet_gateway/src/gateway.rs

@@ -154,8 +154,7 @@ impl ProcessTxBlockingTask {
         )?;
 
         // TODO(Arni): Add the Sierra and the Casm to the mempool input.
-        // TODO(noamsp): put internal_tx instead of executable_tx
-        Ok(AddTransactionArgs { tx: executable_tx, account_state: AccountState { address, nonce } })
+        Ok(AddTransactionArgs { tx: internal_tx, account_state: AccountState { address, nonce } })
     }
 }
 

crates/starknet_mempool/src/mempool.rs

@@ -2,7 +2,6 @@ use std::collections::HashMap;
 
 use starknet_api::block::GasPrice;
 use starknet_api::core::{ContractAddress, Nonce};
-use starknet_api::executable_transaction::AccountTransaction;
 use starknet_api::rpc_transaction::InternalRpcTransaction;
 use starknet_api::transaction::fields::Tip;
 use starknet_api::transaction::TransactionHash;
@@ -157,7 +156,7 @@ impl Mempool {
     /// created.
     // TODO: Consider renaming to `pop_txs` to be more consistent with the standard library.
     #[instrument(skip(self), err)]
-    pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<AccountTransaction>> {
+    pub fn get_txs(&mut self, n_txs: usize) -> MempoolResult<Vec<InternalRpcTransaction>> {
         let mut eligible_tx_references: Vec<TransactionReference> = Vec::with_capacity(n_txs);
         let mut n_remaining_txs = n_txs;
 
@@ -194,17 +193,17 @@ impl Mempool {
         skip(self, args),
         fields( // Log subset of (informative) fields.
             tx_nonce = %args.tx.nonce(),
-            tx_hash = %args.tx.tx_hash(),
-            tx_tip = %tip(&args.tx),
-            tx_max_l2_gas_price = %max_l2_gas_price(&args.tx),
+            tx_hash = %args.tx.tx_hash,
+            tx_tip = %args.tx.tip(),
+            tx_max_l2_gas_price = %args.tx.resource_bounds().l2_gas.max_price_per_unit,
             account_state = %args.account_state
         ),
         err
     )]
     pub fn add_tx(&mut self, args: AddTransactionArgs) -> MempoolResult<()> {
         let AddTransactionArgs { tx, account_state } = args;
         debug!("Adding transaction to mempool: {tx:#?}.");
-        let tx_reference = TransactionReference::deprecated_new(&tx);
+        let tx_reference = TransactionReference::new(&tx);
         self.validate_incoming_tx(tx_reference)?;
 
         self.handle_fee_escalation(&tx)?;
@@ -319,8 +318,8 @@ impl Mempool {
     }
 
     #[instrument(level = "debug", skip(self, incoming_tx), err)]
-    fn handle_fee_escalation(&mut self, incoming_tx: &AccountTransaction) -> MempoolResult<()> {
-        let incoming_tx_reference = TransactionReference::deprecated_new(incoming_tx);
+    fn handle_fee_escalation(&mut self, incoming_tx: &InternalRpcTransaction) -> MempoolResult<()> {
+        let incoming_tx_reference = TransactionReference::new(incoming_tx);
         let TransactionReference { address, nonce, .. } = incoming_tx_reference;
 
         if !self.config.enable_fee_escalation {
@@ -386,15 +385,6 @@ impl Mempool {
     }
 }
 
-// TODO(Elin): move to a shared location with other next-gen node crates.
-fn tip(tx: &AccountTransaction) -> Tip {
-    tx.tip()
-}
-
-fn max_l2_gas_price(tx: &AccountTransaction) -> GasPrice {
-    tx.resource_bounds().get_l2_bounds().max_price_per_unit
-}
-
 /// Provides a lightweight representation of a transaction for mempool usage (e.g., excluding
 /// execution fields).
 /// TODO(Mohammad): rename this struct to `ThinTransaction` once that name
@@ -409,17 +399,6 @@ pub struct TransactionReference {
 }
 
 impl TransactionReference {
-    // TODO(noamsp): remove this once class manager client integration is complete.
-    pub fn deprecated_new(tx: &AccountTransaction) -> Self {
-        TransactionReference {
-            address: tx.contract_address(),
-            nonce: tx.nonce(),
-            tx_hash: tx.tx_hash(),
-            tip: tip(tx),
-            max_l2_gas_price: max_l2_gas_price(tx),
-        }
-    }
-
     pub fn new(tx: &InternalRpcTransaction) -> Self {
         TransactionReference {
             address: tx.contract_address(),

crates/starknet_mempool/src/mempool_test.rs

@@ -7,12 +7,7 @@ use pretty_assertions::assert_eq;
 use rstest::{fixture, rstest};
 use starknet_api::block::GasPrice;
 use starknet_api::core::ContractAddress;
-use starknet_api::executable_transaction::AccountTransaction;
-use starknet_api::rpc_transaction::{
-    RpcDeployAccountTransaction,
-    RpcInvokeTransaction,
-    RpcTransaction,
-};
+use starknet_api::rpc_transaction::InternalRpcTransaction;
 use starknet_api::{contract_address, nonce};
 use starknet_mempool_p2p_types::communication::MockMempoolP2pPropagatorClient;
 use starknet_mempool_types::communication::AddTransactionArgsWrapper;
@@ -88,7 +83,7 @@ impl MempoolContentBuilder {
 
     fn with_pool<P>(mut self, pool_txs: P) -> Self
     where
-        P: IntoIterator<Item = AccountTransaction>,
+        P: IntoIterator<Item = InternalRpcTransaction>,
     {
         self.tx_pool = Some(pool_txs.into_iter().collect());
         self
@@ -140,8 +135,8 @@ impl MempoolContentBuilder {
     }
 }
 
-impl FromIterator<AccountTransaction> for TransactionPool {
-    fn from_iter<T: IntoIterator<Item = AccountTransaction>>(txs: T) -> Self {
+impl FromIterator<InternalRpcTransaction> for TransactionPool {
+    fn from_iter<T: IntoIterator<Item = InternalRpcTransaction>>(txs: T) -> Self {
         let mut pool = Self::default();
         for tx in txs {
             pool.insert(tx).unwrap();
@@ -154,7 +149,7 @@ impl FromIterator<AccountTransaction> for TransactionPool {
 fn builder_with_queue(
     in_priority_queue: bool,
     in_pending_queue: bool,
-    tx: &AccountTransaction,
+    tx: &InternalRpcTransaction,
 ) -> MempoolContentBuilder {
     assert!(
         !(in_priority_queue && in_pending_queue),
@@ -164,11 +159,11 @@ fn builder_with_queue(
     let mut builder = MempoolContentBuilder::new();
 
     if in_priority_queue {
-        builder = builder.with_priority_queue([TransactionReference::deprecated_new(tx)]);
+        builder = builder.with_priority_queue([TransactionReference::new(tx)]);
     }
 
     if in_pending_queue {
-        builder = builder.with_pending_queue([TransactionReference::deprecated_new(tx)]);
+        builder = builder.with_pending_queue([TransactionReference::new(tx)]);
     }
 
     builder
@@ -209,7 +204,7 @@ fn add_tx_and_verify_replacement_in_pool(
 #[track_caller]
 fn add_txs_and_verify_no_replacement(
     mut mempool: Mempool,
-    existing_tx: AccountTransaction,
+    existing_tx: InternalRpcTransaction,
     invalid_replacement_inputs: impl IntoIterator<Item = AddTransactionArgs>,
     in_priority_queue: bool,
     in_pending_queue: bool,
@@ -235,7 +230,7 @@ fn add_txs_and_verify_no_replacement(
 #[track_caller]
 fn add_txs_and_verify_no_replacement_in_pool(
     mempool: Mempool,
-    existing_tx: AccountTransaction,
+    existing_tx: InternalRpcTransaction,
     invalid_replacement_inputs: impl IntoIterator<Item = AddTransactionArgs>,
 ) {
     let in_priority_queue = false;
@@ -276,7 +271,7 @@ fn test_get_txs_returns_by_priority(#[case] n_requested_txs: usize) {
     let tx_tip_30 = tx!(tx_hash: 2, address: "0x1", tip: 30);
     let tx_tip_10 = tx!(tx_hash: 3, address: "0x2", tip: 10);
 
-    let queue_txs = [&tx_tip_20, &tx_tip_30, &tx_tip_10].map(TransactionReference::deprecated_new);
+    let queue_txs = [&tx_tip_20, &tx_tip_30, &tx_tip_10].map(TransactionReference::new);
     let pool_txs = [&tx_tip_20, &tx_tip_30, &tx_tip_10].map(|tx| tx.clone());
     let mut mempool = MempoolContentBuilder::new()
         .with_pool(pool_txs)
@@ -292,7 +287,7 @@ fn test_get_txs_returns_by_priority(#[case] n_requested_txs: usize) {
     assert_eq!(fetched_txs, expected_fetched_txs);
 
     // Assert: non-returned transactions are still in the mempool.
-    let remaining_tx_references = remaining_txs.iter().map(TransactionReference::deprecated_new);
+    let remaining_tx_references = remaining_txs.iter().map(TransactionReference::new);
     let expected_mempool_content =
         MempoolContentBuilder::new().with_priority_queue(remaining_tx_references).build();
     expected_mempool_content.assert_eq(&mempool);
@@ -306,9 +301,7 @@ fn test_get_txs_returns_by_secondary_priority_on_tie() {
 
     let mut mempool = MempoolContentBuilder::new()
         .with_pool([&tx_tip_10_hash_9, &tx_tip_10_hash_15].map(|tx| tx.clone()))
-        .with_priority_queue(
-            [&tx_tip_10_hash_9, &tx_tip_10_hash_15].map(TransactionReference::deprecated_new),
-        )
+        .with_priority_queue([&tx_tip_10_hash_9, &tx_tip_10_hash_15].map(TransactionReference::new))
         .build_into_mempool();
 
     // Test and assert.
@@ -321,7 +314,7 @@ fn test_get_txs_does_not_return_pending_txs() {
     let tx = tx!();
 
     let mut mempool = MempoolContentBuilder::new()
-        .with_pending_queue([TransactionReference::deprecated_new(&tx)])
+        .with_pending_queue([TransactionReference::new(&tx)])
         .with_pool([tx])
         .build_into_mempool();
 
@@ -334,7 +327,7 @@ fn test_get_txs_does_not_remove_returned_txs_from_pool() {
     // Setup.
     let tx = tx!();
 
-    let queue_txs = [TransactionReference::deprecated_new(&tx)];
+    let queue_txs = [TransactionReference::new(&tx)];
     let pool_txs = [tx];
     let mut mempool = MempoolContentBuilder::new()
         .with_pool(pool_txs.clone())
@@ -355,8 +348,7 @@ fn test_get_txs_replenishes_queue_only_between_chunks() {
     let tx_address_0_nonce_1 = tx!(tx_hash: 2, address: "0x0", tx_nonce: 1, tip: 20);
     let tx_address_1_nonce_0 = tx!(tx_hash: 3, address: "0x1", tx_nonce: 0, tip: 10);
 
-    let queue_txs =
-        [&tx_address_0_nonce_0, &tx_address_1_nonce_0].map(TransactionReference::deprecated_new);
+    let queue_txs = [&tx_address_0_nonce_0, &tx_address_1_nonce_0].map(TransactionReference::new);
     let pool_txs =
         [&tx_address_0_nonce_0, &tx_address_0_nonce_1, &tx_address_1_nonce_0].map(|tx| tx.clone());
     let mut mempool = MempoolContentBuilder::new()
@@ -382,7 +374,7 @@ fn test_get_txs_with_nonce_gap() {
     let tx_address_0_nonce_1 = tx!(tx_hash: 2, address: "0x0", tx_nonce: 1);
     let tx_address_1_nonce_0 = tx!(tx_hash: 3, address: "0x1", tx_nonce: 0);
 
-    let queue_txs = [TransactionReference::deprecated_new(&tx_address_1_nonce_0)];
+    let queue_txs = [TransactionReference::new(&tx_address_1_nonce_0)];
     let pool_txs = [tx_address_0_nonce_1, tx_address_1_nonce_0.clone()];
     let mut mempool = MempoolContentBuilder::new()
         .with_pool(pool_txs)
@@ -415,8 +407,8 @@ fn test_add_tx_insertion_sorted_by_priority(mut mempool: Mempool) {
     }
 
     // Assert: transactions are ordered by priority.
-    let expected_queue_txs = [&input_tip_100.tx, &input_tip_80.tx, &input_tip_50.tx]
-        .map(TransactionReference::deprecated_new);
+    let expected_queue_txs =
+        [&input_tip_100.tx, &input_tip_80.tx, &input_tip_50.tx].map(TransactionReference::new);
     let expected_mempool_content =
         MempoolContentBuilder::new().with_priority_queue(expected_queue_txs).build();
     expected_mempool_content.assert_eq(&mempool);
@@ -438,8 +430,8 @@ fn test_add_tx_correctly_places_txs_in_queue_and_pool(mut mempool: Mempool) {
     }
 
     // Assert: only the eligible transactions appear in the queue.
-    let expected_queue_txs = [&input_address_1_nonce_0.tx, &input_address_0_nonce_0.tx]
-        .map(TransactionReference::deprecated_new);
+    let expected_queue_txs =
+        [&input_address_1_nonce_0.tx, &input_address_0_nonce_0.tx].map(TransactionReference::new);
     let expected_pool_txs =
         [input_address_0_nonce_0.tx, input_address_1_nonce_0.tx, input_address_0_nonce_1.tx];
     let expected_mempool_content = MempoolContentBuilder::new()
@@ -504,7 +496,7 @@ fn test_add_tx_with_identical_tip_succeeds(mut mempool: Mempool) {
     }
 
     // Assert: both transactions are in the mempool.
-    let expected_queue_txs = [&input1.tx, &input2.tx].map(TransactionReference::deprecated_new);
+    let expected_queue_txs = [&input1.tx, &input2.tx].map(TransactionReference::new);
     let expected_pool_txs = [input1.tx, input2.tx];
     let expected_mempool_content = MempoolContentBuilder::new()
         .with_pool(expected_pool_txs)
@@ -535,7 +527,7 @@ fn test_add_tx_fills_nonce_gap(mut mempool: Mempool) {
     add_tx(&mut mempool, &input_nonce_0);
 
     // Assert: only the eligible transaction appears in the queue.
-    let expected_queue_txs = [TransactionReference::deprecated_new(&input_nonce_0.tx)];
+    let expected_queue_txs = [TransactionReference::new(&input_nonce_0.tx)];
     let expected_pool_txs = [input_nonce_1.tx, input_nonce_0.tx];
     let expected_mempool_content = MempoolContentBuilder::new()
         .with_pool(expected_pool_txs)
@@ -576,7 +568,7 @@ fn test_commit_block_includes_all_proposed_txs() {
     let tx_address_2_nonce_1 = tx!(tx_hash: 6, address: "0x2", tx_nonce: 1);
 
     let queue_txs = [&tx_address_2_nonce_1, &tx_address_1_nonce_3, &tx_address_0_nonce_4]
-        .map(TransactionReference::deprecated_new);
+        .map(TransactionReference::new);
     let pool_txs = [
         tx_address_0_nonce_3,
         tx_address_0_nonce_4.clone(),
@@ -762,7 +754,7 @@ fn test_update_gas_price_threshold_increases_threshold() {
         &tx!(tx_hash: 0, address: "0x0", max_l2_gas_price: 100),
         &tx!(tx_hash: 1, address: "0x1", max_l2_gas_price: 101),
     ]
-    .map(TransactionReference::deprecated_new);
+    .map(TransactionReference::new);
 
     let mut mempool: Mempool = MempoolContentBuilder::new()
         .with_priority_queue([tx_low_gas, tx_high_gas])
@@ -788,7 +780,7 @@ fn test_update_gas_price_threshold_decreases_threshold() {
         &tx!(tx_hash: 0, address: "0x0", max_l2_gas_price: 89),
         &tx!(tx_hash: 1, address: "0x1", max_l2_gas_price: 90),
     ]
-    .map(TransactionReference::deprecated_new);
+    .map(TransactionReference::new);
 
     let mut mempool: Mempool = MempoolContentBuilder::new()
         .with_pending_queue([tx_low_gas, tx_high_gas])
@@ -814,26 +806,11 @@ async fn test_new_tx_sent_to_p2p(mempool: Mempool) {
     let tx_args = add_tx_input!(tx_hash: 1, address: "0x0", tx_nonce: 2, account_nonce: 2);
     let propagateor_args =
         AddTransactionArgsWrapper { args: tx_args.clone(), p2p_message_metadata: None };
-    // TODO: use regular conversion once we have a compiler component
-    let rpc_tx = match tx_args.tx {
-        AccountTransaction::Declare(_declare_tx) => {
-            panic!("No implementation for converting DeclareTransaction to an RpcTransaction")
-        }
-        AccountTransaction::DeployAccount(deploy_account_transaction) => {
-            RpcTransaction::DeployAccount(RpcDeployAccountTransaction::V3(
-                deploy_account_transaction.clone().into(),
-            ))
-        }
-        AccountTransaction::Invoke(invoke_transaction) => {
-            RpcTransaction::Invoke(RpcInvokeTransaction::V3(invoke_transaction.clone().into()))
-        }
-    };
-
     let mut mock_mempool_p2p_propagator_client = MockMempoolP2pPropagatorClient::new();
     mock_mempool_p2p_propagator_client
         .expect_add_transaction()
         .times(1)
-        .with(predicate::eq(rpc_tx))
+        .with(predicate::eq(tx_args.tx))
         .returning(|_| Ok(()));
     let mut mempool_wrapper =
         MempoolCommunicationWrapper::new(mempool, Arc::new(mock_mempool_p2p_propagator_client));

crates/starknet_mempool/src/transaction_pool.rs

@@ -1,15 +1,15 @@
 use std::collections::{hash_map, BTreeMap, HashMap};
 
 use starknet_api::core::{ContractAddress, Nonce};
-use starknet_api::executable_transaction::AccountTransaction;
+use starknet_api::rpc_transaction::InternalRpcTransaction;
 use starknet_api::transaction::TransactionHash;
 use starknet_mempool_types::errors::MempoolError;
 use starknet_mempool_types::mempool_types::{AccountState, MempoolResult};
 
 use crate::mempool::TransactionReference;
 use crate::utils::try_increment_nonce;
 
-type HashToTransaction = HashMap<TransactionHash, AccountTransaction>;
+type HashToTransaction = HashMap<TransactionHash, InternalRpcTransaction>;
 
 /// Contains all transactions currently held in the mempool.
 /// Invariant: both data structures are consistent regarding the existence of transactions:
@@ -26,8 +26,8 @@ pub struct TransactionPool {
 }
 
 impl TransactionPool {
-    pub fn insert(&mut self, tx: AccountTransaction) -> MempoolResult<()> {
-        let tx_reference = TransactionReference::deprecated_new(&tx);
+    pub fn insert(&mut self, tx: InternalRpcTransaction) -> MempoolResult<()> {
+        let tx_reference = TransactionReference::new(&tx);
         let tx_hash = tx_reference.tx_hash;
 
         // Insert to pool.
@@ -52,21 +52,18 @@ impl TransactionPool {
         Ok(())
     }
 
-    pub fn remove(&mut self, tx_hash: TransactionHash) -> MempoolResult<AccountTransaction> {
+    pub fn remove(&mut self, tx_hash: TransactionHash) -> MempoolResult<InternalRpcTransaction> {
         // Remove from pool.
         let tx =
             self.tx_pool.remove(&tx_hash).ok_or(MempoolError::TransactionNotFound { tx_hash })?;
 
         // Remove from account mapping.
-        self.txs_by_account.remove(TransactionReference::deprecated_new(&tx)).unwrap_or_else(
-            || {
-                panic!(
-                    "Transaction pool consistency error: transaction with hash {tx_hash} appears \
-                     in
+        self.txs_by_account.remove(TransactionReference::new(&tx)).unwrap_or_else(|| {
+            panic!(
+                "Transaction pool consistency error: transaction with hash {tx_hash} appears in
                 main mapping, but does not appear in the account mapping"
-                )
-            },
-        );
+            )
+        });
 
         self.capacity.remove();
 
@@ -95,7 +92,10 @@ impl TransactionPool {
         self.txs_by_account.account_txs_sorted_by_nonce(address)
     }
 
-    pub fn get_by_tx_hash(&self, tx_hash: TransactionHash) -> MempoolResult<&AccountTransaction> {
+    pub fn get_by_tx_hash(
+        &self,
+        tx_hash: TransactionHash,
+    ) -> MempoolResult<&InternalRpcTransaction> {
         self.tx_pool.get(&tx_hash).ok_or(MempoolError::TransactionNotFound { tx_hash })
     }