Skip to content

vendor sc-network-types v0.20.3#573

Merged
illuzen merged 4 commits into
mainfrom
illuzen/vendor-sc-network-types
May 29, 2026
Merged

vendor sc-network-types v0.20.3#573
illuzen merged 4 commits into
mainfrom
illuzen/vendor-sc-network-types

Conversation

@illuzen
Copy link
Copy Markdown
Contributor

@illuzen illuzen commented May 29, 2026

Inline sc-network-types crate from polkadot-sdk to prepare for post-quantum modifications. This is a direct copy of upstream sc-network-types v0.20.3 with Cargo.toml updated to use workspace dependencies where applicable.

This prepares for replacing Ed25519 identity with Dilithium ML-DSA-87 in a follow-up PR.


Note

Medium Risk
Touches core P2P identity and address types used across the node; changes are upstream-equivalent today but expand the surface for future crypto/peer-id edits.

Overview
This PR vendors upstream sc-network-types v0.20.3 as an in-tree crate under client/network-types, instead of pulling it only from crates.io.

Workspace wiring: client/network-types is added to workspace members; sc-network-types is bumped to 0.20.3 and [patch.crates-io] points the dependency at the local path (alongside existing patches for sc-network, sc-network-sync, etc.).

What the new crate provides: Shared P2P types with conversions between libp2p, litep2p, and Substrate—PeerId, Multiaddr/Protocol, Multihash, Kademlia Record/Key, and Ed25519 key material (including zeroizing secret parsing and interoperability tests). Behavior matches upstream; the intent is a forkable copy before post-quantum identity work.

Lockfile: Transitive updates (e.g. bytes 1.11.1, litep2p 0.13.3, yamux 0.13.10) align the graph with the vendored crate’s dependencies.

Reviewed by Cursor Bugbot for commit cbe51d4. Bugbot is set up for automated code reviews on this repo. Configure here.

illuzen added 2 commits May 29, 2026 12:51
Inline sc-network-types crate from polkadot-sdk to prepare for
post-quantum modifications. This is a direct copy of upstream
sc-network-types v0.20.3 with Cargo.toml updated to use workspace
dependencies where applicable.

This prepares for replacing Ed25519 identity with Dilithium ML-DSA-87
in a follow-up PR.
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 026972d. Configure here.

Comment thread Cargo.toml
illuzen added 2 commits May 29, 2026 13:32
Inline sc-network-types crate from polkadot-sdk to prepare for
post-quantum modifications. This is a direct copy of upstream
sc-network-types v0.20.3 with Cargo.toml updated to use workspace
dependencies where applicable.

This prepares for replacing Ed25519 identity with Dilithium ML-DSA-87
in a follow-up PR.
@n13
Copy link
Copy Markdown
Collaborator

n13 commented May 29, 2026

Review: PR #573 — vendor sc-network-types v0.20.3

Verdict: Approve / low-risk. This PR does exactly what it claims — it vendors upstream sc-network-types v0.20.3 as an in-tree crate to set up the eventual Ed25519 → Dilithium ML-DSA-87 identity work. I verified it against the genuine upstream crate rather than taking the description's word for it, and it holds up well.

What I verified

1. The source is a faithful copy of upstream v0.20.3. I downloaded the real crate from crates.io and diffed it against the PR's files. The only differences in all 7 source files are rustfmt brace-style reformatting on a couple of match arms (from the fmt commit) — the logic is byte-identical:

			Ok(Code::Identity) if multihash.digest().len() <= MAX_INLINE_KEY_LENGTH =>
				Ok(PeerId { multihash }),

2. The implicit 0.20.1 → 0.20.3 bump is behaviorally a no-op. The repo was previously on 0.20.1; this vendors 0.20.3. I diffed those two upstream releases — the entire difference is rustfmt brace styling (Parity changed their fmt config between releases). Zero functional change, so the version bump is completely safe.

3. The manifest transformation is correct. The hand-written client/network-types/Cargo.toml concretizes upstream's workspace-inherited deps, and every version/feature matches upstream's published manifest exactly (litep2p 0.13.3 +rsa,websocket, serde_with 3.12.0 +hex,macros, libp2p-kad 0.46.2, etc.). All { workspace = true } deps (bytes, libp2p-identity, log, rand, serde, thiserror, zeroize) exist in the chain's [workspace.dependencies] and their pins match upstream (bytes 1.4.0, thiserror 1.0.64, zeroize 1.7.0).

4. The [patch.crates-io] resolves cleanly. In Cargo.lock, sc-network-types now has no source/checksum (resolved to the local path) and appears once — so the patch fully replaced the crates.io version with no dangerous version-split.

5. License headers preserved (GPL-3.0-or-later WITH Classpath-exception-2.0), consistent with the 3 GPL crates already vendored under client/.

6. CI confirms it compiles: Format, Clippy & Doc, and Bugbot all pass. (Build & Test matrix was still pending when I looked.)

Bugbot's finding — already fixed

Bugbot flagged a workspace-vs-crate version mismatch (0.20.1 vs 0.20.3), but it reviewed the older commit 026972d. A later commit bumped the workspace dep, so the current head correctly reads sc-network-types = { version = "0.20.3" }, matching the crate and the established pattern (sc-network-sync 0.54.0 = 0.54.0, etc.). No action needed.

Minor / optional notes (none blocking)

  • Manifest style divergence: the existing vendored crates (client/network-sync, etc.) use the crates.io auto-generated manifest with fully-pinned versions, whereas this one is hand-written with { workspace = true } inheritance. Both work (Clippy proves it), and the new style is arguably nicer for a monorepo — just flagging it so the team makes a conscious consistency choice. The one trade-off: a future workspace bump of e.g. serde/rand could silently drift from what this upstream code expects (today they match).
  • Incidental lockfile churn: beyond the required litep2p 0.13.0 → 0.13.3 bump, the PR also drifts bytes 1.10.1 → 1.11.1 graph-wide and yamux 0.13.8 → 0.13.10. Harmless (semver-compatible patch bumps), but it's unrelated cargo update noise riding along with the vendor.
  • Licensing (informational): repo LICENSE is MIT while the vendored crate is GPL-3.0+Classpath. This is standard for Substrate chains (the node already links GPL polkadot-sdk client crates) and the per-file SPDX headers are intact, so the GPL files remain correctly licensed within the repo.

Forward-looking note

The eventual PQ identity work will live in client/network-types/src/ed25519.rs. Worth remembering that identity is split across two places — the chain already patches libp2p-identity to the Quantus Dilithium fork (qp-libp2p-identity), so the Dilithium migration will need to touch both this vendored crate and that fork in concert.

Overall this is a clean, well-scoped foundation PR. Reference: PR #573.

Want me to drop any of this onto the PR as a review comment, or leave it here? (I won't post to GitHub unless you ask.)

Copy link
Copy Markdown
Collaborator

@n13 n13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@illuzen illuzen merged commit 66a2dd2 into main May 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants