Skip to content

Abstract over single transaction receivers - #5131

Closed
jmg-duarte wants to merge 4 commits into
mainfrom
jmgd/trait-tx-receiver
Closed

jmg-duarte wants to merge 4 commits into
mainfrom
jmgd/trait-tx-receiver

Conversation

@jmg-duarte

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

Copy link
Copy Markdown
Contributor

Description

Follow up to the discussion in #5114 (comment), this PR introduces the TxReceiver trait, which "covers" the single mempools (and in the future, the single builders) as a single "transaction receiver".

Changes

  • Transactions are signed before being passed to the Receiver layer (old Mempool)
  • Single receivers are abstracted over using TxReceiver (the generic parameter was done on purpose to ensure the collection of receivers only carries a single type!)
  • Adds a NonceSource trait for "nonce-enabled transaction receivers" — e.g. nodes can returns nonces (TxReceiver + NonceSource) but builders can't (TxReceiver)

How to test

Existing tests

@jmg-duarte
jmg-duarte requested a review from a team as a code owner October 21, 2026 11:00
@claude

claude Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 4m 17s —— View job


Review: Abstract over single transaction receivers

Nice refactor — pulling signing out of the submission transport and hiding single mempools/relays behind TxReceiver reads cleanly, and the generic Mempools<R> keeping the collection monomorphic is a nice touch. I verified the metric/label strings (Mempool(name)), the pending_gas_price replacement semantics, chain-id handling, and the account set registered into Wallet are all preserved from the old flow.

A couple of non-blocking notes, posted inline:

  • wallet.rs — the new doc comment claims the tx is signed once and the same bytes sent to every receiver, but signing actually happens per-receiver inside submit (and final_gas_price differs per receiver), so the shared-bytes optimization isn't realized here. Not a regression, just a comment that overstates what this PR does.
  • mempools.rs — nonces are now read from the shared self.ethereum node instead of each receiver's URL (the relay nonce-via-proxy comment was removed). Looks intentional and correct; flagging for confirmation.

Everything else looks good. I couldn't run cargo check/tests in this environment (command not permitted), so compilation relies on your local run / CI.

Comment thread crates/driver/src/infra/wallet.rs Outdated
Comment thread crates/driver/src/domain/mempools.rs Outdated
Comment thread crates/driver/src/domain/mempools.rs Outdated
Comment on lines +13 to +14
/// Returns once the receiver accepted the tx.
async fn send(&self, tx: &SignedTx) -> anyhow::Result<()>;

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.

Wouldn't this interface cause issues for block builders?
Or are you planning to create a temporary struct which knows the block range the tx should be submitted at such that you don't have to pass it here anymore?

@jmg-duarte jmg-duarte closed this Oct 21, 2026
@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