Refactor: fix docs, clean up code quality #843

prestwich · 2025-12-19T16:37:50Z

📝 Summary

No longer based on #842, which has been closed

Improves and corrects documentation in the live_builder, particularly around Order propagation through the OrderPool. Functionality does not change. Most significant API change is around BlobTypeOrderFilter, by moving constructors from free functions to assoc functions

Improve and correct OrderPool documentation
Add Copy impl for ReplacementData<KeyType>
Add missing Copy impl for NonceKey
make implementation of BlobTypeOrderFilter construction idiomatic for Rust
add missing trace! event to BlobTypeOrderFilter when filtering an order
add rule_name to BlobTypeOrderFilter to ensure behavior is clear in tracing event
add From impls for ReplaceableOrderEvent, and remove some relevant .clone invocations
update AutoRemovingOrderPoolSubscriptionId to use Weak to prevent them from holding OrderPool memory alloc for dying subscriptions
use FuturesUnordered instead of Vec<JoinHandle<()>> in the order pool to ensure parallelization of handle reaping

✅ I have completed the following steps:

Run make lint
Run make test
Added tests (if applicable)

Copilot

Pull request overview

This PR refactors the live_builder order_input code with a focus on improving documentation, code quality, and idiomatic Rust patterns. The PR includes documentation improvements for OrderPool propagation, adding Copy trait implementations, cleaning up unnecessary clones, refactoring BlobTypeOrderFilter API from free functions to associated functions, and improving resource management with Weak pointers.

Improved documentation for OrderPool and related components to clarify order propagation behavior
Added Copy trait implementations for types that support it (e.g., ReplacementData<KeyType>, ValidBundleState, BundleReplacementState)
Refactored BlobTypeOrderFilter API from free functions to idiomatic associated functions

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/rbuilder/src/live_builder/order_input/orderpool.rs	Enhanced documentation for OrderPool and related structures; changed `process_commands` to use generic iterator; removed unnecessary `.clone()` calls
crates/rbuilder/src/live_builder/order_input/order_replacement_manager.rs	Improved documentation; added `Copy` trait to `ValidBundleState` and `BundleReplacementState`; removed `.clone()` calls in tests
crates/rbuilder/src/live_builder/order_input/mod.rs	Changed handles storage from `Vec` to `FuturesUnordered` for parallel reaping; updated `AutoRemovingOrderPoolSubscriptionId` to use `Weak` pointer; enhanced documentation
crates/rbuilder/src/live_builder/order_input/blob_type_order_filter.rs	Refactored from free functions to associated functions; added `rule_name` field; added trace event for filtered orders; changed filter logic
crates/rbuilder/src/live_builder/order_flow_tracing/order_flow_tracer.rs	Utilized new `From` trait implementations to remove `.clone()` calls; improved documentation
crates/rbuilder/src/live_builder/order_flow_tracing/events.rs	Added `From` trait implementations for `ReplaceableOrderEvent`
crates/rbuilder/src/live_builder/building/mod.rs	Updated to use new `BlobTypeOrderFilter` associated functions
crates/rbuilder-primitives/src/test_data_generator.rs	Removed unnecessary `.clone()` calls
crates/rbuilder-primitives/src/lib.rs	Added `Copy` impl for `ReplacementData<KeyType>`; removed unnecessary `.clone()` in `replacement_key_and_sequence_number`
crates/rbuilder-primitives/benches/ssz_proof.rs	Removed unnecessary `default()` calls and `.clone()` for unit-like structs

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

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

Copilot · 2025-12-30T14:59:13Z

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

+/// Cancels are treated as highest-priority, and after that we must always
+/// honor the replacement with highest `sequence_number`.
+///
+/// Although all the structs and fields say "bundle" we always refer to Bundle


Spelling error: "reefer" should be "refer"

copilot this error is fixeeed in the diff

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

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

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

prestwich · 2025-12-30T15:35:11Z

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

+    /// [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()


this logic relies on 4844 validity condition that transactions cannot have 0 blobs

See:
https://eips.ethereum.org/EIPS/eip-4844#blob-transaction:~:text=Execution%20layer%20validation,-On

prestwich · 2025-12-30T15:36:18Z

crates/rbuilder-primitives/src/lib.rs

+/// 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


is this comment accurate? there is an AsRef<Recovered<TransactionSigned>> implementation that allows direct access to the internal tx

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-30T19:13:24Z

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

-/// events (Orders and cancellations) for a particular block even if the orders arrived in the past.
-/// Since by infra restrictions bundle cancellations don't have an associated block so we store them for a while and asume
-/// they are valid for all in progress sinks
+/// Repository of ALL orders and cancellations that arrives us via


Grammar issue: "arrives us via" should be "arrive to us via" or simply "that arrive via".

Suggested change

/// Repository of ALL orders and cancellations that arrives us via

/// Repository of ALL orders and cancellations that arrive via

Copilot · 2025-12-30T19:13:25Z

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

-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 struct ValidBundleState {
    /// Current valid  sequence_number (larges we've seen)


Typo: "larges" should be "largest".

Suggested change

/// Current valid sequence_number (larges we've seen)

/// Current valid sequence_number (largest we've seen)

prestwich added 2 commits December 30, 2025 09:54

chore: update docs, clean up code quality

2e7feaf

lint: clippy --all-targets

bc4a166

prestwich force-pushed the prestwich/docs-and-cleanup branch from 1ea2cbd to bc4a166 Compare December 30, 2025 14:54

prestwich marked this pull request as ready for review December 30, 2025 14:55

prestwich requested review from ZanCorDX and dvush as code owners December 30, 2025 14:55

Copilot AI review requested due to automatic review settings December 30, 2025 14:55

Copilot started reviewing on behalf of prestwich December 30, 2025 14:56 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

prestwich added 2 commits December 30, 2025 10:03

docs: improve filter documentation

2fad1e0

doc: update SinkSubscription docs

a7a303d

Copilot AI review requested due to automatic review settings December 30, 2025 15:26

nit: assume

fecdaae

Copilot started reviewing on behalf of prestwich December 30, 2025 15:26 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

fix: filter logic

ffa51ec

prestwich commented Dec 30, 2025

View reviewed changes

fix: typos

dd1647a

Copilot AI review requested due to automatic review settings December 30, 2025 19:09

Copilot started reviewing on behalf of prestwich December 30, 2025 19:09 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

	/// Repository of ALL orders and cancellations that arrives us via
	/// Repository of ALL orders and cancellations that arrive via

	/// Current valid sequence_number (larges we've seen)
	/// Current valid sequence_number (largest we've seen)

Refactor: fix docs, clean up code quality #843

Are you sure you want to change the base?

Refactor: fix docs, clean up code quality #843

Conversation

prestwich commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Summary

✅ I have completed the following steps:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

prestwich Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prestwich Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

prestwich Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

prestwich commented Dec 19, 2025 •

edited

Loading