You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found while reviewing #1343 (per-worker network key validation, closes #1337). Line numbers are against a3f0d001.
Problem
The comment above the worker-list splice in AuthorityFixture::generate (crates/test-utils-committee/src/authority.rs:150-153) reads:
// The node info defaults to one worker; give the fixture `number_of_workers` entries so// its p2p info matches the committee. Worker 0 keeps the key config's worker key (the// key config holds a single worker key); workers 1.. are the SAME nodes the committee's// bootstrap entry advertises for this authority, so committee and node info agree.
Two things in it are no longer true:
"the key config holds a single worker key" — KeyConfig::worker_network_keypair (crates/config/src/keys.rs:512-524) derives a distinct keypair per worker id from worker_network_seed, and since fix(node): validate per-worker network keys and addresses #1343 the committee builder advertises key_config.worker_network_public_key(worker_id) for every worker (crates/test-utils-committee/src/builder.rs:166-171). Workers 1.. carry key-config keys too; that is the whole point of the fixture regression added in that PR.
"so committee and node info agree" — not quite for worker 0. Its node_info entry comes from Config::default_for_test_with_genesis → NodeP2pInfo::default() (crates/types/src/primary/info.rs:60-85), which allocates its own port, so worker 0's address in node_info and in the bootstrap entry differ. Only the key is reconciled, by the update_worker_network_key call a few lines down.
This is the only occurrence of "single worker key" in crates/. Harmless, but it sends the next reader to look for a per-worker key path that does not exist.
Fix
Rewrite the comment to describe what the code does now:
// The node info defaults to one worker; give the fixture `number_of_workers` entries so// its p2p info matches the committee. Worker 0 keeps the default node-info entry (its// key is overwritten below with the key config's worker-0 key); workers 1.. are cloned// from the committee's bootstrap entry, whose keys are derived per worker id from the// same key config, so committee and node info advertise the same worker identities.
Found while reviewing #1343 (per-worker network key validation, closes #1337). Line numbers are against
a3f0d001.Problem
The comment above the worker-list splice in
AuthorityFixture::generate(crates/test-utils-committee/src/authority.rs:150-153) reads:Two things in it are no longer true:
KeyConfig::worker_network_keypair(crates/config/src/keys.rs:512-524) derives a distinct keypair per worker id fromworker_network_seed, and since fix(node): validate per-worker network keys and addresses #1343 the committee builder advertiseskey_config.worker_network_public_key(worker_id)for every worker (crates/test-utils-committee/src/builder.rs:166-171). Workers 1.. carry key-config keys too; that is the whole point of the fixture regression added in that PR.node_infoentry comes fromConfig::default_for_test_with_genesis→NodeP2pInfo::default()(crates/types/src/primary/info.rs:60-85), which allocates its own port, so worker 0's address innode_infoand in the bootstrap entry differ. Only the key is reconciled, by theupdate_worker_network_keycall a few lines down.This is the only occurrence of "single worker key" in
crates/. Harmless, but it sends the next reader to look for a per-worker key path that does not exist.Fix
Rewrite the comment to describe what the code does now: