Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ns/plug zk v2 #1846

Merged
merged 6 commits into from
Dec 16, 2024
Merged

Ns/plug zk v2 #1846

merged 6 commits into from
Dec 16, 2024

Conversation

nsarlin-zama
Copy link
Contributor

@nsarlin-zama nsarlin-zama commented Dec 4, 2024

closes: https://github.com/zama-ai/tfhe-rs-internal/issues/772

PR content/description

Plugs ZK V2. The CompactPkeCrs type is now an enum with a variant for V1 and V2, each holding the right 'PublicParam' type from tfhe_zk_pok. The proof is similarly wrapped in an enum. A prove method has been added on the Crs object that will generate a proof compatible with this Crs. Similarly, a verify method will always reject proofs that are not of the right kind.

Backward compatibility is maintained and old crs/proofs are automatically wrapped in the V1 scheme. A fix was needed in versionable for this, to make it handle type that evolve from #[repr(transparent)] to their own type.

By default new Crs are created using v2, but methods have been added to create a Crs with v1.

Proof conformance checks the version of the scheme.

Check-list:

  • Tests for the changes have been added -> Need to add a v2 proof in data repo
  • Param update
  • Docs have been added / updated (for bug fixes / features)
  • Relevant issues are marked as resolved/closed, related issues are linked in the description
  • Check for breaking changes (including serialization changes) and add them to commit message following the conventional commit specification

@cla-bot cla-bot bot added the cla-signed label Dec 4, 2024
@nsarlin-zama nsarlin-zama force-pushed the ns/plug_zk_v2 branch 2 times, most recently from dd5e5d0 to 74813d9 Compare December 4, 2024 14:13
@nsarlin-zama nsarlin-zama added the data_PR This is a PR that needs to fetch new data for backward compat tests label Dec 4, 2024
@nsarlin-zama nsarlin-zama force-pushed the ns/plug_zk_v2 branch 2 times, most recently from 13a73e4 to 7e57b6a Compare December 5, 2024 10:00
Comment on lines 9 to 12
/// This parameter set should be used when doing zk proof of public key encryption
pub const PARAM_PKE_MESSAGE_2_CARRY_2_KS_PBS_TUNIFORM_2M64: CompactPublicKeyEncryptionParameters =
CompactPublicKeyEncryptionParameters {
encryption_lwe_dimension: LweDimension(1024),
encryption_noise_distribution: DynamicDistribution::new_t_uniform(42),
encryption_lwe_dimension: LweDimension(2048),
Copy link
Member

Choose a reason for hiding this comment

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

I'll check if the keyswitching parameters need an update as well

Copy link
Member

Choose a reason for hiding this comment

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

looks like the keyswitch to big key needs a small update, I'll give you the info to update and have legacy constants there as well

Copy link
Contributor

@mayeul-zama mayeul-zama left a comment

Choose a reason for hiding this comment

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

Just a few comments
FYI I only had a quick look at the "plug zk v2" commit

Copy link
Contributor

@mayeul-zama mayeul-zama left a comment

Choose a reason for hiding this comment

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

Thanks!

@IceTDrinker
Copy link
Member

please don't merge right away, I'd like to have a chance to discuss the keyswitching parameters before you do (and read the PR as well)


pub struct CompactPkeCrsConformanceParams {
lwe_dim: LweDimension,
max_num_message: usize,
Copy link
Member

Choose a reason for hiding this comment

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

new type ?

Copy link
Member

@IceTDrinker IceTDrinker left a comment

Choose a reason for hiding this comment

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

It seems we are missing a check on k_max <= d it seems, otherwise we are looking good, after the parameter stuff

@nsarlin-zama nsarlin-zama force-pushed the ns/plug_zk_v2 branch 2 times, most recently from 3b94cf0 to 64c4476 Compare December 13, 2024 09:58
Comment on lines 9 to 23
/// This parameter set should be used when doing zk proof of public key encryption
pub const PARAM_PKE_MESSAGE_2_CARRY_2_KS_PBS_TUNIFORM_2M64: CompactPublicKeyEncryptionParameters =
CompactPublicKeyEncryptionParameters {
encryption_lwe_dimension: LweDimension(1024),
encryption_noise_distribution: DynamicDistribution::new_t_uniform(42),
encryption_lwe_dimension: LweDimension(2048),
encryption_noise_distribution: DynamicDistribution::new_t_uniform(17),
message_modulus: MessageModulus(4),
carry_modulus: CarryModulus(4),
ciphertext_modulus: CiphertextModulus::new_native(),
expansion_kind: CompactCiphertextListExpansionKind::RequiresCasting,
}
.validate();

/// This legacy parameter set should be used with the v1 pke zk scheme
pub const PARAM_PKE_MESSAGE_2_CARRY_2_KS_PBS_TUNIFORM_2M64_ZK_V1:
CompactPublicKeyEncryptionParameters = CompactPublicKeyEncryptionParameters {
Copy link
Member

Choose a reason for hiding this comment

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

did you do the keyswitch parameters update ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not yet !

Comment on lines 21 to 22
/// This legacy parameter set should be used with the v1 pke zk scheme
pub const PARAM_PKE_MESSAGE_2_CARRY_2_KS_PBS_TUNIFORM_2M64_ZK_V1:
Copy link
Member

Choose a reason for hiding this comment

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

also this does not respect the potential TFHE-rs version being in the name of consts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, I did not reworked the params yet

BREAKING CHANGE:
- The object ZkVerificationOutCome has been renamed ZkVerificationOutcome.
- Conformance of proofs now checks the scheme version of the CRS. This is
breaking at the shortint and core_crypto levels, and for manually built integer
conformance params.

New CRS will be generated with the V2 Scheme by default, but V1 CRS and proofs
are still accepted, so this is not breaking. New methods have been added to
generate a V1 CRS.
This allows to detect unused dispatch enums
@nsarlin-zama nsarlin-zama merged commit 03956a9 into main Dec 16, 2024
100 of 106 checks passed
@nsarlin-zama nsarlin-zama deleted the ns/plug_zk_v2 branch December 16, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved cla-signed data_PR This is a PR that needs to fetch new data for backward compat tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants