feat: copy migrated key from new key ID to the old one - #530
Conversation
5f5e39e to
e08b202
Compare
Consolidated Tests Results 2026-04-30 - 09:48:55Test ResultsDetails
test-reporter: Run #4385
🎉 All tests passed!TestsView All Tests
🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
5c7fbdd to
69f8197
Compare
c4eaa32 to
3f6d2e6
Compare
3f6d2e6 to
d327b58
Compare
dvdplm
left a comment
There was a problem hiding this comment.
Still need to review core/service/src/vault/storage/crypto_material/tests/migration.rs.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in migration path for threshold key generation to copy newly generated compressed key material back onto an existing (old) key ID, so existing clients can keep using the old ID while benefiting from the compressed storage layout.
Changes:
- Introduces
copy_compressed_key_to_originalflow in threshold crypto-material storage and wires it into the threshold key generator. - Extends CLI + gRPC configuration (
KeySetAddedInfo) to request the copy-back migration behavior and documents the new flags. - Adds dedicated migration tests and refactors existing compressed-key test helpers.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/guides/core_client.md | Documents --uncompressed default behavior and migration flags including copy-back to original key ID. |
| core/service/src/vault/storage/crypto_material/threshold.rs | Adds copy_compressed_key_to_original to copy compressed pub material + re-sign and replace ThresholdFheKeys under the old ID. |
| core/service/src/vault/storage/crypto_material/tests/migration.rs | New test suite covering successful copy, overwrite, atomicity, legacy-metadata rejection, and backup-vault update. |
| core/service/src/vault/storage/crypto_material/tests.rs | Adds shared helpers used by migration tests and registers the new test module. |
| core/service/src/engine/threshold/service/key_generator.rs | Calls the new copy-back operation after successful compressed keygen-from-existing when requested. |
| core/service/src/engine/keyset_configuration.rs | Exposes copy_compressed_key_to_original() flag on InternalKeySetConfig. |
| core/service/src/client/tests/threshold/key_gen_tests.rs | Updates test config struct initialization with the new proto field. |
| core/service/src/client/tests/common.rs | Updates shared test helpers to set the new proto field. |
| core/grpc/proto/kms.v1.proto | Adds copy_compressed_key_to_original to KeySetAddedInfo (but currently renumbers an existing field). |
| core-client/src/lib.rs | Adds CLI flag --copy-compressed-key-to-original. |
| core-client/src/keygen.rs | Validates flag combinations and populates the new gRPC field when building the request. |
| .github/workflows/main.yml | Increases CI job timeout from 60 to 120 minutes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1dc1fd9 to
e6e1917
Compare
Description of changes
Implement a function that
Manual testing
Issue ticket number and link
Closes zama-ai/kms-internal#2988
PR Checklist
I attest that all checked items are satisfied. Any deviation is clearly justified above.
chore: ...).TODO(#issue).unwrap/expect/paniconly in tests or for invariant bugs (documented if present).devopslabel + infra notified + infra-team reviewer assigned.!and affected teams notified.Zeroize+ZeroizeOnDropimplemented.unsafe; if unavoidable: minimal, justified, documented, and test/fuzz covered.Dependency Update Questionnaire (only if deps changed or added)
Answer in the
Cargo.tomlnext to the dependency (or here if updating):More details and explanations for the checklist and dependency updates can be found in CONTRIBUTING.md