COSE signatures becomes a map (was singleton value) - #8334
COSE signatures becomes a map (was singleton value)#8334Max (maxtropets) wants to merge 9 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness hazards (notably an assertion that can fire on removal-only writes and legacy signature selection returning empty signatures) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR advances the work towards #7848 by changing the COSE signatures KV table from a singleton ServiceValue to a ServiceMap keyed by IdentityType, enabling multiple concurrent signing identities (currently still using CLASSICAL only) while preserving legacy serialisation compatibility.
Changes:
- Convert
COSE_SIGNATURESfromServiceValue<CoseSignature>toServiceMap<IdentityType, CoseSignature>, and propagate the newCoseSignatureMapthrough receipts, snapshotting, and signature caching. - Update historical query/receipt description logic to select the
CLASSICALCOSE signature for legacy v1 APIs. - Add/extend unit tests and update audit documentation to reflect the new map shape and semantics.
Custom instructions used:
/.github/copilot-instructions.md/.github/instructions/reviewing.instructions.md
File summaries
| File | Description |
|---|---|
| src/service/tables/signatures.h | Converts COSE signatures table to a map keyed by IdentityType and adds extraction helper. |
| src/service/tables/identity_types.h | Defines IdentityType and its blit serialiser to preserve legacy singleton key encoding. |
| src/node/tx_receipt_impl.h | Changes receipt storage from optional single COSE signature to a per-identity signature map. |
| src/node/signature_cache_interface.h | Updates cached signature interface to carry a CoseSignatureMap. |
| src/node/signature_cache_subsystem.h | Updates commit hooks/caching logic to collect COSE signature writes as a map. |
| src/node/snapshotter.h | Threads COSE signatures as a map through snapshot persistence flow. |
| src/node/snapshot_serdes.h | Builds snapshot receipts using the COSE signatures map. |
| src/node/history.h | Writes COSE signatures keyed by IdentityType::CLASSICAL and reads per-key previous write version. |
| src/node/historical_queries.h | Fetches COSE signatures as a map and selects CLASSICAL for legacy description paths. |
| src/node/historical_queries_adapter.cpp | Updates v1 receipt description to return CLASSICAL signature from the map. |
| src/node/rpc/node_frontend.h | Updates receipt endpoint path to read CLASSICAL COSE signature from the map. |
| src/node/node_state.h | Updates internal COSE signature reads and snapshotter hook to use extracted map writes. |
| src/endpoints/endpoint_registry.cpp | Constructs receipts using cached COSE signature maps. |
| src/node/test/snapshotter.cpp | Updates snapshotter test to record COSE signatures as a map. |
| src/node/test/history.cpp | Adds round-trip/compatibility/unit tests for the COSE signature map and extraction helper. |
| src/node/test/historical_queries.cpp | Adds tests validating cached vs historical COSE signature transaction alignment and CLASSICAL selection. |
| doc/audit/builtin_maps.rst | Updates documentation for cose_signatures key/value semantics as an identity-keyed map. |
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cjen1-msft
left a comment
There was a problem hiding this comment.
I think this is fine.
Some testing says that consumers who could try to pull in the "public:ccf.internal.cose_signatures" table will be fine as the sentinel value maps over.
And then they can be migrated later.
Similarly for JS.
The only risk is if someone pulled the signatures alias from the private headers somehow.
Users will be meant to pull those when they are filled up. They aren't now, and won't be until 8.x on AL4, but by then we'll have all the necessary things in. This step is mainly preparatory work, and should not affect KV at all (from the user's perspective). |
Co-authored-by: Amaury Chamayou <amaury@xargs.fr>
Another step towards #7848.
ServiceValue -> ServiceMap.