fix(node): validate per-worker network keys and addresses

[MavenRain]

Closes #1337.

Problem

An operator can configure worker network keys that differ from the identities derived from the loaded BLS keystore, or configure multiple workers with the same network address. Startup previously accepted these configurations, allowing worker authentication failures or address conflicts after network startup. No attacker is required: this is a local configuration error.

Changes

• Validate the complete worker configuration before starting any swarm. Each advertised key must match KeyConfig::worker_network_public_key(worker_id), and worker network addresses must be pairwise distinct under exact Multiaddr equality. Errors identify the offending worker and configuration field. Existing worker-count, fork, event-stream, and RPC validation remains in the same startup check.
• Keep fixtures consistent with worker identities. Committee fixtures derive every advertised worker key from the authority's KeyConfig, including nonzero worker IDs. The fixture regression covers one and three workers across all four authorities and retains the worker-zero identity check.
• Document the operator action. The README explains the new startup errors and how to restore matching worker keys and separate worker addresses.

Integration

Targets main after #1315 merged. Merge commit 6657add9 preserves the parent change and resolves the old branch conflicts; the resulting PR diff contains only the four files for this fix. All 15 source fragments used by the focused tests and mutation checks still match the integrated source byte for byte.

Testing

• Pinned formatting passed. gateledger run -- cargo +nightly-2026-03-20 fmt --all -- --check, using the checkout's rust-nightly pin. git diff --check and independent strict review also passed.
• Focused Rust 1.94 harness passed. From the local tn-1337-validation/baseline directory, ran CARGO_HOME=../cargo-home CARGO_TARGET_DIR=../target RUSTC_WRAPPER= testledger --ledger ../ledger -- cargo +1.94 test --offline --locked -j 2 -- --test-threads 2: 10 passed, none failed or ignored, including the full 65,536-worker boundary. With --features adiri, the two new rejection tests and pre-fork rejection test also passed (3/3). The harness extracts the exact startup helper, its supporting methods, the startup regression tests, and the fixture key-mapping statement. It uses real cryptography and address types, with queue and fixture-construction substitutes. An independent audit checked source equality, direct dependency versions, derivation bytes, and mutation scope.
• Mutation checks passed. Three separate mutants compiled and failed their intended tests: disabling key matching, disabling duplicate-address rejection, and deriving worker zero's key for every fixture worker. Each changes only one expression added by this PR. Production files remained unchanged.
• Integration coverage remains outstanding. The focused harness does not establish full tn-node or committee-fixture integration coverage. The current 21-crate Clippy check completed with exit 101 because of lints in primary and e2e test files outside this PR, including int_plus_one, needless_question_mark, manual_checked_ops, and useless_vec. It used the pinned nightly with --offline --locked -j 2 --all-targets --no-deps -- -D warnings, an isolated target directory, and the compiler wrapper disabled. Workspace test lanes and the full attestation suite have not run.

This remains a draft until the outstanding integration checks and attestation are complete. The PR targets main, but the current workflow skips compile/test lanes for drafts and maintainer-authored PRs. CI run 34378773207 started on this push: its compile/test lanes were skipped and verify-on-chain failed because commit 6657add9884d4b309ab53a9abbcc79571964fbd4 is not attested. The local attested suite is still required. No skipped or absent workflow is counted as validation.

[grantkee]

Looks good - I have a few requests to improve the debugging ux and future proof the validation effort here

[grantkee]

The duplicate-address check compares Multiaddrs byte for byte, so two worker entries that differ only in their trailing /p2p/ suffix pass it and then collide on the same UDP socket when the second one binds.

libp2p-quic's multiaddr_to_socketaddr parses and discards a trailing Protocol::P2p and binds only (ip, udp port). The node-info address reaches Swarm::listen_on with the suffix intact: consensus_config.worker_address(worker_id) (start_epoch.rs:849-851) → NetworkHandle::start_listening (network-libp2p/src/types.rs:574-579) → self.swarm.listen_on(multiaddr) (consensus.rs:812-815). Production already depends on this, since every address keytool writes carries /p2p/<peer_id> (keytool/generate.rs:202-215), and nothing on the config path strips it — NodeP2pInfo's deserializer only enforces a non-empty list, and none of the validate() functions in tn-config look at network_address.

So /ip4/X/udp/9000/quic-v1/p2p/A and /ip4/X/udp/9000/quic-v1/p2p/B are unequal here but the same socket. create_socket binds with no SO_REUSEADDR/SO_REUSEPORT, so the second bind returns AddrInUse → NetworkError::Listen → start_epoch.rs:863 ? → the node exits. That happens after spawn_node_networks has already constructed and spawned every swarm as a critical task, which is the partially-spawned state the doc comment on this function says the up-front check prevents. /ip4/0.0.0.0/udp/9000 vs /ip4/127.0.0.1/udp/9000 slips through the same way.

Two scope notes so this isn't overstated. keytool never writes a multi-worker file (validate() rejects --workers != 1), so a multi-worker node-info.yaml is hand-edited — but the natural way to hand-edit is to copy worker 0's entry, change the key (which this PR now forces) and the /p2p/ suffix to match, and forget the port, which is exactly the shape this misses. And worker-vs-worker collision is unreachable until #557: spawn_worker_node_components hard-codes DEFAULT_WORKER_ID (start_epoch.rs:463-464), so workers 1..N are spawned but never bind today. The same gap between the primary and worker 0 on one port is reachable today, but that's outside #1337's remit.

The existing test prepare_worker_networks_rejects_duplicate_network_addresses copies a byte-identical address, and worker_p2p never appends /p2p/, so the suffix-only variant has no coverage.

Smallest fix I can see is to key the set on the address a QUIC listener actually binds. tn_types already re-exports Protocol. Keeping the raw configured address in the message means the existing contains(&first_address.to_string()) assertion still holds.

use tn_types::{Multiaddr, Protocol};

/// The address a QUIC swarm actually binds: libp2p-quic parses and discards trailing
/// `/p2p/<id>` components on `listen_on`, so two entries that differ only there share a socket.
fn listen_address(addr: &Multiaddr) -> Multiaddr {
    let mut listen = addr.clone();
    while matches!(listen.iter().last(), Some(Protocol::P2p(_))) {
        listen.pop();
    }
    listen
}

let mut addresses: HashSet<Multiaddr> = HashSet::with_capacity(configured);
// ...
eyre::ensure!(
    addresses.insert(listen_address(&p2p.network_address)),
    "node config `node_info.p2p_info.workers[{worker_id}].network_address` ({}) \
     duplicates an earlier worker's listen address (a differing `/p2p/` suffix is still \
     the same socket): every worker swarm needs its own address",
    p2p.network_address
);

Plus one more case in the duplicate-address test where the duplicate is first_address.clone().with(Protocol::P2p(keys.worker_network_public_key(duplicate_id).into())).

A fuller (ip, port) comparison that also treats 0.0.0.0 as colliding within its family and includes the primary address would close the rest, but it changes the signature and all nine call sites and widens the PR past #1337 — I'd take that as a follow-up alongside the primary-key check.

[grantkee]

The key-mismatch error prints the expected key with Debug formatting, which is libp2p's unpadded-hex form rather than the base58 string node-info.yaml uses, so the operator cannot paste the value the message tells them to set.

NetworkPublicKey derives Debug (crates/types/src/crypto/network.rs:11-12), which delegates to libp2p::identity::PublicKey. The ed25519 impl writes PublicKey(compressed): followed by each byte as {byte:x} — unpadded, so 0x0a prints as a and the string isn't even reliably decodable back to 32 bytes by hand. What lands in the log is shaped like:

NetworkPublicKey(PublicKey { publickey: Ed25519(PublicKey(compressed): 5f3a9c…e1) })

The value in node-info.yaml comes from the custom Serialize at network.rs:46-57, which emits bs58::encode(self.encode_protobuf()) for human-readable serializers, and Deserialize::visit_str reverses exactly that. That's the only paste-able form. PeerId renders as a base58 multihash (12D3KooW…), which also isn't it.

There's no Display impl on the type, and this ensure! is the first operator-facing site in the repo that formats a NetworkPublicKey, so {:?} was the only formatter available rather than a convention being followed. The README paragraph this PR adds tells operators to "Restore worker key entries that match the node's BLS keystore", which presumes the error hands them a usable value. The found {:?} half echoes back the value they already have in the file.

Suggest adding Display next to the Serialize branch it mirrors — bs58 is already a tn-types dependency — and switching this message to {}:

/// Renders the key exactly as `node-info.yaml` and genesis files carry it
/// (Base58 of the libp2p protobuf encoding), so error messages are paste-able.
impl fmt::Display for NetworkPublicKey {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.write_str(&bs58::encode(self.encode_protobuf()).into_string())
    }
}

let expected = key_config.worker_network_public_key(worker_id);
eyre::ensure!(
    p2p.network_key == expected,
    "node config `node_info.p2p_info.workers[{worker_id}].network_key` does not \
     match the key derived for worker {worker_id} (expected {expected}, found {}): \
     set this entry to the expected value to match the loaded BLS keystore",
    p2p.network_key
);

prepare_worker_networks_rejects_mismatched_network_keys could then also assert error.to_string().contains(&keys.worker_network_public_key(invalid_id).to_string()) to pin the paste-able format. Debug output for ?-formatted logs is unchanged.

[grantkee]

This test now performs 131,073 uncached BLS-backed key derivations and takes about 19 seconds under the debug test profile, where the version on main did one.

worker_network_public_key → worker_network_keypair (crates/config/src/keys.rs:512-524) → generate_network_keypair (keys.rs:549-555) does one BLS min_sig signature, one blake3 hash, and one ed25519 keygen per call, and nothing caches the result — KeyConfigInner has no derived-key field, and the test at keys.rs:1060 explicitly guards against one being added. Under cargo test, .cargo/config.toml sets [profile.test.package."*"] opt-level = 1, and blst only bumps its C code to -O2 when built without debug_assertions, so the signing runs at -O1.

Count for this test: 65,536 in worker_p2p for 0..=WorkerId::MAX in the block anchored here, one more for the overflow tail on line 1663, and another 65,536 inside prepare_worker_networks's success path when it re-derives every key at line 151 — zero on the overflow path. I measured 18.72 s for this test alone; the whole prepare_worker_networks filter (nine tests) runs in 18.71 s, so this is the entire cost of the module's unit tests. On 58c0a62d the test built one P2pNode and did vec![worker; max_workers + 1].

The two halves aren't equally expensive to keep. The overflow rejection fires at the count check (node.rs:129-132) before the HashSet and the per-worker loop, so it needs no distinct keys — the test already relies on that, since the overflow tail is a clone of worker 0 and would trip the duplicate-address check if the loop ran. The full-range success assertion (prepared.last().worker_id == WorkerId::MAX) genuinely needs 65,536 distinct, correctly keyed entries, and what it proves is that .zip(0..=WorkerId::MAX) is inclusive and agrees with the <= MAX + 1 bound. That's a real off-by-one guard, but not one that needs to run in every default cargo test.

Suggest splitting it — cheap overflow rejection in the default run, full-range success behind #[ignore] with a reason, the way crates/types/src/forks.rs:1339 already does for an expensive case. Building the P2pNode directly also skips the two URL parses per worker that worker_p2p does for an RpcInfo this test immediately discards.

/// One more worker than the `WorkerId` range holds is rejected before any per-worker
/// validation runs, so cloned entries are enough: the count check fires ahead of the loop.
#[test]
fn prepare_worker_networks_rejects_worker_id_overflow() -> eyre::Result<()> {
    let keys = worker_key_config();
    let max_workers = usize::from(WorkerId::MAX) + 1;
    let worker = P2pNode { rpc: None, ..worker_p2p(&keys, 0)? };
    let workers = vec![worker; max_workers + 1];
    let streams = vec![(); max_workers + 1];
    let error = prepare_worker_networks(&workers, &streams, &keys, Epoch::MAX, max_workers + 1)
        .err()
        .ok_or_else(|| eyre!("expected WorkerId overflow"))?;
    assert!(error.to_string().contains("WorkerId range"));
    Ok(())
}

/// Every id in the `WorkerId` range is accepted and the last prepared worker carries
/// `WorkerId::MAX`, so the `0..=WorkerId::MAX` zip cannot silently truncate the layout.
///
/// Each entry must carry its own derived key and a distinct address to pass the per-worker
/// checks, and `prepare_worker_networks` re-derives every key, so this test performs
/// 2 x 65,536 BLS-backed derivations (tens of seconds under the debug test profile).
#[test]
#[ignore = "derives 2 x 65,536 worker network keys; run with `cargo nextest run --run-ignored all`"]
fn prepare_worker_networks_accepts_full_worker_id_range() -> eyre::Result<()> {
    let keys = worker_key_config();
    let max_workers = usize::from(WorkerId::MAX) + 1;
    let workers = (0..=WorkerId::MAX)
        .map(|worker_id| {
            Ok(P2pNode {
                network_address: format!(
                    "/ip4/127.0.{}.1/udp/{}/quic-v1",
                    worker_id / 256,
                    9000 + worker_id % 256
                )
                .parse()?,
                network_key: keys.worker_network_public_key(worker_id),
                rpc: None,
            })
        })
        .collect::<eyre::Result<Vec<_>>>()?;
    let streams = vec![(); max_workers];
    let prepared = prepare_worker_networks(&workers, &streams, &keys, Epoch::MAX, max_workers)?;
    assert_eq!(prepared.len(), max_workers);
    assert_eq!(prepared.last().map(|worker| worker.worker_id), Some(WorkerId::MAX));
    Ok(())
}

The alternative is a production-side eyre::ensure!(prepared.len() == configured, ...) after the collect, which turns a silent zip truncation into a startup error covered by every small-N test already here and lets the full-range test go away entirely. That's a production change for a regression the bound at 129-132 already makes unlikely, so I'd leave it to your judgment.

[grantkee]

With the builder's default randomize_ports == false, every advertised worker here gets the identical placeholder address /ip4/127.0.0.1/udp/0/quic-v1, which the duplicate-address check this PR adds to prepare_worker_networks rejects for any fixture of three or more workers.

The code producing the address is the worker_address closure at builder.rs:157-164, which this PR doesn't change — anchoring here because this is the loop that calls it once per worker. The closure takes no worker id and yields port 0 unless randomizing, so every call returns the same string. Under randomize_ports == true the ports are distinct, because get_available_udp_port only returns a port after claiming it against the process-global USED_PORTS; the placeholder path has no such guard.

Those entries reach node_info verbatim. AuthorityFixture::generate (authority.rs:145-165) takes worker 0 from Config::default_for_test_with_genesis → NodeP2pInfo::default(), which allocates a real claimed port, then chains bootstrap.workers.iter().skip(1).cloned() for the rest. Default layout for N workers is [127.0.0.1:<rand>, 127.0.0.1:0, 127.0.0.1:0, …], so two workers pass and three fail the new HashSet insert at node.rs:145-150.

This is latent rather than broken today: no test feeds a CommitteeFixture config through prepare_worker_networks. .number_of_workers( has exactly one caller (fixture_tests.rs:15, which only compares keys), and the e2e harness builds its TnBuilder from on-disk keytool output with one worker per validator. It will bite the first time #557 adds a multi-worker EpochManager test on a default fixture, and the failure will look like a bug in the new check rather than in the fixture.

I don't think the production check should bend for it — /udp/0 is also the advertised external address, so two of them in a real config is a genuine misconfiguration. The fixture is the right place to fix this. Passing the worker id into the closure and varying the loopback host in placeholder mode keeps everything unbound (no fixed port) and distinct for any N. 127.{id / 256}.{id % 256}.1 maps every WorkerId to a valid octet pair; a 1 + id % 256 scheme would hit an invalid .256 at id % 256 == 255.

            let randomize_ports = self.randomize_ports;
            // Placeholder addresses (`randomize_ports == false`) must still be distinct per
            // worker: `prepare_worker_networks` rejects duplicate worker addresses, and a
            // three-worker fixture would otherwise advertise `/udp/0` twice. Vary the loopback
            // host rather than the port so nothing binds a fixed port. Randomized addresses stay
            // on 127.0.0.1 with allocator-claimed (already distinct) ports.
            let worker_address = move |worker_id: WorkerId| -> Multiaddr {
                let (host, port) = if randomize_ports {
                    (host.to_string(), get_available_udp_port(host).unwrap_or(DEFAULT_WORKER_PORT))
                } else {
                    (format!("127.{}.{}.1", worker_id / 256, worker_id % 256), 0)
                };
                format!("/ip4/{host}/udp/{port}/quic-v1").parse().unwrap()
            };
            // Advertise the same derived identity each worker swarm uses to authenticate.
            let worker_nodes: Vec<P2pNode> = (0..=WorkerId::MAX)
                .take(self.number_of_workers.get())
                .map(|worker_id| {
                    let key = key_config.worker_network_public_key(worker_id);
                    (worker_address(worker_id), key).into()
                })
                .collect();

The fixture regression in fixture_tests.rs could pin it with a HashSet over worker_p2p_nodes() asserting len() == number_of_workers.get() per authority. One collateral to be aware of: 127.0.1.1 and above aren't bound by default on macOS, so a test that sets randomize_ports(false) and then actually listens on worker ≥ 1 would fail there — but the non-randomized mode has never promised bindable addresses, and worker 0 (the only worker any current test listens on) keeps 127.0.0.1.

[sstanfield]

LGTM but fill need to re-approve after you address Grant's feedback.