Tm/seed pub enc

[tmontaigu]

This allows to create CompactCiphertextList and
ProvenCompactCiphertextList using a seed, so that the
encryption can be reproduced

• Follows NIST submission:

• Create XofSeed from some seed bytes
• Then init a NoiseRandomGenerator from the XofSeed
• Use the gnerator to do the public encryption
• When a zk proof is needed, for each chunk create the seed for the
  zk-proof by taking the next 16 bytes of noise_random_generator.
  This is custom to tfhe-rs as NIST submission does not cover this case

• JS API + tests included
• Backward compatibility tests

Backward compatibility tests are included, as since this produces seeded
data, we need to be able to guarantee backward compatibility.

====
AI used for the tests

---

This change is 

[IceTDrinker]

There are a few things that are very fishy to me (looks to be in tests, so could be the AI doing weird stuff)

@IceTDrinker reviewed 33 files and all commit messages, and made 16 comments.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on mayeul-zama, nsarlin-zama, SouchonTheo, and tmontaigu).

---

tfhe/src/core_crypto/commons/generators/encryption/noise_random_generator.rs line 13 at r1 (raw file):

};
#[cfg(feature = "zk-pok")]
use rand_core::RngCore;

nit: import could be in the function needing it directly, avoids having to keep too many feature gates

---

tfhe/js_on_wasm_tests/test-hlapi-signed.js line 635 at r1 (raw file):

  let seed_a = new Uint8Array(16);
  seed_a[0] = 42;

could be a full 128 bits seed maybe ?

same in the other test

if needed seed can be printed to logs

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 34 at r1 (raw file):

const SEEDED_COMPACT_LIST_SEED: &[u8] = &167644343036794213320094654445260117732u128.to_le_bytes();

fn hl_seeded_compactlist_test() -> HlSeededCompactCiphertextListTest {

can't be a const ?, is it the seed to vec stuff ?

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 49 at r1 (raw file):

        test_filename: Cow::Borrowed("hl_seeded_proven_compact_list"),
        key_filename: Cow::Borrowed("seeded_proven_client_key"),
        public_key_filename: Cow::Borrowed("seeded_proven_compact_public_key"),

redundant ?

just might be a footgun if both pk files are not in sync

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 132 at r1 (raw file):

        {
            let config = tfhe::ConfigBuilder::with_custom_parameters(
                INSECURE_SMALL_TEST_PARAMS_KS32.convert(),

let's use a param with larger LWE dim please (64 is ok)

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 169 at r1 (raw file):

        {
            let config = tfhe::ConfigBuilder::with_custom_parameters(
                INSECURE_SMALL_TEST_PARAMS_KS32.convert(),

same remark

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 182 at r1 (raw file):

            let crs = CompactPkeCrs::from_config(config, 64).unwrap();

            store_versioned_auxiliary(&hl_client_key, &dir, &seeded_proven_test.key_filename);

server key not saved ? I thought the backward test did some expand, I must be wrong then

---

tests/backward_compatibility/high_level_api.rs line 297 at r1 (raw file):

    key: &ClientKey,
) -> Result<(), String> {
    for (idx, (value, kind)) in clear_values.iter().zip(data_kinds.iter()).enumerate() {

would need a length check or use zip_eq, you could have a silent failure on len mismatch

---

tests/backward_compatibility/high_level_api.rs line 356 at r1 (raw file):

    .map_err(|e| test.failure(e, format))?;

    let server_key = key.generate_server_key();

is this a pattern we usually have ? I have a comment elsewhere wondering why we are not storing the server key

---

tests/backward_compatibility/high_level_api.rs line 404 at r1 (raw file):

    format: DataFormat,
) -> Result<TestSuccess, TestFailure> {
    #[cfg(feature = "zk-pok")]

do we ever run the tests without ZK poks ? seems weird to me

this was written by AI ?

---

tests/backward_compatibility/high_level_api.rs line 414 at r1 (raw file):

        .map_err(|e| test.failure(e, format))?;

        let server_key = key.generate_server_key();

weird pattern

---

tfhe/src/high_level_api/compact_list.rs line 174 at r1 (raw file):

            (Self::Cpu(a), Self::Cpu(b)) => a == b,
            #[cfg(feature = "gpu")]
            (Self::Cuda(a), Self::Cuda(b)) => with_cuda_internal_keys(|keys| {

would it be desirable to handle mixed cases ? CPU/GPU

since GPU lists are turned into CPU lists anyways below ?

---

tfhe/src/high_level_api/compact_list.rs line 523 at r1 (raw file):

                (Self::Cpu(a), Self::Cpu(b)) => a == b,
                #[cfg(feature = "gpu")]
                (Self::Cuda(a), Self::Cuda(b)) => a.h_proved_lists == b.h_proved_lists,

could handle mixed cases ?

---

tfhe/src/high_level_api/compact_list.rs line 1255 at r1 (raw file):

        let seed_a = 42u128.to_le_bytes();
        let seed_b = 1337u128.to_le_bytes();

maybe get a random seed ? unless 42 and 1337 are used for something in particular, could be printed out, same remark for the other test

---

tfhe/src/high_level_api/compact_list.rs line 1739 at r1 (raw file):

        set_server_key(sks);

        let crs = CompactPkeCrs::from_config(config, 32).unwrap();

FheUint64 across two lists will be handled correctly ? 32 bits max for CRS might be a bit constrained here

[nsarlin-zama]

@nsarlin-zama reviewed all commit messages and made 2 comments.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on IceTDrinker, mayeul-zama, SouchonTheo, and tmontaigu).

---

tests/backward_compatibility/high_level_api.rs line 356 at r1 (raw file):

Previously, IceTDrinker wrote…

is this a pattern we usually have ? I have a comment elsewhere wondering why we are not storing the server key

this is a pattern used in all the other tests, I guess the key is not stored because it was not needed

---

tests/backward_compatibility/high_level_api.rs line 404 at r1 (raw file):

Previously, IceTDrinker wrote…

do we ever run the tests without ZK poks ? seems weird to me

this was written by AI ?

no this is also done on the other tests I wrote before the age of AI, to make it build correctly with or without the feature. This is maybe not really useful.

[IceTDrinker]

tests/backward_compatibility/high_level_api.rs line 356 at r1 (raw file):

Previously, nsarlin-zama (Nicolas Sarlin) wrote…

this is a pattern used in all the other tests, I guess the key is not stored because it was not needed

ok then

[IceTDrinker]

tests/backward_compatibility/high_level_api.rs line 404 at r1 (raw file):

Previously, nsarlin-zama (Nicolas Sarlin) wrote…

no this is also done on the other tests I wrote before the age of AI, to make it build correctly with or without the feature. This is maybe not really useful.

in that case I think we should prefer backward compat that will always run the tests even with ZK, so maybe worth removing the feature and make it "always on" by default

[nsarlin-zama]

@nsarlin-zama reviewed 2 files and made 3 comments.
Reviewable status: all files reviewed, 17 unresolved discussions (waiting on mayeul-zama, SouchonTheo, and tmontaigu).

---

tests/backward_compatibility/high_level_api.rs line 291 at r1 (raw file):

}

fn verify_expanded_values(

it looks like it could be factorized with the function above?

---

tests/backward_compatibility/high_level_api.rs line 347 at r1 (raw file):

pub fn test_hl_seeded_compact_ciphertext_list(
    dir: &Path,
    test: &HlSeededCompactCiphertextListTest,

I think you could reuse code between the proven and not proven test. For the unseeded case it's even the same test with an optional proof metadata.

---

tfhe/src/shortint/public_key/compact.rs line 524 at r1 (raw file):

}

fn noise_generator_from_seed(

Maybe this could be a method on the NoiseRandomGenerator?

[mayeul-zama]

@mayeul-zama reviewed 33 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on nsarlin-zama, SouchonTheo, and tmontaigu).

---

tfhe/js_on_wasm_tests/test-hlapi-signed.js line 717 at r1 (raw file):

  let list_a2 = builder_a2.build_packed_seeded(seed_a);

  let builder_b = CompactCiphertextList.builder(publicKey);

Could use a closure build as done in Rust tests to avoid some repetition
In the end call build(seed_a) (twice) and build(seed_b)

---

tests/backward_compatibility/high_level_api.rs line 321 at r1 (raw file):

                    return Err(format!(
                        "Invalid decrypted signed at index {idx}: expected {:?}, got {clear:?}",
                        *value as i8

Converting a u64 to an i8 is not very intuitive to me
Couldn't clear_values be i64s?

[tmontaigu]

@tmontaigu made 5 comments and resolved 3 discussions.
Reviewable status: all files reviewed, 16 unresolved discussions (waiting on IceTDrinker, nsarlin-zama, and SouchonTheo).

---

tests/backward_compatibility/high_level_api.rs line 414 at r1 (raw file):

Previously, IceTDrinker wrote…

weird pattern

Its the same pattern used in existing test_hl_heterogeneous_ciphertext_list

---

tfhe/src/high_level_api/compact_list.rs line 174 at r1 (raw file):

Previously, IceTDrinker wrote…

would it be desirable to handle mixed cases ? CPU/GPU

since GPU lists are turned into CPU lists anyways below ?

yeah we could to a.on_cpu() == b.on_cpu()

---

tfhe/src/high_level_api/compact_list.rs line 1739 at r1 (raw file):

Previously, IceTDrinker wrote…

FheUint64 across two lists will be handled correctly ? 32 bits max for CRS might be a bit constrained here

Why would it be mishandled ?
Should I increase the CRS to >32+64+1 bit ?

---

tfhe/src/shortint/public_key/compact.rs line 524 at r1 (raw file):

Previously, nsarlin-zama (Nicolas Sarlin) wrote…

Maybe this could be a method on the NoiseRandomGenerator?

There's already a new_from_seed that we can use to replace the last 2 lines of the function, but we still need this local function as the transformation into xof with the harcoded separtor is specific to this function.

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 182 at r1 (raw file):

Previously, IceTDrinker wrote…

server key not saved ? I thought the backward test did some expand, I must be wrong then

There is an expand but the key is generated during the test as is done in the existing heterogeneous list test

[IceTDrinker]

@IceTDrinker made 1 comment and resolved 2 discussions.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on nsarlin-zama, SouchonTheo, and tmontaigu).

---

tfhe/src/high_level_api/compact_list.rs line 1739 at r1 (raw file):

Previously, tmontaigu (tmontaigu) wrote…

Why would it be mishandled ?
Should I increase the CRS to >32+64+1 bit ?

It's just I don't recall if that works well when you have a type across several lists, it may be perfectly fine, just wanted to point out here that it looked unusual to me (but I guess it's managed via the DataKind, so in the end might be perfectly fine), if this works, it's fine by me

[tmontaigu]

@tmontaigu made 3 comments and resolved 8 discussions.
Reviewable status: 17 of 37 files reviewed, 6 unresolved discussions (waiting on IceTDrinker, mayeul-zama, nsarlin-zama, and SouchonTheo).

---

tests/backward_compatibility/high_level_api.rs line 321 at r1 (raw file):

Previously, mayeul-zama wrote…

Converting a u64 to an i8 is not very intuitive to me
Couldn't clear_values be i64s?

Done.

---

tfhe/js_on_wasm_tests/test-hlapi-signed.js line 717 at r1 (raw file):

Previously, mayeul-zama wrote…

Could use a closure build as done in Rust tests to avoid some repetition
In the end call build(seed_a) (twice) and build(seed_b)

Done.

---

utils/tfhe-backward-compat-data/crates/generate_1_6/src/lib.rs line 49 at r1 (raw file):

Previously, IceTDrinker wrote…

redundant ?

just might be a footgun if both pk files are not in sync

Yeah I defined a ZkProofAuxiliaryInfo that does not have a Pke as its already defined above

[IceTDrinker]

@IceTDrinker resolved 2 discussions.
Reviewable status: 17 of 37 files reviewed, 4 unresolved discussions (waiting on mayeul-zama, nsarlin-zama, and SouchonTheo).

[IceTDrinker]

Looks good to me !

Thanks a lot

I'll let the others check, but I think we can get a head start on the CI

@IceTDrinker reviewed 20 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on mayeul-zama, nsarlin-zama, SouchonTheo, and tmontaigu).

---

tests/backward_compatibility/high_level_api.rs line 342 at r3 (raw file):

    zk_proof_info: Option<&ZkProofAuxiliaryInfo>,
) -> Result<TestSuccess, TestFailure> {
    #[cfg(not(feature = "zk-pok"))]

let's plan a PR afterwards to remove the ZK feature from this crate

[nsarlin-zama]

@nsarlin-zama reviewed 3 files and made 2 comments.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on mayeul-zama, SouchonTheo, and tmontaigu).

---

tests/backward_compatibility/high_level_api.rs line 276 at r3 (raw file):

    if clear_values.len() != list.len() {
        return Err(format!(
            "Number of clear values ({}) is not the same if the number of values in the expander({})",

I think there is a typo here

---

tests/backward_compatibility/high_level_api.rs line 457 at r3 (raw file):

pub fn test_hl_seeded_proven_compact_ciphertext_list(
    dir: &Path,
    test: &HlSeededProvenCompactCiphertextListTest,

So maybe you could have only one type with the option proof info as an Option inside like for the unseeded list ?