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

feat(crypto): CRP-2597 Extend NiDkgTag with HighThresholdForKey variant #2445

Draft
wants to merge 32 commits into
base: franzstefan/CRP-2597-move-masterpublickeyid-proto-to-types
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a4e5d35
feat(crypto): CRP-2597 make NiDkgTag non-Copy
fspreiss Oct 31, 2024
808aa1d
More fixes
fspreiss Oct 31, 2024
9cb80e8
Take NiDkgId as ref in combine_threshold_sig_shares
fspreiss Nov 1, 2024
4461616
Take NiDkgId as ref in verify_threshold_sig_combined
fspreiss Nov 1, 2024
0ae9cdc
Take NiDkgId as ref at more places
fspreiss Nov 1, 2024
3a8675b
Fmt
fspreiss Nov 1, 2024
8c447fd
Merge remote-tracking branch 'origin' into franzstefan/CRP-2597-make-…
fspreiss Nov 1, 2024
5e466de
Remove unused dkg_id param of NiDkgCspClient::load_threshold_signing_key
fspreiss Nov 1, 2024
b5343d6
Cleanup benches
fspreiss Nov 1, 2024
64a7b7b
NiDkgConfig::dkg_id now returns ref
fspreiss Nov 1, 2024
e0e60b9
Remove obsolete dkg_id param in NiDkgCspClient::create_dealing
fspreiss Nov 1, 2024
26e2b6a
Remove obsolete dkg_id param in NiDkgCspClient::verify_dealing
fspreiss Nov 1, 2024
14fec1d
Remove obsolete dkg_id param in NiDkgCspClient::verify_resharing_dealing
fspreiss Nov 1, 2024
7339b90
Cleanup
fspreiss Nov 1, 2024
7fda226
Merge remote-tracking branch 'origin' into franzstefan/CRP-2597-make-…
fspreiss Nov 4, 2024
e98b7d8
Merge branch 'franzstefan/CRP-2597-make-nidkgtag-non-copy' into franz…
fspreiss Nov 4, 2024
dbaf13a
feat(crypto): CRP-2597-extend-nidkg-tag-with-high-threshold-for-keyid…
fspreiss Nov 5, 2024
c51dc23
Fix deps
fspreiss Nov 5, 2024
fce1c63
Remove obsolete conversion i32->NiDkgTag
fspreiss Nov 5, 2024
ac819a5
Merge branch 'franzstefan/CRP-2597-move-masterpublickeyid-proto-to-ty…
fspreiss Nov 6, 2024
06f027f
Merge branch 'franzstefan/CRP-2597-move-masterpublickeyid-proto-to-ty…
fspreiss Nov 6, 2024
6a5a578
Cleanup
fspreiss Nov 6, 2024
07fb339
Tests
fspreiss Nov 6, 2024
0c1c7a7
Do not benchmark NiDkgTag::HighThresholdForKey for now
fspreiss Nov 6, 2024
b17d8f9
Do not support new tag in new_with_inverted_threshold
fspreiss Nov 6, 2024
ac37697
Add test should_return_error_if_ni_dkg_tag_invalid_with_invalid_maste…
fspreiss Nov 6, 2024
7cfe2f7
Extend TODO description
fspreiss Nov 6, 2024
14cfb31
Add comment
fspreiss Nov 7, 2024
4fadc02
Store: rename capacity and improve docu
fspreiss Nov 7, 2024
3ad52e4
Typo in docu
fspreiss Nov 7, 2024
770c4e4
Typo
fspreiss Nov 7, 2024
cd99051
Custom ExhaustiveSet impl for NiDkgTag
fspreiss Nov 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion rs/consensus/src/consensus/batch_delivery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ pub fn generate_responses_to_setup_initial_dkg_calls(
for (id, callback_id, transcript) in transcripts_for_remote_subnets.iter() {
let add_transcript = |transcript_results: &mut TranscriptResults| {
let value = Some(transcript.clone());
match id.dkg_tag {
match &id.dkg_tag {
NiDkgTag::LowThreshold => {
if transcript_results.low_threshold.is_some() {
error!(
Expand All @@ -327,6 +327,12 @@ pub fn generate_responses_to_setup_initial_dkg_calls(
}
transcript_results.high_threshold = value;
}
NiDkgTag::HighThresholdForKey(master_public_key_id) => {
error!(
log,
"Implementation error: NiDkgTag::HighThresholdForKey({master_public_key_id}) used in SetupInitialDKG for callback ID {callback_id}",
);
}
}
};
match transcripts.get_mut(callback_id) {
Expand Down
9 changes: 6 additions & 3 deletions rs/consensus/src/consensus/dkg_key_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,9 +535,12 @@ impl Drop for DkgKeyManager {

/// Print the information about a [`NiDkgId`] in a concise way for logging
fn dkg_id_log_msg(id: &NiDkgId) -> String {
let tag = match id.dkg_tag {
NiDkgTag::LowThreshold => "low",
NiDkgTag::HighThreshold => "high",
let tag = match &id.dkg_tag {
NiDkgTag::LowThreshold => "low".to_string(),
NiDkgTag::HighThreshold => "high".to_string(),
NiDkgTag::HighThresholdForKey(master_public_key_id) => {
format!("highForKey({master_public_key_id})")
}
};

// If the target is local (which it is usually), we don't log the target
Expand Down
27 changes: 25 additions & 2 deletions rs/consensus/src/dkg/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,21 @@ pub(crate) fn get_configs_for_local_transcripts(
resharing_transcript.cloned(),
)
}
////////////////////////////////////////////////////////////////////////////////
// TODO: how to behave here? The code below is copied+adapted from the HighThreshold case. However,
// this code currently won't be executed because we iterate over TAGS, which is a const that does not
// and cannot contain an NiDkgTag::HighThresholdForKey entry
///////////////////////////////////////////////////////////////////////////////
NiDkgTag::HighThresholdForKey(master_public_key_id) => {
let resharing_transcript = reshared_transcripts
.get(&NiDkgTag::HighThresholdForKey(master_public_key_id.clone()));
(
resharing_transcript
.map(|transcript| transcript.committee.get().clone())
.unwrap_or_else(|| node_ids.clone()),
resharing_transcript.cloned(),
)
}
};
let threshold =
NumberOfNodes::from(tag.threshold_for_subnet_of_size(node_ids.len()) as u32);
Expand Down Expand Up @@ -1206,9 +1221,13 @@ mod tests {
assert_eq!(conf.max_corrupt_dealers().get(), 2);
assert_eq!(
conf.threshold().get().get(),
match *tag {
match tag {
NiDkgTag::LowThreshold => 3,
NiDkgTag::HighThreshold => 5,
/////////////////////////////////////////
// TODO: check with Consensus team
NiDkgTag::HighThresholdForKey(_) => panic!("not applicable"),
/////////////////////////////////////////
}
);
}
Expand Down Expand Up @@ -1311,9 +1330,13 @@ mod tests {
assert_eq!(conf.max_corrupt_dealers().get(), 2);
assert_eq!(
conf.threshold().get().get(),
match *tag {
match tag {
NiDkgTag::LowThreshold => 3,
NiDkgTag::HighThreshold => 5,
/////////////////////////////////////////
// TODO: check with Consensus team
NiDkgTag::HighThresholdForKey(_) => panic!("not applicable"),
/////////////////////////////////////////
}
);

Expand Down
1 change: 1 addition & 0 deletions rs/crypto/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ DEPENDENCIES = [
"//rs/registry/helpers",
"//rs/registry/keys",
"//rs/types/base_types",
"//rs/types/management_canister_types",
"//rs/types/types",
"@crate_index//:bincode",
"@crate_index//:hex",
Expand Down
1 change: 1 addition & 0 deletions rs/crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ ic-crypto-tls-cert-validation = { path = "node_key_validation/tls_cert_validatio
ic-crypto-tls-interfaces = { path = "tls_interfaces" }
ic-crypto-utils-basic-sig = { path = "utils/basic_sig" }
ic-crypto-utils-tls = { path = "utils/tls" }
ic-management-canister-types = { path = "../types/management_canister_types" }
ic-interfaces = { path = "../interfaces" }
ic-interfaces-registry = { path = "../interfaces/registry" }
ic-logger = { path = "../monitoring/logger" }
Expand Down
2 changes: 2 additions & 0 deletions rs/crypto/benches/ni_dkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ impl TestCase {
let tag_name = match self.dkg_tag {
NiDkgTag::LowThreshold => "low",
NiDkgTag::HighThreshold => "high",
NiDkgTag::HighThresholdForKey(_) => unimplemented!(),
};
format!(
"crypto_nidkg_{}_nodes_{}_dealers_{}",
Expand All @@ -281,6 +282,7 @@ impl TestCase {
num_of_dealers: num_of_nodes,
dkg_tag,
},
NiDkgTag::HighThresholdForKey(_) => unimplemented!(),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions rs/crypto/internal/crypto_service_provider/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ DEV_DEPENDENCIES = [
"//rs/crypto/utils/basic_sig",
"//rs/test_utilities/in_memory_logger",
"//rs/test_utilities/time",
"//rs/types/management_canister_types",
"//rs/types/types_test_utils",
"@crate_index//:assert_matches",
"@crate_index//:lazy_static",
Expand Down
1 change: 1 addition & 0 deletions rs/crypto/internal/crypto_service_provider/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ ic-crypto-test-utils-local-csp-vault = { path = "../../../crypto/test_utils/loca
ic-crypto-test-utils-metrics = { path = "../../../crypto/test_utils/metrics" }
ic-crypto-test-utils-reproducible-rng = { path = "../../../crypto/test_utils/reproducible_rng" }
ic-crypto-utils-basic-sig = { path = "../../utils/basic_sig" }
ic-management-canister-types = { path = "../../../types/management_canister_types" }
ic-test-utilities-compare-dirs = { path = "../../../test_utilities/compare_dirs" }
ic-test-utilities-in-memory-logger = { path = "../../../test_utilities/in_memory_logger" }
ic-test-utilities-time = { path = "../../../test_utilities/time" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::{
CspNiDkgDealing, CspNiDkgTranscript, Epoch,
};
use ic_crypto_internal_types::sign::threshold_sig::public_key::CspThresholdSigPublicKey;
use ic_management_canister_types::{
EcdsaCurve, EcdsaKeyId, MasterPublicKeyId, SchnorrAlgorithm, SchnorrKeyId, VetKdCurve,
VetKdKeyId,
};
use ic_types::crypto::threshold_sig::ni_dkg::config::dealers::NiDkgDealers;
use ic_types::crypto::threshold_sig::ni_dkg::config::receivers::NiDkgReceivers;
use ic_types::crypto::threshold_sig::ni_dkg::config::NiDkgThreshold;
Expand All @@ -22,7 +26,7 @@ use rand::Rng;
use rand_chacha::ChaCha20Rng;
use std::collections::BTreeMap;
use std::sync::Arc;
use strum::IntoEnumIterator;
use strum::{EnumCount, IntoEnumIterator};

// Generate random data structures:
// Alternatively we could implement Distribution for all of these types.
Expand All @@ -33,10 +37,45 @@ pub fn random_height(rng: &mut ChaCha20Rng) -> Height {
pub fn random_subnet_id(rng: &mut ChaCha20Rng) -> SubnetId {
subnet_test_id(rng.gen::<u64>())
}
pub fn random_master_public_key_id(rng: &mut ChaCha20Rng) -> MasterPublicKeyId {
assert_eq!(MasterPublicKeyId::COUNT, 3);
assert_eq!(EcdsaCurve::iter().count(), 1);
assert_eq!(SchnorrAlgorithm::iter().count(), 2);
assert_eq!(VetKdCurve::iter().count(), 1);

use rand::prelude::SliceRandom;
[
MasterPublicKeyId::Ecdsa(EcdsaKeyId {
curve: EcdsaCurve::Secp256k1,
name: "some key".to_string(),
}),
MasterPublicKeyId::Schnorr(SchnorrKeyId {
algorithm: SchnorrAlgorithm::Bip340Secp256k1,
name: "some key".to_string(),
}),
MasterPublicKeyId::Schnorr(SchnorrKeyId {
algorithm: SchnorrAlgorithm::Ed25519,
name: "some key".to_string(),
}),
MasterPublicKeyId::VetKd(VetKdKeyId {
curve: VetKdCurve::Bls12_381_G2,
name: "some key".to_string(),
}),
]
.choose(rng)
.cloned()
.expect("Could not choose a MasterPublicKeyId")
}
pub fn random_ni_dkg_tag(rng: &mut ChaCha20Rng) -> NiDkgTag {
NiDkgTag::iter()
.choose(rng)
.expect("Could not choose a NiDkgTag")
use rand::prelude::SliceRandom;
[
NiDkgTag::LowThreshold,
NiDkgTag::HighThreshold,
NiDkgTag::HighThresholdForKey(random_master_public_key_id(rng)),
]
.choose(rng)
.cloned()
.expect("Could not choose a NiDkgTag")
}
pub fn random_ni_dkg_id(rng: &mut ChaCha20Rng) -> NiDkgId {
NiDkgId {
Expand Down
22 changes: 10 additions & 12 deletions rs/crypto/src/sign/threshold_sig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ mod tests;
pub struct ThresholdSignerInternal {}

impl ThresholdSignerInternal {
////////////////////////////////////////
#[allow(clippy::result_large_err)]
////////////////////////////////////////
pub fn sign_threshold<C: ThresholdSignatureCspClient, H: Signable>(
lockable_threshold_sig_data_store: &LockableThresholdSigDataStore,
threshold_sig_csp_client: &C,
Expand Down Expand Up @@ -70,6 +73,9 @@ fn sig_data_not_found_error(dkg_id: NiDkgId) -> ThresholdSigDataNotFoundError {
ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { dkg_id }
}

////////////////////////////////////////
#[allow(clippy::result_large_err)]
////////////////////////////////////////
fn threshold_sig_share_or_panic<H: Signable>(
csp_signature: CspSignature,
) -> Result<ThresholdSigShareOf<H>, ThresholdSignError> {
Expand Down Expand Up @@ -441,12 +447,8 @@ impl ThresholdSigVerifierInternal {
H: Signable,
{
let csp_signature = CspSignature::try_from(signature)?;
let transcript = initial_ni_dkg_transcript_from_registry(
registry,
subnet_id,
version,
NiDkgTag::HighThreshold,
)?;
let transcript =
initial_high_threshold_ni_dkg_transcript_from_registry(registry, subnet_id, version)?;
let csp_pub_coeffs = CspPublicCoefficients::from(&transcript);
threshold_sig_csp_client
.threshold_verify_combined_signature(
Expand All @@ -459,20 +461,16 @@ impl ThresholdSigVerifierInternal {
}
}

fn initial_ni_dkg_transcript_from_registry(
fn initial_high_threshold_ni_dkg_transcript_from_registry(
registry: &dyn RegistryClient,
subnet_id: SubnetId,
registry_version: RegistryVersion,
dkg_tag: NiDkgTag,
) -> CryptoResult<NiDkgTranscript> {
let maybe_transcripts = registry
.get_initial_dkg_transcripts(subnet_id, registry_version)
.map_err(CryptoError::RegistryClient)?;
match maybe_transcripts.value {
Some(transcripts) => Ok(match dkg_tag {
NiDkgTag::LowThreshold => transcripts.low_threshold,
NiDkgTag::HighThreshold => transcripts.high_threshold,
}),
Some(transcripts) => Ok(transcripts.high_threshold),
None => Err(CryptoError::DkgTranscriptNotFound {
subnet_id,
registry_version,
Expand Down
Loading