Skip to content

Move common submission arguments to the submission configuration - #5110

Merged
jmg-duarte merged 2 commits into
mainfrom
jmgd/submission-tip-params
Oct 16, 2026
Merged

jmg-duarte merged 2 commits into
mainfrom
jmgd/submission-tip-params

Conversation

@jmg-duarte

Copy link
Copy Markdown
Contributor

Description

Before we move the builder submission further, it's time we take care of some tech debt that we accrued over time by cleaning up the configuration file. To start, we move the max-additional-tip and additional-tip-percentage that we always set to the same values up to the common submission section.

Changes

  • Move max-additional-tip and additional-tip-percentage to the common submission section

How to test

Existing tests shouldn't break, namely E2E ones

@jmg-duarte
jmg-duarte requested a review from a team as a code owner October 16, 2026 09:14
@claude

claude Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 1m 15s —— View job


PR Review

Clean, well-scoped tech-debt cleanup — moving max-additional-tip / additional-tip-percentage from the per-mempool section up to the common [submission] section. The serde_as/default attributes moved correctly, load.rs sources both fields from config.submission, and the test + example.toml were updated consistently.

I left one inline comment flagging a silent config-migration hazard: Mempool lacks deny_unknown_fields, so stale per-mempool keys in deployment configs won't error — they'll be silently ignored and fall back to defaults (3 gwei / 0.05). Worth migrating deployment configs in lockstep.

Otherwise LGTM. The loss of per-mempool tip granularity is intentional per the PR description.

  • Read PR diff
  • Analyze config changes & usages
  • Post inline comments for any issues
  • Post summary
    · jmgd/submission-tip-params


/// The mempools to submit settlement transactions to. Can be the public
/// mempool of a node or the private MEVBlocker mempool.
#[serde(rename = "mempool", default)]

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.

Heads-up on a silent config-migration hazard with this move: unlike SubmissionConfig, the Mempool struct does not have #[serde(deny_unknown_fields)]. So any deployment config that still carries max-additional-tip / additional-tip-percentage under [[submission.mempool]] after this change will not error — the keys are silently ignored, and the values fall back to the defaults (default_max_additional_tip = 3 gwei, default_additional_tip_percentage = 0.05).

Concretely: if a production config currently sets e.g. max-additional-tip = "5000000000" per-mempool and isn't migrated to [submission] in lockstep with this binary, the effective max tip silently drops from 5 gwei to 3 gwei. Worth ensuring all deployment configs are updated alongside this PR (and ideally calling it out in the release notes), since there's no compile- or load-time guard to catch a stale config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MartinquaXD MartinquaXD left a comment

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.

Good change but doesn't address the ugly configuration fully.

Comment thread crates/driver/src/infra/config/file/load.rs Outdated
@jmg-duarte
jmg-duarte marked this pull request as draft October 16, 2026 11:40
@jmg-duarte
jmg-duarte force-pushed the jmgd/submission-tip-params branch from 86c39bf to e7e4e6c Compare October 16, 2026 12:09
@jmg-duarte
jmg-duarte marked this pull request as ready for review October 16, 2026 12:24
@claude

claude Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 1m 28s —— View job


PR Review

Re-reviewed after the latest push (e7e4e6c9 Address Martin's comments). This is now a clean, well-scoped refactor.

Looks good:

  • max_additional_tip / additional_tip_percentage moved to the common [submission] section with the serde_as/default attributes carried over correctly.
  • The follow-up concern from Martin is fully addressed: adjustments() is gone, GasPriceParameters is now a dedicated #[derive(Clone)] config field built directly from config.submission in load.rs, and run.rs just clones it. This removes the per-mempool coupling that previously blocked cleaning up gas::adjustments().
  • Dead code cleaned up consistently: mempool::Config loses min_priority_fee/gas_price_cap/max_additional_tip/additional_tip_percentage and the test_config helper; tests and example.toml updated in lockstep.
  • No behavior change: gas_price_cap/min_priority_fee were already sourced from config.submission for every mempool, so the old min/max-across-mempools aggregation was a no-op; defaults (3 gwei / 0.05) are preserved.

Note (already acknowledged, not blocking): Mempool still lacks #[serde(deny_unknown_fields)], so a stale max-additional-tip/additional-tip-percentage under [[submission.mempool]] would be silently ignored rather than erroring. The author confirmed deployment configs are migrated in infrastructure#7285, so this is handled.

LGTM. ✅


  • Read PR diff
  • Analyze config changes & gas.rs cleanup
  • Verify no stale references / unused imports
  • Post summary
    · jmgd/submission-tip-params

@jmg-duarte
jmg-duarte added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit b389eb1 Oct 16, 2026
27 checks passed
@jmg-duarte
jmg-duarte deleted the jmgd/submission-tip-params branch October 16, 2026 13:25
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants