Skip to content

Tore/refactor/2983/rewrite storage helpers#562

Draft
jot2re wants to merge 19 commits intomainfrom
tore/refactor/2983/rewrite-storage-helpers
Draft

Tore/refactor/2983/rewrite storage helpers#562
jot2re wants to merge 19 commits intomainfrom
tore/refactor/2983/rewrite-storage-helpers

Conversation

@jot2re
Copy link
Copy Markdown
Collaborator

@jot2re jot2re commented Apr 30, 2026

Description of changes

Issue ticket number and link

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

@cla-bot cla-bot Bot added the cla-signed The CLA has been signed. label Apr 30, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

Consolidated Tests Results 2026-05-05 - 20:24:53

Test Results

passed 45 passed

Details

tests 45 tests
clock not captured
tool junit-to-ctrf
build main arrow-right test-reporter link #4481
pull-request Tore/refactor/2983/rewrite storage helpers link #562

test-reporter: Run #4481

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

🎉 All tests passed!

Tests

View All Tests
Test Name Status Flaky Duration
config_conformance_client_local_threshold 6ms
test_threshold_abort_crs_gen 561ms
test_threshold_abort_key_gen 561ms
test_threshold_custodian_backup 639ms
test_threshold_insecure_default_keygen 909ms
test_threshold_mpc_context_switch 2.1s
test_threshold_concurrent_crs 18.1s
test_threshold_mpc_context_init 4m 48s
test_threshold_restore_from_backup 2.6s
test_threshold_reshare 4m 43s
test_threshold_concurrent_preproc_keygen 5m 4s
test_threshold_default_preproc_keygen 6m 49s
test_threshold_mpc_context_switch_6 11m 46s
test_threshold_insecure 16m 25s
kms_gen_keys_binary_test::threshold_signing_key 52ms
kms_gen_keys_binary_test::threshold_signing_key_wrong_party_id 137ms
kms_gen_keys_binary_test::threshold_wrong_num_parties 139ms
kms_gen_keys_binary_test::gen_key_threshold 1.1s
kms_gen_keys_binary_test::gen_key_tempdir_threshold 1.1s
kms_server_binary_test::subcommand_dev_threshold 6.2s
test_backward_compatibility_threshold_fhe 111ms
config_conformance_client_local_centralized 6ms
test_centralized_abort_key_gen 694ms
test_centralized_abort_crs_gen 695ms
test_centralized_insecure_default_keygen 714ms
test_centralized_custodian_backup 826ms
test_centralized_crsgen_secure 991ms
test_centralized_restore_from_backup 911ms
test_centralized_insecure 42.0s
test_private_keyset_v2_upgrade_sets_missing_oprf_share_to_none 4ms
test_backward_compatibility_kms_grpc 4ms
kms_custodian_binary_tests::sunshine_verify 21ms
kms_custodian_binary_tests::sunshine_generate 21ms
kms_gen_keys_binary_test::central_signing_address_format 19ms
kms_gen_keys_binary_test::central_s3 36ms
kms_gen_keys_binary_test::central_signing_keys_overwrite 32ms
kms_gen_keys_binary_test::help 21ms
kms_init_binary_test::help 9ms
kms_server_binary_test::help 6ms
kms_custodian_binary_tests::sunshine_decrypt_custodian 348ms
kms_init_binary_test::init 561ms
kms_gen_keys_binary_test::gen_key_tempdir_centralized 736ms
kms_gen_keys_binary_test::gen_key_centralized 775ms
kms_server_binary_test::subcommand_dev_centralized 5.8s
test_backward_compatibility_kms 1.1s

🍂 No flaky tests in this run.

Github Test Reporter by CTRF 💚

🔄 This comment has been updated

"Failed to delete private type {cur_priv_type} for request {req_id} and epoch {inner_epoch}: {e}"
);
}
} else {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewer: should we maybe actually return an error here instead of continuing?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Or maybe even panicking

@@ -136,7 +136,7 @@ pub enum PublicKeyMaterialV0 {
#[versionize(PublicKeyMaterialVersioned)]
pub enum PublicKeyMaterial {
Uncompressed {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do we actually need to store the server key? It seems we are just storing it and wasting a lot of space currently.
Can we just remove it?

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.

1 participant