fix(l1): update existing contact ENR on NODES response

[ElFantasma]

Motivation

new_contact_records() only inserted new contacts (Entry::Vacant), silently discarding updated ENRs for already-known peers. This made the FINDNODE(distance=0) request from #5910 ineffective — the request was sent but the response couldn't update the existing record.

Description

• Handle Entry::Occupied in new_contact_records(): when the incoming record has a higher seq than the existing one, update the contact's node, record, and is_fork_id_valid.
• Add ENR signature validation (verify_signature()) upfront for all records, which was previously missing even for new contacts.
• Hoist shared logic (discarded contacts check, fork ID validation) before the match to avoid duplication.

Closes #6166

[ElFantasma]

Addressing agent reviews

Fixes applied

1. FINDNODE spam from != + > mismatch (Codex, Claude) — handle_pong used != while new_contact_records used >. If a peer's PONG had a lower seq than cached, we'd send FINDNODE every time but never update, causing infinite requests. Fixed by changing handle_pong to use > (committed on base branch discv5-server-enr-update-on-pong). Added test case for lower seq scenario.

2. Fork-id validation for stale ENRs (Codex, Claude) — is_fork_id_valid was computed (async, hits storage) before knowing whether the ENR would actually be used. Moved the validation inside each match arm so it only runs when the record will be inserted/updated.

3. record = None with seq=0 edge case (Claude) — If an existing contact had record = None (mapped to existing_seq = 0) and the incoming ENR also had seq = 0, the > check would fail and the record would never be stored. Fixed by matching on Option directly: None => true (always accept first ENR), Some(r) => node_record.seq > r.seq.

False positives dismissed

• Kimi docs: add milestones #1 ("Missing signature verification for occupied") — Wrong. verify_signature() is already called upfront at line 846, before the Entry match. It applies to all records regardless of whether the contact exists.

• Kimi docs: update milestones. #4 ("Inefficient re-fetching of contact") — Wrong. The code uses occupied_entry.get_mut() directly from the match arm. There's no re-fetch.

• Kimi build: add Github actions boilerplate #3 ("OsRng panic") — OsRng doesn't fail on any real OS. The suggested Lazy<Mutex<OsRng>> adds unnecessary complexity for a non-issue.

• Claude "Incomplete state preservation" — Fields like validation_timestamp, ping_req_id, knows_us etc. reflect the ongoing relationship with the peer, not the ENR content. They should be preserved as-is when only the ENR record updates. Resetting validation state on IP change would be over-engineering since a PONG was just received (proving reachability).

• Claude/Codex "Missing metrics/test gaps" — Nice-to-have, not blocking.

[iovoid]

Duplicated code

[ElFantasma]

Extracted an evaluate_fork_id helper to remove the duplication: 1f39ea9

[iovoid]

When updating an existing contact in the Occupied branch, contact.node = node can change the contact's IP address, but validation_timestamp is not reset. The validate_contact method (line 798) relies on was_validated() + an IP match to prevent amplification attacks (see the anti-amplification comment on lines 806-809). After this update, a previously-validated contact now appears validated at a new IP it was never actually pinged at.

Attack scenario: A malicious node M, previously validated at IP_M, creates a legitimately-signed ENR with IP_victim and higher seq. When this ENR arrives in a NODES response, we update M's contact: node.ip = IP_victim, while validation_timestamp is preserved. A spoofed FindNode packet with src = IP_victim now passes validate_contact, causing us to send a large NODES response to the victim.

The fix would be to check if the IP changed and, if so, reset validation_timestamp (and ping_req_id). This is a new concern — on main, new_contact_records only inserted vacant entries, so a contact's IP could never change through this path.

[ElFantasma]

The specific attack scenario doesn't quite work as described. Let me trace through it:

1. In discv5, messages are encrypted with session keys established via ECDH handshake. A third party can't forge a FindNode as node M without M's session key.

2. More importantly, validate_contact (which checks sender_ip == contact.node.ip) is not called in the discv5 handle_find_node handler at all — that check only exists in discv4. So validation_timestamp is irrelevant to this path.

3. handle_find_node responds to contact.node.udp_addr() (the stored IP from the ENR), not to the actual sender_addr from the UDP packet. This is a pre-existing behavior — before this PR, the handler was already responding to the stored contact IP without any IP check.

What our PR changes: it makes it possible for a peer to update its stored IP via a NODES response (previously only possible during initial discovery). This marginally broadens the existing issue but doesn't create it.

The real pre-existing issue is that discv5's handle_find_node lacks the anti-amplification defense that discv4 has. The proper fix (as a separate PR) would be:

1. Pass sender_addr to handle_find_node and respond to it instead of contact.node.udp_addr()
2. Add a sender_addr.ip() == contact.node.ip check (like discv4's validate_contact)

Neither alone is sufficient (responding to stored IP allows ENR-based redirection; responding to sender_addr alone allows UDP spoofing), but together they make amplification much harder.

That said, resetting validation_timestamp on IP change is cheap and reasonable as defense-in-depth — happy to add it. But it won't fix the actual gap since validate_contact isn't used in the discv5 FindNode path.

[ElFantasma]

Added the validation_timestamp + ping_req_id reset when IP or UDP port changes: 5d52f9b

Also opened #6199 for the pre-existing anti-amplification gap in discv5's handle_find_node.