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: op-reth #4377

Merged
merged 460 commits into from
Nov 5, 2023
Merged

feat: op-reth #4377

merged 460 commits into from
Nov 5, 2023

Conversation

clabby
Copy link
Collaborator

@clabby clabby commented Aug 28, 2023

Overview

This PR contains the full diff for op-reth.

Note: 29,725 lines of this diff consists of large genesis files for base goerli and base mainnet.

Resources

All Future Issues (tracking)

op-geth mappings

The below toggles contain mappings of all op-geth diff changes to their respective places in op-reth.

(if you have questions on this stuff ask @refcell - wrote all these toggles out)

op-geth Diff Mapping: Core Modifications +901 | -52
op-geth Diff Mapping: Node modifications +158 | -10
op-geth Diff Mapping: User API enhancements +995 | -63
op-geth Diff Mapping: Geth extras (not mapped 1:1 since we will have "reth extras") +325 | -25
op-geth Diff Mapping: Other changes +690 | -16

@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #4377 (71836cd) into main (4dc15c3) will increase coverage by 21.63%.
Report is 93 commits behind head on main.
The diff coverage is 49.53%.

❗ Current head 71836cd differs from pull request most recent head 62f8111. Consider uploading reports for the commit 62f8111 to get more accurate results

Impacted file tree graph

Files Coverage Δ
bin/reth/src/args/utils.rs 69.04% <100.00%> (+52.60%) ⬆️
bin/reth/src/cli/config.rs 0.00% <ø> (-100.00%) ⬇️
bin/reth/src/init.rs 91.05% <100.00%> (+42.69%) ⬆️
crates/interfaces/src/executor.rs 20.00% <ø> (+20.00%) ⬆️
crates/net/network-api/src/lib.rs 65.00% <ø> (ø)
crates/net/network-api/src/noop.rs 66.66% <100.00%> (-4.77%) ⬇️
crates/net/network/src/manager.rs 45.91% <100.00%> (-9.68%) ⬇️
crates/net/network/src/test_utils/testnet.rs 81.21% <ø> (-6.37%) ⬇️
crates/payload/builder/src/error.rs 0.00% <ø> (ø)
crates/primitives/src/basefee.rs 99.11% <100.00%> (+68.92%) ⬆️
... and 56 more

... and 390 files with indirect coverage changes

Flag Coverage Δ
integration-tests 14.18% <5.69%> (-11.89%) ⬇️
unit-tests 41.06% <45.17%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 21.34% <17.72%> (-4.44%) ⬇️
blockchain tree 32.12% <ø> (+3.66%) ⬆️
pipeline 66.24% <ø> (+61.19%) ⬆️
storage (db) 52.38% <0.00%> (+22.40%) ⬆️
trie 52.18% <ø> (+29.65%) ⬆️
txpool 40.12% <79.20%> (-1.26%) ⬇️
networking 55.14% <62.96%> (+24.24%) ⬆️
rpc 40.33% <5.55%> (+13.85%) ⬆️
consensus 42.51% <32.43%> (+17.45%) ⬆️
revm 21.07% <74.55%> (+11.22%) ⬆️
payload builder 6.10% <0.00%> (-8.06%) ⬇️
primitives 64.78% <66.50%> (+35.61%) ⬆️

Copy link
Member

@rkrasiuk rkrasiuk left a comment

Choose a reason for hiding this comment

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

leaving first round of comments

bin/reth/Cargo.toml Outdated Show resolved Hide resolved
bin/reth/src/args/utils.rs Outdated Show resolved Hide resolved
bin/reth/src/node/mod.rs Outdated Show resolved Hide resolved
crates/primitives/src/receipt.rs Show resolved Hide resolved
crates/primitives/src/serde_helper/mod.rs Outdated Show resolved Hide resolved
crates/primitives/src/transaction/mod.rs Show resolved Hide resolved
crates/primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
crates/revm/src/optimism/mod.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-types/src/eth/transaction/receipt.rs Outdated Show resolved Hide resolved
crates/storage/provider/src/post_state/mod.rs Outdated Show resolved Hide resolved
@rkrasiuk rkrasiuk added the A-op-reth Related to Optimism and op-reth label Aug 28, 2023
@refcell
Copy link
Contributor

refcell commented Nov 5, 2023

@clabby Do you support syncing Optimism bedrock mainnet now? Why not add support for routing historic queries to l2geth for pre bedrock blocks?

Historical RPC Daisy Chaining is a feature that should be tackled once this lands since it's not critical. I went ahead and opened up an issue #5300.

Further, not supporting historical (pre-bedrock) rpc daisy chaining doesn't prevent a mainnet bedrock node from syncing and following tip successfully.

@ghost
Copy link

ghost commented Nov 5, 2023

Further, not supporting historical (pre-bedrock) rpc daisy chaining doesn't prevent a mainnet bedrock node from syncing and following tip successfully.

@refcell Yes, have you tested or are you testing it now? Due to the receipt RPC fixed for reth #5084 I think it is now possible to sync from op-node at the faster speed

refcell and others added 2 commits November 5, 2023 12:51
…tiation

fix(rpc): Move reqwest client into EthApiInner
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.

I think this is fine now,
needs followups for things discussed separately.

would appreciate if @DaniPopes could take a look at the CI changes before we send it

.github/workflows/unit.yml Outdated Show resolved Hide resolved
.github/workflows/unit.yml Outdated Show resolved Hide resolved
.github/workflows/unit.yml Outdated Show resolved Hide resolved
.github/workflows/lint.yml Outdated Show resolved Hide resolved
.github/workflows/integration.yml Outdated Show resolved Hide resolved
.github/workflows/integration.yml Outdated Show resolved Hide resolved
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@rkrasiuk rkrasiuk added this pull request to the merge queue Nov 5, 2023
Merged via the queue into paradigmxyz:main with commit 52670a8 Nov 5, 2023
26 checks passed
mattsse pushed a commit that referenced this pull request Nov 8, 2023
Co-authored-by: Roberto Bayardo <[email protected]>
Co-authored-by: refcell.eth <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: refcell <[email protected]>
Co-authored-by: nicolas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth
Projects
None yet
Development

Successfully merging this pull request may close these issues.