diff --git a/Makefile b/Makefile index 44f92eadf7..38e62e2b75 100644 --- a/Makefile +++ b/Makefile @@ -33,37 +33,38 @@ test-backward-compatibility: pull-lfs-files test-backward-compatibility-local: cargo test --test backward_compatibility_* -- --include-ignored --no-capture +# Frozen — generators may be non-deterministic. Their data is committed and +# must NOT be deleted or regenerated by `generate-backward-compatibility-all`. +FROZEN_BWC_VERSIONS := 0.11.0 0.11.1 0.13.0 0.13.10 0.13.20 + +# Deterministic — re-running produces byte-identical output. +# `generate-backward-compatibility-all` cleans and regenerates these. +DETERMINISTIC_BWC_VERSIONS := 0.14.0 + +# Derived data-dir paths (e.g. 0.14.0 -> backward-compatibility/data/0_14_0) +DETERMINISTIC_BWC_DIRS := $(addprefix backward-compatibility/data/,$(subst .,_,$(DETERMINISTIC_BWC_VERSIONS))) + clean-backward-compatibility-data: - rm -f backward-compatibility/data/kms.ron - rm -f backward-compatibility/data/kms-grpc.ron - rm -f backward-compatibility/data/threshold-fhe.ron - rm -rf backward-compatibility/data/0_11_0 - rm -rf backward-compatibility/data/0_11_1 - rm -rf backward-compatibility/data/0_13_0 - rm -rf backward-compatibility/data/0_13_10 - rm -rf backward-compatibility/data/0_13_20 - rm -rf backward-compatibility/data/0_14_0 - -generate-backward-compatibility-v0.11.0: - cd backward-compatibility/generate-v0.11.0 && cargo run --release - -generate-backward-compatibility-v0.11.1: - cd backward-compatibility/generate-v0.11.1 && cargo run --release + @# Only deterministic-version dirs are removed. Frozen dirs and the shared + @# .ron files are preserved so committed frozen entries survive. + @if [ -n "$(DETERMINISTIC_BWC_DIRS)" ]; then rm -rf $(DETERMINISTIC_BWC_DIRS); fi +# Use the version-specific generators only if you really need it, +# e.g., when a type is added in a minor release. generate-backward-compatibility-v0.13.0: cd backward-compatibility/generate-v0.13.0 && cargo run --release generate-backward-compatibility-v0.13.10: cd backward-compatibility/generate-v0.13.10 && cargo run --release - + generate-backward-compatibility-v0.13.20: cd backward-compatibility/generate-v0.13.20 && cargo run --release generate-backward-compatibility-v0.14.0: cd backward-compatibility/generate-v0.14.0 && cargo run --release -generate-backward-compatibility-all: clean-backward-compatibility-data generate-backward-compatibility-v0.11.0 generate-backward-compatibility-v0.11.1 generate-backward-compatibility-v0.13.0 generate-backward-compatibility-v0.13.10 generate-backward-compatibility-v0.13.20 generate-backward-compatibility-v0.14.0 - @echo "Generated backward compatibility data for all versions" +generate-backward-compatibility-all: clean-backward-compatibility-data $(addprefix generate-backward-compatibility-v,$(DETERMINISTIC_BWC_VERSIONS)) + @echo "Generated backward compatibility data for deterministic versions: $(DETERMINISTIC_BWC_VERSIONS)" # Test material generation targets generate-test-material-all: diff --git a/ai-docs/COMMANDS.md b/ai-docs/COMMANDS.md index f3f5dbc3b5..c9b554ed98 100644 --- a/ai-docs/COMMANDS.md +++ b/ai-docs/COMMANDS.md @@ -80,24 +80,26 @@ Direct cargo invocation (what the make targets call under the hood): cargo test --test 'backward_compatibility_*' -- --include-ignored ``` -Regenerate vectors for all historical versions (cleans first, then runs each generator): +Regenerate vectors. Versions are split into two lists in the `Makefile`: + +- `FROZEN_BWC_VERSIONS` — currently `0.11.0`, `0.11.1`, `0.13.0`, `0.13.10`, `0.13.20`. Generators were non-deterministic across runs, so the committed `.bcode` files and `.ron` entries are the source of truth and must not be regenerated as part of normal workflow. +- `DETERMINISTIC_BWC_VERSIONS` — `0.14.0` and future versions. Re-running produces byte-identical output. + +Regenerate all deterministic versions (cleans only deterministic data dirs first, then runs their generators; frozen versions are left untouched): ``` make generate-backward-compatibility-all ``` -Regenerate for a single version. **Warning**: removes other versions' entries from the `.ron` files — do not commit the result: +Regenerate for a single deterministic version: ``` -make generate-backward-compatibility-v0.11.0 -make generate-backward-compatibility-v0.11.1 -make generate-backward-compatibility-v0.13.0 -make generate-backward-compatibility-v0.13.10 -make generate-backward-compatibility-v0.13.20 make generate-backward-compatibility-v0.14.0 ``` -Clean all generated BC data: +Per-version targets also exist for the frozen versions `v0.13.0`, `v0.13.10`, and `v0.13.20` (no targets for `v0.11.0` / `v0.11.1` — their generator crates are kept only for historical inspection). These frozen-version targets are for exceptional investigation only; running them can produce non-deterministic bytes and append duplicate metadata to the shared `.ron` files, so their output must not be committed. + +Remove generated BC data for deterministic versions only — frozen data dirs and all shared `.ron` files are preserved: ``` make clean-backward-compatibility-data diff --git a/backward-compatibility/ADDING_NEW_VERSIONS.md b/backward-compatibility/ADDING_NEW_VERSIONS.md index 6452f8c9df..40be91cd9d 100644 --- a/backward-compatibility/ADDING_NEW_VERSIONS.md +++ b/backward-compatibility/ADDING_NEW_VERSIONS.md @@ -2,7 +2,7 @@ ## Quick Start -When adding a new KMS version (e.g., v0.12.0), follow these steps: +When adding a new KMS version (e.g., v0.14.0), follow these steps: ### Step 1: Check Dependency Compatibility @@ -10,10 +10,10 @@ Before adding a new version, verify it's compatible with existing generator vers ```bash # Check key dependencies in the new release -curl -s "https://raw.githubusercontent.com/zama-ai/kms/v0.12.0/Cargo.toml" | grep -E "serde|alloy|tfhe" +curl -s "https://raw.githubusercontent.com/zama-ai/kms/v0.14.0/Cargo.toml" | grep -E "serde|alloy|tfhe" -# Compare with the most recent generator (v0.11.1) -grep -E "serde|alloy|tfhe" backward-compatibility/generate-v0.11.1/Cargo.toml +# Compare with the most recent deterministic generator (currently v0.14.0) +grep -E "serde|alloy|tfhe" backward-compatibility/generate-v0.14.0/Cargo.toml ``` **Key dependencies to check:** @@ -24,20 +24,22 @@ grep -E "serde|alloy|tfhe" backward-compatibility/generate-v0.11.1/Cargo.toml ### Step 2A: If Dependencies Are Compatible -Add the version to the most recent compatible generator (e.g., `generate-v0.11.1`): +Add the version to the most recent compatible deterministic generator (e.g., `generate-v0.14.0`): -1. **Add dependencies** to `backward-compatibility/generate-v0.11.1/Cargo.toml`: +1. **Add dependencies** to `backward-compatibility/generate-v0.14.0/Cargo.toml`: ```toml -kms_0_12_0 = { git = "https://github.com/zama-ai/kms.git", package = "kms", rev = "v0.12.0" } -kms_grpc_0_12_0 = { git = "https://github.com/zama-ai/kms.git", package = "kms-grpc", rev = "v0.12.0" } -threshold_fhe_0_12_0 = { git = "https://github.com/zama-ai/kms.git", package = "threshold-fhe", rev = "v0.12.0", features = ["testing"] } +kms_0_14_0 = { git = "https://github.com/zama-ai/kms.git", package = "kms", rev = "v0.14.0" } +kms_grpc_0_14_0 = { git = "https://github.com/zama-ai/kms.git", package = "kms-grpc", rev = "v0.14.0" } +threshold_fhe_0_14_0 = { git = "https://github.com/zama-ai/kms.git", package = "threshold-fhe", rev = "v0.14.0", features = ["testing"] } ``` -2. **Create** `backward-compatibility/generate-v0.11.1/src/data_0_12.rs` implementing `KMSCoreVersion` trait +2. **Create** `backward-compatibility/generate-v0.14.0/src/data_0_14.rs` implementing `KMSCoreVersion` trait -3. **Update** `backward-compatibility/generate-v0.11.1/src/main.rs` to generate data for v0.12.0 +3. **Update** `backward-compatibility/generate-v0.14.0/src/main.rs` to generate data for v0.14.0 -4. **Update** root `Makefile` to add v0.12.0 target (optional, for individual generation) +4. **Update** root `Makefile`: + - Add v0.14.0 to `DETERMINISTIC_BWC_VERSIONS` + - Add a `generate-backward-compatibility-v0.14.0` target if this version has its own generator crate 5. **Generate the data**: ```bash @@ -50,27 +52,29 @@ make generate-backward-compatibility-all make test-backward-compatibility-local ``` -⚠️ **Important**: Always use `make generate-backward-compatibility-all` to ensure all versions are regenerated with merged metadata! +⚠️ **Important**: Always use `make generate-backward-compatibility-all` for normal regeneration. It skips frozen versions and refreshes only the deterministic versions listed in `DETERMINISTIC_BWC_VERSIONS`. ### Step 2B: If Dependencies Are Incompatible Create a new generator crate for the incompatible version: -1. **Copy the most recent generator**: +1. **Copy the closest previous generator**: ```bash -cp -r backward-compatibility/generate-v0.11.1 backward-compatibility/generate-v0.12.0 +cp -r backward-compatibility/generate-v0.13.20 backward-compatibility/generate-v0.14.0 ``` -2. **Update** `backward-compatibility/generate-v0.12.0/Cargo.toml`: - - Package name: `backward-compatibility-generate-v0-12-0` (use dashes, include patch version) - - Package version: `0.12.0` (matches the KMS version exactly) - - Update all KMS dependencies to v0.12.0 - - Update dependency versions (serde, alloy, tfhe) to match v0.12.0's requirements +2. **Update** `backward-compatibility/generate-v0.14.0/Cargo.toml`: + - Package name: `backward-compatibility-generate-v0-14-0` (use dashes, include patch version) + - Package version: `0.14.0` (matches the KMS version exactly) + - Update all KMS dependencies to v0.14.0 (or to the release commit if the tag is not available yet) + - Update dependency versions (serde, alloy, tfhe) to match v0.14.0's requirements -3. **Update** `backward-compatibility/generate-v0.12.0/src/data_0_11.rs`: - - Change imports from `kms_0_11_1` to `kms_0_12_0` (or rename file to `data_0_12.rs`) - - Update `VERSION_NUMBER` to `"0.12.0"` - - Fix any API changes between v0.11.1 and v0.12.0 +3. **Update** the copied source files: + - Rename `backward-compatibility/generate-v0.14.0/src/data_0_13.rs` to `data_0_14.rs` + - Change imports from `kms_0_13_20`, `kms_grpc_0_13_20`, and sibling aliases to their `0_14_0` names + - Update `VERSION_NUMBER` to `"0.14.0"` + - Update `src/lib.rs` and `src/main.rs` module names and imports to use `data_0_14` + - Fix any API changes between v0.13.20 and v0.14.0 4. **Update** root `Cargo.toml` to exclude the new generator: ```toml @@ -78,31 +82,23 @@ exclude = [ "backward-compatibility", "backward-compatibility/generate-v0.11.0", "backward-compatibility/generate-v0.11.1", - "backward-compatibility/generate-v0.12.0" # Add this + "backward-compatibility/generate-v0.13.0", + "backward-compatibility/generate-v0.13.10", + "backward-compatibility/generate-v0.13.20", + "backward-compatibility/generate-v0.14.0", # Add this ] ``` 5. **Update** root `Makefile`: ```makefile -generate-backward-compatibility-v0.12.0: - cd backward-compatibility/generate-v0.12.0 && cargo run --release - -generate-backward-compatibility-all: clean-backward-compatibility-data \ - generate-backward-compatibility-v0.11.0 \ - generate-backward-compatibility-v0.11.1 \ - generate-backward-compatibility-v0.12.0 # Add this - @echo "✅ Generated backward compatibility data for all versions" -``` +DETERMINISTIC_BWC_VERSIONS := 0.14.0 -6. **Update** `clean-backward-compatibility-data` target in Makefile: -```makefile -clean-backward-compatibility-data: - rm -f backward-compatibility/data/*.ron - rm -rf backward-compatibility/data/0_11_0 - rm -rf backward-compatibility/data/0_11_1 - rm -rf backward-compatibility/data/0_12_0 # Add this +generate-backward-compatibility-v0.14.0: + cd backward-compatibility/generate-v0.14.0 && cargo run --release ``` +6. **Do not add the new version to `FROZEN_BWC_VERSIONS`** unless the generator is known to be non-deterministic and the generated data is intentionally frozen. `clean-backward-compatibility-data` derives deterministic data directories from `DETERMINISTIC_BWC_VERSIONS`. + 7. **Test the new generator**: ```bash make generate-backward-compatibility-all @@ -115,12 +111,12 @@ make test-backward-compatibility-local | Generator Crate | Package Name | KMS Versions | Key Dependencies | Status | |----------------|--------------|--------------|------------------|--------| -| `generate-v0.11.0` | `backward-compatibility-generate-v0-11-0` | v0.11.0 | serde 1.0.219, alloy 1.1.2, tfhe 1.3.2 | ✅ Active | -| `generate-v0.11.1` | `backward-compatibility-generate-v0-11-1` | v0.11.1 | serde 1.0.226, alloy 1.3.1, tfhe 1.3.3 | ✅ Active | -| `generate-v0.13.0` | `backward-compatibility-generate-v0-13-0` | v0.13.0 | — | ✅ Active | -| `generate-v0.13.10` | `backward-compatibility-generate-v0-13-10` | v0.13.10 | — | ✅ Active | -| `generate-v0.13.20` | `backward-compatibility-generate-v0-13-20` | v0.13.20 | — | ✅ Active | -| `generate-v0.14.0` | `backward-compatibility-generate-v0-14-0` | v0.14.0 | tfhe-versionable 0.7.0, tfhe 1.6.1, alloy 1.4.1, serde 1.0.228 | ✅ Active | +| `generate-v0.11.0` | `backward-compatibility-generate-v0-11-0` | v0.11.0 | serde 1.0.219, alloy 1.1.2, tfhe 1.3.2 | Frozen | +| `generate-v0.11.1` | `backward-compatibility-generate-v0-11-1` | v0.11.1 | serde 1.0.226, alloy 1.3.1, tfhe 1.3.3 | Frozen | +| `generate-v0.13.0` | `backward-compatibility-generate-v0-13-0` | v0.13.0 | — | Frozen | +| `generate-v0.13.10` | `backward-compatibility-generate-v0-13-10` | v0.13.10 | — | Frozen | +| `generate-v0.13.20` | `backward-compatibility-generate-v0-13-20` | v0.13.20 | — | Frozen | +| `generate-v0.14.0` | `backward-compatibility-generate-v0-14-0` | v0.14.0 | tfhe-versionable 0.7.0, tfhe 1.6.1, alloy 1.4.1, serde 1.0.228 | Deterministic | **Note**: v0.11.0 and v0.11.1 require separate generators due to incompatible alloy and tfhe versions. @@ -137,7 +133,7 @@ Create a new generator crate when: Before committing, verify the generator builds: ```bash -cd backward-compatibility/generate-v0.11 +cd backward-compatibility/generate-v0.14.0 cargo check cargo build --release ``` @@ -159,17 +155,16 @@ make generate-backward-compatibility-all ``` This will: -1. Clean old data files -2. Run each generator in sequence (v0.11.0, v0.11.1, etc.) -3. Merge metadata from all versions into combined `.ron` files +1. Clean deterministic-version data directories +2. Run the deterministic generators listed in `DETERMINISTIC_BWC_VERSIONS` +3. Replace metadata entries for regenerated versions while preserving frozen entries Or run individual generators: ```bash -make generate-backward-compatibility-v0.11.0 -make generate-backward-compatibility-v0.11.1 +make generate-backward-compatibility-v0.14.0 ``` -⚠️ **Important**: When running individual generators, they will append to existing metadata. Always clean first with `make clean-backward-compatibility-data` to avoid duplicates. +⚠️ **Important**: Frozen generator crates can still be run directly for historical investigation, but their output may be non-deterministic and should not be committed. ## Best Practices @@ -180,7 +175,7 @@ make generate-backward-compatibility-v0.11.1 5. **Test after adding** - verify both generation and loading work 6. **Update this document** - keep the compatibility matrix current 7. **Version the generator crate** - set the crate version to match the KMS version exactly (e.g., `generate-v0.11.0` should be version `0.11.0`) -8. **Use `make generate-backward-compatibility-all`** - ensures proper metadata merging +8. **Use `make generate-backward-compatibility-all`** - refreshes deterministic data while preserving frozen entries ## Troubleshooting diff --git a/backward-compatibility/generate-v0.14.0/src/generate.rs b/backward-compatibility/generate-v0.14.0/src/generate.rs index 492e83fd80..a5dc48a05d 100644 --- a/backward-compatibility/generate-v0.14.0/src/generate.rs +++ b/backward-compatibility/generate-v0.14.0/src/generate.rs @@ -2,6 +2,7 @@ use std::{ borrow::Cow, + collections::HashSet, fs, path::{Path, PathBuf}, }; @@ -15,7 +16,7 @@ use backward_compatibility::{ ClassicPBSParametersTest, DKGParamsRegularTest, DKGParamsSnSTest, SwitchAndSquashCompressionParametersTest, SwitchAndSquashParametersTest, }, - TestMetadataDD, TestMetadataKMS, TestMetadataKmsGrpc, + Testcase, TestMetadataDD, TestMetadataKMS, TestMetadataKmsGrpc, }; // Parameters set for tests in kms-core 0.9, found in `PARAMS_TEST_BK_SNS`. However, for stability @@ -115,9 +116,9 @@ macro_rules! define_store_versioned_auxiliary_fn { } define_store_versioned_auxiliary_fn!(store_versioned_auxiliary_05, Versionize_0_7); -pub fn store_metadata(new_data: &Vec, path: P) +pub fn store_metadata(new_data: &Vec>, path: P) where - Meta: Serialize + serde::de::DeserializeOwned + Clone, + T: Serialize + serde::de::DeserializeOwned + Clone, P: AsRef, { let path = path.as_ref(); @@ -130,15 +131,25 @@ where return; } - // Load existing metadata + // Replace any existing entries for the versions being regenerated; leave + // every other version (notably the frozen ones) untouched. Without this, + // re-running the generator would append duplicate rows for the same + // version on every invocation. + let versions_being_written: HashSet<&str> = new_data + .iter() + .map(|t| t.kms_core_version_min.as_str()) + .collect(); + let existing_content = fs::read_to_string(path).unwrap(); - let mut combined_data: Vec = + let existing: Vec> = ron::from_str(&existing_content).expect("Failed to deserialize existing metadata"); - // Append new entries + let mut combined_data: Vec> = existing + .into_iter() + .filter(|t| !versions_being_written.contains(t.kms_core_version_min.as_str())) + .collect(); combined_data.extend_from_slice(new_data); - // Write combined data let serialized = ron::ser::to_string_pretty(&combined_data, ron::ser::PrettyConfig::default()).unwrap(); fs::write(path, serialized).unwrap(); diff --git a/core/service/src/backup/custodian.rs b/core/service/src/backup/custodian.rs index 8ca2b026c7..6956bb39a9 100644 --- a/core/service/src/backup/custodian.rs +++ b/core/service/src/backup/custodian.rs @@ -376,14 +376,22 @@ impl Custodian { }) } - // We allow the following lints because we are fine with mutating the rng even if - // we end up returning an error when signing the encrypted share. - #[allow(unknown_lints)] - #[allow(non_local_effect_before_error_return)] pub fn generate_setup_message( &self, rng: &mut R, custodian_name: String, // This is the human readable name of the custodian to be used in the setup message + ) -> Result { + let timestamp = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); + self.generate_setup_message_with_timestamp(rng, custodian_name, timestamp) + } + + // The timestamp is taken as an explicit argument so that callers needing deterministic + // output (e.g. backward-compatibility data generators) can pass a fixed value. + pub fn generate_setup_message_with_timestamp( + &self, + rng: &mut R, + custodian_name: String, + timestamp: u64, ) -> Result { let mut random_value = [0u8; 32]; rng.fill_bytes(&mut random_value); @@ -392,7 +400,7 @@ impl Custodian { header: HEADER.to_string(), custodian_role: self.role, random_value, - timestamp: SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(), + timestamp, public_enc_key: self.enc_key.clone(), public_verf_key: self.verification_key().clone(), name: custodian_name, diff --git a/core/service/src/engine/base.rs b/core/service/src/engine/base.rs index 42333d6cbc..90b26983d7 100644 --- a/core/service/src/engine/base.rs +++ b/core/service/src/engine/base.rs @@ -34,6 +34,7 @@ use kms_grpc::solidity_types::{ use kms_grpc::utils::tonic_result::BoxedStatus; use rand::{RngCore, SeedableRng}; use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::str::FromStr; @@ -345,7 +346,7 @@ pub(crate) fn compute_info_standard_keygen_from_digests( Ok(KeyGenMetadata::new( *key_id, *prep_id, - HashMap::from([ + BTreeMap::from([ (PubDataType::ServerKey, server_key_digest), (PubDataType::PublicKey, public_key_digest), ]), @@ -374,7 +375,7 @@ pub(crate) fn compute_info_decompression_keygen( Ok(KeyGenMetadata::new( *key_id, *prep_id, - HashMap::from([(PubDataType::DecompressionKey, key_digest)]), + BTreeMap::from([(PubDataType::DecompressionKey, key_digest)]), external_signature, extra_data, )) @@ -436,7 +437,7 @@ pub(crate) fn compute_info_compressed_keygen_from_digests( Ok(KeyGenMetadata::new( *key_id, *prep_id, - HashMap::from([ + BTreeMap::from([ (PubDataType::CompressedXofKeySet, compressed_keyset_digest), (PubDataType::PublicKey, public_key_digest), ]), @@ -940,12 +941,22 @@ pub(crate) fn retrieve_parameters(fhe_parameter: Option) -> Result>, + pub extra_data: Option>, + pub external_signature: Vec, +} + +#[derive(Clone, Serialize, Deserialize, Version)] +pub struct KeyGenMetadataInnerV1 { pub key_id: RequestId, pub preprocessing_id: RequestId, pub key_digest_map: HashMap>, @@ -953,11 +964,11 @@ pub struct KeyGenMetadataInner { pub external_signature: Vec, } -impl Upgrade for KeyGenMetadataInnerQ126 { +impl Upgrade for KeyGenMetadataInnerQ126 { type Error = std::convert::Infallible; - fn upgrade(self) -> Result { - Ok(KeyGenMetadataInner { + fn upgrade(self) -> Result { + Ok(KeyGenMetadataInnerV1 { key_id: self.key_id, preprocessing_id: self.preprocessing_id, key_digest_map: self.key_digest_map, @@ -966,6 +977,21 @@ impl Upgrade for KeyGenMetadataInnerQ126 { }) } } + +impl Upgrade for KeyGenMetadataInnerV1 { + type Error = std::convert::Infallible; + + fn upgrade(self) -> Result { + Ok(KeyGenMetadataInner { + key_id: self.key_id, + preprocessing_id: self.preprocessing_id, + key_digest_map: self.key_digest_map.into_iter().collect(), + extra_data: self.extra_data, + external_signature: self.external_signature, + }) + } +} + #[derive(Clone, Serialize, Deserialize, Version)] pub struct KeyGenMetadataInnerQ126 { pub key_id: RequestId, @@ -996,7 +1022,7 @@ impl KeyGenMetadata { pub fn new( key_id: RequestId, preprocessing_id: RequestId, - key_digest_map: HashMap>, + key_digest_map: BTreeMap>, external_signature: Vec, extra_data: Vec, ) -> Self { @@ -1151,6 +1177,7 @@ pub type UserDecryptCallValues = (UserDecryptionResponsePayload, Vec, Vec V2: the HashMap is converted to a BTreeMap. Field-by-field structural + // equality remains, modulo container type. + let upgraded_v2: KeyGenMetadataInner = upgraded.upgrade().unwrap(); + assert_eq!(upgraded_v2.extra_data, None); + assert_eq!(upgraded_v2.key_id, q126.key_id); + assert_eq!(upgraded_v2.preprocessing_id, q126.preprocessing_id); + assert_eq!( + upgraded_v2.key_digest_map, + q126.key_digest_map + .iter() + .map(|(k, v)| (*k, v.clone())) + .collect::>(), + ); + assert_eq!(upgraded_v2.external_signature, q126.external_signature); } #[test] diff --git a/core/service/src/engine/threshold/service/kms_impl.rs b/core/service/src/engine/threshold/service/kms_impl.rs index 6183a4141e..6811ec231b 100644 --- a/core/service/src/engine/threshold/service/kms_impl.rs +++ b/core/service/src/engine/threshold/service/kms_impl.rs @@ -897,6 +897,7 @@ fn update_threshold_kms_system_metrics( mod tests { use aes_prng::AesRng; use rand::SeedableRng; + use std::collections::BTreeMap; use tfhe::safe_serialization::{safe_deserialize, safe_serialize}; use tfhe_versionable::Upgrade; use threshold_execution::tfhe_internals::{ @@ -1024,7 +1025,7 @@ mod tests { meta_data: KeyGenMetadata::new( RequestId::zeros(), RequestId::zeros(), - HashMap::new(), + BTreeMap::new(), vec![], vec![], ), @@ -1072,7 +1073,7 @@ mod tests { KeyGenMetadata::new( RequestId::zeros(), RequestId::zeros(), - HashMap::new(), + BTreeMap::new(), vec![], vec![], ), @@ -1136,7 +1137,7 @@ mod tests { meta_data: KeyGenMetadata::new( RequestId::zeros(), RequestId::zeros(), - HashMap::new(), + BTreeMap::new(), vec![], vec![], ), diff --git a/core/service/src/engine/utils.rs b/core/service/src/engine/utils.rs index 0e604589f6..e436a03422 100644 --- a/core/service/src/engine/utils.rs +++ b/core/service/src/engine/utils.rs @@ -9,7 +9,7 @@ use observability::metrics::METRICS; use observability::metrics_names::{ ERR_ASYNC, OP_KEY_MATERIAL_AVAILABILITY, map_tonic_code_to_metric_err_tag, }; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::Display; use tonic::Status; @@ -135,7 +135,7 @@ pub(crate) fn verify_crs_digest_from_bytes( async fn verify_digests( storage: &S, id: &RequestId, - key_digest_map: &HashMap>, + key_digest_map: &BTreeMap>, ) -> anyhow::Result<()> { let pub_data_types: HashSet<_> = key_digest_map.keys().cloned().collect(); match classify_current_public_material(&pub_data_types)? { @@ -653,7 +653,7 @@ mod tests { use aes_prng::AesRng; use kms_grpc::rpc_types::{PubDataType, SignedPubDataHandleInternal}; use rand::SeedableRng; - use std::collections::HashMap; + use std::collections::{BTreeMap, HashMap}; use tfhe::core_crypto::prelude::NormalizedHammingWeightBound; use tfhe::shortint::ClassicPBSParameters; use tfhe::xof_key_set::CompressedXofKeySet; @@ -712,7 +712,7 @@ mod tests { struct TestStoredMaterial { key_id: RequestId, preproc_id: RequestId, - key_digest_map: HashMap>, + key_digest_map: BTreeMap>, } impl TestStoredMaterial { @@ -1282,7 +1282,7 @@ mod tests { TestStoredMaterial { key_id, preproc_id, - key_digest_map: HashMap::from_iter([ + key_digest_map: BTreeMap::from_iter([ (PubDataType::ServerKey, server_key_digest), (PubDataType::PublicKey, public_key_digest), ]), @@ -1350,7 +1350,7 @@ mod tests { TestStoredMaterial { key_id, preproc_id, - key_digest_map: HashMap::from_iter([ + key_digest_map: BTreeMap::from_iter([ (PubDataType::CompressedXofKeySet, compressed_keyset_digest), (PubDataType::PublicKey, public_key_digest), ]), diff --git a/core/service/src/vault/storage/crypto_material/tests.rs b/core/service/src/vault/storage/crypto_material/tests.rs index c4b2713538..e80aeee819 100644 --- a/core/service/src/vault/storage/crypto_material/tests.rs +++ b/core/service/src/vault/storage/crypto_material/tests.rs @@ -80,7 +80,7 @@ where fn dummy_info() -> KeyGenMetadata { let req_id = derive_request_id("dummy_info").unwrap(); - KeyGenMetadata::new(req_id, req_id, HashMap::new(), vec![], vec![]) + KeyGenMetadata::new(req_id, req_id, BTreeMap::new(), vec![], vec![]) } fn ram_threshold_storage( diff --git a/core/service/src/vault/storage/crypto_material/tests/migration.rs b/core/service/src/vault/storage/crypto_material/tests/migration.rs index cae244ceb5..f78ee14bcb 100644 --- a/core/service/src/vault/storage/crypto_material/tests/migration.rs +++ b/core/service/src/vault/storage/crypto_material/tests/migration.rs @@ -642,7 +642,7 @@ async fn test_copy_compressed_key_validation_failure_is_atomic() { let bad_fhe_keys = ThresholdFheKeys::new( old_fhe_keys.private_keys.clone(), PublicKeyMaterial::new(compressed_keyset.clone()), - KeyGenMetadata::new(new_key_id, prep_id, HashMap::new(), vec![], vec![]), + KeyGenMetadata::new(new_key_id, prep_id, BTreeMap::new(), vec![], vec![]), ); store_migrated_compressed_material( &crypto_storage, diff --git a/core/service/tests/backward_compatibility_determinism.rs b/core/service/tests/backward_compatibility_determinism.rs new file mode 100644 index 0000000000..971a1e80e4 --- /dev/null +++ b/core/service/tests/backward_compatibility_determinism.rs @@ -0,0 +1,103 @@ +//! Regression tests for byte-stable backward-compatibility fixture serialization. +//! +//! Each test builds a representative fixture object multiple times with fixed +//! entropy and fixed timestamps, then asserts that versionized bincode bytes +//! are identical. The goal is to catch accidental nondeterminism in KMS fixture +//! construction or persisted fields, not to test stdlib container behavior. + +use aes_prng::AesRng; +use kms_grpc::RequestId; +use kms_grpc::rpc_types::PubDataType; +use kms_lib::backup::custodian::Custodian; +use kms_lib::cryptography::encryption::{Encryption, PkeScheme, PkeSchemeType}; +use kms_lib::cryptography::signatures::gen_sig_keys; +use kms_lib::engine::base::{KeyGenMetadata, KeyGenMetadataInner}; +use rand::SeedableRng; +use std::collections::BTreeMap; +use tfhe_versionable::Versionize; +use threshold_types::role::Role; + +const REPEATS: usize = 10; +const FIXED_TIMESTAMP: u64 = 1_700_000_000; +const SEED: u64 = 0xdeadbeef; + +fn encode_versioned(value: &T) -> Vec { + // Mirror backward-compatibility/generate-vX.Y.Z/src/generate.rs::save_bcode. + let versioned = value.versionize(); + let config = bincode::config::legacy().with_fixed_int_encoding(); + bincode::serde::encode_to_vec(&versioned, config).expect("encode") +} + +fn assert_all_equal(label: &str, runs: Vec>) { + let first = &runs[0]; + for (i, other) in runs.iter().enumerate().skip(1) { + assert_eq!( + first, + other, + "{label}: serialization differs between run 0 and run {i} \ + ({} bytes vs {} bytes). This usually means an unordered field or \ + ambient input reached fixture construction or serialization.", + first.len(), + other.len(), + ); + } +} + +fn build_key_gen_metadata_inner() -> KeyGenMetadataInner { + let mut rng = AesRng::seed_from_u64(SEED); + let key_id = RequestId::new_random(&mut rng); + let preprocessing_id = RequestId::new_random(&mut rng); + + let mut digest_map: BTreeMap> = BTreeMap::new(); + digest_map.insert(PubDataType::ServerKey, vec![0xAA; 32]); + digest_map.insert(PubDataType::PublicKey, vec![0xBB; 32]); + digest_map.insert(PubDataType::CompressedXofKeySet, vec![0xCC; 32]); + digest_map.insert(PubDataType::DecompressionKey, vec![0xDD; 32]); + + match KeyGenMetadata::new( + key_id, + preprocessing_id, + digest_map, + vec![0xEE; 64], + b"extra".to_vec(), + ) { + KeyGenMetadata::Current(inner) => inner, + KeyGenMetadata::LegacyV0(_) => unreachable!("::new constructs Current"), + } +} + +#[test] +fn key_gen_metadata_inner_serialization_is_deterministic() { + let runs: Vec> = (0..REPEATS) + .map(|_| encode_versioned(&build_key_gen_metadata_inner())) + .collect(); + assert_all_equal("KeyGenMetadataInner", runs); +} + +#[test] +fn internal_custodian_setup_message_serialization_is_deterministic() { + let runs: Vec> = (0..REPEATS) + .map(|_| { + let mut rng = AesRng::seed_from_u64(SEED); + let (_verification_key, signing_key) = gen_sig_keys(&mut rng); + let mut encryption = Encryption::new(PkeSchemeType::MlKem512, &mut rng); + let (private_key, public_key) = encryption.keygen().expect("keygen"); + let custodian = Custodian::new( + Role::indexed_from_one(1), + signing_key, + public_key, + private_key, + ) + .expect("Custodian::new"); + let setup_message = custodian + .generate_setup_message_with_timestamp( + &mut rng, + "custodian-1".to_string(), + FIXED_TIMESTAMP, + ) + .expect("generate_setup_message_with_timestamp"); + encode_versioned(&setup_message) + }) + .collect(); + assert_all_equal("InternalCustodianSetupMessage", runs); +} diff --git a/core/service/tests/backward_compatibility_kms.rs b/core/service/tests/backward_compatibility_kms.rs index 6c011f539e..3f09e9358e 100644 --- a/core/service/tests/backward_compatibility_kms.rs +++ b/core/service/tests/backward_compatibility_kms.rs @@ -195,7 +195,7 @@ fn test_key_gen_metadata( let preprocessing_id: RequestId = RequestId::new_random(&mut rng); let key_id: RequestId = RequestId::new_random(&mut rng); - let mut key_digest_map: HashMap> = HashMap::new(); + let mut key_digest_map: BTreeMap> = BTreeMap::new(); let mut new_legacy: HashMap = HashMap::new(); let server_key_digest = safe_serialize_hash_element_versioned(b"TESTTEST", &pretend_server_key).unwrap(); @@ -354,7 +354,7 @@ fn test_key_gen_metadata_with_extra_data( let key_id: RequestId = RequestId::new_random(&mut rng); let extra_data = test.extra_data.to_vec(); - let mut key_digest_map: HashMap> = HashMap::new(); + let mut key_digest_map: BTreeMap> = BTreeMap::new(); let server_key_digest = safe_serialize_hash_element_versioned(b"TESTTEST", &pretend_server_key).unwrap(); let pub_key_digest = diff --git a/docs/developer/backward_compatibility.md b/docs/developer/backward_compatibility.md index aa6b75255f..75d72073bb 100644 --- a/docs/developer/backward_compatibility.md +++ b/docs/developer/backward_compatibility.md @@ -40,21 +40,15 @@ However, this does not allow changes to be made to the test metadata scheme itse Each generator uses the exact dependencies from its target KMS version. -To re-generate data for **all versions** (recommended): +To re-generate data for all **deterministic** versions (recommended — frozen versions are intentionally skipped, see [Data Determinism](#data-determinism) below): ```shell make generate-backward-compatibility-all ``` -Or generate for specific versions: +Or generate for a specific version with an existing Makefile target: ```shell -# Generate only v0.11.0 data -make generate-backward-compatibility-v0.11.0 - -# Generate only v0.11.1 data -make generate-backward-compatibility-v0.11.1 - # Generate only v0.13.0 data make generate-backward-compatibility-v0.13.0 @@ -68,15 +62,16 @@ make generate-backward-compatibility-v0.13.20 make generate-backward-compatibility-v0.14.0 ``` -WARNING: Generating for specific versions removes previously generated data from the `.ron` files, effectively ignoring other versions in the tests! -Thus, changes based on generating a specific version should NEVER be committed to the repo. +WARNING: Frozen-version targets are for exceptional investigation only. They can produce non-deterministic bytes and may append duplicate metadata with older generator code. Changes based on generating a frozen version should NEVER be committed to the repo. **Direct cargo commands:** -The make commands are aliases for changing the directory to the respective `generate-vX.Y.Z` directory and then running `cargo run --release`, i.e. for `v0.11.0` the command is: +The make commands are aliases for changing the directory to the respective `generate-vX.Y.Z` directory and then running `cargo run --release`, i.e. for `v0.14.0` the command is: ```shell -cd backward-compatibility/generate-v0.11.0 && cargo run --release +cd backward-compatibility/generate-v0.14.0 && cargo run --release ``` +Older frozen generator crates such as `generate-v0.11.0` and `generate-v0.11.1` still exist for historical inspection, but they do not have Makefile targets. + ### Testing Generated Data After generating new testing data, test it **without** pulling LFS (which would overwrite your changes): @@ -93,7 +88,12 @@ cargo test --test 'backward_compatibility_*' -- --include-ignored ### Data Determinism -TFHE-rs' prng is seeded with a fixed seed, so the data should be identical at each generation. However, the actual serialized objects might be different because bincode does not serialize HashMap in a deterministic way (see: [this issue](https://github.com/TyOverby/bincode/issues/514)). +Generator output falls into two categories: + +- **Frozen** (currently `0.11.0`, `0.11.1`, `0.13.0`, `0.13.10`, `0.13.20`): produced by generators that were non-deterministic across runs (e.g. `HashMap` iteration order — see [this bincode issue](https://github.com/TyOverby/bincode/issues/514) — or `SystemTime::now()` baked into serialized fields). Their `.bcode` files and `.ron` entries are committed via Git LFS and **must not** be regenerated as part of normal workflow — re-running their generators would produce different bytes for the same logical content. `make generate-backward-compatibility-all` skips these. +- **Deterministic** (any version added going forward): produced by generators that yield byte-identical output across runs. Their entries can be safely regenerated by `-all`. + +The two lists live in the root `Makefile` as `FROZEN_BWC_VERSIONS` and `DETERMINISTIC_BWC_VERSIONS`. ### Why separate generator crates? @@ -113,8 +113,7 @@ By maintaining separate generator crates per version, we can: ### Metadata Merging -Generators **append** to existing metadata files rather than overwriting them. -This allows multiple versions to coexist: +Generators merge into existing metadata files rather than overwriting them, so multiple versions can coexist in a single `.ron`: ```ron // backward-compatibility/data/kms.ron @@ -124,7 +123,11 @@ This allows multiple versions to coexist: ] ``` -The `make generate-backward-compatibility-all` target cleans old data first, then runs all generators in sequence. +Deterministic generators (`generate-v0.14.0` and later) works as follows: any existing entries whose `kms_core_version_min` matches a version being written are dropped first, then the freshly generated entries are appended. Entries for other versions — notably the frozen ones — are preserved verbatim. This is what makes `make generate-backward-compatibility-all` idempotent: re-running it does not accumulate duplicate rows for the deterministic versions it regenerates, while still leaving frozen-version entries intact. + +The older frozen generators (`generate-v0.11.0` … `generate-v0.13.20`) use a plain append, which is one reason re-running them can produce duplicate `.ron` rows and why they must not be run as part of normal workflow (see [Data Determinism](#data-determinism)). + +The `make generate-backward-compatibility-all` target only cleans and regenerates **deterministic** versions (listed in `DETERMINISTIC_BWC_VERSIONS` in the root `Makefile`). **Frozen** versions (`FROZEN_BWC_VERSIONS`, currently everything up to and including `0.13.20`) are left untouched — their data dirs are preserved and their generators are not invoked. The shared `.ron` files are never deleted by `-all`, so committed frozen entries survive across regenerations. ## Adding a test for an existing type @@ -359,6 +362,8 @@ Tracking ticket for replacing this manual list with structural reachability chec ⚠️ **Important**: Before adding a new version, check for dependency compatibility. See [`backward-compatibility/ADDING_NEW_VERSIONS.md`](../../backward-compatibility/ADDING_NEW_VERSIONS.md) for detailed instructions. +Any non-determinism the new generator inherits from `kms-core` itself (e.g., a new versioned struct that contains a `HashMap`, or a struct field populated from `SystemTime::now()`) must be fixed in `kms-core` via a fresh version bump before the corresponding generator can be added to `DETERMINISTIC_BWC_VERSIONS`. The freeze convention exists precisely because retroactively fixing non-determinism in old generators is not safe — old data on disk would no longer match. + To add data for a new released version of kms-core, you should: - **Check compatibility**: Verify the new version's dependencies (especially `serde`, `alloy`, `tfhe`) are compatible with existing generator versions @@ -371,7 +376,10 @@ See [`backward-compatibility/ADDING_NEW_VERSIONS.md`](../../backward-compatibili 1. Copy an existing generator: `cp -r generate-v0.11.1 generate-v0.12.0` 2. Update `Cargo.toml`: package name, version, and KMS dependencies 3. Update `src/data_0_11.rs`: imports and `VERSION_NUMBER` -4. Add to root `Makefile` and `Cargo.toml` exclude list +4. Add to `Makefile`: + - Append the new version number (e.g. `0.14.0`) to `DETERMINISTIC_BWC_VERSIONS`. **Do not** add it to `FROZEN_BWC_VERSIONS` — that list is closed at `0.13.20`. + - Add a `generate-backward-compatibility-v0.14.0` recipe matching the existing pattern. + - Add the crate to the root `Cargo.toml` exclude list. 5. Test: `make generate-backward-compatibility-all` In the generator's `src/data_x_y.rs`: