feat(l1): store validated ENR from handshake in peer table

[mikhailofff]

Motivation

When a handshake is received from an unknown peer, we verify their ENR but do not add it to the peer table. According to the discv5 specification, once an ENR signature has been validated, the record should be stored.

Description

1. Added the peer to the peer table using new_contact_records().
2. Updated existing contacts if the received ENR has a higher seq.

Checklist

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

Closes #6056.

[greptile-apps]

Greptile Overview

Greptile Summary

Implements ENR storage during handshake validation per discv5 spec by calling new_contact_records() for validated ENRs and updating existing contacts with higher sequence numbers.

Key changes:

• Stores validated ENR in peer table during handshake (server.rs:306-309)
• Updates existing contacts when receiving ENR with higher seq number (peer_table.rs:867-878)
• Resolves TODO for handling already-present contacts

Issues found:

• Logic error: peer added to table before node_id verification completes, allowing invalid peers to persist if verification fails

Confidence Score: 3/5

• Safe to merge after fixing node_id verification ordering issue
• Implementation correctly handles ENR updates and follows spec, but contains a logic error where peers are added before complete validation
• crates/networking/p2p/discv5/server.rs requires attention for validation ordering

Important Files Changed

Filename	Overview
crates/networking/p2p/discv5/server.rs	Adds validated ENR to peer table during handshake, but stores record before verifying node_id matches src_id
crates/networking/p2p/discv5/peer_table.rs	Updates existing contacts with higher seq ENRs, properly resolving the TODO

Sequence Diagram

sequenceDiagram
    participant Remote as Remote Peer
    participant Server as DiscoveryServer
    participant PeerTable as PeerTable
    
    Remote->>Server: Handshake packet with ENR
    Server->>Server: Verify ENR signature
    alt ENR signature invalid
        Server-->>Remote: Return (no response)
    end
    
    Server->>Server: Extract public key from ENR
    Server->>PeerTable: new_contact_records(ENR)
    Note over PeerTable: Add/Update peer in contacts
    
    Server->>Server: Verify node_id matches src_id
    alt node_id mismatch
        Server-->>Remote: Return (no response)
        Note right of PeerTable: Invalid peer remains in table
    end
    
    Server->>Server: Verify id-signature
    alt id-signature invalid
        Server-->>Remote: Return (no response)
    end
    
    Server->>Server: Derive session keys
    Server->>PeerTable: set_session_info()
    Server->>Server: Decrypt and handle message
    Server-->>Remote: Handshake complete

Loading

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Peer added to table before verifying node_id matches src_id (line 315). If verification fails at line 317, the invalid peer remains in the table.

Move this call after the node_id verification (after line 319).

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/networking/p2p/discv5/server.rs
Line: 306:309

Comment:
Peer added to table before verifying node_id matches src_id (line 315). If verification fails at line 317, the invalid peer remains in the table.

Move this call after the node_id verification (after line 319).

How can I resolve this? If you propose a fix, please make it concise.

[ElFantasma]

Thanks for your contribution, but a previous PR fixing this exists.

[ElFantasma]

Sorry, there is no previous PR for this issue, I got mixed up. I'm reopening this

[greptile-apps]

Greptile Overview

Greptile Summary

This PR implements ENR storage during handshake verification per the discv5 spec. The implementation adds validated ENRs to the peer table and updates existing contacts when receiving higher sequence numbers.

Key changes:

• Stores validated ENR in peer table after signature verification in server.rs:306-309
• Updates existing contacts with higher seq ENRs in peer_table.rs:868-878

Issues identified:

• Critical ordering issue (already noted): Peer added to table at line 307 before node_id verification at line 315. If verification fails, invalid peer remains stored
• Fork ID validation missing when updating existing contacts with new ENRs (lines 870-877)

Confidence Score: 2/5

• This PR has a critical security issue that allows invalid peers to be stored
• The peer is added to the table before node_id verification completes. If verification fails (lines 315-317), the invalid peer remains in the table with no cleanup mechanism. Additionally, fork_id validation is skipped when updating existing contacts.
• Pay close attention to crates/networking/p2p/discv5/server.rs - the ordering issue must be fixed before merge

Important Files Changed

Filename	Overview
crates/networking/p2p/discv5/server.rs	Added peer to table during handshake, but placement before node_id verification creates security issue where invalid peers remain stored
crates/networking/p2p/discv5/peer_table.rs	Updated existing contacts with higher seq ENRs, but missing fork_id validation when updating records

Sequence Diagram

sequenceDiagram
    participant Remote as Remote Peer
    participant Server as DiscoveryServer
    participant PeerTable as PeerTableServer
    
    Remote->>Server: Handshake with ENR
    Server->>Server: Verify ENR signature
    alt ENR signature valid
        Server->>PeerTable: new_contact_records(ENR)
        PeerTable->>PeerTable: Check if contact exists
        alt Contact does not exist
            PeerTable->>PeerTable: Validate fork_id
            PeerTable->>PeerTable: Insert new contact
        else Contact exists with ENR
            alt New seq > existing seq
                PeerTable->>PeerTable: Update record (no fork_id check)
            end
        else Contact exists without ENR
            PeerTable->>PeerTable: Add record (no fork_id check)
        end
        Server->>Server: Verify node_id matches src_id
        alt node_id mismatch
            Server->>Server: Return early (peer remains in table)
        else node_id valid
            Server->>Server: Verify id-signature
            Server->>PeerTable: set_session_info()
            Server->>Server: Process message
        end
    else ENR signature invalid
        Server->>Server: Return early
    end

Loading

_{Last reviewed commit: 9c73c70}

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[ElFantasma]

Peer added before full handshake authentication. This call runs before the derived_node_id != src_id check (line 316) and before the id-signature verification (line 337). A malicious node claiming src_id=X but sending a valid ENR for node Y would get Y inserted into the peer table — the ENR signature is valid (it's Y's real ENR), so new_contact_records accepts it. The function returns early at line 317, but the peer table entry persists.

Move this call after the id-signature verification at line 346 — there's no reason to store anything until the handshake is fully authenticated.

[ElFantasma]

This was already implemented more thoroughly on a previous PR
This PR will conflict once that merges. Consider rebasing on top of it and focusing only on the handle_handshake insertion.