feat(node,config,network-libp2p): per-worker network swarms (#555, PR 2) - #1315
Conversation
Give each configured worker its own long-running libp2p swarm. The EpochManager's singular worker_network_handle and worker_event_stream become vectors indexed by WorkerId, and spawn_node_networks spawns one ConsensusNetwork per entry in node_info.p2p_info.workers, each on its own listen address with its own derived network keypair and gossip topics. KeyConfig now derives worker network keypairs per id. Worker 0 keeps the legacy bare-seed derivation so its PeerId stays stable for deployed nodes (the identity is advertised on-chain and cached in peers' kad stores). Higher ids append the id to the seed for a distinct keypair per swarm. The per-epoch code still drives worker 0 only; #557 adds the loop over worker components. With num_workers = 1 behavior is unchanged. Closes #555. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…k-swarms Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…k-swarms Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…k-swarms Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv> # Conflicts: # crates/config/src/keys.rs
…k-swarms Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…k-swarms Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv> # Conflicts: # crates/config/src/keys.rs
grantkee
left a comment
There was a problem hiding this comment.
The code is clean. I left feedback here because the swarm is still limited to one worker. The codebase still requires 1 worker, so I'm okay if you prefer to address this in follow up PRs or if you want to stay true to the goal of "per-worker network swarms" and address it here. Good work @MavenRain
| network_config, | ||
| worker_event_stream.clone(), | ||
| self.key_config.clone(), | ||
| self.consensus_db.clone(), |
There was a problem hiding this comment.
All worker swarms share a single kad record table, so sibling swarms overwrite and purge each other's node records.
This is the blocker. The code I'm describing is crates/network-libp2p/src/kad.rs:224-234, :248-252 and :429-510, which this PR doesn't change — commenting here because self.consensus_db.clone() inside the new per-worker loop is what hands every swarm the same store, and the loop is what makes a second worker swarm reachable for the first time.
KadStore routes every NetworkType::Worker(_) to one table pair regardless of the id. All 15 members that branch on kad_type write the worker arm as NetworkType::Worker(_): new (:224-232), evict_expired_records (:256-289), evict_expired_providers (:293-333), scrub_corrupt_providers (:341-363), get (:429-441), put (:443-483), remove (:485-502), records (:504-510), add_provider (:512-583), :585-602, provided (:604-620), remove_provider (:622-659). key_to_hash (:248-252) takes &self but never reads self.kad_type, so the row hash carries no worker-id component either. There are only four tables (crates/storage/src/lib.rs:112-115) and their CF names are compile-time &'static str consts with no id interpolation (storage/src/lib.rs:68-71).
Every swarm then keys its own record on the same bytes. get_peer_record keys on the primary BLS public key (consensus.rs:618), and both the primary at node.rs:1029 and the worker here pass self.key_config.clone(). So worker 0 and worker 1 write the same row, in the same table, from the same Arc<Inner<DB>>.
What makes the collision destructive rather than merely redundant is that the values are not interchangeable. RecordDomain::role_parts folds the worker id into the signed bytes (types.rs:928-933, Worker(id) => (1, id), and :944-947), and the domain is never transmitted (types.rs:900-908) — the verifier reconstructs it from its own identity. So a Worker(1)-signed record fails verification under Worker(0)'s domain and vice versa.
Two consequences, both worse than a lost row:
Honest peers ban this node. A remote worker-0 peer that receives a Worker(1)-signed record fails peer_record_valid and applies Penalty::Fatal (consensus.rs:1881-1890, and the put path at :2074-2079). The node gets banned for records it published correctly.
It recurs, and it self-purges on every restart. run calls provide_our_data() unconditionally at task start (consensus.rs:718-721) — one run shared by primary and worker, with no node-mode, committee, peer-count or worker-id guard. libp2p's PutRecordJob then re-runs it every kad_publication_interval, 12h by default (crates/config/src/network.rs:202). Separately, the startup loop in ConsensusNetwork::new (consensus.rs:465-489) runs decode_and_verify over every row and pushes failures to corrupt, then calls kad_store.remove(&key) — so each swarm deletes its sibling's rows on load, wiping the persisted peer cache every restart. Observer transaction forwarding (#804) then can't resolve targets on such a node.
One imprecision worth noting so nobody re-derives it: key_to_hash hashes the BCS encoding of the RecordKey, not the raw bytes. Immaterial to the collision.
Fix, part 1 — namespace the row key
fn key_to_hash(&self, key: &RecordKey) -> BlockHash {
// Namespace the row by (role, worker id). `NetworkType::Worker(_)` selects one shared pair
// of tables for EVERY worker, and every ConsensusNetwork keys its own record on the same
// primary BLS key (consensus.rs:618). Without this, two worker swarms land on one row:
// they publish NodeRecords signed for different `RecordDomain`s, so each overwrites the
// other's value and each purges the other's rows in the verify-and-remove loop in
// `ConsensusNetwork::new`. Discriminants match `RecordDomain::role_parts`.
let (role, worker_id): (u8, tn_types::WorkerId) = match self.kad_type {
NetworkType::Primary => (0, 0),
NetworkType::Worker(id) => (1, id),
};
let mut h = DefaultHashFunction::new();
h.update(&[role]);
h.update(&worker_id.to_le_bytes());
h.update(encode(key).as_ref());
BlockHash::from_slice(h.finalize().as_bytes())
}Fix, part 2 — ownership filter on whole-table scans
Namespacing the key alone doesn't stop a scan from touching a sibling's rows. KadRecord already persists its key field (kad.rs:33), so ownership is re-derivable with no schema change:
fn owns(&self, key: &RecordKey, hash: &BlockHash) -> bool { self.key_to_hash(key) == *hash }Apply it in records() (:504-510), evict_expired_records (:256-289), evict_expired_providers (:293-333), scrub_corrupt_providers (:341-363), and the counter seed in new (:224-232). This also resolves the counter-drift problem in my other comment.
Migration — your call
| Option | Pro | Con |
|---|---|---|
A. Bump the CF name consts (crates/storage/src/lib.rs:68-71, "kad_record" → "kad_record_v2") |
Cannot half-apply; least code; kad state self-heals via republication | Old tables left as dead space |
B. One-time startup purge of rows whose key doesn't re-derive, gated on a schema version |
No dead space | More code; needs a schema-version field that doesn't exist yet |
I'd go with A. Kad rows are TTL'd discovery data re-learned on first connect via process_kad_put_request, so the cost is one discovery round. No determinism impact — kad state is node-local and never enters consensus — and no NodeRecord serialization change.
Tests
All three fail today, and no test anywhere currently constructs NetworkType::Worker(1) — test_kad_store (kad.rs:813-984) pins only the Primary↔Worker(0) axis that already works.
- Two
KadStores on onedbwithWorker(0)/Worker(1)must not see, clobber or evict each other (sibling oftest_kad_store). - A
Worker(0)-signedNodeRecordmust faildecode_and_verifyunderRecordDomain::new(chain, Worker(1))(sibling oftest_cross_role_replay_rejected,tests/types.rs:149-179). - Seed
KadWorkerRecordswith aWorker(1)record, construct aWorker(0)network, assert the row survives.
There was a problem hiding this comment.
Addressed in 1748aba: Kademlia keys now include the role and worker ID, and scans, eviction, cleanup, and initial counters only consider owned rows. Used option A for migration with fresh *_v2 cache tables. Added shared-DB worker isolation, cross-worker signature rejection, and startup sibling-preservation tests; ebf7da6 adds the required persistence barriers to the deletion assertions.
| Ok(res?) | ||
| // create one long-running swarm per configured worker | ||
| // the per-epoch code still drives worker 0 only (#557 loops over worker components) | ||
| let workers = self.builder.tn_config.node_info.p2p_info.workers.clone(); |
There was a problem hiding this comment.
The number of worker swarms comes straight from local node config, with no cross-check against the committee's worker count or the multi-workers fork gate.
This line decides how many swarms get spawned, and nothing between here and run_epochs compares it to anything. spawn_node_networks is called at node.rs:765; run_epochs is at node.rs:991; check_committee_worker_count runs at start_epoch.rs:365 — about 226 lines and a full async epoch-entry chain later.
The gap is wider than "validation happens late". check_committee_worker_count (start_epoch.rs:1133-1157) reads the on-chain count only, and returns Ok(()) on its first line when that count is 1. So in the local=3 / chain=1 / pre-fork case it never fires and emits no diagnostic at all. For the local-vs-chain relationship, the check doesn't exist. Grepping p2p_info.workers, num_workers(), workers.len() and number_of_workers across the repo turns up no expression that compares the two; the only production number_of_workers() comparison (run_epoch.rs:341-348) is chain-vs-chain.
A multi-worker local config is reachable without any code change. pub workers: Vec<P2pNode> (crates/types/src/primary/info.rs:25) has exactly one repo-wide constraint — deserialize_non_empty_workers (crates/types/src/committee.rs:193-203) — min 1, max unbounded. There are zero deny_unknown_fields hits in crates/, Config::load / load_from_path_or_default (crates/config/src/node.rs:76-172, crates/config/src/traits.rs:56-86) are pure serde with no post-load hook, and NodeInfo/NodeP2pInfo have no validate(). An in-tree test already round-trips a 2-worker list through the exact serde path Config::load uses (info.rs:111-122 asserts decoded.num_workers() == 2). The keytool's if self.workers != 1 { return Err(...) } (keytool/generate.rs:227-229) checks a CLI flag, not the on-disk file, and keytool set-rpc / generate pop preserve p2p_info verbatim (pinned by keytool/mod.rs:459), so a hand-added entry survives them. NetworkGenesis::validate → agreed_num_workers (crates/config/src/genesis.rs:132-186) is a ceremony-host check over other validators' files and never runs on a node's own datadir. On non-adiri builds multi_workers_build_fork_active returns true unconditionally (crates/types/src/forks.rs:496-500).
So: duplicate the entry under p2p_info.workers in node-info.yaml (rpc is #[serde(default)], so it can be omitted), start the node, and it starts silently. It needs deliberate hand-editing, it isn't remotely triggerable and it isn't consensus-affecting — but a paste-duplication typo is enough, and there's no diagnostic.
The chain count is already available before this point: catchup_accumulator (node.rs:756) sizes gas_accumulator from WorkerConfigs. Passing it into spawn_node_networks (node.rs:765, :1004) would let this fail loudly:
let configured = self.builder.tn_config.node_info.p2p_info.num_workers();
eyre::ensure!(
configured == 1 || tn_types::forks::multi_workers_fork_active(epoch),
"node config `node_info.p2p_info.workers` lists {configured} workers but the \
multi-workers fork is not active at epoch {epoch}: configure exactly one worker"
);
let on_chain = gas_accumulator.num_workers();
eyre::ensure!(
configured == on_chain,
"node config `node_info.p2p_info.workers` lists {configured} workers but the \
chain-derived count for epoch {epoch} is {on_chain}: every validator must run the \
worker count the committee carries (see `Committee::number_of_workers`)"
);Worth also adding a third configured_workers: usize parameter to check_committee_worker_count (start_epoch.rs:1133) with eyre::ensure!(configured_workers == num_workers.get(), ...) at the top, so a governance-driven count change mid-process is caught at the epoch boundary too. The call site at start_epoch.rs:365 would pass self.builder.tn_config.node_info.p2p_info.num_workers().
One deployment-visible consequence: a node whose local config disagrees with the chain would now fail to start rather than start degraded. That's the intent, but it's worth a release note.
| )); | ||
| // create long-running network task for this worker | ||
| let worker_network = ConsensusNetwork::new_for_worker( | ||
| worker_id, |
There was a problem hiding this comment.
Kad record accounting is per-store but the underlying table is shared, so one worker's eviction desynchronizes another worker's counter and can wedge it permanently.
Each ConsensusNetwork::new_for_worker call here builds its own KadStore, and each one gets its own private record counters. The code is in crates/network-libp2p/src/kad.rs:201-203, :224-236 and :443-483, which this PR doesn't change — commenting here because this is the call that now constructs more than one of them against a single table.
num_records (kad.rs:201) and num_providers (:203) are plain per-instance usize — no atomics, no lock, no interior mutability. They're seeded by counting the whole table (:224-232), because nothing in a row identifies its writer. They're mutated only locally (:234, :286, :469, :499) and nothing ever re-synchronizes them. max_records is enforced against the private counter at :462 and re-checked at :465; max_provided_keys does the same at :550/:555.
The interesting failure isn't the cap bypass, it's the wedge. Worker 1's evict_expired_records (:256-289) scans the whole shared table and deletes worker 0's expired rows, then subtracts only from its own counter. Worker 0's counter still counts those rows. Worker 0's next new-key put hits the cap at :462, evicts (nothing left to evict), re-checks at :465, and returns Err(Error::MaxRecords) — forever, against an effectively empty table, until the process restarts. The roughly N × max_records aggregate bypass holds too, in the other direction.
One correction to flag, since it changes the scope: max_providers_per_key is not counter-based. kad.rs:385 compares against the length of the actual decoded DB row, so that particular cap is drift-immune.
The ownership filter from my key_to_hash comment resolves this — once a scan only touches rows it owns, the counters stop diverging. Worth a test alongside it: two stores on one open_db, fill worker 0 to max_records, have worker 1 run an eviction, then assert worker 0's next put still succeeds.
There was a problem hiding this comment.
Addressed in 1748aba. Startup counts and eviction now operate only on the store's owned rows, with consistent accounting for corrupt-row repair/removal. Added a regression that fills worker 0 to capacity, runs eviction on worker 1, and verifies worker 0 can still insert records and providers.
|
|
||
| // spawn long-running worker network task | ||
| node_task_spawner.spawn_critical_task( | ||
| format!("Worker Network {worker_id}"), |
There was a problem hiding this comment.
The network metrics label collapses every Worker(id) to the string "worker", so per-worker swarms silently overwrite each other's gauges in the process-global registry.
The task name here already carries the worker id, which is the right instinct — the metrics don't. network_label (crates/network-libp2p/src/metrics.rs:13-18) returns &'static str with Worker(_) => "worker". That file isn't in this diff; I'm commenting here because this loop is what first creates more than one worker swarm in a process.
The registry is process-global: static RECORDER_HANDLE: OnceLock<PrometheusHandle> (crates/tn-metrics/src/recorder.rs:44), one set_global_recorder (:68), installed once at crates/telcoin-network-cli/src/node.rs:163. metrics-rs keys series by (name, labels) — the repo documents this itself at crates/consensus/worker/src/metrics.rs:59-61.
So every worker swarm writes the same series. tn_network.kad_records (kad.rs:239-246) is a gauge each store .set()s with its own num_records, called from :235, :287, :470 and :500, and the collision is continuous rather than at startup only: set_pending runs once per event-loop iteration (consensus.rs:744) and set_peer_counts on every peer-manager heartbeat (peers/manager.rs:364).
Gauges that collide (last writer wins): px_disconnects_pending (:31), outbound_requests_pending (:33), connected_peers / known_peers / discovery_peers / banned_peers (:89-96), and kad_records. Counters that silently sum across swarms: gossip_*_total (:25-30), connections_closed_total / dial_failures_total / peers_banned_total (:97-103), outbound_request_failures_total (:75-80), connections_established_total (:142-147), peer_penalties_total (:173-178).
The rest of the codebase already does this correctly — WorkerMetrics::new_for_worker uses ("worker", worker_id.to_string()) (crates/consensus/worker/src/metrics.rs:74), as does BatchBuilderMetrics (crates/batch-builder/src/metrics.rs:30-31). Only the libp2p layer collapses it.
Fix is to change network_label to return String with Worker(id) => format!("worker-{id}"), change the cached network: &'static str field to String in SwarmMetrics (metrics.rs:42) and PeerManagerMetrics (:113), and clone at the three metrics::counter! sites (:77, :144, :175). kad.rs:243 needs no change.
Collateral worth calling out in the PR description: dashboards and alerts querying network="worker" would need network=~"worker.*". And metrics.rs:192-247 could be extended with a Worker(0) vs Worker(1) pair to pin it.
There was a problem hiding this comment.
Implemented in 1748aba: network_label and cached labels now use String, with worker-{id} labels and clones at the dynamic counter sites. Added worker-0/worker-1 gauge and counter isolation coverage, and documented the network=~"worker.*" dashboard selector update in the PR.
| self.key_config.worker_network_keypair() | ||
| /// The derived network keypair for this fixture's worker id. | ||
| pub fn keypair(&self) -> NetworkKeypair { | ||
| self.key_config.worker_network_keypair(self.id) |
There was a problem hiding this comment.
WorkerFixture is built with the authority index as its worker id, so fixtures for authorities 1 and up now derive a keypair that matches nothing in the committee.
This change is what makes it matter. On main, keypair() ignored self.id entirely, so the mislabelled field was inert; now it feeds the derivation. The i as u16 itself is pre-existing — added 2025-08-01 in d2ba05a4 — but this PR makes it load-bearing.
The mislabelling is at crates/test-utils-committee/src/builder.rs:193-196, which this PR doesn't change. authorities is a BTreeMap<BlsPublicKey, ...> (builder.rs:143), so the i from .enumerate() is the authority's rank in sorted-pubkey order, not a worker id. With committee_size = 4 the fixtures come out with id = 0,1,2,3, and each AuthorityFixture holds a single WorkerFixture field (authority.rs:20) which by construction is worker 0.
| k | worker().keypair().public() |
Committee bootstrap (builder.rs:170) |
Node info worker 0 (authority.rs:169-172) |
Match |
|---|---|---|---|---|
| 0 | derive(S) |
derive(S) |
derive(S) |
yes |
| 1-3 | derive("S k") |
derive(S) |
derive(S) |
no |
Additional workers at index ≥1 in a multi-worker fixture get fresh random ed25519 keys (builder.rs:171-173), so derive("S k") doesn't match those either.
Nothing is broken today, which is why I'd call this low priority rather than a blocker: a repo-wide grep finds zero callers of WorkerFixture::keypair(), zero readers of WorkerFixture.id outside keypair(), and zero callers of AuthorityFixture::worker(). The crate has no tests of its own, and the tests that do check worker-key/committee agreement (network_tests.rs:726, :756, :856, :881) call key_config().worker_network_keypair(DEFAULT_WORKER_ID) directly. It's a trap for the next person who reaches for the fixture, not a live failure.
Fix is at builder.rs:193-196 — drop the .enumerate() and pass DEFAULT_WORKER_ID, which is already imported at builder.rs:12:
for (_, (primary_keypair, key_config, authority)) in authorities.iter_mut() {
let worker = WorkerFixture::generate(key_config.clone(), DEFAULT_WORKER_ID);Optionally an assert_eq!(id, DEFAULT_WORKER_ID, ...) in WorkerFixture::generate to pin it. This is test-only code, so a hard panic is the point — not debug_assert!.
There was a problem hiding this comment.
Fixed in 1748aba by dropping the authority-index enumeration and passing DEFAULT_WORKER_ID for every authority's single-worker fixture. A regression checks all four fixtures against the committee's advertised worker-0 public keys.
| warn!(target: "epoch-manager", ?res, "worker network stopped"); | ||
| Ok(res?) | ||
| // create one long-running swarm per configured worker | ||
| // the per-epoch code still drives worker 0 only (#557 loops over worker components) |
There was a problem hiding this comment.
Workers 1 and up get full swarms that never listen or dial, which costs a failed startup publish and a permanent heartbeat log line per extra swarm.
This comment is accurate and the staging is clearly deliberate — I traced it and it holds. start_epoch.rs:459-460 hard-codes let worker_id = DEFAULT_WORKER_ID; and everything downstream is scoped through it: gas slots (:465, :470), handle lookups (:474-477, :526-530), engine init (:489-500), batch validator (:531-533), and spawn_worker_network_for_epoch (:534-543). So start_listening (:859), dialling (:872) and topic subscription (:898) are worker-0-only. It's documented in four places (node.rs:1058-1059, start_epoch.rs:438, :234, plus the runtime warn! at :1145-1153), and spawn_worker_network_for_epoch is already written generically over worker_id (:829-834, :848-857) and just never called with a non-zero id. That's the pre-wiring this PR exists to deliver.
Two things I checked that are fine, recorded so nobody re-derives them. consensus.rs:560-561 adds the external address unconditionally in new, before any bind — but there's no protocol-visible effect, because the crate has no identify behaviour (TNBehavior at consensus.rs:135-162 is peer_manager, connection_limits, gossipsub, req_res, peer_exchange, kademlia, stream) and an id ≥1 swarm has zero connections, so there's nobody to propagate the address to.
The real cost per extra swarm is one failing startup provide_our_data() (PutRecord(Err) debug! at consensus.rs:1920-1922, StartProviding(Err) warn! at :1930-1937), a near-zero 1s gossipsub heartbeat with no topics, and a 30s peer-manager heartbeat emitting an info! line forever (peers/manager.rs:346-378). Cosmetic, but the log line is permanent and per-swarm, so it's worth knowing about before someone runs a 4-worker config and wonders about the noise.
The sequencing question is the one that actually needs an answer, and it's yours to make:
- Land this as-is and fix the shared kad table here. My preference — the fix is confined to
kad.rsand doesn't touch the diff under review. - Defer the kad fix to feat: Multi-Worker Support — PR 4:
EpochManagerMulti-Worker Loop #557. That ships a window where a hand-edited 2-worker config silently corrupts its own DHT records and gets the node banned by honest peers. - Gate the loop on
idx == 0. Removes the risk but trades away the pre-wiring this PR is for.
One note on a claim you may hear elsewhere: it is not the case that the kad store is keyed by NetworkType in a way that avoids collision. The table pair is selected by NetworkType, but the worker id inside it is discarded and the row hash has no discriminator. See my comment on self.consensus_db.clone().
There was a problem hiding this comment.
| worker_network_keypair: NetworkKeypair, | ||
| // Seed string for worker network keypairs. Per-worker keypairs are derived on demand from | ||
| // the primary_keypair and this seed; see `KeyConfig::worker_network_keypair`. | ||
| worker_network_seed: String, |
There was a problem hiding this comment.
The derived Debug on KeyConfigInner renders the worker network seed, and this PR dropped the positive assertion that kept the worker side of the leak test honest.
KeyConfigInner is #[derive(Debug)] (keys.rs:211) and now holds worker_network_seed: String on this line. KeyConfig is #[derive(Debug, Clone)] (:234) over Arc<KeyConfigInner>, and Arc's Debug forwards to the inner value — so format!("{:?}", key_config) does render the seed.
This is informational rather than urgent, for two reasons. The seed is a domain-separation label, not key material: it's operator-configurable, read from node_keys_path().join(WORKER_NETWORK_SEED_FILE) at keys.rs:356-358. And no production log path debug-prints a KeyConfig — I grepped every tracing macro plus format!, dbg! and println! and found zero sites.
Still worth closing, since the type is one ?key_config away from leaking, and the swap to a stored seed removed the only thing that would have caught a regression. A manual Debug for KeyConfigInner rendering the seed as "<redacted>" handles the first part.
For the second: the diff replaces the worker positive anchor in debug_does_not_leak_key_material with a comment explaining that the worker side has no anchor now. That's true as written, but it leaves the worker half of the test with no assertion at all. A negative assertion would restore coverage and would fail if the seed ever starts rendering:
assert!(
!rendered.contains("worker network keypair"),
"worker network seed should not be rendered: {rendered}"
);That pins the redaction directly, rather than relying on the absence of a field that could come back.
There was a problem hiding this comment.
Added the manual Debug implementation in 1748aba, rendering the stored seed as [REDACTED]. The leak test now explicitly rejects the seed and checks positive anchors for redaction and public fields.
| // create one long-running swarm per configured worker | ||
| // the per-epoch code still drives worker 0 only (#557 loops over worker components) | ||
| let workers = self.builder.tn_config.node_info.p2p_info.workers.clone(); | ||
| self.worker_network_handles = workers |
There was a problem hiding this comment.
No test covers the multi-swarm spawn path this PR introduces.
This assignment is the new behaviour — one swarm per configured worker — and nothing exercises it. worker_network_handles and worker_event_streams appear only at node.rs:95, :131, :616, :647, :655, :1061, :1063 and start_epoch.rs:475, :527, :809. spawn_node_networks has exactly one call site (node.rs:765), inside run(), which is called once at crates/node/src/lib.rs:60. No test constructs an EpochManager at all, so the loop, the zip, the u16::try_from bound and the per-worker RPC validation are all uncovered.
The gap that matters most is downstream of that: no test anywhere in the repo constructs NetworkType::Worker(1). That's the same absence that lets the shared kad record table go unnoticed — test_kad_store (kad.rs:813-984) only pins the Primary↔Worker(0) axis, which already worked. The three store-level tests I suggested on the self.consensus_db.clone() comment would cover the storage half cheaply, without needing an EpochManager harness.
test_worker_network_keypair_per_id_derivation in keys.rs is the right shape for the derivation half and it's good to have — it's the spawn and storage halves that are still open.
There was a problem hiding this comment.
Added coverage in 1748aba for the production worker-preparation helper: per-worker IDs/keys/addresses/streams, invalid RPC descriptors, stream/count mismatches, and WorkerId bounds. The storage tests now exercise Worker(0)/Worker(1) on one DB, cross-worker signature rejection, and sibling preservation during network construction. This covers preparation and construction; a full live two-swarm harness remains outside this PR.
Namespace Kademlia records and providers by role and worker ID, with ownership-aware scans, consistent counters, and fresh v2 cache tables. Validate every worker descriptor and the authoritative epoch count before network construction, then recheck the count at epoch boundaries. Give worker metrics distinct labels, redact the stored worker seed, and correct fixture identities. Cover the changes with default and Adiri regressions and confirm the new guards through deliberate mutations. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
The layered database queues disk deletions, so an immediate read can still return the persisted row. Add persistence barriers around the two worker isolation tests' deletion checks, retaining every assertion and explicitly exercising rows that have reached disk. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
grantkee
left a comment
There was a problem hiding this comment.
LGTM! I added issues #1336 #1331 and #1337 as follow-ups based on this review. Good work @MavenRain
Resolve Kademlia store conflicts while preserving worker namespaces and provider envelopes alongside database error handling and eviction throttling. Validation: pinned nightly formatting, merge integrity checks, and independent static review passed. Full dynamic validation remains with the attestation workflow. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
Closes #555.
Summary
Each configured worker gets its own persistent libp2p swarm, derived identity, handle, event stream, and discovery namespace. Worker 0 keeps its existing PeerId. This follows the merged config and LocalNetwork foundations (#554 and #556); epoch components still run for worker 0 until #557.
Grant's review changes are included here. Two worker swarms sharing a database no longer overwrite, serve, purge, or count each other's discovery records. This was an honest-node failure: sibling records have different signed worker domains, so sharing a row could cause peers to reject valid publications and ban the sender.
Changes
network="worker-0",network="worker-1", and subsequent IDs for swarm, peer-manager, and Kademlia series.Deployment notes
*_v2names. Old tables remain unused on disk; discovery data is learned again through peer connections and publication. NodeRecord wire serialization and signed domains are unchanged.network="worker"tonetwork=~"worker.*", or select a specific worker ID.Testing
The attestation run exposed two deletion-read races in the new worker isolation tests. The layered database queues disk deletion, so these tests now persist the seeded rows and wait for deletion to persist before checking absence. Every assertion is retained. Both failures were reproduced individually before the fix; afterward all 25 Kademlia tests passed under nextest with retries disabled, and each of the two formerly failing tests passed 20 repetitions. The earlier grouped libtest run had not exposed this timing issue.
The selected regression suite passed under Rust 1.94 with default features (45 tests) and with
--features tn-node/adiri,tn-config/adiri(45 tests). This includes all 25 Kademlia unit tests. Runs usedtestledger,CARGO_INCREMENTAL=0, andCARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0:cargo +1.94 test --locked -p tn-config -p tn-network-libp2p -p tn-node \ --lib -j 2 --target-dir .validation/target -- \ kad:: metrics::tests fixture_tests \ test_worker_startup_preserves_sibling_kad_records \ test_cross_worker_replay_rejected test_cross_role_replay_rejected \ debug_does_not_leak_key_material test_worker_network_keypair_per_id_derivation \ prepare_worker_networks epoch_entryEight deliberate mutation scenarios produced the expected regression failures: collapsed worker storage identities, unfiltered ownership scans, corrupt-row accounting, disabled worker preparation guards, bypassed epoch checks, leaked seed Debug output, incorrect fixture worker IDs, and collapsed metric labels. The pre-fork guard mutation was also checked under Adiri. All mutations were restored, and the final Adiri test binaries matched the passing baseline in the byte-identity ledger.
cargo +nightly-2026-03-20 fmt --all -- --checkpassed. Scoped Clippy includes all 23 packages selected bybuildplan, uses the pinned nightly and--no-deps -- -D warnings, and passed with both default features and--all-featuresusing CI's target settings.The broader Clippy check with
--all-targets --keep-goingfailed on 131 existing test lint errors. Independent review confirmed every diagnostic points outside the modified files. CI's Clippy commands do not use--all-targets; no CI settings or lint allowances were changed.The latest PR head still requires the full
make attestsuite and on-chain attestation, followed by a review request to rerun verification. Maintainer pull requests skip the GitHub compile/test lanes, so a workflow run alone does not supply that coverage.Follow-up scope
EpochManagerMulti-Worker Loop #557 adds the per-worker epoch loop; feat: Multi-Worker Support — PR 5: Execution Engine Initialization #558 covers execution initialization.