Skip to content

Abstract over transaction execution with TransactionReceiver trait - #5114

Merged
jmg-duarte merged 1 commit into
mainfrom
jmgd/submission-submitter
Oct 21, 2026
Merged

jmg-duarte merged 1 commit into
mainfrom
jmgd/submission-submitter

Conversation

@jmg-duarte

@jmg-duarte jmg-duarte commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

Places the settlement submission behind a trait to simplify the addition of builders in a follow up PR (and lower the amount of changes). Note that the diff for the mempools.rs file is quite large but mostly whitespace.

Changes

  • TransactionReceiver trait. Mempools becomes an Arc<dyn TransactionReceiver>.

How to test

Existing tests

@jmg-duarte
jmg-duarte force-pushed the jmgd/submission-submitter branch from 78e419d to d1f779a Compare October 16, 2026 15:39
Base automatically changed from jmgd/submission-receiver to main October 19, 2026 10:36
@jmg-duarte
jmg-duarte marked this pull request as ready for review October 20, 2026 12:35
@jmg-duarte
jmg-duarte requested a review from a team as a code owner October 20, 2026 12:35
@claude

claude Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 2m 13s —— View job


Review: Put settlement submission behind a Submitter trait

  • Read the diff and changed files
  • Analyze the Submitter trait refactor
  • Check for behavioral/compile regressions
  • Post review

This is a clean, mechanical refactor — I didn't find any correctness issues worth a change request, so no inline comments. The pull request CI job (build + tests) is green, which backs that up.

What I checked

  • Delegation is faithful. Mempools::execute now forwards to Submitter::execute, and the mempool race (select_ok, inline logging, stats, update_metrics) moves verbatim into impl Submitter for Vec<infra::Mempool>. The extracted free functions (submit, cancel, minimum_replacement_gas_price, update_metrics) just swap self.ethereum/self.mempools for passed-in args — no logic change.
  • Clone semantics are safe. Mempools now shares the submitter via Arc<dyn Submitter> instead of deep-cloning Vec<infra::Mempool>. Since infra::Mempool already shares its mutable state (last_submissions: Arc<DashMap<..>>, transport: Web3) across clones, cloned Mempools observe the same submission/nonce state either way — so the Arc wrapping is behaviorally equivalent, not a regression. (Mempools is cloned per-request in api/mod.rs.)
  • No dangling references. observe::mempool_submission_result dropping its &Mempool param for &str still compiles — Mempool remains imported/used by mempool_log. update_metrics passes &mempool.to_string(), preserving the old label.
  • run.rs factoring hoists accounts out of the per-mempool closure and clone()s it, which is equivalent and slightly cheaper than re-collecting each iteration.

Minor (non-blocking, optional)

  • Mempools::execute lost its doc comment (it migrated to the impl Submitter block). The public method is now undocumented — a one-line /// Submits the settlement via the configured submitter. wouldn't hurt.
  • The ethereum is held by both Mempools and threaded into Submitter::execute(&self.ethereum, ..). That's a reasonable seam for the follow-up builders PR, just noting the slight redundancy.

Nothing here needs to block the merge. 👍
· branch jmgd/submission-submitter

Comment thread crates/driver/src/domain/mempools.rs Outdated
@jmg-duarte
jmg-duarte force-pushed the jmgd/submission-submitter branch from 9f7da35 to d678601 Compare October 21, 2026 13:31
@jmg-duarte jmg-duarte changed the title Put settlement submission behind a Submitter trait Abstract over transaction execution with TransactionReceiver trait Oct 21, 2026

#[derive(Clone, Copy)]
enum Outcome {
pub(crate) enum Outcome {

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.

Eventually it would be nice to move the trait and the metrics we expect it to populate into a separate file such that the 3 variants (blockbuilder, mempools, private_relay) can then be put into a separate directory nicely.

@jmg-duarte
jmg-duarte added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit eb3c745 Oct 21, 2026
25 checks passed
@jmg-duarte
jmg-duarte deleted the jmgd/submission-submitter branch October 21, 2026 13:43
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 21, 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