cli(ethp2p): unique per-node identity + operator-configurable peering + CI coverage - #1047
Merged
Merged
Conversation
… + CI Addresses review on #1046. The experimental ethp2p adapter was wired in the CLI with a hard-coded `.local_peer_id = "zeam-ethp2p"` and no other config, so every node that enabled it (e.g. via the Dockerfile `--build-arg ETHP2P=true` + `ZEAM_ETHP2P=1`) started the transport with the SAME identity and dial-only / no-peers — an identity collision that made the transport unusable and confusing across a fleet. - Derive a unique identity: `ZEAM_ETHP2P_PEER_ID` if set, else `zeam-ethp2p-<node_key_index>`. - Wire the previously-inaccessible peering fields from env (matching the existing `ZEAM_ETHP2P` env-toggle convention): `ZEAM_ETHP2P_LISTEN`, `ZEAM_ETHP2P_SERVER_CERT`, `ZEAM_ETHP2P_SERVER_KEY`, `ZEAM_ETHP2P_STATIC_PEERS` (comma-separated), `ZEAM_ETHP2P_SERVER_NAME`. Strings are allocated from the node's long-lived allocator (borrowed by the adapter for the process; startup-once). - CI: add a compile-only `zig build -Dethp2p=true` step. The adapter and its lazy `zig_ethp2p` dep are only imported under that flag, so default CI could stay green while the path was broken. Runtime activation is unchanged: still off unless built with `-Dethp2p=true` AND `ZEAM_ETHP2P` is truthy.
Contributor
|
Yes, this addresses the comments I left on #1046. Checked against the two concrete blockers:
Scope is also much cleaner here: this PR is just the ethp2p CLI/runtime config + CI follow-up, not another mixed sync/release/hashsig change. Only caveat: GitHub build jobs are still in progress as I write this, so my "happy" is conditional on the new CI step actually passing. I did not submit a formal GitHub approval; per repo convention this is a regular PR comment. |
g11tech
approved these changes
Jul 17, 2026
ch4r10t33r
added a commit
that referenced
this pull request
Jul 17, 2026
…evnet cert Revives #1045 on top of main (which already carries the ethp2p adapter via #1046 and the env-config layer via #1047). Adds the pieces that were only on the old #1045 branch, so an operator can enable ethp2p with just `ZEAM_ETHP2P=1` — no manual port/cert wiring: - `buildEthp2pConfig` derives the ethp2p listen address and static peers from the node's OWN libp2p QUIC addresses, shifted by `ZEAM_ETHP2P_PORT_OFFSET` (default +1, the ethlambda "ethp2p = gossipsub port + 1" convention). Every field keeps explicit-env precedence (`ZEAM_ETHP2P_LISTEN`, `ZEAM_ETHP2P_STATIC_PEERS`, ...) over the derived default. - Identity is the node's `node_key` (unique per node; retained by the RS engine and outlives the process — so the shared TLS cert below is NOT the peer id and cannot cause identity collisions). - Bundled self-signed devnet TLS cert/key at `/app/resources/ethp2p/{cert,key}.pem` (shipped via the existing `COPY resources/`), overridable with `ZEAM_ETHP2P_SERVER_CERT` / `ZEAM_ETHP2P_SERVER_KEY`. - Owned strings (listen_addr, static_peers) are freed via `freeEthp2pConfig` after `beam_node.init` — the adapter's `start` binds/dials synchronously and retains only `local_peer_id` (= long-lived `node_key`). Deliberately does NOT include the old #1045 `pkgs/node/src/node.zig` hunk, which predated and would revert the blocks_by_range sync-wedge fix (#1046). Runtime activation unchanged and doubly gated: `-Dethp2p=true` build AND `ZEAM_ETHP2P` truthy. Default build/test unaffected (adapter comptime-excluded); `-Dethp2p=true` compiles.
ch4r10t33r
added a commit
that referenced
this pull request
Jul 18, 2026
…ts + per-node runtime TLS cert (#1049) * cli(ethp2p): auto-derive endpoints from libp2p QUIC ports + bundled devnet cert Revives #1045 on top of main (which already carries the ethp2p adapter via #1046 and the env-config layer via #1047). Adds the pieces that were only on the old #1045 branch, so an operator can enable ethp2p with just `ZEAM_ETHP2P=1` — no manual port/cert wiring: - `buildEthp2pConfig` derives the ethp2p listen address and static peers from the node's OWN libp2p QUIC addresses, shifted by `ZEAM_ETHP2P_PORT_OFFSET` (default +1, the ethlambda "ethp2p = gossipsub port + 1" convention). Every field keeps explicit-env precedence (`ZEAM_ETHP2P_LISTEN`, `ZEAM_ETHP2P_STATIC_PEERS`, ...) over the derived default. - Identity is the node's `node_key` (unique per node; retained by the RS engine and outlives the process — so the shared TLS cert below is NOT the peer id and cannot cause identity collisions). - Bundled self-signed devnet TLS cert/key at `/app/resources/ethp2p/{cert,key}.pem` (shipped via the existing `COPY resources/`), overridable with `ZEAM_ETHP2P_SERVER_CERT` / `ZEAM_ETHP2P_SERVER_KEY`. - Owned strings (listen_addr, static_peers) are freed via `freeEthp2pConfig` after `beam_node.init` — the adapter's `start` binds/dials synchronously and retains only `local_peer_id` (= long-lived `node_key`). Deliberately does NOT include the old #1045 `pkgs/node/src/node.zig` hunk, which predated and would revert the blocks_by_range sync-wedge fix (#1046). Runtime activation unchanged and doubly gated: `-Dethp2p=true` build AND `ZEAM_ETHP2P` truthy. Default build/test unaffected (adapter comptime-excluded); `-Dethp2p=true` compiles. * cli(ethp2p): generate per-node TLS cert at runtime, drop the committed keypair Follow-up to review feedback: shipping a single self-signed cert/key for all nodes is wrong — libp2p mints its QUIC TLS cert at runtime from the node's identity, and ethp2p must do the same. - Remove the committed resources/ethp2p/{cert,key}.pem. - Add EthLibp2p.generateAuxQuicCertPems() — mints a fresh self-signed cert bound to the node's secp256k1 host identity via the SAME facility the primary libp2p QUIC transport uses (libp2p_tls_cert.generate + a fresh ephemeral cert key). Each call is unique per node and per process; nothing is shipped or shared. - buildEthp2pConfig now, when it listens and no explicit cert env is set, generates the cert at startup and writes the PEMs under the data dir (ethp2p wants file paths — it has no in-memory PEM entry point). Explicit ZEAM_ETHP2P_SERVER_CERT / _KEY still override. Cert/key paths are heap-owned and freed by freeEthp2pConfig after beam_node.init. The runtime cert can't be *literally* reused: the primary transport's cert is generated in EthLibp2p.run(), which happens after beam_node.init sets up the ethp2p listener — so at that point no cert exists yet. Generating a dedicated per-node cert from the same host identity via the same facility gives the identical security property (unique, runtime, never committed). zig build -Dethp2p=true ✅ · default zig build ✅ · zig fmt --check ✅ * ethp2p: skip self-entry when deriving static peers from genesis The genesis peer list (nodes.yaml) always contains this node itself, so the derived ethp2p static_peers included our own listen port. connect() is synchronous and blocks start() in its handshake-poll loop; a self-dial never completes because the server-side accept only runs later in tick(). That stalled the dial loop before it reached the real peers, so no ethp2p peer ever connected. Skip any connect-peer whose libp2p QUIC port matches our own.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the review findings on #1046 (which merged the experimental ethp2p adapter alongside the sync-wedge fix).
Finding 1 — shared identity / no operator config
pkgs/cli/src/node.zighard-coded.local_peer_id = "zeam-ethp2p"and set no other fields, so every node that enabled the adapter (Dockerfile--build-arg ETHP2P=true+ZEAM_ETHP2P=1) started it with the same identity and dial-only / no peers — an identity collision that makes the transport unusable/confusing across a fleet.Fix:
ZEAM_ETHP2P_PEER_IDif set, elsezeam-ethp2p-<node_key_index>.ZEAM_ETHP2Penv-toggle convention):ZEAM_ETHP2P_LISTEN,ZEAM_ETHP2P_SERVER_CERT,ZEAM_ETHP2P_SERVER_KEY,ZEAM_ETHP2P_STATIC_PEERS(comma-separated),ZEAM_ETHP2P_SERVER_NAME. Strings are allocated from the node's long-lived allocator (the adapter borrows them for the process; this runs once at startup).Finding 2 — no CI coverage for the flag
The adapter and its lazy
zig_ethp2pdependency are only imported under-Dethp2p=true, so default CI could stay green while that path was broken. Added a compile-onlybuild (ethp2p adapter)step runningzig build --fetch -Dethp2p=true && zig build -Dethp2p=true.Safety
Runtime activation is unchanged and still doubly-gated: off unless the binary was built with
-Dethp2p=trueANDZEAM_ETHP2Pis truthy. A node that hasn't setZEAM_ETHP2Pnever constructs the config.Validation
zig build -Dethp2p=truecompiles locally (verified).zig fmt --check pkgs/cli/src/node.zigclean.zig build/zig build testunaffected (adapter comptime-excluded).Scope is limited to the ethp2p CLI wiring + CI; the sync-wedge fix already landed in #1046.