Skip to content

Commit

Permalink
Merge branch 'mathias-CRP-1043-remove-nidkg-placeholder-to-delete' in…
Browse files Browse the repository at this point in the history
…to 'master'

chore(crypto): CRP-1043: Remove placeholder_to_delete from CspNiDkgTranscript production code

Remove the `CspNiDkgTranscript::placeholder_to_delete` from the production code. In the places where it was used:
- In the test utilities, inline the code
- For the unit tests in the `ic-types` crate, add it as a standalone crate-internal function 

See merge request dfinity-lab/public/ic!15668
  • Loading branch information
mbjorkqvist committed Nov 1, 2023
2 parents 53f1eb3 + e650225 commit 5bd3b0d
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 19 deletions.
Expand Up @@ -3,7 +3,6 @@ pub use crate::encrypt::forward_secure::{CspFsEncryptionPop, CspFsEncryptionPubl
use crate::sign::threshold_sig::public_coefficients::CspPublicCoefficients;
use phantom_newtype::AmountOf;
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
use std::hash::Hash;
use strum_macros::IntoStaticStr;

Expand Down Expand Up @@ -71,18 +70,6 @@ pub enum CspNiDkgTranscript {
Groth20_Bls12_381(ni_dkg_groth20_bls12_381::Transcript),
}
impl CspNiDkgTranscript {
/// Generates an instance of a transcript, for use in stub implementations.
/// TODO (CRP-824): Delete when stub implementations are complete.
pub fn placeholder_to_delete() -> Self {
use crate::sign::threshold_sig::public_key::bls12_381::PublicKeyBytes;
CspNiDkgTranscript::Groth20_Bls12_381(ni_dkg_groth20_bls12_381::Transcript {
public_coefficients: ni_dkg_groth20_bls12_381::PublicCoefficientsBytes {
coefficients: vec![PublicKeyBytes([0; PublicKeyBytes::SIZE])],
},
receiver_data: BTreeMap::new(),
})
}

/// From a general transcript to general public coefficients.
pub fn public_coefficients(&self) -> CspPublicCoefficients {
match &self {
Expand Down
20 changes: 19 additions & 1 deletion rs/crypto/test_utils/ni-dkg/src/lib.rs
Expand Up @@ -192,6 +192,17 @@ pub fn dummy_transcript_for_tests_with_params(
threshold: u32,
registry_version: u64,
) -> NiDkgTranscript {
// The functionality in this function, and the one in `dummy_transcript_for_tests` below, are
// copied from the `impl NiDkgTranscript` block in `ic_types::crypto::threshold_sig::ni_dkg`.
// The reasons for not being able to reuse the code are:
// - The functions/methods should not be available to production code, i.e., they should either
// have a `#[cfg(test)]` annotation, or be in a `test-utils` crate.
// - We can annotate the method in the `ic-types` crate with `#[cfg(test)]`, since it is only
// used for tests within that crate.
// - We cannot use the functions in this `ic-crypto-test-utils-ni-dkg` crate from the `ic-types`
// crate even just for tests, due to the quasi-circular dependency that it would introduce.
// Since the code is not very complex and a dozen lines, duplicating it is not a big issue.
use ic_crypto_internal_types::sign::threshold_sig::public_key::bls12_381::PublicKeyBytes;
NiDkgTranscript {
dkg_id: NiDkgId {
start_block_height: Height::from(0),
Expand All @@ -204,7 +215,14 @@ pub fn dummy_transcript_for_tests_with_params(
committee: NiDkgReceivers::new(committee.into_iter().collect())
.expect("Couldn't create non-interactive DKG committee"),
registry_version: RegistryVersion::from(registry_version),
internal_csp_transcript: CspNiDkgTranscript::placeholder_to_delete(),
internal_csp_transcript: CspNiDkgTranscript::Groth20_Bls12_381(
ni_dkg_groth20_bls12_381::Transcript {
public_coefficients: ni_dkg_groth20_bls12_381::PublicCoefficientsBytes {
coefficients: vec![PublicKeyBytes([0; PublicKeyBytes::SIZE])],
},
receiver_data: BTreeMap::new(),
},
),
}
}
pub fn dummy_transcript_for_tests() -> NiDkgTranscript {
Expand Down
15 changes: 13 additions & 2 deletions rs/types/types/src/crypto/threshold_sig/ni_dkg.rs
Expand Up @@ -219,7 +219,6 @@ impl From<&NiDkgTranscript> for CspNiDkgTranscript {

#[cfg(test)]
impl NiDkgTranscript {
#[allow(clippy::new_without_default)]
pub fn dummy_transcript_for_tests_with_params(
committee: Vec<NodeId>,
dkg_tag: NiDkgTag,
Expand All @@ -238,7 +237,7 @@ impl NiDkgTranscript {
committee: NiDkgReceivers::new(committee.into_iter().collect())
.expect("Couldn't create non-interactive DKG committee"),
registry_version: RegistryVersion::from(registry_version),
internal_csp_transcript: CspNiDkgTranscript::placeholder_to_delete(),
internal_csp_transcript: dummy_internal_transcript(),
}
}
pub fn dummy_transcript_for_tests() -> Self {
Expand Down Expand Up @@ -278,3 +277,15 @@ impl From<NiDkgTranscript> for InitialNiDkgTranscriptRecord {
}
}
}

#[cfg(test)]
fn dummy_internal_transcript() -> CspNiDkgTranscript {
use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::ni_dkg_groth20_bls12_381;
use ic_crypto_internal_types::sign::threshold_sig::public_key::bls12_381::PublicKeyBytes;
CspNiDkgTranscript::Groth20_Bls12_381(ni_dkg_groth20_bls12_381::Transcript {
public_coefficients: ni_dkg_groth20_bls12_381::PublicCoefficientsBytes {
coefficients: vec![PublicKeyBytes([0; PublicKeyBytes::SIZE])],
},
receiver_data: std::collections::BTreeMap::new(),
})
}
@@ -1,7 +1,7 @@
use super::*;
use crate::crypto::threshold_sig::ni_dkg::dummy_internal_transcript;
use crate::crypto::threshold_sig::ni_dkg::{NiDkgTag, NiDkgTargetSubnet};
use crate::{Height, PrincipalId, SubnetId};
use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::CspNiDkgTranscript;

pub const NODE_1: u64 = 1;
pub const NODE_2: u64 = 2;
Expand All @@ -23,7 +23,7 @@ fn should_succeed_creating_valid_config_for_single_node() {
threshold: dkg_threshold(1),
committee: NiDkgReceivers::new(set_of(&[node_id(NODE_1)])).unwrap(),
registry_version: REG_V1,
internal_csp_transcript: CspNiDkgTranscript::placeholder_to_delete(),
internal_csp_transcript: dummy_internal_transcript(),
};
let config_data = NiDkgConfigData {
dkg_id: dkg_id(1),
Expand Down Expand Up @@ -374,7 +374,7 @@ fn transcript() -> NiDkgTranscript {
]))
.unwrap(),
registry_version: REG_V2,
internal_csp_transcript: CspNiDkgTranscript::placeholder_to_delete(),
internal_csp_transcript: dummy_internal_transcript(),
}
}

Expand Down

0 comments on commit 5bd3b0d

Please sign in to comment.