Skip to content

Commit

Permalink
fix(schnorr): Revert 'CON-1255 Make MasterPublicKey in `EcdsaReshar…
Browse files Browse the repository at this point in the history
…eRequest` mandatory'
  • Loading branch information
eichhorl authored and sasa-tomic committed May 13, 2024
1 parent 2c4566b commit 30bf45e
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 34 deletions.
6 changes: 3 additions & 3 deletions rs/consensus/src/ecdsa/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,9 @@ fn create_summary_payload_helper(
.retain(|_, pre_sig| !new_key_transcripts.contains(&pre_sig.key_id()));
// This will clear the current ongoing reshares, and the execution requests will be restarted
// with the new key and different transcript IDs.
ecdsa_summary
.ongoing_xnet_reshares
.retain(|request, _| !new_key_transcripts.contains(&request.master_key_id));
ecdsa_summary.ongoing_xnet_reshares.retain(|request, _| {
!new_key_transcripts.contains(&MasterPublicKeyId::Ecdsa(request.key_id.clone()))
});

ecdsa_summary.uid_generator.update_height(height)?;
update_summary_refs(height, &mut ecdsa_summary, block_reader)?;
Expand Down
12 changes: 6 additions & 6 deletions rs/consensus/src/ecdsa/payload_builder/resharing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ fn reshare_request_from_dealings_context(
context: &EcdsaDealingsContext,
) -> idkg::EcdsaReshareRequest {
idkg::EcdsaReshareRequest {
key_id: Some(context.key_id.clone()),
master_key_id: MasterPublicKeyId::Ecdsa(context.key_id.clone()),
key_id: context.key_id.clone(),
master_key_id: Some(MasterPublicKeyId::Ecdsa(context.key_id.clone())),
receiving_node_ids: context.nodes.iter().copied().collect(),
registry_version: context.registry_version,
}
Expand All @@ -180,8 +180,8 @@ pub mod test_utils {
registry_version: u64,
) -> idkg::EcdsaReshareRequest {
idkg::EcdsaReshareRequest {
key_id: Some(key_id.clone()),
master_key_id: MasterPublicKeyId::Ecdsa(key_id),
key_id: key_id.clone(),
master_key_id: Some(MasterPublicKeyId::Ecdsa(key_id)),
receiving_node_ids: (0..num_nodes).map(node_test_id).collect::<Vec<_>>(),
registry_version: RegistryVersion::from(registry_version),
}
Expand Down Expand Up @@ -250,8 +250,8 @@ mod tests {
let make_key_id =
|i: u64| EcdsaKeyId::from_str(&format!("Secp256k1:some_key_{i}")).unwrap();
let make_reshare_request = |i| EcdsaReshareRequest {
key_id: Some(make_key_id(i)),
master_key_id: MasterPublicKeyId::Ecdsa(make_key_id(i)),
key_id: make_key_id(i),
master_key_id: Some(MasterPublicKeyId::Ecdsa(make_key_id(i))),
receiving_node_ids: vec![node_test_id(i)],
registry_version: RegistryVersion::from(i),
};
Expand Down
10 changes: 3 additions & 7 deletions rs/consensus/src/ecdsa/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use ic_test_utilities_types::messages::RequestBuilder;
use ic_types::artifact::EcdsaMessageId;
use ic_types::consensus::certification::Certification;
use ic_types::consensus::idkg::common::PreSignatureRef;
use ic_types::consensus::idkg::HasMasterPublicKeyId;
use ic_types::consensus::idkg::{
self,
ecdsa::{PreSignatureQuadrupleRef, ThresholdEcdsaSigInputsRef},
Expand Down Expand Up @@ -67,12 +66,9 @@ use super::utils::get_context_request_id;
pub(crate) fn dealings_context_from_reshare_request(
request: idkg::EcdsaReshareRequest,
) -> EcdsaDealingsContext {
let MasterPublicKeyId::Ecdsa(key_id) = request.key_id() else {
panic!("Expected ECDSA key Id");
};
EcdsaDealingsContext {
request: RequestBuilder::new().build(),
key_id,
key_id: request.key_id,
nodes: request.receiving_node_ids.into_iter().collect(),
registry_version: request.registry_version,
time: time::UNIX_EPOCH,
Expand Down Expand Up @@ -1485,8 +1481,8 @@ pub(crate) fn fake_ecdsa_key_id() -> EcdsaKeyId {
pub(crate) fn create_reshare_request(num_nodes: u64, registry_version: u64) -> EcdsaReshareRequest {
let key_id = fake_ecdsa_key_id();
EcdsaReshareRequest {
key_id: Some(key_id.clone()),
master_key_id: MasterPublicKeyId::Ecdsa(key_id),
key_id: key_id.clone(),
master_key_id: Some(MasterPublicKeyId::Ecdsa(key_id)),
receiving_node_ids: (0..num_nodes).map(node_test_id).collect::<Vec<_>>(),
registry_version: RegistryVersion::from(registry_version),
}
Expand Down
27 changes: 11 additions & 16 deletions rs/types/types/src/consensus/idkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,8 @@ impl TryFrom<&pb::KeyTranscriptCreation> for KeyTranscriptCreation {
/// Internal format of the resharing request from execution.
#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq, Serialize, Deserialize)]
pub struct EcdsaReshareRequest {
pub key_id: Option<EcdsaKeyId>,
pub master_key_id: MasterPublicKeyId,
pub key_id: EcdsaKeyId,
pub master_key_id: Option<MasterPublicKeyId>,
pub receiving_node_ids: Vec<NodeId>,
pub registry_version: RegistryVersion,
}
Expand All @@ -793,10 +793,10 @@ impl Hash for EcdsaReshareRequest {
receiving_node_ids,
registry_version,
} = self;
if let Some(key_id) = key_id {
key_id.hash(state);
key_id.hash(state);
if let Some(master_key_id) = master_key_id {
master_key_id.hash(state);
}
master_key_id.hash(state);
receiving_node_ids.hash(state);
registry_version.hash(state);
}
Expand All @@ -809,8 +809,8 @@ impl From<&EcdsaReshareRequest> for pb::EcdsaReshareRequest {
receiving_node_ids.push(node_id_into_protobuf(*node));
}
Self {
key_id: request.key_id.as_ref().map(|key_id| key_id.into()),
master_key_id: Some(request.master_key_id.clone().into()),
key_id: Some((&request.key_id).into()),
master_key_id: request.master_key_id.clone().map(|key_id| key_id.into()),
receiving_node_ids,
registry_version: request.registry_version.get(),
}
Expand All @@ -826,17 +826,12 @@ impl TryFrom<&pb::EcdsaReshareRequest> for EcdsaReshareRequest {
.map(|node| node_id_try_from_option(Some(node.clone())))
.collect::<Result<Vec<_>, ProxyDecodeError>>()?;

let key_id = request
.key_id
let key_id = try_from_option_field(request.key_id.clone(), "EcdsaReshareRequest::key_id")?;
let master_key_id = request
.master_key_id
.clone()
.map(|key_id| key_id.try_into())
.transpose()?;

let master_key_id = try_from_option_field(
request.master_key_id.clone(),
"EcdsaReshareRequest::master_key_id",
)?;

Ok(Self {
key_id,
master_key_id,
Expand Down Expand Up @@ -2148,7 +2143,7 @@ impl HasMasterPublicKeyId for PreSignatureRef {

impl HasMasterPublicKeyId for EcdsaReshareRequest {
fn key_id(&self) -> MasterPublicKeyId {
self.master_key_id.clone()
MasterPublicKeyId::Ecdsa(self.key_id.clone())
}
}

Expand Down
4 changes: 2 additions & 2 deletions rs/types/types/src/exhaustive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,8 @@ impl ExhaustiveSet for EcdsaReshareRequest {
DerivedEcdsaReshareRequest::exhaustive_set(rng)
.into_iter()
.map(|r| EcdsaReshareRequest {
key_id: Some(r.key_id.clone()),
master_key_id: MasterPublicKeyId::Ecdsa(r.key_id),
key_id: r.key_id.clone(),
master_key_id: Some(MasterPublicKeyId::Ecdsa(r.key_id)),
receiving_node_ids: r.receiving_node_ids,
registry_version: r.registry_version,
})
Expand Down

0 comments on commit 30bf45e

Please sign in to comment.