Skip to content

chore: test of migration with prf key#572

Merged
kc1212 merged 4 commits intomainfrom
kc1212/chore/3010/prf-key-in-migration
May 6, 2026
Merged

chore: test of migration with prf key#572
kc1212 merged 4 commits intomainfrom
kc1212/chore/3010/prf-key-in-migration

Conversation

@kc1212
Copy link
Copy Markdown
Contributor

@kc1212 kc1212 commented May 5, 2026

Description of changes

It turned out that by adding prf keygen, we're already doing the migration implicitly. This PR adds a proper test of migration keygen.

Issue ticket number and link

Closes zama-ai/kms-internal#3010

PR Checklist

I attest that all checked items are satisfied. Any deviation is clearly justified above.

  • Title follows conventional commits (e.g. chore: ...).
  • Tests added for every new pub item and test coverage has not decreased.
  • Public APIs and non-obvious logic documented; unfinished work marked as TODO(#issue).
  • unwrap/expect/panic only in tests or for invariant bugs (documented if present).
  • No dependency version changes OR (if changed) only minimal required fixes.
  • No architectural protocol changes OR linked spec PR/issue provided.
  • No breaking deployment config changes OR devops label + infra notified + infra-team reviewer assigned.
  • No breaking gRPC / serialized data changes OR commit marked with ! and affected teams notified.
  • No modifications to existing versionized structs OR backward compatibility tests updated.
  • No critical business logic / crypto changes OR ≥2 reviewers assigned.
  • No new sensitive data fields added OR Zeroize + ZeroizeOnDrop implemented.
  • No new public storage data OR data is verifiable (signature / digest).
  • No unsafe; if unavoidable: minimal, justified, documented, and test/fuzz covered.
  • Strongly typed boundaries: typed inputs validated at the edge; no untyped values or errors cross modules.
  • Self-review completed.

Dependency Update Questionnaire (only if deps changed or added)

Answer in the Cargo.toml next to the dependency (or here if updating):

  1. Ownership changes or suspicious concentration?
  2. Low popularity?
  3. Unusual version jump?
  4. Lacking documentation?
  5. Missing CI?
  6. No security / disclosure policy?
  7. Significant size increase?

More details and explanations for the checklist and dependency updates can be found in CONTRIBUTING.md

@kc1212 kc1212 self-assigned this May 5, 2026
@kc1212 kc1212 requested a review from a team as a code owner May 5, 2026 12:25
@cla-bot cla-bot Bot added the cla-signed The CLA has been signed. label May 5, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Consolidated Tests Results 2026-05-06 - 11:09:22

Test Results

passed 12 passed

Details

tests 12 tests
clock not captured
tool junit-to-ctrf
build build-and-test arrow-right test-reporter link #1926
pull-request chore: test of migration with prf key link #572

test-reporter: Run #1926

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
12 12 0 0 0 0 0 not captured

🎉 All tests passed!

Tests

View All Tests
Test Name Status Flaky Duration
k8s_test_crs_uniqueness 32.3s
k8s_test_insecure_keygen_encrypt_and_public_decrypt 2m 8s
k8s_test_insecure_keygen_encrypt_multiple_types 2m 21s
k8s_test_keygen_and_crs 2m 3s
k8s_test_keygen_uniqueness 5m 20s
k8s_test_crs_uniqueness 34.4s
k8s_test_insecure_keygen_encrypt_and_public_decrypt 2m 30s
k8s_test_insecure_keygen_encrypt_multiple_types 2m 32s
k8s_test_keygen_and_crs 2m 12s
k8s_test_keygen_uniqueness 5m 46s
k8s_test_centralized_insecure 1m 2s
nightly_full_gen_tests_default_k8s_centralized_sequential_crs 1.9s

🍂 No flaky tests in this run.

Github Test Reporter by CTRF 💚

🔄 This comment has been updated

Comment thread core/service/src/client/tests/threshold/key_gen_tests.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds explicit slow-tests coverage for the “compressed keygen from existing shares” migration path when OPRF private-key shares may already exist or be missing (legacy material), and updates test helpers to control copy_compressed_key_to_original behavior.

Changes:

  • Split the existing “compressed keygen from existing” test into two slow-tests to cover both “OPRF present” and “OPRF missing” scenarios.
  • Add utilities to rewrite stored key material to simulate legacy fixtures missing dedicated OPRF shares and restart servers from disk material.
  • Extend keygen_config_from_existing test helper to pass through copy_compressed_key_to_original.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
core/service/src/client/tests/threshold/key_gen_tests.rs Adds two migration-focused tests plus helper routines to simulate legacy missing-OPRF material and restart servers from persisted storage.
core/service/src/client/tests/common.rs Extends the test helper to configure copy_compressed_key_to_original in KeySetAddedInfo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/service/src/client/tests/threshold/key_gen_tests.rs Outdated
Comment thread core/service/src/client/tests/threshold/key_gen_tests.rs Outdated
jot2re
jot2re previously approved these changes May 6, 2026
Copy link
Copy Markdown
Collaborator

@jot2re jot2re left a comment

Choose a reason for hiding this comment

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

LGTM besides Daniel's comment

@kc1212 kc1212 force-pushed the kc1212/chore/3010/prf-key-in-migration branch 2 times, most recently from c99ad62 to 285c740 Compare May 6, 2026 10:15
@kc1212 kc1212 force-pushed the kc1212/chore/3010/prf-key-in-migration branch from 285c740 to ca11423 Compare May 6, 2026 10:16
Copy link
Copy Markdown
Member

@dd23 dd23 left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

Copy link
Copy Markdown
Collaborator

@jot2re jot2re left a comment

Choose a reason for hiding this comment

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

LGTM

@kc1212 kc1212 merged commit 3733c60 into main May 6, 2026
64 checks passed
@kc1212 kc1212 deleted the kc1212/chore/3010/prf-key-in-migration branch May 6, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The CLA has been signed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants