Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions crates/rbuilder/src/live_builder/block_output/relay_submit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ struct BuiltBlockInfo {
/// How submission works:
/// 0. We divide relays into optimistic and non-optimistic (defined in config file)
/// 1. We schedule submissions with non-optimistic key for all non-optimistic relays.
/// 1.1 If "optimistic_enabled" is false or bid_value >= "optimistic_max_bid_value" we schedule submissions with non-optimistic key
/// 1.1 If "optimistic_enabled" is false or bid_value >= "optimistic_v3_max_bid_eth" we schedule submissions with non-optimistic key

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this doc comment says >= but the actual code on line 432 uses > (strictly greater). Since this PR touched this line to rename the field, it would be a good opportunity to fix the inconsistency.

Suggested change
/// 1.1 If "optimistic_enabled" is false or bid_value >= "optimistic_v3_max_bid_eth" we schedule submissions with non-optimistic key
/// 1.1 If "optimistic_enabled" is false or bid_value > "optimistic_v3_max_bid_eth" we schedule submissions with non-optimistic key

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc comment still says >= but the code on line 432 uses > (strictly greater than). This was flagged in the previous review. Please either update the doc to match the code or vice versa.

Suggested change
/// 1.1 If "optimistic_enabled" is false or bid_value >= "optimistic_v3_max_bid_eth" we schedule submissions with non-optimistic key
/// 1.1 If "optimistic_enabled" is false or bid_value > "optimistic_v3_max_bid_eth" we schedule submissions with non-optimistic key

/// returns the best bid made
#[allow(clippy::too_many_arguments)]
async fn run_submit_to_relays_job(
Expand Down Expand Up @@ -427,21 +427,32 @@ fn submit_block_to_relays(

let mut optimistic_v3 = None;
if relay.optimistic_v3() {
if let Some(config) = optimistic_v3_config {
optimistic_v3 = create_optimistic_v3_request(
&config.builder_url,
request.as_ref(),
maybe_adjustment_data,
relay.optimistic_v3_bid_adjustment_required(),
)
.map(|request| (config.clone(), SubmitHeaderRequestWithMetadata {
submission: request,
metadata: bid_metadata.clone()
}))
.inspect_err(|error| {
error!(parent: submission_span, ?error, "Unable to create optimistic V3 request");
})
.ok();
if relay
.optimistic_v3_max_bid()
.is_some_and(|cap| bid_value > cap)
{
info!(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

info! will fire for every bid above the cap on every block submission cycle. Given that high-value bids may be common in practice, this could produce significant log volume. Consider using trace! or debug! instead, matching the silent approach used by the existing max_bid check on line 411.

Comment on lines +430 to +434

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two items still open from previous reviews:

  1. >= vs "above": The code uses >= (line 432), matching the function-level doc on line 141. But the field-level doc comments in mod.rs (lines 138 and 264) say "above this", which reads as strictly greater than (>). These should be aligned — either change the docs to say "at or above" or change the operator to >.

  2. Log level: info! fires on every bid above the cap per submission cycle. This was flagged in the two previous reviews. debug! would be more appropriate, consistent with the silent max_bid check on line 411 which skips bids without logging at all.

bid_value = format_ether(bid_value),
cap = format_ether(relay.optimistic_v3_max_bid().unwrap()),
"Optimistic V3 disabled: bid exceeds max bid cap"
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues here:

  1. Log level: info! will fire on every bid above the cap per submission cycle. This was flagged in the previous review — debug! or trace! would be more appropriate, consistent with the silent max_bid check on line 411.

  2. Redundant getter + unwrap: relay.optimistic_v3_max_bid() is called twice — once in is_some_and and again with .unwrap(). Using if let Some(cap) avoids both the duplicate call and the unwrap:

if let Some(cap) = relay.optimistic_v3_max_bid() {
    if bid_value > cap {
        debug!(
            bid_value = format_ether(bid_value),
            cap = format_ether(cap),
            "Optimistic V3 disabled: bid exceeds max bid cap"
        );
    } else if let Some(config) = optimistic_v3_config {
        // ... existing logic
    }
} else if let Some(config) = optimistic_v3_config {
    // ... existing logic
}

Or keep the current structure but bind the cap:

if let Some(cap) = relay.optimistic_v3_max_bid().filter(|&cap| bid_value > cap) {
    debug!(
        bid_value = format_ether(bid_value),
        cap = format_ether(cap),
        "Optimistic V3 disabled: bid exceeds max bid cap"
    );
} else if let Some(config) = optimistic_v3_config {

} else {
if let Some(config) = optimistic_v3_config {
optimistic_v3 = create_optimistic_v3_request(
&config.builder_url,
request.as_ref(),
maybe_adjustment_data,
relay.optimistic_v3_bid_adjustment_required(),
)
.map(|request| (config.clone(), SubmitHeaderRequestWithMetadata {
submission: request,
metadata: bid_metadata.clone()
}))
.inspect_err(|error| {
error!(parent: submission_span, ?error, "Unable to create optimistic V3 request");
})
.ok();
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: else { if ... } can be collapsed into else if ... to reduce nesting. This is also a clippy::collapsible_else_if lint.

Suggested change
} else {
if let Some(config) = optimistic_v3_config {
optimistic_v3 = create_optimistic_v3_request(
&config.builder_url,
request.as_ref(),
maybe_adjustment_data,
relay.optimistic_v3_bid_adjustment_required(),
)
.map(|request| (config.clone(), SubmitHeaderRequestWithMetadata {
submission: request,
metadata: bid_metadata.clone()
}))
.inspect_err(|error| {
error!(parent: submission_span, ?error, "Unable to create optimistic V3 request");
})
.ok();
}
}
} else if let Some(config) = optimistic_v3_config {
optimistic_v3 = create_optimistic_v3_request(
&config.builder_url,
request.as_ref(),
maybe_adjustment_data,
relay.optimistic_v3_bid_adjustment_required(),
)
.map(|request| (config.clone(), SubmitHeaderRequestWithMetadata {
submission: request,
metadata: bid_metadata.clone()
}))
.inspect_err(|error| {
error!(parent: submission_span, ?error, "Unable to create optimistic V3 request");
})
.ok();
}

}

Expand Down
5 changes: 5 additions & 0 deletions crates/rbuilder/src/live_builder/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,7 @@ lazy_static! {
optimistic: false,
interval_between_submissions_ms: Some(250),
max_bid_eth: None,
optimistic_v3_max_bid_eth: None,
optimistic_v3: false,
optimistic_v3_bid_adjustment_required: false,
}),
Expand Down Expand Up @@ -985,6 +986,7 @@ lazy_static! {
optimistic: true,
interval_between_submissions_ms: None,
max_bid_eth: None,
optimistic_v3_max_bid_eth: None,
optimistic_v3: false,
optimistic_v3_bid_adjustment_required: false,
}),
Expand Down Expand Up @@ -1013,6 +1015,7 @@ lazy_static! {
optimistic: true,
interval_between_submissions_ms: None,
max_bid_eth: None,
optimistic_v3_max_bid_eth: None,
optimistic_v3: false,
optimistic_v3_bid_adjustment_required: false,
}),
Expand Down Expand Up @@ -1041,6 +1044,7 @@ lazy_static! {
optimistic: true,
interval_between_submissions_ms: None,
max_bid_eth: None,
optimistic_v3_max_bid_eth: None,
optimistic_v3: false,
optimistic_v3_bid_adjustment_required: false,
}),
Expand Down Expand Up @@ -1069,6 +1073,7 @@ lazy_static! {
optimistic: false,
interval_between_submissions_ms: None,
max_bid_eth: None,
optimistic_v3_max_bid_eth: None,
optimistic_v3: false,
optimistic_v3_bid_adjustment_required: false,
}),
Expand Down
16 changes: 16 additions & 0 deletions crates/rbuilder/src/mev_boost/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ pub struct RelaySubmitConfig {
/// Max bid we can submit to this relay. Any bid above this will be skipped.
/// None -> No limit.
pub max_bid_eth: Option<String>,
/// Do not use optimistic V3 when bid value is above this (ETH). None -> no cap.
#[serde(default)]
pub optimistic_v3_max_bid_eth: Option<String>,
}

impl RelayConfig {
Expand Down Expand Up @@ -258,6 +261,8 @@ pub struct MevBoostRelayBidSubmitter {
/// Max bid we can submit to this relay. Any bid above this will be skipped.
/// None -> No limit.
max_bid: Option<U256>,
/// Do not use optimistic V3 when bid value is above this. None -> no cap.
optimistic_v3_max_bid: Option<U256>,
/// This is not a real relay so we can send blocks to it even if it does not have any validator registered.
test_relay: bool,
}
Expand All @@ -275,6 +280,12 @@ impl MevBoostRelayBidSubmitter {
.map(|s| parse_ether(s))
.transpose()
.map_err(|e| eyre::eyre!("Failed to parse max bid: {}", e))?;
let optimistic_v3_max_bid = config
.optimistic_v3_max_bid_eth
.as_ref()
.map(|s| parse_ether(s))
.transpose()
.map_err(|e| eyre::eyre!("Failed to parse optimistic v3 max bid: {}", e))?;
let submission_rate_limiter = config.interval_between_submissions_ms.map(|d| {
Arc::new(RateLimiter::direct(
Quota::with_period(Duration::from_millis(d)).expect("Rate limiter time period"),
Expand All @@ -291,6 +302,7 @@ impl MevBoostRelayBidSubmitter {
optimistic_v3: config.optimistic_v3,
optimistic_v3_bid_adjustment_required: config.optimistic_v3_bid_adjustment_required,
max_bid,
optimistic_v3_max_bid,
test_relay,
})
}
Expand Down Expand Up @@ -319,6 +331,10 @@ impl MevBoostRelayBidSubmitter {
self.max_bid
}

pub fn optimistic_v3_max_bid(&self) -> Option<U256> {
self.optimistic_v3_max_bid
}

/// false -> rate limiter don't allow
pub fn can_submit_bid(&self) -> bool {
if let Some(limiter) = &self.submission_rate_limiter {
Expand Down
1 change: 1 addition & 0 deletions docs/CONFIG.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Every field has a default if omitted.
|RelayConfig.adjustment_fee_payer|optional string| Address that pays bid adjustment fees for this relay.|None|
|RelayConfig.submit_config.optimistic_v3|bool| Use optimistic V3 submissions for this relay.|false|
|RelayConfig.submit_config.optimistic_v3_bid_adjustment_required|bool| Whether bid adjustments are required for optimistic V3.|false|
|RelayConfig.submit_config.optimistic_v3_max_bid_eth|optional string| Do not use optimistic V3 when bid value is above this (ETH). None = no cap.|None|
|RelayConfig.is_bloxroute|bool|Set to `true` for bloxroute relays to add extra headers.|false|
|RelayConfig.bloxroute_rproxy_regions|vec[string]| Bloxroute rproxy regions to try, in order of preference.|[]|
|RelayConfig.bloxroute_rproxy_only|bool| If true, only submit to bloxroute rproxy endpoints when available.|false|
Expand Down
Loading