Remove tx default debug (#166)

## 📝 Summary

This PR tries to minimize the access to
TransactionSignedEcRecoveredWithBlobs::tx to avoid leaking information
by mistake:
- tx is not pub anymore.
- Replaced the default fmt::Debug implementation for just the hash.
- Removed unnecessary AsRef


## 💡 Motivation and Context

We are trying to avoid leaking tx info for future TEE deployments.

## ✅ I have completed the following steps:

* [x] Run `make lint`
* [x] Run `make test`
* [ ] Added tests (if applicable)

crates/rbuilder/src/backtest/backtest_build_block.rs

@@ -243,8 +243,7 @@ fn print_order_and_timestamp(orders_with_ts: &[OrdersWithTimestamp], block_data:
             )
         );
         for (tx, optional) in owt.order.list_txs() {
-            let tx = &tx.tx;
-            println!("    {:?} {:?}", tx.hash, optional);
+            println!("    {:?} {:?}", tx.hash(), optional);
             println!(
                 "        from: {:?} to: {:?} nonce: {}",
                 tx.signer(),
@@ -326,15 +325,15 @@ fn print_onchain_block_data(
     let txs_to_idx: HashMap<_, _> = tx_sim_results
         .iter()
         .enumerate()
-        .map(|(idx, tx)| (tx.tx.hash(), idx))
+        .map(|(idx, tx)| (tx.hash(), idx))
         .collect();
 
     println!("Onchain block txs:");
     for (idx, tx) in tx_sim_results.into_iter().enumerate() {
         println!(
             "{:>4}, {:>74} revert: {:>5} profit: {}",
             idx,
-            tx.tx.hash(),
+            tx.hash(),
             !tx.receipt.success,
             format_ether(tx.coinbase_profit)
         );
@@ -352,7 +351,7 @@ fn print_onchain_block_data(
             }
         }
         executed_orders.push(ExecutedBlockTx::new(
-            tx.tx.hash,
+            tx.hash(),
             tx.coinbase_profit,
             tx.receipt.success,
         ))

crates/rbuilder/src/backtest/redistribute/mod.rs

@@ -247,7 +247,7 @@ fn restore_available_landed_orders(
     .into_iter()
     .map(|executed_tx| {
         ExecutedBlockTx::new(
-            executed_tx.tx.hash,
+            executed_tx.hash(),
             executed_tx.coinbase_profit,
             executed_tx.receipt.success,
         )
@@ -949,7 +949,7 @@ fn order_redistribution_address(order: &Order, protect_signers: &[Address]) -> O
         Some(signer) => signer,
         None => {
             return if order.is_tx() {
-                Some(order.list_txs().first()?.0.tx.signer())
+                Some(order.list_txs().first()?.0.signer())
             } else {
                 None
             }
@@ -964,7 +964,7 @@ fn order_redistribution_address(order: &Order, protect_signers: &[Address]) -> O
         Order::Bundle(bundle) => {
             // if its just a bundle we take origin tx of the first transaction
             let tx = bundle.txs.first()?;
-            Some(tx.tx.signer())
+            Some(tx.signer())
         }
         Order::ShareBundle(bundle) => {
             // if it is a share bundle we take either
@@ -977,7 +977,7 @@ fn order_redistribution_address(order: &Order, protect_signers: &[Address]) -> O
 
             let txs = bundle.list_txs();
             let (first_tx, _) = txs.first()?;
-            Some(first_tx.tx.signer())
+            Some(first_tx.signer())
         }
         Order::Tx(_) => {
             unreachable!("Mempool tx order can't have signer");

crates/rbuilder/src/backtest/restore_landed_orders/find_landed_orders.rs

@@ -36,7 +36,7 @@ impl SimplifiedOrder {
                         } else {
                             TxRevertBehavior::NotAllowed
                         };
-                        (tx.tx.hash, revert)
+                        (tx.hash(), revert)
                     })
                     .collect();
                 SimplifiedOrder::new(id, vec![OrderChunk::new(txs, false, 0)])

crates/rbuilder/src/backtest/restore_landed_orders/resim_landed_block.rs

@@ -11,18 +11,24 @@ use alloy_primitives::{Address, B256, I256};
 use eyre::Context;
 use reth_chainspec::ChainSpec;
 use reth_db::DatabaseEnv;
-use reth_primitives::{Receipt, TransactionSignedEcRecovered};
+use reth_primitives::{Receipt, TransactionSignedEcRecovered, TxHash};
 use reth_provider::ProviderFactory;
 use std::sync::Arc;
 
 #[derive(Debug)]
 pub struct ExecutedTxs {
-    pub tx: TransactionSignedEcRecovered,
+    tx: TransactionSignedEcRecovered,
     pub receipt: Receipt,
     pub coinbase_profit: I256,
     pub conflicting_txs: Vec<(B256, Vec<SlotKey>)>,
 }
 
+impl ExecutedTxs {
+    pub fn hash(&self) -> TxHash {
+        self.tx.hash()
+    }
+}
+
 pub fn sim_historical_block(
     provider_factory: ProviderFactory<Arc<DatabaseEnv>>,
     chain_spec: Arc<ChainSpec>,
@@ -98,7 +104,7 @@ pub fn sim_historical_block(
         };
 
         results.push(ExecutedTxs {
-            tx: tx.tx,
+            tx: tx.into_internal_tx_unsecure(),
             receipt: result.receipt,
             coinbase_profit,
             conflicting_txs,
@@ -115,7 +121,7 @@ fn find_suggested_fee_recipient(
     let coinbase = block.header.miner;
     let (last_tx_signer, last_tx_to) = if let Some((signer, to)) = txs
         .last()
-        .map(|tx| (tx.tx.signer(), tx.tx.to().unwrap_or_default()))
+        .map(|tx| (tx.signer(), tx.to().unwrap_or_default()))
     {
         (signer, to)
     } else {

crates/rbuilder/src/bin/debug-bench-machine.rs

@@ -89,11 +89,7 @@ async fn main() -> eyre::Result<()> {
             let tx = tx
                 .try_ecrecovered()
                 .ok_or_else(|| eyre::eyre!("Failed to recover tx"))?;
-            let tx = TransactionSignedEcRecoveredWithBlobs {
-                tx,
-                blobs_sidecar: Default::default(),
-                metadata: Default::default(),
-            };
+            let tx = TransactionSignedEcRecoveredWithBlobs::new_for_testing(tx);
             let tx = MempoolTx::new(tx);
             Ok::<_, eyre::Error>(Order::Tx(tx))
         })

crates/rbuilder/src/building/built_block_trace.rs

@@ -116,7 +116,6 @@ impl BuiltBlockTrace {
                 &mut executed_tx_hashes_scratchpad
             };
             for (tx, receipt) in res.txs.iter().zip(res.receipts.iter()) {
-                let tx = &tx.tx;
                 executed_tx_hashes.push((tx.hash(), receipt.success));
                 if blocklist.contains(&tx.signer())
                     || tx.to().map(|to| blocklist.contains(&to)).unwrap_or(false)

crates/rbuilder/src/building/mod.rs

@@ -376,7 +376,7 @@ impl ExecutionError {
                     tx: tx_nonce,
                     ..
                 }),
-            )) => Some((order.list_txs().first()?.0.tx.signer(), *tx_nonce)),
+            )) => Some((order.list_txs().first()?.0.signer(), *tx_nonce)),
             ExecutionError::OrderError(OrderErr::Bundle(BundleErr::InvalidTransaction(
                 hash,
                 TransactionErr::InvalidTransaction(InvalidTransaction::NonceTooHigh {
@@ -387,9 +387,8 @@ impl ExecutionError {
                 let signer = order
                     .list_txs()
                     .iter()
-                    .find(|(tx, _)| &tx.tx.hash == hash)?
+                    .find(|(tx, _)| TransactionSignedEcRecoveredWithBlobs::hash(tx) == *hash)?
                     .0
-                    .tx
                     .signer();
                 Some((signer, *tx_nonce))
             }
@@ -629,11 +628,10 @@ impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
 
         // double check blocked txs
         for tx_with_blob in &self.executed_tx {
-            let tx = &tx_with_blob.tx;
-            if ctx.blocklist.contains(&tx.signer()) {
+            if ctx.blocklist.contains(&tx_with_blob.signer()) {
                 return Err(eyre::eyre!("To from blocked address."));
             }
-            if let Some(to) = tx.to() {
+            if let Some(to) = tx_with_blob.to() {
                 if ctx.blocklist.contains(&to) {
                     return Err(eyre::eyre!("Tx to blocked address"));
                 }
@@ -681,7 +679,11 @@ impl<Tracer: SimulationTracer> PartialBlock<Tracer> {
 
         let block = Block {
             header,
-            body: self.executed_tx.into_iter().map(|t| t.tx.into()).collect(),
+            body: self
+                .executed_tx
+                .into_iter()
+                .map(|t| t.into_internal_tx_unsecure().into())
+                .collect(),
             ommers: vec![],
             withdrawals,
             requests,

crates/rbuilder/src/building/order_commit.rs

@@ -396,7 +396,7 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
         }
 
         let mut db = self.state.new_db_ref();
-        let tx = &tx_with_blobs.tx;
+        let tx = &tx_with_blobs.internal_tx_unsecure();
         if ctx.blocklist.contains(&tx.signer())
             || tx
                 .to()
@@ -420,7 +420,7 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
         }
 
         let mut tx_env = TxEnv::default();
-        let tx_signed = tx_with_blobs.tx.clone().into_signed();
+        let tx_signed = tx_with_blobs.internal_tx_unsecure().clone().into_signed();
         tx_signed.fill_tx_env(&mut tx_env, tx_signed.recover_signer().unwrap());
 
         let env = Env {
@@ -557,7 +557,6 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
             original_order_ids: Vec::new(),
         };
         for tx_with_blobs in &bundle.txs {
-            let tx = &tx_with_blobs.tx;
             let result = self.commit_tx(
                 tx_with_blobs,
                 ctx,
@@ -567,8 +566,10 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
             )?;
             match result {
                 Ok(res) => {
-                    if !res.receipt.success && !bundle.reverting_tx_hashes.contains(&tx.hash()) {
-                        return Ok(Err(BundleErr::TransactionReverted(tx.hash())));
+                    if !res.receipt.success
+                        && !bundle.reverting_tx_hashes.contains(&tx_with_blobs.hash())
+                    {
+                        return Ok(Err(BundleErr::TransactionReverted(tx_with_blobs.hash())));
                     }
 
                     insert.gas_used += res.gas_used;
@@ -581,10 +582,13 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
                 }
                 Err(err) => {
                     // if optional transaction, skip
-                    if allow_tx_skip && bundle.reverting_tx_hashes.contains(&tx.hash()) {
+                    if allow_tx_skip && bundle.reverting_tx_hashes.contains(&tx_with_blobs.hash()) {
                         continue;
                     } else {
-                        return Ok(Err(BundleErr::InvalidTransaction(tx.hash(), err)));
+                        return Ok(Err(BundleErr::InvalidTransaction(
+                            tx_with_blobs.hash(),
+                            err,
+                        )));
                     }
                 }
             }
@@ -792,9 +796,7 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
                             if !res.receipt.success {
                                 match sbundle_tx.revert_behavior {
                                     crate::primitives::TxRevertBehavior::NotAllowed => {
-                                        return Ok(Err(BundleErr::TransactionReverted(
-                                            tx.tx.hash(),
-                                        )));
+                                        return Ok(Err(BundleErr::TransactionReverted(tx.hash())));
                                     }
                                     crate::primitives::TxRevertBehavior::AllowedIncluded => {}
                                     crate::primitives::TxRevertBehavior::AllowedExcluded => {
@@ -828,7 +830,7 @@ impl<'a, 'b, Tracer: SimulationTracer> PartialBlockFork<'a, 'b, Tracer> {
                             if allow_tx_skip && sbundle_tx.revert_behavior.can_revert() {
                                 continue;
                             } else {
-                                return Ok(Err(BundleErr::InvalidTransaction(tx.tx.hash(), err)));
+                                return Ok(Err(BundleErr::InvalidTransaction(tx.hash(), err)));
                             }
                         }
                     }

crates/rbuilder/src/live_builder/order_input/txpool_fetcher.rs

@@ -176,7 +176,7 @@ mod test {
         }
         .unwrap();
 
-        assert_eq!(tx_with_blobs.tx.hash(), *pending_tx.tx_hash());
+        assert_eq!(tx_with_blobs.hash(), *pending_tx.tx_hash());
         assert_eq!(tx_with_blobs.blobs_sidecar.blobs.len(), 1);
 
         // send another tx without blobs
@@ -198,7 +198,7 @@ mod test {
         }
         .unwrap();
 
-        assert_eq!(tx_without_blobs.tx.hash(), *pending_tx.tx_hash());
+        assert_eq!(tx_without_blobs.hash(), *pending_tx.tx_hash());
         assert_eq!(tx_without_blobs.blobs_sidecar.blobs.len(), 0);
     }
 }

crates/rbuilder/src/primitives/mod.rs

@@ -395,28 +395,34 @@ impl ShareBundle {
     }
 }
 
-/// First idea to handle blobs might change.
+/// First idea to handle blobs, might change.
 /// Don't like the fact that blobs_sidecar exists no matter if TransactionSignedEcRecovered contains a non blob tx.
+/// Great effort was put in avoiding simple access to the internal tx so we don't accidentally leak information on logs (particularly the tx sign).
 #[derive(Derivative)]
-#[derivative(Debug, Clone, PartialEq, Eq)]
+#[derivative(Clone, PartialEq, Eq)]
 pub struct TransactionSignedEcRecoveredWithBlobs {
-    pub tx: TransactionSignedEcRecovered,
+    tx: TransactionSignedEcRecovered,
     /// Will have a non empty BlobTransactionSidecar if TransactionSignedEcRecovered is 4844
     pub blobs_sidecar: Arc<BlobTransactionSidecar>,
 
     #[derivative(PartialEq = "ignore", Hash = "ignore")]
     pub metadata: Metadata,
 }
 
-impl AsRef<TransactionSignedEcRecovered> for TransactionSignedEcRecoveredWithBlobs {
-    fn as_ref(&self) -> &TransactionSignedEcRecovered {
+impl AsRef<TransactionSigned> for TransactionSignedEcRecoveredWithBlobs {
+    fn as_ref(&self) -> &TransactionSigned {
         &self.tx
     }
 }
 
-impl AsRef<TransactionSigned> for TransactionSignedEcRecoveredWithBlobs {
-    fn as_ref(&self) -> &TransactionSigned {
-        &self.tx
+/// Custom fmt to avoid leaking information.
+impl std::fmt::Debug for TransactionSignedEcRecoveredWithBlobs {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "TransactionSignedEcRecoveredWithBlobs {{ hash: {} }}",
+            self.hash(),
+        )
     }
 }
 
@@ -436,11 +442,16 @@ impl TransactionSignedEcRecoveredWithBlobs {
         if tx.transaction.blob_versioned_hashes().is_some() {
             return None;
         }
-        Some(Self {
+        Some(Self::new_for_testing(tx))
+    }
+
+    /// Creates a Self with empty blobs sidecar. No consistency check is performed,
+    pub fn new_for_testing(tx: TransactionSignedEcRecovered) -> Self {
+        Self {
             tx,
-            blobs_sidecar: Arc::new(BlobTransactionSidecar::default()),
+            blobs_sidecar: Default::default(),
             metadata: Default::default(),
-        })
+        }
     }
 
     pub fn hash(&self) -> TxHash {
@@ -451,8 +462,27 @@ impl TransactionSignedEcRecoveredWithBlobs {
         self.tx.signer()
     }
 
+    pub fn to(&self) -> Option<Address> {
+        self.tx.to()
+    }
+
+    pub fn nonce(&self) -> u64 {
+        self.tx.nonce()
+    }
+
+    /// USE CAREFULLY since this exposes the signed tx.
+    pub fn internal_tx_unsecure(&self) -> &TransactionSignedEcRecovered {
+        &self.tx
+    }
+
+    /// USE CAREFULLY since this exposes the signed tx.
+    pub fn into_internal_tx_unsecure(self) -> TransactionSignedEcRecovered {
+        self.tx
+    }
+
     /// Encodes the "raw" canonical format of transaction (NOT the one used in `eth_sendRawTransaction`) BLOB DATA IS NOT ENCODED.
     /// I intensionally omitted the version with blob data since we don't use it and may lead to confusions/bugs.
+    /// USE CAREFULLY since this exposes the signed tx.
     pub fn envelope_encoded_no_blobs(&self) -> Bytes {
         self.tx.envelope_encoded()
     }