feat(l1): dual-stack IPv6 support and ENR-based outbound connections

[azteca1998]

Closes #5354

Summary

• Dual-stack RLPx listeners: --p2p.addr now accepts a comma-separated list (e.g. --p2p.addr 0.0.0.0,::) and start_network() spawns one TCP listener per entry.
• Dual-stack ENR advertisement: NodeRecord::from_network_config() emits both ip/tcp/udp (IPv4) and ip6/tcp6/udp6 (IPv6) ENR pairs when both families are configured.
• ENR-based outbound connections: Contact::record_enr_response_received() now updates node.ip and node.tcp_port from the received ENR, so the RLPx initiator can connect to IPv6-only peers.
• NodeRecordPairs::connection_addr(): helper that picks the best TCP address from an ENR (IPv4 preferred, IPv6 fallback).
• NetworkConfig refactor: rlpx_bind_addr/rlpx_external_addr (scalar) replaced by rlpx_bind_addrs/rlpx_external_addrs (Vec<IpAddr>).

Depends on

• Support different addresses for discv4 and RLPx #5424 (decouples discv4 address from RLPx address)
• Decouple local interface address from externally announced address #5425 (decouples bind address from external address)

Test plan

• cargo check passes (verified locally)
• Single-stack IPv4 default: --p2p.addr 0.0.0.0 behaves identically to before
• Dual-stack: --p2p.addr 0.0.0.0,:: binds two listeners
• NAT: --p2p.addr 0.0.0.0 --nat.extip <pub_ip> still announces correct external address
• ENR from an IPv6-only peer updates contact.node.ip so outbound connection uses IPv6

[github-actions]

🤖 Kimi Code Review

Review Summary

This PR adds dual-stack IPv4/IPv6 support for P2P networking by allowing multiple bind addresses and properly handling IPv6 in ENRs. The changes are well-structured and follow Rust best practices.

Issues Found

1. Potential panic in initializers.rs:395-397

let rlpx_bind_addrs: Vec<IpAddr> = if opts.p2p_addr.is_empty() {
    // No explicit bind address — default to one entry.
    let bind = if nat_extip.is_some() {
        IpAddr::V4(Ipv4Addr::UNSPECIFIED)
    } else {
        local_ip().unwrap_or_else(|_| {
            local_ipv6().expect("Neither ipv4 nor ipv6 local address found")
        })
    };

The expect here could panic if both local_ip() and local_ipv6() fail. Consider returning a proper error instead of panicking.

2. Missing validation for address count in initializers.rs:383-387

} else {
    opts.p2p_addr
        .iter()
        .map(|a| a.parse().expect("Failed to parse --p2p.addr address"))
        .collect()
};

The code allows any number of addresses but the help text mentions "one or two comma-separated addresses". Add validation to enforce this limit.

3. Inconsistent default handling in NetworkConfig::from_node()
The method creates a single-entry vector for both rlpx_bind_addrs and rlpx_external_addrs, but uses the same IP for both. This might not be appropriate for dual-stack scenarios.

4. Missing IPv6 validation in NodeRecord::from_network_config()
When handling IPv6 addresses, there's no validation that the address is actually a valid IPv6 address before encoding it.

Minor Suggestions

1. Consider using IpAddr::is_loopback() check
In initializers.rs, when determining external addresses, consider checking if the bind address is loopback and warn users, as this would make the node unreachable.

2. Add debug logging for address selection
The address selection logic is complex. Adding debug logs when addresses are auto-detected would help with troubleshooting.

3. Consider using std::net::ToSocketAddrs
For address parsing, consider using ToSocketAddrs which provides better error messages and handles more edge cases.

Security Considerations

The changes appear safe from a security perspective. The dual-stack support properly handles both IPv4 and IPv6 without introducing new attack vectors. The ENR construction correctly validates and sorts keys before signing.

Performance Impact

The changes are minimal performance-wise. The vector operations for multiple addresses are efficient, and the dual-stack support adds negligible overhead.

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

Lines of code report

Total lines added: 107
Total lines removed: 0
Total lines changed: 107

Detailed view

+--------------------------------------------+-------+------+
| File                                       | Lines | Diff |
+--------------------------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs                   | 987   | +6   |
+--------------------------------------------+-------+------+
| ethrex/cmd/ethrex/initializers.rs          | 665   | +22  |
+--------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/network.rs    | 664   | +3   |
+--------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_table.rs | 1326  | +4   |
+--------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/types.rs      | 569   | +72  |
+--------------------------------------------+-------+------+

[github-actions]

🤖 Claude Code Review

Here is the review of PR #6376:

---

Review: feat(p2p): dual-stack IPv6 support and ENR-based outbound connections

The overall structure is clean and the approach is sound. Below are the issues found, ordered by severity.

---

Correctness / Potential Bugs

1. Silent no-op when rlpx_bind_addrs is empty (network.rs)

bind_tcp_addrs() returns Vec<SocketAddr>. If rlpx_bind_addrs is somehow empty (e.g., from a programmatic NetworkConfig construction bypassing the CLI), start_network() silently spawns zero TCP listeners with no error or warning:

// network.rs
for tcp_addr in context.network_config.bind_tcp_addrs() {
    context.tracker.spawn(serve_p2p_requests(context.clone(), tcp_addr));
}

The node would run with discovery but no RLPx endpoint — a very confusing failure mode. Either add an assertion in bind_tcp_addrs(), or return an Err from start_network() when the list is empty.

---

2. Broken ENR when rlpx_external_addrs is empty (types.rs, ~line 495)

from_network_config iterates network_config.rlpx_external_addrs to populate ip/ip6 pairs. If that vec is empty (same construction path as above), the resulting ENR contains only id and secp256k1 — no reachability info at all. Remote peers would receive a valid-looking but completely unusable ENR. An early guard with a NodeError return would be safer.

---

3. dedup_by keeps first insertion, not first ENR-spec-priority address (types.rs, ~line 539)

record.pairs.sort_by(|a, b| a.0.cmp(&b.0));
record.pairs.dedup_by(|a, b| a.0 == b.0);

Per Rust's dedup_by semantics, a is the later element and is removed when the closure returns true — so the first occurrence of each key wins. After sort_by, the first occurrence corresponds to whichever address was pushed first into pairs during the loop. Since the loop iterates rlpx_external_addrs in order, the first IPv4 entry wins for ip/tcp/udp. This is correct but the comment says "in case both families map to the same family (e.g. two IPv4 addrs)" — it would be clearer to say "keeps the first address per family" so the behavior is obvious to a future reader.

---

4. Wildcard external address may be announced to peers (initializers.rs, ~line 395)

In the wildcard bind path:

local_ip().unwrap_or(IpAddr::V4(Ipv4Addr::UNSPECIFIED))

If local_ip() fails, the fallback 0.0.0.0 is stored in rlpx_external_addrs and will appear in the ENR advertised to peers. This is pre-existing behavior for the scalar case but is now more impactful because it propagates into the dual-stack ENR. A warn! log here would at least make the misconfiguration visible.

---

Design Gap

5. No way to specify dual NAT external addresses

--nat.extip accepts a single IP. In dual-stack mode (--p2p.addr 0.0.0.0,::), if the node is behind both an IPv4 NAT and an IPv6 NAT, there is no way to specify the IPv6 external address. The code silently falls back to local_ipv6() for the IPv6 external address, which may not be the publicly routable address. This limitation should be documented in the help text or as a known limitation.

---

Minor / Nits

6. --p2p.addr help text no longer mentions UDP discovery

The old help text said "Bind address for the P2P protocol (UDP discovery and TCP RLPx)"; the new text says only "Bind address(es) for the TCP RLPx transport." But UDP discovery still follows the first entry of rlpx_bind_addrs (via default_discovery_bind). A user passing --p2p.addr :: expecting IPv6-only would get an IPv6 discovery socket too without it being documented.

7. No automated tests

The test plan is entirely manual. Given the address-family filtering logic in rlpx_external_addrs derivation is non-trivial (the nat_extip family-match filter, the is_unspecified branch), unit tests covering the four combinations (IPv4-only, IPv6-only, dual-stack, dual-stack with NAT) would prevent regressions.

---

What's Good

• The NodeRecordPairs::connection_addr() IPv4-preference logic is clean and matches common client behavior.
• Updating self.node.ip/tcp_port from ENR before storing the record is the right place to do it — it's gated behind the hash check.
• from_network_config correctly sorts pairs before signing (unlike from_node which relies on insertion order).
• The NetworkConfig::from_node() migration to Vec fields is backward-compatible for callers that only ever pass a single node.

---

Automated review by Claude (Anthropic) · custom prompt

[github-actions]

🤖 Codex Code Review

1. --p2p.addr validation gap can create duplicate listeners on the same socket (correctness/perf risk).
   p2p_addr is documented as “one or two” addresses, but parsing accepts unbounded entries and there is no dedup/family validation. With duplicated IPv4 (or IPv6) entries, the node spawns multiple listeners for the same addr:port and SO_REUSEPORT is enabled, so this can silently split accepts across redundant sockets.
   Refs: cmd/ethrex/cli.rs:257, cmd/ethrex/initializers.rs:384, crates/networking/p2p/types.rs:52, crates/networking/p2p/network.rs:191, crates/networking/p2p/network.rs:231.
   Suggested change: enforce 1..=2 values when flag is present, require unique IP families (at most one v4 + one v6), reject duplicates.

2. Bare --p2p.addr is accepted (operator error is silently ignored).
   num_args = 0.. allows passing --p2p.addr with no value; this falls back to default bind behavior instead of failing fast. For infra config, this is risky because typos become silent runtime behavior changes.
   Refs: cmd/ethrex/cli.rs:257, cmd/ethrex/initializers.rs:373.
   Suggested change: require at least one arg (num_args = 1..=2) and keep “not provided” as empty vec.

3. ENR builder silently drops same-family extra addresses instead of rejecting config.
   from_network_config sorts/dedups by ENR key, so multiple IPv4 (or IPv6) addresses collapse to one without warning. That can advertise an unintended endpoint and complicate debugging peer reachability.
   Refs: crates/networking/p2p/types.rs:506, crates/networking/p2p/types.rs:539.
   Suggested change: validate rlpx_external_addrs shape before ENR construction and return an explicit error for unsupported combinations.

Overall direction looks good (dual-stack listener support + ENR-based contact refresh is useful), but the input-shape validation should be tightened to avoid silent misconfiguration.

I couldn’t run cargo check in this environment because rustup needs write access outside the workspace (/home/runner/.rustup is read-only here).

---

Automated review by OpenAI Codex · custom prompt