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.

Sign up for GitHub

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

Jump to bottom

clients/reth: adding reth client definition #717

Merged
merged 23 commits into from
Aug 22, 2023
Merged

clients/reth: adding reth client definition #717

merged 23 commits into from
Aug 22, 2023

Conversation

onbjerg
Copy link
Contributor

@onbjerg onbjerg commented Feb 17, 2023

This is a work-in-progress PR to add support for reth.

reth is still early stages, and we will gradually add support for more simulators. Support is being tracked in paradigmxyz/reth#851

Before moving out of draft I will clean up the commit history in this pull request.

@onbjerg onbjerg mentioned this pull request Feb 17, 2023
Closed
@fjl
Copy link
Collaborator

fjl commented Jun 5, 2023 •
edited
Loading

Can we get this merged? It's OK to have clients that do not pass all tests in main branch. Having reth in would make it easier for us to apply ongoing maintenance to the client definition.

For example, we just now renamed the branch build argument to tag. This change also needs to be applied to clients/reth, but we can't do that when it's in a Draft status PR.

gakonst and others added 21 commits August 3, 2023 16:20
@gakonst @onbjerg @Rjected
feat: reth (#1)
14c8ceb 
* wip

* wip

* wip

* feat: add initial block import part of script (#1)

* set up init and import commands

* reorder reth commands

* change command style

* feat: load rest of test chain

* feat: enable rpc

* fix: correct genesis hex mapper

* use apt instead of apk

---------

Co-authored-by: Oliver Nordbjerg <[email protected]>
Co-authored-by: Dan Cline <[email protected]>
@Rjected
revert mapper changes (#2)
30a3419
@Rjected
feat: add shanghai fork timestamp to mapper
9ab46aa
@Rjected
fix: use new reth import UX (#4)
9a2e390
@Rjected
use bootnodes in online steps only (#5)
6fdaaba
@Rjected
uncomment JWT section (#6)
72ec39d 
remove 0x from JWT secret
@Rjected
use patched chain files for rpc-compat (#7)
b2c4bb5 
use fork with correct withdrawals chain
@Rjected
add shanghai timestamp to rpc-compat tests (#8)
578c3b1
@Rjected
add continuous condition for sync test (#9)
4385a1e
@Rjected
fix: add remaining forks to mapper and rpc-compat suite (#10)
0e02303 
* fix: add remaining forks to mapper and rpc-compat suite

* go fmt
@mattsse @Rjected
chore: enable ws (#11)
d97991f
@mattsse @Rjected
chore: disable ascii (#12)
4955f41
@Rjected
feat: ignore error difference when comparing rpc output (#16)
aa7f74f 
* feat: ignore error difference when comparing rpc output

* only ignore rpc error message
@mattsse @Rjected
feat: enable --auto-mine if clique is set (#15)
0b745c5
@mattsse @Rjected
chore: move RUST_LOG_STYLE to top (#19)
79d8197
@Rjected
feat: ignore error codes in error expects (#20)
f29fd09
@Rjected
fix: use proper bash existence check for continuous (#21)
7f7de8d
@Rjected
fix: use proper bash existence check for auto-mine (#22)
362b3e8
@mattsse @Rjected
chore: remove duplicate output (#23)
452485a
@Rjected
use datadir instead of db arg in hive (#24)
152d2da
@mattsse @Rjected
fix: only run --auto-mine when clique requested (#25)
e49ad30
@Rjected
Remove broad error checks and use upstream execution-apis (#27)
e5a04a5 
remove broad error checks and use upstream execution-apis
@Rjected
Copy link
Contributor

Rjected commented Aug 4, 2023

Can we get this merged? It's OK to have clients that do not pass all tests in main branch. Having reth in would make it easier for us to apply ongoing maintenance to the client definition.

Hi @fjl ! Sorry it's been a while, but I'm trying to prepare this so it can be merged. I just changed from branch to tag, to stay up to date, but noticed now that there are .git and .local dockerfiles. After adding those, what else would we need to for this to get merged?

@fjl
Copy link
Collaborator

fjl commented Aug 4, 2023

Alternative Dockerfiles are not mandatory. They are only needed to test the non-default branch of a client. I think the client definition is OK as is.

@fjl
Copy link
Collaborator

fjl commented Aug 4, 2023

The thing about error checks in the rpc-compat suite is on my TODO list, btw!

@Rjected
Copy link
Contributor

Rjected commented Aug 4, 2023

The thing about error checks in the rpc-compat suite is on my TODO list, btw!

awesome! I removed those changes from this PR, since I'd like to just add the client definitions in this PR

@fjl fjl changed the title clients: add reth support clients/reth: adding reth client definition Aug 4, 2023
@fjl fjl marked this pull request as ready for review August 5, 2023 13:32
@fjl fjl merged commit afbaf25 into ethereum:master Aug 22, 2023
github-merge-queue bot pushed a commit to kkrt-labs/hive that referenced this pull request Aug 22, 2023
@bajpai244 @onbjerg
@gakonst @Rjected @mattsse @fjl
dev: sync with latest changes on ethereum/hive (#1)
da860b4 
* clients/reth: adding reth client definition (ethereum#717)


Co-authored-by: Georgios Konstantopoulos <[email protected]>
Co-authored-by: Dan Cline <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>

* simulators/devp2p: enable discv5 test suite (ethereum#835)

This adds support for running the discv5 test suite from go-ethereum's cmd/devp2p against EL and CL clients.

---------

Co-authored-by: Bjerg <[email protected]>
Co-authored-by: Georgios Konstantopoulos <[email protected]>
Co-authored-by: Dan Cline <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
@holiman
Copy link
Collaborator

holiman commented Jan 17, 2024

Does paradigmxyz/reth exist? Please see #970

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@onbjerg @fjl @Rjected @holiman @gakonst @mattsse