reactivate test_transaction_by_hash and upstream `transaction_by_ha…

…sh` (#1401)

* reactivate test_transaction_by_hash and upstream transaction_by_hash

* clean up

* fix nightly

* fix

* fix clippy

* add random tx to the test

* clean up

* Update rust-toolchain

rust-toolchain

@@ -1 +1 @@
-nightly-2024-09-15
+nightly-2024-09-15

src/client/mod.rs

@@ -8,7 +8,7 @@ use crate::{
             database::Database,
             error::{EthApiError, EthereumDataFormatError, KakarotError, SignatureError},
             provider::{EthApiResult, EthDataProvider},
-            TxPoolProvider,
+            TransactionProvider, TxPoolProvider,
         },
         sn_provider::StarknetProvider,
     },
@@ -18,6 +18,7 @@ use async_trait::async_trait;
 use num_traits::ToPrimitive;
 use reth_chainspec::ChainSpec;
 use reth_primitives::{Address, Bytes, TransactionSigned, TransactionSignedEcRecovered, B256};
+use reth_rpc_eth_types::TransactionSource;
 use reth_rpc_types::{txpool::TxpoolContent, Transaction, WithOtherFields};
 use reth_transaction_pool::{
     blobstore::NoopBlobStore, AllPoolTransactions, EthPooledTransaction, PoolConfig, PoolTransaction,
@@ -32,6 +33,12 @@ pub trait KakarotTransactions {
     async fn send_raw_transaction(&self, transaction: Bytes) -> EthApiResult<B256>;
 }
 
+#[async_trait]
+pub trait TransactionHashProvider {
+    /// Returns the transaction by hash.
+    async fn transaction_by_hash(&self, hash: B256) -> EthApiResult<Option<WithOtherFields<Transaction>>>;
+}
+
 /// Provides a wrapper structure around the Ethereum Provider
 /// and the Mempool.
 #[derive(Debug, Clone)]
@@ -147,3 +154,17 @@ where
         Ok(self.content())
     }
 }
+
+#[async_trait]
+impl<SP> TransactionHashProvider for EthClient<SP>
+where
+    SP: starknet::providers::Provider + Send + Sync,
+{
+    async fn transaction_by_hash(&self, hash: B256) -> EthApiResult<Option<WithOtherFields<Transaction>>> {
+        Ok(self
+            .pool
+            .get(&hash)
+            .map(|transaction| TransactionSource::Pool(transaction.transaction.transaction().clone()).into())
+            .or(self.eth_provider.transaction_by_hash(hash).await?))
+    }
+}

src/eth_rpc/servers/eth_rpc.rs

@@ -1,7 +1,7 @@
 #![allow(clippy::blocks_in_conditions)]
 
 use crate::{
-    client::{EthClient, KakarotTransactions},
+    client::{EthClient, KakarotTransactions, TransactionHashProvider},
     eth_rpc::api::eth_api::EthApiServer,
     providers::eth_provider::{
         constant::MAX_PRIORITY_FEE_PER_GAS, error::EthApiError, BlockProvider, ChainProvider, GasProvider, LogProvider,
@@ -123,7 +123,7 @@ where
 
     #[tracing::instrument(skip(self), ret, err)]
     async fn transaction_by_hash(&self, hash: B256) -> Result<Option<WithOtherFields<Transaction>>> {
-        Ok(self.eth_client.eth_provider().transaction_by_hash(hash).await?)
+        Ok(self.eth_client.transaction_by_hash(hash).await?)
     }
 
     #[tracing::instrument(skip(self), ret, err)]

src/providers/eth_provider/transactions.rs

@@ -1,14 +1,13 @@
 use super::{
-    constant::HASH_HEX_STRING_LEN,
-    database::{filter::EthDatabaseFilterBuilder, types::transaction::StoredTransaction, CollectionName},
+    database::{filter::EthDatabaseFilterBuilder, types::transaction::StoredTransaction},
     error::ExecutionError,
     starknet::kakarot_core::{account_contract::AccountContractReader, starknet_address},
     utils::{contract_not_found, entrypoint_not_found},
 };
 use crate::{
     into_via_wrapper,
     providers::eth_provider::{
-        database::filter::{self, format_hex},
+        database::filter::{self},
         provider::{EthApiResult, EthDataProvider},
         ChainProvider,
     },
@@ -50,39 +49,8 @@ where
     SP: starknet::providers::Provider + Send + Sync,
 {
     async fn transaction_by_hash(&self, hash: B256) -> EthApiResult<Option<WithOtherFields<Transaction>>> {
-        // TODO: modify this for the tests to pass because now we don't have a pending transactions collection anymore.
-        // TODO: So we need to remove the unionWith part and we need to search inside the final transactions collection + inside the mempool.
-        let pipeline = vec![
-            doc! {
-                // Union with pending transactions with only specified hash
-                "$unionWith": {
-                    "coll": StoredTransaction::collection_name(),
-                    "pipeline": [
-                        {
-                            "$match": {
-                                "tx.hash": format_hex(hash, HASH_HEX_STRING_LEN)
-                            }
-                        }
-                    ]
-                },
-            },
-            // Only specified hash in the transactions collection
-            doc! {
-                "$match": {
-                    "tx.hash": format_hex(hash, HASH_HEX_STRING_LEN)
-                }
-            },
-            // Sort in descending order by block number as pending transactions have null block number
-            doc! {
-                "$sort": { "tx.blockNumber" : -1 }
-            },
-            // Only one document in the final result with priority to the final transactions collection if available
-            doc! {
-                "$limit": 1
-            },
-        ];
-
-        Ok(self.database().get_one_aggregate::<StoredTransaction>(pipeline).await?.map(Into::into))
+        let filter = EthDatabaseFilterBuilder::<filter::Transaction>::default().with_tx_hash(&hash).build();
+        Ok(self.database().get_one::<StoredTransaction>(filter, None).await?.map(Into::into))
     }
 
     async fn transaction_by_block_hash_and_index(

tests/tests/debug_api.rs

@@ -2,7 +2,8 @@
 #![cfg(feature = "testing")]
 use alloy_rlp::Encodable;
 use kakarot_rpc::{
-    providers::eth_provider::{BlockProvider, ReceiptProvider, TransactionProvider},
+    client::TransactionHashProvider,
+    providers::eth_provider::{BlockProvider, ReceiptProvider},
     test_utils::{
         fixtures::{katana, setup},
         katana::Katana,
@@ -154,7 +155,7 @@ async fn test_raw_transactions(#[future] katana: Katana, _setup: ()) {
         .enumerate()
     {
         // Fetch the transaction for the current transaction hash.
-        let tx = eth_provider.transaction_by_hash(actual_tx.hash).await.unwrap().unwrap();
+        let tx = katana.eth_client.transaction_by_hash(actual_tx.hash).await.unwrap().unwrap();
         let signature = tx.signature.unwrap();
 
         // Convert the transaction to a primitives transactions and encode it.

tests/tests/eth_provider.rs

@@ -2,12 +2,14 @@
 #![cfg(feature = "testing")]
 use alloy_primitives::{address, bytes};
 use alloy_sol_types::{sol, SolCall};
+use arbitrary::Arbitrary;
 use kakarot_rpc::{
-    client::KakarotTransactions,
+    client::{KakarotTransactions, TransactionHashProvider},
     into_via_try_wrapper,
     models::felt::Felt252Wrapper,
     providers::eth_provider::{
         constant::{MAX_LOGS, STARKNET_MODULUS},
+        database::{ethereum::EthereumTransactionStore, types::transaction::StoredTransaction},
         provider::EthereumProvider,
         starknet::relayer::LockedRelayer,
         BlockProvider, ChainProvider, GasProvider, LogProvider, ReceiptProvider, StateProvider, TransactionProvider,
@@ -20,6 +22,7 @@ use kakarot_rpc::{
         tx_waiter::watch_tx,
     },
 };
+use rand::Rng;
 use reth_primitives::{
     sign_message, transaction::Signature, Address, BlockNumberOrTag, Bytes, Transaction, TransactionSigned, TxEip1559,
     TxKind, TxLegacy, B256, U256, U64,
@@ -28,7 +31,7 @@ use reth_rpc_types::{
     request::TransactionInput, serde_helpers::JsonStorageKey, state::AccountOverride, Filter, FilterBlockOption,
     FilterChanges, Log, RpcBlockHash, Topic, TransactionRequest,
 };
-use reth_transaction_pool::TransactionPool;
+use reth_transaction_pool::{TransactionOrigin, TransactionPool};
 use rstest::*;
 use starknet::{
     accounts::Account,
@@ -38,6 +41,8 @@ use starknet::{
 use std::{collections::HashMap, sync::Arc};
 use tokio::sync::Mutex;
 
+use crate::tests::mempool::create_sample_transactions;
+
 #[rstest]
 #[awt]
 #[tokio::test(flavor = "multi_thread")]
@@ -1330,60 +1335,91 @@ async fn test_call_with_state_override_bytecode(#[future] plain_opcodes: (Katana
 #[rstest]
 #[awt]
 #[tokio::test(flavor = "multi_thread")]
-#[ignore = "failing because of relayer change"]
-async fn test_transaction_by_hash(#[future] katana: Katana, _setup: ()) {
-    // Given
-    // Retrieve an instance of the Ethereum provider from the test environment
-    let eth_provider = katana.eth_provider();
-    let eth_client = katana.eth_client();
-    let chain_id = eth_provider.chain_id().await.unwrap().unwrap_or_default().to();
-
-    // Retrieve the first transaction from the test environment
-    let first_transaction = katana.first_transaction().unwrap();
+async fn test_transaction_by_hash(#[future] katana_empty: Katana, _setup: ()) {
+    // Create a sample transaction
+    let (transaction, transaction_signed) = create_sample_transactions(&katana_empty, 1)
+        .await
+        .expect("Failed to create sample transaction")
+        .pop()
+        .expect("Expected at least one transaction");
+
+    // Insert the transaction into the mempool
+    let tx_hash = katana_empty
+        .eth_client
+        .mempool()
+        .add_transaction(TransactionOrigin::Local, transaction.clone())
+        .await
+        .expect("Failed to insert transaction into the mempool");
 
     // Check if the first transaction is returned correctly by the `transaction_by_hash` method
-    assert_eq!(eth_provider.transaction_by_hash(first_transaction.hash).await.unwrap().unwrap(), first_transaction);
+    assert!(katana_empty.eth_client.transaction_by_hash(transaction.transaction().hash).await.unwrap().is_some());
 
     // Check if a non-existent transaction returns None
-    assert!(eth_provider.transaction_by_hash(B256::random()).await.unwrap().is_none());
+    assert!(katana_empty.eth_client.transaction_by_hash(B256::random()).await.unwrap().is_none());
 
-    // Generate a pending transaction to be stored in the pending transactions collection
-    // Create a sample transaction
-    let transaction = Transaction::Eip1559(TxEip1559 {
-        chain_id,
-        gas_limit: 21000,
-        to: TxKind::Call(Address::random()),
-        value: U256::from(1000),
-        max_fee_per_gas: 875_000_000,
-        ..Default::default()
-    });
+    // Remove the transaction from the mempool
+    katana_empty.eth_client.mempool().remove_transactions(vec![tx_hash]);
 
-    // Sign the transaction
-    let signature = sign_message(katana.eoa().private_key(), transaction.signature_hash()).unwrap();
-    let transaction_signed = TransactionSigned::from_transaction_and_signature(transaction, signature);
+    // Check that the transaction is no longer in the mempool
+    assert_eq!(katana_empty.eth_client.mempool().pool_size().total, 0);
 
     // Send the transaction
-    let _ = eth_client
+    let _ = katana_empty
+        .eth_client
         .send_raw_transaction(transaction_signed.envelope_encoded())
         .await
         .expect("failed to send transaction");
 
-    // TODO: need to write this with the mempool
-    // // Retrieve the pending transaction from the database
-    // let mut stored_transaction: StoredPendingTransaction =
-    //     eth_provider.database().get_first().await.expect("Failed to get transaction").unwrap();
+    // Prepare the relayer
+    let relayer_balance = katana_empty
+        .eth_client
+        .starknet_provider()
+        .balance_at(katana_empty.eoa.relayer.address(), BlockId::Tag(BlockTag::Latest))
+        .await
+        .expect("Failed to get relayer balance");
+    let relayer_balance = into_via_try_wrapper!(relayer_balance).expect("Failed to convert balance");
+
+    let nonce = katana_empty
+        .eth_client
+        .starknet_provider()
+        .get_nonce(BlockId::Tag(BlockTag::Latest), katana_empty.eoa.relayer.address())
+        .await
+        .unwrap_or_default();
 
-    // let tx = stored_transaction.clone().tx;
+    let current_nonce = Mutex::new(nonce);
 
-    // // Check if the pending transaction is returned correctly by the `transaction_by_hash` method
-    // assert_eq!(eth_provider.transaction_by_hash(tx.hash).await.unwrap().unwrap(), tx);
+    // Relay the transaction
+    let _ = LockedRelayer::new(
+        current_nonce.lock().await,
+        katana_empty.eoa.relayer.address(),
+        relayer_balance,
+        &(*(*katana_empty.eth_client.starknet_provider())),
+        katana_empty.eth_client.starknet_provider().chain_id().await.expect("Failed to get chain id"),
+    )
+    .relay_transaction(&transaction_signed)
+    .await
+    .expect("Failed to relay transaction");
+
+    // Retrieve the current size of the mempool
+    let mempool_size_after_send = katana_empty.eth_client.mempool().pool_size();
+    // Assert that the number of pending transactions in the mempool is 1
+    assert_eq!(mempool_size_after_send.pending, 1);
+    assert_eq!(mempool_size_after_send.total, 1);
 
-    // // Modify the block number of the pending transaction
-    // stored_transaction.tx.block_number = Some(1111);
+    // Check if the pending transaction is returned correctly by the `transaction_by_hash` method
+    assert!(katana_empty.eth_client.transaction_by_hash(transaction.transaction().hash).await.unwrap().is_some());
 
-    // // Insert the transaction into the final transaction collection
-    // eth_provider.database().upsert_transaction(stored_transaction.into()).await.expect("Failed to insert documents");
+    // Generate a random transaction
+    let mut bytes = [0u8; 1024];
+    rand::thread_rng().fill(bytes.as_mut_slice());
+    let transaction = StoredTransaction::arbitrary(&mut arbitrary::Unstructured::new(&bytes)).unwrap();
 
-    // // Check if the final transaction is returned correctly by the `transaction_by_hash` method
-    // assert_eq!(eth_provider.transaction_by_hash(tx.hash).await.unwrap().unwrap().block_number, Some(1111));
+    // Insert the transaction into the database to simulate a transaction that has been indexed
+    katana_empty.eth_provider().database().upsert_transaction(transaction.clone().tx).await.unwrap();
+
+    // Check if the indexed transaction is returned correctly by the `transaction_by_hash` method
+    assert_eq!(
+        katana_empty.eth_client.transaction_by_hash(transaction.tx.hash).await.unwrap().unwrap(),
+        transaction.tx
+    );
 }