From ff9f02d524130b0d456e6f9c6da234356755d2ea Mon Sep 17 00:00:00 2001
From: Matthias Seitz <matthias.seitz@outlook.de>
Date: Mon, 13 Mar 2023 14:46:13 +0100
Subject: [PATCH] style(rpc): some transaction by touch ups (#1711)

---
 crates/primitives/src/transaction/mod.rs   |  4 +-
 crates/rpc/rpc/src/eth/api/transactions.rs | 83 +++++++++++-----------
 2 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs
index 23885b8b0c..f9d43f6a30 100644
--- a/crates/primitives/src/transaction/mod.rs
+++ b/crates/primitives/src/transaction/mod.rs
@@ -551,7 +551,7 @@ impl TransactionSigned {
 
     /// Recover signer from signature and hash.
     ///
-    /// Returns `None` if the transaction's signature is invalid.
+    /// Returns `None` if the transaction's signature is invalid, see also [Self::recover_signer].
     pub fn recover_signer(&self) -> Option<Address> {
         let signature_hash = self.signature_hash();
         self.signature.recover_signer(signature_hash)
@@ -559,7 +559,7 @@ impl TransactionSigned {
 
     /// Devour Self, recover signer and return [`TransactionSignedEcRecovered`]
     ///
-    /// Returns `None` if the transaction's signature is invalid.
+    /// Returns `None` if the transaction's signature is invalid, see also [Self::recover_signer].
     pub fn into_ecrecovered(self) -> Option<TransactionSignedEcRecovered> {
         let signer = self.recover_signer()?;
         Some(TransactionSignedEcRecovered { signed_transaction: self, signer })
diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs
index 72ce26a63f..bece1772f1 100644
--- a/crates/rpc/rpc/src/eth/api/transactions.rs
+++ b/crates/rpc/rpc/src/eth/api/transactions.rs
@@ -3,7 +3,7 @@ use crate::{
     eth::error::{EthApiError, EthResult},
     EthApi,
 };
-use reth_primitives::{BlockId, Bytes, FromRecoveredTransaction, TransactionSigned, H256, U256};
+use reth_primitives::{BlockId, Bytes, FromRecoveredTransaction, TransactionSigned, H256};
 use reth_provider::{BlockProvider, EvmEnvProvider, StateProviderFactory};
 use reth_rlp::Decodable;
 use reth_rpc_types::{Index, Transaction, TransactionRequest};
@@ -19,56 +19,55 @@ where
         unimplemented!()
     }
 
-    /// Finds a given trasaction by its hash.
-    pub(crate) async fn transaction_by_hash(
-        &self,
-        hash: H256,
-    ) -> EthResult<Option<reth_rpc_types::Transaction>> {
-        let tx_by_hash = match self.client().transaction_by_hash(hash)? {
-            Some(item) => item,
-            None => return Ok(None),
-        };
+    /// Finds a given [Transaction] by its hash.
+    ///
+    /// Returns `Ok(None)` if no matching transaction was found.
+    pub(crate) async fn transaction_by_hash(&self, hash: H256) -> EthResult<Option<Transaction>> {
+        match self.client().transaction_by_hash(hash)? {
+            None => Ok(None),
+            Some(tx) => {
+                let tx = tx.into_ecrecovered().ok_or(EthApiError::InvalidTransactionSignature)?;
 
-        if let Some(tx) = tx_by_hash.into_ecrecovered() {
-            Ok(Some(Transaction::from_recovered_with_block_context(
-                tx,
-                // TODO: this is just stubbed out for now still need to fully implement tx => block
-                H256::default(),
-                u64::default(),
-                U256::from(usize::from(Index::default())),
-            )))
-        } else {
-            Ok(None)
+                let tx = Transaction::from_recovered_with_block_context(
+                    tx,
+                    // TODO: this is just stubbed out for now still need to fully implement tx =>
+                    // block
+                    H256::default(),
+                    u64::default(),
+                    Index::default().into(),
+                );
+                Ok(Some(tx))
+            }
         }
     }
 
-    /// Get Transaction by Block and tx index within that Block.
-    /// Used for both:
-    ///     transaction_by_block_hash_and_index() &
-    ///     transaction_by_block_number_and_index()
+    /// Get Transaction by [BlockId] and the index of the transaction within that Block.
+    ///
+    /// Returns `Ok(None)` if the block does not exist, or the block as fewer transactions
     pub(crate) async fn transaction_by_block_and_tx_index(
         &self,
         block_id: impl Into<BlockId>,
         index: Index,
-    ) -> EthResult<Option<reth_rpc_types::Transaction>> {
+    ) -> EthResult<Option<Transaction>> {
         let block_id = block_id.into();
-        let Some(block) = self.client().block(block_id)? else {
-            return Ok(None);
-        };
-        let block_hash =
-            self.client().block_hash_for_id(block_id)?.ok_or(EthApiError::UnknownBlockNumber)?;
-        let Some(tx_signed) = block.body.into_iter().nth(usize::from(index)) else {
-            return Ok(None);
-        };
-        let Some(tx) = tx_signed.into_ecrecovered() else {
-            return Ok(None);
-        };
-        Ok(Some(Transaction::from_recovered_with_block_context(
-            tx,
-            block_hash,
-            block.header.number,
-            U256::from(usize::from(index)),
-        )))
+        if let Some(block) = self.client().block(block_id)? {
+            let block_hash = self
+                .client()
+                .block_hash_for_id(block_id)?
+                .ok_or(EthApiError::UnknownBlockNumber)?;
+            if let Some(tx_signed) = block.body.into_iter().nth(index.into()) {
+                let tx =
+                    tx_signed.into_ecrecovered().ok_or(EthApiError::InvalidTransactionSignature)?;
+                return Ok(Some(Transaction::from_recovered_with_block_context(
+                    tx,
+                    block_hash,
+                    block.header.number,
+                    index.into(),
+                )))
+            }
+        }
+
+        Ok(None)
     }
 
     /// Decodes and recovers the transaction and submits it to the pool.