Litep2p clean

[illuzen]

• vendors the litep2p code
• switches the networking stack over to litep2p exclusively
• adds post-quantum support to litep2p
• turns off libp2p

The litep2p code was first copied from the official source, then modified to add dilithium and kyber support, then elliptic curve support was removed.

The idea behind litep2p and our adoption of it is to reduce the attack surface in the networking stack.

The node connects to testnet and syncs without issues because the wire protocol is identical to the libp2p stack, but we should probably run a few more tests to be sure.

---

Note

High Risk
Replaces the entire Substrate networking stack and changes peer authentication/encryption (Dilithium, pqXX Noise, PQ TLS); interoperability and security regressions need thorough testnet and adversarial validation.

Overview
This PR replaces libp2p with a vendored litep2p stack as the only P2P backend: workspace and lockfile drop libp2p / patched identity-noise crates, add a new client/litep2p crate (patched via [patch.crates-io]), and wire sc-network through Litep2pNetworkBackend while removing the old libp2p swarm/transport modules.

Node identity and handshakes move to post-quantum crypto: Dilithium ML-DSA-87 for local keys and PeerId, pqXX Noise with ML-KEM 768 (clatter), and QUIC/TLS via rustls-post-quantum / aws-lc-rs with libp2p-style certificate extensions. CLI defaults and generate-node-key / inspect-node-key now use litep2p types; --network-backend defaults to Litep2p (libp2p option removed).

Smaller lockfile/pallet dependency trims are secondary to the networking swap.

^{Reviewed by Cursor Bugbot for commit bc0f44a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit bc0f44a. Configure here.}

[n13]

v12 pls audit

[v12-auditor]

Warning

GitHub user @n13 is not connected to an V12 account. Please sign in at the dashboard and connect your GitHub account.

[n13]

Security & correctness review

Focused on the novel post-quantum code (Dilithium identity, pqXX/ML-KEM Noise, QUIC TLS) and the identity/key plumbing — not the bulk-vendored litep2p internals. Overall the Noise identity-binding design is sound (each side signs its static ML-KEM key, verified against the session's authenticated remote static before any transport socket is returned). One critical issue stands out, plus several lower-severity items.

---

🔴 CRITICAL — generate-node-key derives the node identity seed from the current timestamp

client/cli/src/commands/generate_node_key.rs

fn hash_current_time_to_hex() -> [u8; 32] {
    let timestamp = SystemTime::now().duration_since(UNIX_EPOCH)
        .expect("Time went backwards").as_millis() as u64;
    blake2_256(&timestamp.to_le_bytes())
}
// ...
let mut hashed_timestamp = hash_current_time_to_hex();
let entropy = SensitiveBytes32::from(&mut hashed_timestamp);
let keypair = Keypair::generate(entropy);

The Dilithium keypair is generated deterministically from a 32-byte seed that is just blake2_256(millisecond_timestamp). ML-DSA-87 keygen is deterministic in the seed, so the entire private identity key is determined by the generation instant.

Impact: the effective entropy is log2(plausible generation window in ms) — roughly 2^35 for a one-year window, and far less if the window is known (deployment date, testnet launch, onboarding time). Given a node's public PeerId, an attacker can enumerate candidate timestamps, run blake2_256 + ML-DSA-87 keygen, and recover the private identity key. That allows full impersonation, forged Noise/TLS identity proofs, and MITM — completely defeating the post-quantum identity guarantees regardless of how strong Dilithium/ML-KEM are.

Worse: node_key_params.rs makes authorities/validators provide a pre-generated key file (it errors with NetworkKeyNotFound otherwise), and generate-node-key is the documented way to create it — so the highest-value nodes are funneled straight into the weak generator.

Note the secure path already exists: dilithium::SecretKey::generate() and Keypair::generate() use OsRng (and the node's auto-generation via Secret::File correctly uses it). generate-node-key should do the same:

let keypair = ml_dsa_87::Keypair::generate(SensitiveBytes32::from(&mut OsRng.gen::<[u8; 32]>()));

Any keys already created with the current generate-node-key must be considered compromised and regenerated.

---

🟠 MEDIUM — "Post-quantum QUIC" authentication is still classical (ECDSA P-256)

client/litep2p/src/crypto/tls/certificate.rs, tls/verifier.rs, tls/mod.rs

• The X.509 cert keypair is PKCS_ECDSA_P256_SHA256 and verification_schemes() only offers ECDSA/RSA/Ed25519. The TLS 1.3 CertificateVerify (the signature that authenticates the live connection) is therefore classical. Only the key exchange (rustls_post_quantum::provider(), ML-KEM hybrid) and the identity-binding extension (Dilithium over the cert SPKI) are PQ.
• A quantum adversary that can break P-256 can forge the handshake signature and impersonate/MITM a peer over QUIC despite the Dilithium binding.

Mitigating fact (please confirm intent): QUIC is enabled as a Cargo feature (litep2p default ["quic","websocket"], and sc-network depends with features = ["quic","websocket"]), but the transport is never wired up — Litep2pNetworkBackend::configure_transport only calls .with_tcp(...) / .with_websocket(...) and the address parser explicitly rejects non-tcp protocols. So the TLS/ECDSA code is currently compiled-but-dead. Given the PR's stated goal ("reduce the attack surface"), consider either dropping the quic feature entirely, or don't ship it until cert-level auth is PQ. As-is it's attack surface (and a misleading "post-quantum QUIC") with no functional benefit.

---

🟡 LOW — Dilithium try_from_bytes trusts the stored public key (also flagged by Bugbot)

client/litep2p/src/crypto/dilithium.rs and client/network-types/src/dilithium.rs (both copies)

In the seed + public branch, the public key is parsed from the file and stored without checking it matches the key derived from the seed. sign() derives from the seed; public() returns the stored bytes. A corrupted/mismatched file yields a node that advertises one PeerId but signs with another key → its own handshake signatures fail to verify (self-DoS / confusing failures). Since the seed fully determines the keypair, the stored public key is redundant — simplest fix is to always derive the public key from the seed (and accept seed-only), or verify equality and error on mismatch.

---

🟡 LOW — Duplicated Dilithium key module (also flagged by Bugbot)

client/network-types/src/dilithium.rs reimplements the same Keypair/PublicKey/SecretKey logic as client/litep2p/src/crypto/dilithium.rs. They are already diverging (network-types added Hash/Ord/PartialOrd and a different DecodingError; litep2p uses Error/ParseError), and both carry the unchecked-public-key issue above — exactly the divergent-fix risk duplication creates. sc-network-types should wrap litep2p's dilithium types (it already depends on the crate and defines From conversions) instead of maintaining a parallel implementation.

---

🟡 LOW — PeerId::try_from_multiaddr is inconsistent between layers (also flagged by Bugbot)

• client/litep2p/src/peer_id.rs: address.iter().last() — only matches /p2p/ as the last component.
• client/network-types/src/peer_id.rs: address.iter().find(...) — matches /p2p/ anywhere.

The same multiaddr (e.g. /p2p/<id>/p2p-circuit, or /p2p/<id> followed by anything) resolves to a PeerId in one layer and None in the other, which can break address validation / reserved-peer / public-address handling. Align both on the same rule.

---

🔵 INFO / nits

• unsafe self-referential RNG in noise/protocol.rs (ClatterSession::ensure_handshake_initialized casts Box<StdRng> to &'static mut). It works in practice (stable heap address, field unused afterward) but is a soundness smell — two live &mut paths to the RNG exist. Prefer restructuring (e.g. owned-RNG handshake API or Pin/self-referential helper).
• Handshake security depends on clatter noise_pqxx() (v1.1.0) providing mutual static-key authentication. The identity binding is only as sound as that pattern. Worth pinning + noting the audit status of clatter/ml-kem for a consensus-critical path. (The client/litep2p crate is not under the workspace deny(expect_used/panic) lints, so the many .expect() calls on handshake/identity paths won't be caught — worth auditing those that touch remote-controlled input.)
• Silent Ed25519 stubs: PeerId::into_ed25519/from_ed25519 return None unconditionally (for sc-mixnet compile compat). If mixnet is ever enabled this silently misbehaves rather than failing loudly — fine while mixnet is off, but a latent "silent failure".

---

I'm happy to open targeted fix PRs — the critical generate-node-key fix is a one-liner and should land regardless of the rest.

_{Manual review by Cursor agent (Claude). Not a substitute for the formal audit.}

[n13]

Review complete and posted to the PR: #574 (comment)

Summary

I reviewed the 186-file PR by fetching the branch into a throwaway worktree (your working tree was untouched) and focusing on the novel security-critical code — Dilithium identity, pqXX/ML-KEM Noise, QUIC TLS, and the key/identity plumbing — rather than the bulk-vendored litep2p internals.

The headline finding is a critical key-generation flaw that the Bugbot pass missed:

• 🔴 generate-node-key seeds the Dilithium identity from blake2_256(millisecond_timestamp) (client/cli/src/commands/generate_node_key.rs). ML-DSA-87 keygen is deterministic in the seed, so the private identity key has only ~timestamp-worth of entropy (~2^35 for a 1-year window, far less if the time is known). Given the public PeerId, an attacker can brute-force the timestamp and recover the private key → full impersonation/MITM, defeating the PQ guarantees. Worse, validators are funneled into this exact tool (authorities must pre-generate a key file). The secure OsRng path already exists and is used by the node's auto-generation — the CLI just needs to use it. Any keys made with it should be regenerated.

Plus, with severity calibrated against actual reachability:

• 🟠 Medium — "Post-quantum QUIC" is only PQ for key-exchange + identity binding; the TLS cert/CertificateVerify is classical ECDSA P-256. It's currently dead code (QUIC is compiled in but never wired into the transport — only TCP/WS are), so I framed it as latent and recommended dropping the quic feature to actually reduce attack surface as the PR intends.
• 🟡 Low ×3 — validated and contextualized Bugbot's three findings: unchecked stored public key in try_from_bytes, the duplicated Dilithium module (already diverging), and the try_from_multiaddr last() vs find() inconsistency.
• 🔵 Info — unsafe self-referential RNG in the Noise wrapper, the dependence on clatter's pqXX for mutual auth (and that client/litep2p is exempt from the workspace deny(expect_used/panic) lints), and the silent Ed25519 None stubs.

I confirmed the good parts too: the Noise handshake verifies the remote's Dilithium signature over its authenticated static ML-KEM key before returning a transport socket, secret files are written 0o600, and the node's automatic key generation correctly uses OsRng.

One note in line with your "don't comment on GitHub automatically" rule — I only posted because you explicitly asked. I did not push any code or open fix PRs; happy to do the one-line generate-node-key fix (and the others) if you'd like.

[n13]

Review complete. I fetched PR #574 into a throwaway worktree (your working tree on main was untouched), verified the follow-up commits against the earlier audit findings, and focused on the novel security-critical code plus the key plumbing.

Verdict

The follow-up commits resolved the earlier critical and most lower-severity findings — nice work. However, those same fixes introduced one new release-blocker: the generate-node-key / inspect-node-key CLI now writes/reads a key format the node itself can no longer load. CI is green only because no test crosses that boundary.

What got fixed since the last review (great progress)

• Timestamp-seed key generation (was CRITICAL) — fixed. generate_key now uses OsRng.fill_bytes(&mut seed).

	let mut seed = [0u8; 32];
	rand::rngs::OsRng.fill_bytes(&mut seed);
	let entropy = SensitiveBytes32::from(&mut seed);
	let keypair = Keypair::generate(entropy);

• "Post-quantum QUIC" with classical ECDSA (was MEDIUM) — resolved by removing QUIC/TLS entirely. The crypto module is now just dilithium + noise, and sc-network pulls litep2p with only features = ["websocket"].
• Unchecked stored public key (was LOW) — fixed. Keypair stores only the 32-byte seed and derives the public key on demand, so advertised PeerId and signing key can't diverge.
• Duplicated Dilithium module (was LOW) — fixed. client/network-types/src/dilithium.rs is gone; network-types now wraps litep2p::crypto::dilithium.
• try_from_multiaddr inconsistency (was LOW) — fixed. Both litep2p and network-types now match /p2p/ only as the last component via .iter().last().
• Noise identity binding — confirmed still sound: each side signs STATIC_KEY_DOMAIN || its ML-KEM static pubkey, and parse_and_verify_peer_id verifies the remote's Dilithium signature against the session-authenticated get_remote_static() before a NoiseSocket is returned.

New CRITICAL — generate-node-key output cannot be loaded by the node

The seed-only serialization cleanup updated the runtime loader but not the CLI key tooling, so they now speak different formats.

• The CLI generates via the raw qp_rusty_crystals_dilithium crate and writes ml_dsa_87::Keypair::to_bytes():

	let dilithium_pk = DilithiumPublicKey::try_from_bytes(&keypair.public.to_bytes())
		.expect("Valid Dilithium public key");
	let public_key = PublicKey::from(dilithium_pk);
	let peer_id = litep2p::PeerId::from_public_key(&public_key);

That to_bytes() is SECRETKEYBYTES + PUBLICKEYBYTES = 4896 + 2592 = 7488 bytes (the expanded secret+public key).

• The node loads the key (client/network/src/litep2p/mod.rs:264 → into_litep2p_keypair) via the litep2p keypair, which now requires exactly 32 bytes (the generation seed):

			Dilithium(Secret::File(f)) => get_secret(
				f,
				|b| {
					let mut bytes = if is_hex_data(b) { /* hex-decode */ } else { b.to_vec() };
					litep2p::crypto::dilithium::Keypair::try_from_bytes(&mut bytes)
						.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, format!("{e:?}")))
				},

	pub fn try_from_bytes(kp: &mut [u8]) -> Result<Keypair, Error> {
		let seed: [u8; SEED_BYTES] = kp.try_into().map_err(|_| {
			Error::Other(format!(
				"Invalid Dilithium seed length: expected {} bytes, got {}",
				SEED_BYTES, kp.len()
			))
		})?;

So a node started with a generate-node-key file fails immediately with InvalidData: "Invalid Dilithium seed length: expected 32 bytes, got 7488". The two formats aren't even reconcilable by truncation — the first 32 bytes of the expanded secret key are rho (public), not the generation seed xi, so the seed is genuinely absent from the file.

Why it's high impact: authorities/validators are forced to pre-generate a key file (otherwise node_key_params.rs returns NetworkKeyNotFound), and generate-node-key is the documented tool to do it — exactly mirroring the previous critical finding's "highest-value nodes funneled into the broken path." Dev nodes that auto-generate (Secret::File missing → writes a 32-byte seed) work fine, which is why this hides easily. inspect-node-key reads the 7488-byte format too, so generate→inspect round-trips and masks the problem further.

Why CI didn't catch it: the tests are siloed — generate_node_key/inspect_node_key tests round-trip within the ml_dsa_87 format, while node_key_params/config tests write a litep2p 32-byte seed directly. Nothing exercises generate-node-key → into_litep2p_keypair.

Suggested fix (also DRYs up the CLI): have generate-node-key and inspect-node-key use litep2p::crypto::dilithium::Keypair like the rest of the node (write/read Keypair::to_bytes() = 32-byte seed) instead of the raw ml_dsa_87 crate, and add a test that feeds the CLI output through into_litep2p_keypair.

Lower-severity / carried-over

• LOW — old on-disk keys won't load. try_from_bytes now requires exactly 32 bytes, so any key written by an earlier commit of this branch (the interim 2624-byte seed+public format) fails to load. Acceptable for a testnet reset, but worth a one-line note in docs/ / release notes that node keys must be regenerated.
• INFO — unsafe self-referential RNG still present in ClatterSession::ensure_handshake_initialized (Box<StdRng> cast to &'static mut, client/litep2p/src/crypto/noise/protocol.rs:91-98). Works in practice but remains a soundness smell; an owned-RNG handshake API would remove the unsafe.
• INFO — client/litep2p is exempt from the workspace deny(expect_used/panic) lints, and identity/handshake paths use .expect() (e.g. sign() "Signing should not fail"). Remote-controlled input paths (from_protobuf_encoding → try_from_bytes → verify) are panic-free, which is the important part, but the local .expect()s are worth an audit pass.
• INFO — silent Ed25519 stubs (PeerId::into_ed25519/from_ed25519 return None) — fine while sc-mixnet is off, but a latent silent-failure if mixnet is ever enabled (slightly at odds with your "fail loudly" rule).

If you'd like, I can draft the one-file fix for generate-node-key/inspect-node-key (switch to litep2p's dilithium::Keypair + add the cross-boundary test) so the validator key workflow actually round-trips.

[n13]

Re-reviewed the 4 new commits (edd2b719 → 4d06dd3b) in a fresh throwaway worktree. Great news: my critical finding and all the carried-over items are addressed, and CI is green on the new head.

Fixes verified

CRITICAL — generate-node-key key format (now resolved). d540c005 does exactly the right thing: both generate-node-key and inspect-node-key now use litep2p::crypto::dilithium::Keypair instead of the raw ml_dsa_87 crate, so they read/write the 32-byte seed the node's loader expects. It also DRYs up the CLI (no more parallel key handling) and adds the precise cross-boundary test I recommended:

	fn generate_key_compatible_with_node_loader() {
		// Generate a key using the CLI
		let cmd = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", path_str]);
		assert!(cmd.run("test", &String::from("test")).is_ok());
		// This is the same call the node makes
		let keypair = Keypair::try_from_bytes(&mut bytes)
			.expect("Generated key should be loadable by the node");

The generate test now also asserts the file is exactly 32 bytes. This closes the test gap that hid the original bug.

sign().expect() panic (now resolved). 4d06dd3b makes Keypair::sign return Result<Vec<u8>, SignError>, adds a NegotiationError::SigningFailed variant (+ a "signing-failed" metrics label), and all four call sites propagate it (Noise identity-payload signing, signature.rs, litep2p/service.rs). Fully fail-loud now.

Silent Ed25519 stubs (now resolved). 9a35b539 turns into_ed25519/from_ed25519 into loud panics with actionable messages. I confirmed there are no runtime callers of these in client/ or node/, so the panics aren't reachable while mixnet is off — purely a guard against future misuse.

One small new item (LOW)

The RNG refactor (8f98aecf) is cleaner overall (eager init, no more Option/ensure_* plumbing), but its own safety contract is now self-contradictory:

/// 4. Struct fields drop in declaration order: `handshake` drops before `_rng`
pub struct ClatterSession {
	_rng: Box<rand::rngs::StdRng>,
	handshake: PqHandshake<'static, MlKem768, MlKem768, ChaChaPoly, Sha256, rand::rngs::StdRng>,
}

Rust drops fields in declaration order, so _rng (declared first) actually drops before handshake — the opposite of what comment #4 claims. It isn't UB today (dropping handshake's &'static mut reference is inert, and PqHandshake has no Drop that touches the RNG), but the documented invariant the unsafe relies on is inverted, which is fragile in consensus-critical crypto.

Trivial fix: declare handshake before _rng so the drop order genuinely matches the comment (borrower dropped before owner). If they'd rather drop the unsafe entirely, self_cell/ouroboros would express this self-referential pattern safely — but the field reorder is a one-liner and sufficient.

Status

• Critical CLI key-loading regression: fixed and tested.
• Signing panic, Ed25519 silent stubs: fixed (fail-loud).
• Earlier fixes (OsRng seed, QUIC/TLS removal, dedup, multiaddr alignment, seed-only keypair) still in place.
• Noise identity binding remains sound.
• CI: all checks passing.

The only thing I'd still nudge before merge is the one-line field reorder in ClatterSession (or a note acknowledging it). Want me to draft that change?

[n13]

Push confirmed and everything lines up. Final pass below.

Sync state

• Local HEAD = remote origin/illuzen/pq-litep2p-clean = 296e9721 — fully in sync, no ahead/behind. The "not synced" worry is gone now that you've pushed.
• PR head is 296e9721; the only delta vs the prior reviewed head is the ClatterSession field-order fix — nothing else snuck in.
• mergeable: MERGEABLE (no conflicts with main).

CI: freshly re-triggered by the push and currently running (config passed, format pending; the build/test matrix takes ~40 min). The fix is purely a struct field reorder + comments, which can't change compilation, and the parent commit 4d06dd3b was fully green — so I expect it to pass. Happy to watch it to completion and report back if you'd like.

Final review verdict — all blocking items resolved:

Finding (severity)	Status
generate-node-key writes a key the node can't load (CRITICAL)	Fixed — both CLI cmds use litep2p's 32-byte-seed Keypair; cross-boundary test added
Timestamp-derived key seed (CRITICAL, earlier)	Fixed — OsRng
sign() panics via .expect() (INFO)	Fixed — returns Result, all call sites propagate
Silent Ed25519 stubs (INFO)	Fixed — fail loudly; no runtime callers
ClatterSession drop-order vs safety contract (LOW)	Fixed — your 296e9721
"PQ QUIC" was classical ECDSA (MEDIUM, earlier)	Fixed — QUIC/TLS removed
Duplicate Dilithium module / multiaddr mismatch (LOW, earlier)	Fixed
Noise identity binding	Verified sound (Dilithium sig over authenticated remote static, before transport)

Two non-blocking notes for later (not gating this PR):

• Pin the exact clatter / ml-kem versions and note their audit status — the handshake's mutual authentication rests on clatter's noise_pqxx, and this is now a consensus-critical path.
• Eventually bring client/litep2p under the workspace deny(expect_used/panic) lints. The remote-controlled input paths are already panic-free, so this is hygiene, not a hole.

Remaining merge gate: reviewDecision: REVIEW_REQUIRED — it still needs an approving review (you're the requested reviewer). I won't approve or comment on GitHub for you, per your rules.

So: code-wise, I'm comfortable signing off — this is in good shape. The only thing left is CI going green and your approval. Want me to monitor CI and ping you when it finishes?

[n13]

GTG