Skip to content

refactor(rpc): add TxEnv converter to RpcCoverter#17792

Merged
mattsse merged 14 commits intoparadigmxyz:mainfrom
lean-apple:refactor-rpc-converter-tx-env
Aug 26, 2025
Merged

refactor(rpc): add TxEnv converter to RpcCoverter#17792
mattsse merged 14 commits intoparadigmxyz:mainfrom
lean-apple:refactor-rpc-converter-tx-env

Conversation

@lean-apple
Copy link
Contributor

@lean-apple lean-apple commented Aug 11, 2025

Part 1 to go towards #17545.

Still to do:

  • Add closure support,
  • Update doc

Add convertion support only for TxEnv, other conversions will be done in next prs.

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Aug 11, 2025
@lean-apple lean-apple changed the title refactor(rpc): add TxEnv converter to RpcCoverter refactor(rpc): add TxEnv converter to RpcCoverter Aug 11, 2025
@lean-apple lean-apple marked this pull request as ready for review August 11, 2025 16:27
@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Aug 12, 2025
@lean-apple lean-apple requested a review from gakonst as a code owner August 13, 2025 11:30
@lean-apple lean-apple requested a review from mattsse August 13, 2025 11:32
@codspeed-hq
Copy link

codspeed-hq bot commented Aug 13, 2025

CodSpeed Performance Report

Merging #17792 will not alter performance

Comparing lean-apple:refactor-rpc-converter-tx-env (2541c1a) with main (b50eb7e)

Summary

✅ 77 untouched benchmarks

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

can we undo the function signature change

@lean-apple lean-apple force-pushed the refactor-rpc-converter-tx-env branch from ef54dcc to 4e6c9fd Compare August 17, 2025 18:05
@lean-apple lean-apple requested a review from mattsse August 18, 2025 04:24
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

question about the closure impl

this doesnt seem useful because cfg<()> isnt a thing I believe?

@lean-apple lean-apple requested a review from mattsse August 19, 2025 17:39
@mattsse mattsse requested a review from klkvr August 25, 2025 14:18
///
/// The `TxEnvConverter` has a blanket implementation:
/// * `()` assuming `TxReq` implements [`TryIntoTxEnv`] and is used as default for [`RpcConverter`].
pub trait TxEnvConverter<TxReq, TxEnv>: Debug + Send + Sync + Unpin + Clone + 'static {
Copy link
Member

@klkvr klkvr Aug 26, 2025

Choose a reason for hiding this comment

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

can we add a blanket implementation for closures in scope of this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this works because this requires Spec generic,

should we make this an AT as well?

Copy link
Member

Choose a reason for hiding this comment

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

ah right

yeah we can make Spec an AT on RpcConvert, and do TxEnvConverter<TxReq, TxEnv, Spec>. we should know the spec type from the Evm

@RomanHodulak RomanHodulak requested a review from fgimenez as a code owner August 26, 2025 13:25
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm!

@mattsse mattsse added this pull request to the merge queue Aug 26, 2025
Merged via the queue into paradigmxyz:main with commit 8c8ffd4 Aug 26, 2025
41 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Aug 26, 2025
@lean-apple lean-apple deleted the refactor-rpc-converter-tx-env branch September 3, 2025 00:22
lwedge99 pushed a commit to sentioxyz/reth that referenced this pull request Sep 16, 2025
)

Co-authored-by: Roman Hodulák <roman.hodulak@polyglot-software.com>
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 22, 2026
…th#17792)

Co-authored-by: Roman Hodulák <roman.hodulak@polyglot-software.com>
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Feb 11, 2026
…th#17792)

Co-authored-by: Roman Hodulák <roman.hodulak@polyglot-software.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants