Introduce identity type map - #8263
Merged
Merged
Conversation
Max (maxtropets)
force-pushed
the
f/multisign-part-1
branch
from
September 2, 2026 12:09
e2b5ee1 to
615438e
Compare
Max (maxtropets)
marked this pull request as ready for review
September 2, 2026 12:12
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The persisted Identity field name differs from the linked target schema.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Introduces foundational identity types for the planned post-quantum identity migration.
Changes:
- Adds identity enums, values, and JSON serialization.
- Preserves the legacy EC384 KV key encoding.
- Adds serialization and compatibility tests.
Custom instructions used:
.github/copilot-instructions.md.github/instructions/reviewing.instructions.md
File summaries
| File | Description |
|---|---|
src/service/tables/identity_types.h |
Defines identity types and serialization. |
src/node/test/identity_types.cpp |
Tests JSON and KV serialization. |
CMakeLists.txt |
Registers the new unit test. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ES384 is the COSE algorithm the identity signs with, matching MLDSA65, which is likewise named after its signature scheme rather than its key. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Amaury Chamayou (achamayou)
approved these changes
Sep 2, 2026
IdentityType drops the parameter from its names: ES384 becomes ES, and MLDSA65 becomes MLDSA. The size cannot be part of the name because it is not a property of the slot. The curve of the service identity comes from node_certificate.curve_id (default Secp384R1, also accepts Secp256R1), which is passed straight to NetworkIdentity when a service is started or recovered. A service configured with Secp256R1 therefore signs with ES256 over SHA-256, and, since the identity is recreated during recovery from the recovering node's configuration, a service can change curve between epochs. An enum value asserting 384 would be wrong in both cases. ES and MLDSA name the signature scheme family, which is fixed for the slot; the parameters follow the key and are already recoverable from it. IdentityKind now names the encoding precisely: RawX509Cert becomes X509_CERT_DER, and RawX509Key becomes X509_SPKI_DER. The latter is not an X.509 certificate but a SubjectPublicKeyInfo, the structure produced by i2d_PUBKEY and carried in a PEM public key, so the old name suggested more than it held. Spelling out DER also makes it explicit that these are the binary encodings, not PEM. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The comment named encodings which may never be added, and says nothing about the two values which exist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ES is the JOSE and COSE prefix for ECDSA with SHA-2, and is only meaningful with a digest size attached: ES256, ES384, ES512 are registered, ES alone is not. This enum names an identity rather than a signature, so it takes the name of the scheme the identity belongs to. ECDSA pairs symmetrically with MLDSA, and neither implies a curve or parameter set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The identity in slot 0 is whatever the service was already signing with before multiple identities existed, and that is not a fixed algorithm. Its curve comes from node_certificate.curve_id, which accepts Secp384R1 and Secp256R1, and the identity is recreated from the recovering node's configuration during recovery, so it can differ between epochs. Naming the slot after an algorithm makes a claim about existing ledgers which cannot be checked and need not hold. CLASSICAL and PQ name the property the slot exists to express: whether the identity is vulnerable to a cryptographically relevant quantum computer. The concrete algorithm, curve and parameter set stay where they are already recorded, in the key itself. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…into f/multisign-part-1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opening #7848. This PR is part 1 of unknown.
Disclaimer. The new headers aren't exposed under public interface for now on purpose. It will be a job for the closing PR(s) to advertise this to the users, add the necessary changelog entries and documentation.
Adds (currently unused)
IdentityType,IdentityKind, andIdentity=IdentityKind+bytes.