Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(perf): integrate OnStateHook in executor #11345

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

fgimenez
Copy link
Member

@fgimenez fgimenez commented Sep 30, 2024

Closes: #11330

It can be used for instance with state root calculation like this

let (tx, rx) = channel();
// spawn SR
p.executor().execute_with_state_hook(.., |state| tx.send(state.clone()));

@fgimenez fgimenez force-pushed the fgimenez/integrate-on-state-hook-in-executor branch from 238b4e8 to 86ab37e Compare September 30, 2024 17:50
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.

cool, this will do it,
I wonder if the hook generic will just be code bloat, so we could consider using dyn here

Comment on lines 175 to 176
evm.db_mut().commit(state.clone());
system_caller.on_state(&ResultAndState { result: result.clone(), state });
Copy link
Collaborator

Choose a reason for hiding this comment

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

these should be swapped, and we can delay the resultandstate destruct on L167, then we can get rid of the clone

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch, done

/// Execute a single block and apply the state changes to the internal state.
///
/// Returns the receipts of the transactions in the block, the total gas used and the list of
/// EIP-7685 [requests](Request).
///
/// Returns an error if execution fails.
fn execute_without_verification(
fn execute_without_verification_with_state_hook<F>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah cool, this is smarter than integrating this in the Executor type as field

Comment on lines +322 to +327
/// Delegate to stored `OnStateHook`, noop if hook is `None`.
pub fn on_state(&mut self, state: &ResultAndState) {
if let Some(ref mut hook) = &mut self.hook {
hook.on_state(state);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for now this is perfectly fine, I assume there's a better approach to this once we try to redesign the executor parts

@fgimenez fgimenez marked this pull request as ready for review September 30, 2024 18:22
@fgimenez fgimenez force-pushed the fgimenez/integrate-on-state-hook-in-executor branch from 57042ad to bdf77c5 Compare October 1, 2024 10:15
@fgimenez fgimenez requested a review from mattsse October 2, 2024 07:10
@fgimenez fgimenez force-pushed the fgimenez/integrate-on-state-hook-in-executor branch from bdf77c5 to 7d1907e Compare October 3, 2024 08:48
@mattsse mattsse added this pull request to the merge queue Oct 4, 2024
Merged via the queue into main with commit af1eb61 Oct 4, 2024
36 checks passed
@mattsse mattsse deleted the fgimenez/integrate-on-state-hook-in-executor branch October 4, 2024 08:24
Thegaram added a commit to scroll-tech/reth that referenced this pull request Oct 10, 2024
* chore(provider): use `get_in_memory_or_storage` on `transactions_by_block_range` (paradigmxyz#11453)

* chore(exex): adjust WAL gauge metric names (paradigmxyz#11454)

* chore(provider): use `get_in_memory_or_storage_by_block` on `fn block_body_indices` (paradigmxyz#11452)

* chore(provider): find `last_database_block_number` with `BlockState` anchor instead (paradigmxyz#11455)

* feat: add metrics for failed deliveries (paradigmxyz#11456)

* chore: release 1.0.8 (paradigmxyz#11457)

* chore(provider): use `block_ref` instead on `BlockState` (paradigmxyz#11458)

* feat(rpc): Add codes in execution witness return (paradigmxyz#11443)

* fix: ensure the request's gas limit does not exceed the target gas limit (paradigmxyz#11462)

* feat(grafana): ExEx WAL (paradigmxyz#11461)

* fix: use correct rpc errors (paradigmxyz#11463)

* chore(provider): clone after filtering on `sealed_headers_while`  (paradigmxyz#11459)

      Co-authored-by: Matthias Seitz <[email protected]>

* Map `TransferKind::EofCreate` => `OperationType::OpEofCreate` (paradigmxyz#11090)

Co-authored-by: Matthias Seitz <[email protected]>

* fix: windows build (paradigmxyz#11465)

* chore: use `block_ref` on `CanonicalInMemoryState`  (paradigmxyz#11467)

* chore(rpc): remove include_preimage param on debug_execution_witness (paradigmxyz#11466)

* feat: make addons stateful (paradigmxyz#11204)

Co-authored-by: Arsenii Kulikov <[email protected]>

* Relax Trait Bounds on TransactionPool::Transaction and EthPoolTransaction (paradigmxyz#11079)

Co-authored-by: Matthias Seitz <[email protected]>

* test: add unit tests for `CanonicalChain` (paradigmxyz#11472)

* feat(perf): integrate OnStateHook in executor (paradigmxyz#11345)

* feat: cleaned up prepare_call_env() (paradigmxyz#11469)

Co-authored-by: Matthias Seitz <[email protected]>

* chore: use block.body directly (paradigmxyz#11474)

* feat: Add metrics to track transactions by type in txpool  (paradigmxyz#11403)

Co-authored-by: garwah <garwah@garwah>
Co-authored-by: Matthias Seitz <[email protected]>

* chore: op chainspec (paradigmxyz#11415)

* chore(exex): more backfill debug logs (paradigmxyz#11476)

* fix(exex): use thresholds in stream backfill (paradigmxyz#11478)

* chore(db): capture tx opening backtrace in debug mode (paradigmxyz#11477)

* chore(sdk): `SealedHeader` generic over header (paradigmxyz#11429)

* chore(lint): fix lint primitives (paradigmxyz#11487)

* chore(provider): add more test coverage on `BlockchainProvider::*by_tx_range` queries (paradigmxyz#11480)

* test: ensure default hash matches (paradigmxyz#11486)

* chore: rm deposit contract config for op (paradigmxyz#11479)

* chore(lint): fix lint storage (paradigmxyz#11485)

* chore(provider): add more test coverage on `BlockchainProvider::*by_block_range` queries (paradigmxyz#11488)

* fix(rpc-eth-types): incorrect error msg(; -> :) (paradigmxyz#11503)

Signed-off-by: jsvisa <[email protected]>

* Reexport optimism specific crates from `op-reth` (paradigmxyz#11499)

* feat: rpc replace function created (paradigmxyz#11501)

Co-authored-by: Matthias Seitz <[email protected]>

* Add metrics for failed deliveries to Grafana dashboard (paradigmxyz#11481)

* chore: Remove duplicate EthereumChainSpecParser in favor of existing EthChainSpecParser  (paradigmxyz#11412)

Co-authored-by: garwah <garwah@garwah>
Co-authored-by: Matthias Seitz <[email protected]>

* chore(lint): fix `clippy::needles_lifetimes` (paradigmxyz#11496)

* chore: rm from genesis impl (paradigmxyz#11509)

* fix: cap gas limit properly (paradigmxyz#11505)

* feat: add PoolBuilderConfigOverrides (paradigmxyz#11507)

* feat: expose Op node network_config helper (paradigmxyz#11506)

* chore(deps): weekly `cargo update` (paradigmxyz#11518)

Co-authored-by: github-merge-queue <[email protected]>

* test: add unit tests for `PruneLimiter` (paradigmxyz#11517)

* feat(provider): add `test_race` to `BlockchainProvider2` tests (paradigmxyz#11523)

* fix(tree): make state methods work for historical blocks (paradigmxyz#11265)

Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: Federico Gimenez <[email protected]>

* rpc: use `eth_api()` method (paradigmxyz#11516)

* chore: delete rpc-types (paradigmxyz#11528)

* feat: add get_highest_tx_by_sender to pools (paradigmxyz#11514)

Co-authored-by: Matthias Seitz <[email protected]>

* ci: add `windows` cargo check  (paradigmxyz#11468)

* fix: acquire permit first (paradigmxyz#11537)

* chore: dont fail on ttd (paradigmxyz#11539)

* grafana: add metrics of all transactions in pool by type  (paradigmxyz#11515)

Co-authored-by: Emilia Hane <[email protected]>
Co-authored-by: Emilia Hane <[email protected]>

* feat(exex): subscribe to notifications with head using `ExExContext` (paradigmxyz#11500)

* chore: enforce window (paradigmxyz#11540)

* Refactor get_payload_bodies_by_hash_with to be non-blocking (paradigmxyz#11511)

Co-authored-by: Matthias Seitz <[email protected]>

* Introduce Eth PayloadTypes Impl (paradigmxyz#11519)

Co-authored-by: Matthias Seitz <[email protected]>

* fix(op-reth): add jemalloc feature to optimism-cli for version (paradigmxyz#11543)

* fix(grafana): remove rate function from panel "Transactions by Type in Pool" (paradigmxyz#11542)

* chore: move ethfiltererror (paradigmxyz#11552)

* chore: rm redundant type hint (paradigmxyz#11557)

* Introduce Op PayloadTypes Impl (paradigmxyz#11558)

Co-authored-by: Matthias Seitz <[email protected]>

* chore: chain manual serialisation implementation (paradigmxyz#11538)

* chore: rm unused optimism feature (paradigmxyz#11559)

* feat(trie): expose storage proofs (paradigmxyz#11550)

* chore: rm unused optimism feature from compat (paradigmxyz#11560)

* Added InternalBlockExecutionError to execute.rs exports (paradigmxyz#11525)

Co-authored-by: Matthias Seitz <[email protected]>

* docs(exex): include code for ExEx book from real files (paradigmxyz#11545)

* fix: actually configure the custom gas limit (paradigmxyz#11565)

* chore: relax trait bound for `EthTransactions` (paradigmxyz#11571)

* fix(provider): fix sub overflow on `tx_range` queries for empty blocks (paradigmxyz#11568)

* chore(provider): add more test coverage on `BlockchainProvider` non-range queries (paradigmxyz#11564)

* feat: adding a new method to network config builder (paradigmxyz#11569)

* chore: rm unused optimism feature from engine api (paradigmxyz#11577)

* chore: replace some revm deps (paradigmxyz#11579)

* chore: rm bad cap function (paradigmxyz#11562)

* feat: impl `Encodable2718` and `Decodable2718` for `PooledTransactionsElement` (paradigmxyz#11482)

* fix(exex): exhaust backfill job when using a stream (paradigmxyz#11578)

* fix: in-memory trie updates pruning (paradigmxyz#11580)

Co-authored-by: Matthias Seitz <[email protected]>

* chore(providers): test race condition on all `BlockchainProvider2` macro tests (paradigmxyz#11574)

* chore: also derive arb for test (paradigmxyz#11588)

* chore: bump alloy primitives 0 8 7 (paradigmxyz#11586)

* chore(ci): remove expected failures related to checksummed addresses (paradigmxyz#11589)

* chore(rpc): use `block_hash` instead on fetching `debug_trace_block` block (paradigmxyz#11587)

* feat: add mul support for SubPoolLimit (paradigmxyz#11591)

Co-authored-by: Dan Cline <[email protected]>

* docs: delete missing part path (paradigmxyz#11590)

Co-authored-by: Dan Cline <[email protected]>

* fix: simplify reorg handling (paradigmxyz#11592)

* fix: use original bytes for codes (paradigmxyz#11593)

* perf(rpc): use `Arc<BlockWithSenders` on `full_block_cache` (paradigmxyz#11585)

* fix: active inflight count (paradigmxyz#11598)

* fix(net): max inflight tx reqs default (paradigmxyz#11602)

* fix: set deposit gasprice correctly (paradigmxyz#11603)

* fix: set system tx correctly (paradigmxyz#11601)

* fix(trie): prefix set extension (paradigmxyz#11605)

* feat: add tx propagation mode (paradigmxyz#11594)

* feat: add helper function to provde the tx manager config (paradigmxyz#11608)

* fix(grafana): set instance variable from `reth_info` metric (paradigmxyz#11607)

* fix(net): add concurrency param from config to `TransactionFetcherInfo` (paradigmxyz#11600)

Co-authored-by: Matthias Seitz <[email protected]>

* perf(rpc): optimistically retrieve block if near the tip on `eth_getLogs` (paradigmxyz#11582)

* update fork base commit

* remove new book tests

---------

Signed-off-by: jsvisa <[email protected]>
Co-authored-by: joshieDo <[email protected]>
Co-authored-by: Alexey Shekhirin <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
Co-authored-by: Francis Li <[email protected]>
Co-authored-by: Emilia Hane <[email protected]>
Co-authored-by: Arsenii Kulikov <[email protected]>
Co-authored-by: Eric Woolsey <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Federico Gimenez <[email protected]>
Co-authored-by: Varun Doshi <[email protected]>
Co-authored-by: garwah <[email protected]>
Co-authored-by: garwah <garwah@garwah>
Co-authored-by: greged93 <[email protected]>
Co-authored-by: Delweng <[email protected]>
Co-authored-by: Parikalp Bhardwaj <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-merge-queue <[email protected]>
Co-authored-by: Dan Cline <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: crazykissshout <[email protected]>
Co-authored-by: tedison <[email protected]>
Co-authored-by: David <[email protected]>
Co-authored-by: Emilia Hane <[email protected]>
Co-authored-by: Steven <[email protected]>
Co-authored-by: Debjit Bhowal <[email protected]>
Co-authored-by: Oliver <[email protected]>
Co-authored-by: Luca Provini <[email protected]>
Co-authored-by: John <[email protected]>
vandenbogart pushed a commit to testmachine-ai/reth that referenced this pull request Oct 14, 2024
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.

Integrate OnStateHook in executor
2 participants