Skip to content

Conversation

@austinchandra
Copy link

Motivation

Currently, Reth executes metered blocks with an in-lined implementation of execute_block.

This defies expectations that the BlockExecutor owns its execution.

Solution

Create a method execute_block_with_transaction_closure in BlockExecutor.

Callers—for instance—may wrap execute_transaction in a trace.

This clarifies ownership divides.

Reth PR.

Create a method `execute_block_with_transaction_closure` in
`BlockExecutor` that lets callers process transactions.

This returns the result of `finish` for persistence.

Callers may use this to collect metrics for each transaction.
Copy link
Member

@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.

wdyt @klkvr

Comment on lines +327 to +328
transactions: impl IntoIterator<Item = Tx>,
mut f: F,
Copy link
Member

Choose a reason for hiding this comment

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

looking at the reth pr, then what would be more useful here if this accepts an iterator of results, then we can avoid the collect

Copy link
Member

Choose a reason for hiding this comment

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

yeah, it's actually quite important because doing collect on the iterator in reth is pretty expensive, because that iterator is yielding transactions that are decoded in parallel

Copy link
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

not sure if this makes sense to me yet, the code on reth side didn't really become much simpler

it's not very complex to manually iterate through transactions and execute_block helper wasn't designed as a primary API so I'm not sure if we should try to make it such

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants