Skip to content

Conversation

@ElFantasma
Copy link
Contributor

Motivation

Ethereum nodes are expected to run discv5 for early 2026.

Description

On one hand, this is work in progress, but in the other we don't want to have a huge PR at the end of development. So we plan to start merging discv5 work into main, but behind a feature flag so nothing is compiled or run, except for development. The flag is already present in this commit, so we should be clear to merge.

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

Closes #4491

ElFantasma and others added 23 commits December 9, 2025 10:31
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves #111, Resolves #222 -->

**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

Closes #5574 and #5575.

Co-authored-by: Esteban Dimitroff Hodi <[email protected]>
**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

Closes #5580 and #5581
**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

Closes #5578 and #5579
**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

Closes #5576 and closes #5577
**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

Closes #issue_number
**Motivation**
I saw current new_nonce impl allocated a vector when it can just return
a fixed size array.

**Description**

Removes the needless vec

**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.
**Motivation**

In order to start merging discv5 code into main, to avoid having a huge
PR at the end of the development, we should create a feature flag
disabled by default.

Closes #5639
**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.

Closes #5586
Closes #5587
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves #111, Resolves #222 -->

**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
includes breaking changes to the `Store` requiring a re-sync.
Copy link
Contributor

@Oppen Oppen left a comment

Choose a reason for hiding this comment

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

Left a few comments but given it won't be enabled until it's ready, I think it's mostly safe to approve.
If you're gonna apply anything I suggested, the most important right now is the feature name suggestion IMO.

edg-l and others added 4 commits December 18, 2025 11:07
… tests (#5673)

**Motivation**

Adds a Session struct and related functions to the discv5 module, this
represents a session established from a handshake. This PR does not
implement the handshake protocol, but it lays a path to doing so easily.

This is based on reading
https://github.com/ethereum/devp2p/blob/master/discv5/discv5-theory.md#sessions

This allows us to add the remaining test vectors related to
cryptography, which is done now.

Also renamed the PacketDecodeErr to PacketCodecError, since the error
was also used when encoding and for other things.

Fixed nonce generation to be secure according to the spec
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Session {
pub keys: SessionKeys,
pub role: SessionRole,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be kept? I think the role can be removed if we switch the keys in one of the two cases (maybe rename the keys to local and remote too).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it in a future PR

@ElFantasma ElFantasma enabled auto-merge December 19, 2025 19:28
@github-project-automation github-project-automation bot moved this to In Review in ethrex_l1 Dec 19, 2025
@ElFantasma ElFantasma added this pull request to the merge queue Dec 19, 2025
Merged via the queue into main with commit 1629632 Dec 19, 2025
51 checks passed
@ElFantasma ElFantasma deleted the discv5 branch December 19, 2025 21:12
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Tracking] Implement discv5

7 participants