Refactor: fix docs, clean up code quality

crates/rbuilder-primitives/benches/ssz_proof.rs

@@ -77,9 +77,9 @@ mod impls {
         let tx_size = 1_024;
         let mut runner = TestRunner::deterministic();
 
-        let mut vanilla = VanillaSszTxProof::default();
+        let mut vanilla = VanillaSszTxProof;
         let mut vanilla_buf = VanillaBufferedSszTxProof::default();
-        let mut compact = CompactSszTxProof::default();
+        let mut compact = CompactSszTxProof;
         for _ in 0..100 {
             let txs = generate_test_data(&mut runner, num_txs, tx_size);
             let expected = vanilla.generate(&txs, proof_target);
@@ -135,7 +135,7 @@ mod impls {
         for idx in 0..MAX_CHUNK_COUNT {
             let leaf = txs
                 .get(idx)
-                .map(|tx| tx_ssz_leaf_root(&tx))
+                .map(|tx| tx_ssz_leaf_root(tx))
                 .unwrap_or(B256::ZERO);
             current_buf.insert(idx, leaf);
         }

crates/rbuilder-primitives/src/lib.rs

@@ -144,6 +144,8 @@ pub struct ReplacementData<KeyType> {
     pub sequence_number: u64,
 }
 
+impl<KeyType: Copy> Copy for ReplacementData<KeyType> {}
+
 impl<KeyType: Clone> ReplacementData<KeyType> {
     /// Next sequence_number, useful for testing.
     pub fn next(&self) -> Self {
@@ -439,13 +441,17 @@ impl FakeSidecar for BlobTransactionSidecar {
 }
 
 /// First idea to handle blobs, might change.
-/// Don't like the fact that blobs_sidecar exists no matter if Recovered<TransactionSigned> 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).
+///
+/// Don't like the fact that blobs_sidecar exists no matter if
+/// [`Recovered<TransactionSigned>`] 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(Clone, PartialEq, Eq)]
 pub struct TransactionSignedEcRecoveredWithBlobs {
     tx: Recovered<TransactionSigned>,
-    /// Will have a non empty BlobTransactionSidecarVariant if Recovered<TransactionSigned> is 4844
+    /// Will have a non empty [`BlobTransactionSidecarVariant`] if [`Recovered<TransactionSigned>`] is 4844
     pub blobs_sidecar: Arc<BlobTransactionSidecarVariant>,
 
     #[derivative(PartialEq = "ignore", Hash = "ignore")]
@@ -858,7 +864,7 @@ impl Order {
             Order::Bundle(bundle) => bundle
                 .replacement_data
                 .as_ref()
-                .map(|r| (r.clone().key, r.sequence_number)),
+                .map(|r| (r.key, r.sequence_number)),
             Order::Tx(_) => None,
         }
     }

crates/rbuilder-primitives/src/test_data_generator.rs

@@ -93,7 +93,7 @@ impl TestDataGenerator {
             reverting_tx_hashes: vec![],
             hash: B256::default(),
             uuid: Uuid::default(),
-            replacement_data: replacement_data.clone(),
+            replacement_data,
             signer: replacement_data.as_ref().and_then(|r| r.key.signer),
             refund_identity: None,
             metadata: Default::default(),
@@ -130,7 +130,7 @@ impl TestDataGenerator {
             reverting_tx_hashes,
             hash: B256::default(),
             uuid: Uuid::default(),
-            replacement_data: replacement_data.clone(),
+            replacement_data,
             signer: replacement_data.as_ref().and_then(|r| r.key.signer),
             refund_identity: None,
             metadata: Default::default(),

crates/rbuilder/src/live_builder/building/mod.rs

@@ -8,8 +8,12 @@ use crate::{
     live_builder::{
         building::built_block_cache::BuiltBlockCache,
         order_flow_tracing::order_flow_tracer_manager::OrderFlowTracerManager,
-        order_input::replaceable_order_sink::ReplaceableOrderSink,
-        payload_events::MevBoostSlotData, simulation::SlotOrderSimResults,
+        order_input::{
+            blob_type_order_filter::BlobTypeOrderFilter,
+            replaceable_order_sink::ReplaceableOrderSink,
+        },
+        payload_events::MevBoostSlotData,
+        simulation::SlotOrderSimResults,
     },
     provider::StateProviderFactory,
 };
@@ -112,13 +116,13 @@ where
             .chain_spec
             .is_osaka_active_at_timestamp(block_ctx.attributes.timestamp)
         {
-            Box::new(order_input::blob_type_order_filter::new_fusaka(Box::new(
+            Box::new(BlobTypeOrderFilter::new_fusaka(Box::new(
                 order_replacement_manager,
             )))
         } else {
-            Box::new(order_input::blob_type_order_filter::new_pre_fusaka(
-                Box::new(order_replacement_manager),
-            ))
+            Box::new(BlobTypeOrderFilter::new_pre_fusaka(Box::new(
+                order_replacement_manager,
+            )))
         };
 
         let mempool_txs_detector_sniffer =

crates/rbuilder/src/live_builder/order_flow_tracing/events.rs

@@ -54,4 +54,22 @@ pub enum ReplaceableOrderEvent {
     RemoveBundle(BundleReplacementData),
 }
 
+impl From<InsertOrderData> for ReplaceableOrderEvent {
+    fn from(data: InsertOrderData) -> Self {
+        ReplaceableOrderEvent::InsertOrder(data)
+    }
+}
+
+impl From<&BundleReplacementData> for ReplaceableOrderEvent {
+    fn from(data: &BundleReplacementData) -> Self {
+        ReplaceableOrderEvent::RemoveBundle(*data)
+    }
+}
+
+impl From<BundleReplacementData> for ReplaceableOrderEvent {
+    fn from(data: BundleReplacementData) -> Self {
+        ReplaceableOrderEvent::RemoveBundle(data)
+    }
+}
+
 pub type ReplaceableOrderEventWithTimestamp = EventWithTimestamp<ReplaceableOrderEvent>;

crates/rbuilder/src/live_builder/order_flow_tracing/order_flow_tracer.rs

@@ -7,8 +7,8 @@ use crate::{
     live_builder::{
         block_output::bidding_service_interface::SlotBlockId,
         order_flow_tracing::events::{
-            InsertOrderData, ReplaceableOrderEvent, ReplaceableOrderEventWithTimestamp,
-            SimulatedOrderData, SimulationEvent, SimulationEventWithTimestamp,
+            InsertOrderData, ReplaceableOrderEventWithTimestamp, SimulatedOrderData,
+            SimulationEvent, SimulationEventWithTimestamp,
         },
         order_input::replaceable_order_sink::ReplaceableOrderSink,
         simulation::simulation_job_tracer::SimulationJobTracer,
@@ -24,15 +24,18 @@ pub struct OrderFlowTracer {
     order_input_events: Mutex<Vec<ReplaceableOrderEventWithTimestamp>>,
 }
 
+/// Report generated by the [`OrderFlowTracer`].
 #[derive(Debug, Serialize, Deserialize)]
 pub struct OrderFlowTracerReport {
     pub sim_events: Vec<SimulationEventWithTimestamp>,
     pub order_input_events: Vec<ReplaceableOrderEventWithTimestamp>,
 }
 
 impl OrderFlowTracer {
-    /// Takes the next ReplaceableOrderSink on the chain and returns the one that will be used to forward the events.
-    /// Also returns the OrderFlowTracer itself.
+    /// Takes the next [`ReplaceableOrderSink`] on the chain and returns the
+    /// one that will be used to forward the events.
+    ///
+    /// Also returns the [`OrderFlowTracer`] itself.
     pub fn new(
         id: SlotBlockId,
         sink: Box<dyn ReplaceableOrderSink>,
@@ -53,19 +56,18 @@ impl OrderFlowTracer {
     }
 
     fn insert_order(&self, order: &Order) {
-        let event = ReplaceableOrderEventWithTimestamp::new(ReplaceableOrderEvent::InsertOrder(
+        let event = ReplaceableOrderEventWithTimestamp::new(
             InsertOrderData {
                 order_id: order.id(),
                 replacement_key_and_sequence_number: order.replacement_key_and_sequence_number(),
                 tx_hashes: order.list_txs().iter().map(|(tx, _)| tx.hash()).collect(),
-            },
-        ));
+            }
+            .into(),
+        );
         self.order_input_events.lock().push(event);
     }
     fn remove_bundle(&self, replacement_data: &BundleReplacementData) {
-        let event = ReplaceableOrderEventWithTimestamp::new(ReplaceableOrderEvent::RemoveBundle(
-            replacement_data.clone(),
-        ));
+        let event = ReplaceableOrderEventWithTimestamp::new(replacement_data.into());
         self.order_input_events.lock().push(event);
     }
 

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

@@ -1,62 +1,82 @@
-use alloy_eips::{eip7594::BlobTransactionSidecarVariant, Typed2718};
-
 use crate::live_builder::order_input::replaceable_order_sink::ReplaceableOrderSink;
 use rbuilder_primitives::{BundleReplacementData, Order, TransactionSignedEcRecoveredWithBlobs};
+use tracing::trace;
 
-/// Filters out Orders with incorrect blobs (pre/post fusaka).
-/// Since it's very unlikely what we have many wrong blobs we only filter on insert_order without take note of filtered orders.
-/// If remove_bundle is called we just forward the call to the sink so it might try to remove a filtered order.
-pub struct BlobTypeOrderFilter<FilterFunc> {
+/// Filters out [`Order`]s based on their blob type. [`Order`]s that do not
+/// pass the filter are dropped not inserted. Preconfigured filters are
+/// provided to remove pre-Fusaka [EIP-4844] blobs ([`Self::new_fusaka`]) or
+/// post-Fusaka [EIP-7594] blobs ([`Self::new_pre_fusaka`]).
+///
+/// Since it's very unlikely that we have many wrong blobs we only filter on
+/// [`ReplaceableOrderSink::insert_order`] without taking note of filtered
+/// [`Order`]s.
+///
+/// [`ReplaceableOrderSink::remove_bundle`] calls are simply forwarded to the
+/// sink, so it might try to remove a filtered [`Order`].
+///
+/// [EIP-4844]: https://eips.ethereum.org/EIPS/eip-4844
+/// [EIP-7594]: https://eips.ethereum.org/EIPS/eip-7594
+pub struct BlobTypeOrderFilter {
     sink: Box<dyn ReplaceableOrderSink>,
-    ///true if it likes the blob sidecar, false if it doesn't (Order gets filtered).
-    filter_func: FilterFunc,
+
+    /// Name of the filter for logging purposes.
+    rule_name: &'static str,
+
+    /// `true` if it likes the blob sidecar, `false` if it doesn't ([`Order`]
+    /// gets filtered).
+    filter_func: fn(&TransactionSignedEcRecoveredWithBlobs) -> bool,
 }
 
-impl<FilterFunc> std::fmt::Debug for BlobTypeOrderFilter<FilterFunc> {
+impl std::fmt::Debug for BlobTypeOrderFilter {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("BlobTypeOrderFilter")
             .field("sink", &"<dyn ReplaceableOrderSink>")
+            .field("rule", &self.rule_name)
             .finish()
     }
 }
 
-/// Filters out EIP-7594 style blobs, supports only EIP-4844 style.
-pub fn new_pre_fusaka(
-    sink: Box<dyn ReplaceableOrderSink>,
-) -> BlobTypeOrderFilter<impl Fn(&TransactionSignedEcRecoveredWithBlobs) -> bool + Send + Sync> {
-    BlobTypeOrderFilter::new(sink, |tx| {
-        if tx.is_eip4844() {
-            matches!(*tx.blobs_sidecar, BlobTransactionSidecarVariant::Eip4844(_))
-        } else {
-            true
+impl BlobTypeOrderFilter {
+    /// Creates a new [`BlobTypeOrderFilter`], using the given `filter_func` to
+    /// filter out orders.
+    pub const fn new(
+        sink: Box<dyn ReplaceableOrderSink>,
+        rule_name: &'static str,
+        filter_func: fn(&TransactionSignedEcRecoveredWithBlobs) -> bool,
+    ) -> Self {
+        Self {
+            sink,
+            rule_name,
+            filter_func,
         }
-    })
-}
+    }
 
-/// Filters out EIP-4844 style, supports only EIP-7594 style blobs.
-pub fn new_fusaka(
-    sink: Box<dyn ReplaceableOrderSink>,
-) -> BlobTypeOrderFilter<impl Fn(&TransactionSignedEcRecoveredWithBlobs) -> bool + Send + Sync> {
-    BlobTypeOrderFilter::new(sink, |tx| {
-        if tx.is_eip4844() {
-            matches!(*tx.blobs_sidecar, BlobTransactionSidecarVariant::Eip7594(_))
-        } else {
-            true
+    /// Filters out [EIP-4844] style, allowing only [EIP-7594] style blobs.
+    ///
+    /// [EIP-4844]: https://eips.ethereum.org/EIPS/eip-4844
+    /// [EIP-7594]: https://eips.ethereum.org/EIPS/eip-7594
+    pub const fn new_fusaka(sink: Box<dyn ReplaceableOrderSink>) -> Self {
+        fn fusaka(tx: &TransactionSignedEcRecoveredWithBlobs) -> bool {
+            !tx.as_ref().is_eip4844() || tx.blobs_sidecar.is_eip7594()
+        }
+
+        Self::new(sink, "fusaka", fusaka)
+    }
+
+    /// Filters out [EIP-7594] style blobs, allowing only [EIP-4844] style.
+    ///
+    /// [EIP-4844]: https://eips.ethereum.org/EIPS/eip-4844
+    /// [EIP-7594]: https://eips.ethereum.org/EIPS/eip-7594
+    pub const fn new_pre_fusaka(sink: Box<dyn ReplaceableOrderSink>) -> Self {
+        fn pre_fusaka(tx: &TransactionSignedEcRecoveredWithBlobs) -> bool {
+            !tx.as_ref().is_eip4844() || tx.blobs_sidecar.is_eip4844()
         }
-    })
-}
 
-impl<FilterFunc: Fn(&TransactionSignedEcRecoveredWithBlobs) -> bool>
-    BlobTypeOrderFilter<FilterFunc>
-{
-    fn new(sink: Box<dyn ReplaceableOrderSink>, filter_func: FilterFunc) -> Self {
-        Self { sink, filter_func }
+        Self::new(sink, "pre-fusaka", pre_fusaka)
     }
 }
 
-impl<FilterFunc: Fn(&TransactionSignedEcRecoveredWithBlobs) -> bool + Send + Sync>
-    ReplaceableOrderSink for BlobTypeOrderFilter<FilterFunc>
-{
+impl ReplaceableOrderSink for BlobTypeOrderFilter {
     fn insert_order(&mut self, order: Order) -> bool {
         if order
             .list_txs()
@@ -65,6 +85,7 @@ impl<FilterFunc: Fn(&TransactionSignedEcRecoveredWithBlobs) -> bool + Send + Syn
         {
             self.sink.insert_order(order)
         } else {
+            trace!(order_id = ?order.id(), rule = self.rule_name, "Order filtered out by BlobTypeOrderFilter");
             true
         }
     }