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.

Sign up for GitHub

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

Additional Key Handover Tests #3165

Merged
merged 5 commits into from
May 3, 2023
Merged

Additional Key Handover Tests #3165

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b20e58
refactor: key handover test takes parameters
msgmaxim May 2, 2023
62a0385
test: key handover with disjoint validator sets
msgmaxim May 2, 2023
51cc33b
test: key handover with different set sizes
msgmaxim May 2, 2023
4870a2c
refactor: extract prepare handover function
msgmaxim May 2, 2023
69ffe9e
test: recovery during stage 0 handover
msgmaxim May 2, 2023
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
283 changes: 184 additions & 99 deletions engine/multisig/src/client/keygen/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,50 +919,42 @@ async fn initially_incompatible_keys_can_sign() {

mod key_handover {

use super::*;
use cf_primitives::PublicKeyBytes;

#[tokio::test]
async fn key_handover() {
// The high level idea of this test is to generate some key with some
// nodes, then introduce new nodes who the key will be handed over to.
// There is an overlap between the two sets of nodes, which is going
// to be common in practice. The resulting aggregate keys should match.
use super::*;

type Scheme = BtcSigning;
type Point = <Scheme as CryptoScheme>::Point;
type Scalar = <Point as ECPoint>::Scalar;
fn to_account_id_set<T: AsRef<[u8]>>(ids: T) -> BTreeSet<AccountId> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be worth moving this into helpers so we can use it in other places. Could make the ACCOUNT_IDS lazy_static use it.
The db tests could use it, but it will need to be outside of helpers so we can put it under the "test" feature flag. Maybe we need a "common_helpers" file.
We can do it in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, let me make another PR.

ids.as_ref().iter().map(|i| AccountId::new([*i; 32])).collect()
}

let all_account_ids: Vec<AccountId> =
[1, 2, 3, 4, 5].iter().map(|i| AccountId::new([*i; 32])).collect();
#[derive(Default)]
struct HandoverTestOptions {
nodes_sharing_invalid_secret: BTreeSet<AccountId>,
}

// Accounts (1), (2) and (3) will hold the original key
let original_set: BTreeSet<_> = all_account_ids.iter().take(3).cloned().collect();
async fn prepare_handover_test<Scheme: CryptoScheme>(
original_set: BTreeSet<AccountId>,
sharing_subset: BTreeSet<AccountId>,
receiving_set: BTreeSet<AccountId>,
options: HandoverTestOptions,
) -> (KeygenCeremonyRunner<Scheme>, PublicKeyBytes) {
use crate::client::common::ParticipantStatus;

// Accounts (3), (4) and (5) will receive the key as the result of this ceremony.
// (Note that (3) appears in both sets.)
let new_set: BTreeSet<_> = all_account_ids.iter().skip(2).take(3).cloned().collect();
assert!(sharing_subset.is_subset(&original_set));

// Perform a regular keygen to generate initial keys:
let (initial_key, mut key_infos) = keygen::generate_key_data::<Scheme>(
original_set.clone().into_iter().collect(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
original_set.clone().into_iter().collect(),
original_set.clone(),

&mut Rng::from_seed(DEFAULT_KEYGEN_SEED),
);

// Only 2 and 3 will contribute their secret shares
let sharing_participants: BTreeSet<AccountId> =
original_set.clone().into_iter().skip(1).collect();

// Sanity check: we have (just) enough participants to re-share the key
assert_eq!(
key_infos.values().next().as_ref().unwrap().params.threshold + 1,
sharing_participants.len() as u32
sharing_subset.len() as u32
);

let receiving_participants: BTreeSet<AccountId> = new_set.clone().into_iter().collect();
// Accounts (2), (3), (4) and (5) will participate, with (2) and (3)
// re-sharing their key to (3), (4) and (5)
let all_participants: BTreeSet<_> =
sharing_participants.union(&receiving_participants).cloned().collect();
let all_participants: BTreeSet<_> = sharing_subset.union(&receiving_set).cloned().collect();

let mut ceremony = KeygenCeremonyRunner::<Scheme>::new(
new_nodes(all_participants),
Expand All @@ -974,20 +966,52 @@ mod key_handover {

for (id, node) in &mut ceremony.nodes {
// Give the right context type depending on whether they have keys
let context = if sharing_participants.contains(id) {
let mut context = if sharing_subset.contains(id) {
let key_info = key_infos.remove(id).unwrap();
ResharingContext::from_key(
&key_info,
id,
&sharing_participants,
&receiving_participants,
)
ResharingContext::from_key(&key_info, id, &sharing_subset, &receiving_set)
} else {
ResharingContext::without_key(&sharing_participants, &receiving_participants)
ResharingContext::without_key(&sharing_subset, &receiving_set)
};

// Handle the case where a node sends an invalid secret share
if options.nodes_sharing_invalid_secret.contains(id) {
// Adding a small tweak to the share to make it incorrect
match &mut context.party_status {
ParticipantStatus::Sharing(secret_share, _) => {
*secret_share =
secret_share.clone() + &<Scheme::Point as ECPoint>::Scalar::from(1);
},
_ => panic!("Unexpected status"),
}
}

node.request_key_handover(ceremony_details.clone(), context).await;
}

(ceremony, initial_key)
}

/// Run key handover (preceded by a keygen) with the provided parameters
/// and ensure that it is successful
async fn ensure_successful_handover(
original_set: BTreeSet<AccountId>,
sharing_subset: BTreeSet<AccountId>,
receiving_set: BTreeSet<AccountId>,
) {
type Scheme = BtcSigning;
// The high level idea of this test is to generate some key with some
// nodes, then introduce new nodes who the key will be handed over to.
// The resulting aggregate keys should match, and the new nodes should
// be able to sign with their newly generated shares.

let (mut ceremony, initial_key) = prepare_handover_test(
original_set,
sharing_subset,
receiving_set.clone(),
HandoverTestOptions::default(),
)
.await;

let messages = ceremony.gather_outgoing_messages::<keygen::PubkeyShares0<Point>, _>().await;

let messages = run_stages!(
Expand All @@ -1009,95 +1033,156 @@ mod key_handover {

// Ensure that the new key shares can be used for signing:
let mut signing_ceremony = SigningCeremonyRunner::<Scheme>::new_with_all_signers(
new_nodes(receiving_participants),
new_nodes(receiving_set),
DEFAULT_SIGNING_CEREMONY_ID,
vec![PayloadAndKeyData::new(Scheme::signing_payload_for_test(), new_key, new_shares)],
Rng::from_entropy(),
);
standard_signing(&mut signing_ceremony).await;
}

// Test that a party who doesn't perform re-sharing correctly
// (commits to an unexpected secret) gets reported
#[tokio::test]
async fn key_handover_with_incorrect_commitment() {
use crate::client::common::ParticipantStatus;
async fn with_disjoint_sets_of_nodes() {
// Test that key handover can be performed even if there no overlap
// between sharing (old) and receiving (new) validators.

// These parties will hold the original key
let original_set = to_account_id_set([1, 2, 3]);

// A subset of them will contribute their secret shares
let sharing_subset = to_account_id_set([2, 3]);

// These parties will receive the key as the result of key handover.
// (Note no overlap with the original set.)
let new_set = to_account_id_set([4, 5, 6]);

assert!(original_set.is_disjoint(&new_set));

ensure_successful_handover(original_set, sharing_subset, new_set).await;
}

#[tokio::test]
async fn with_sets_of_nodes_overlapping() {
// In practice it is going to be common to have an overlap between
// sharing and receiving participants

// These parties will hold the original key
let original_set = to_account_id_set([1, 2, 3]);

// A subset of them will contribute their secret shares
let sharing_subset = to_account_id_set([2, 3]);

// These parties will receive the key as the result of key handover.
// (Note that (3) appears in both sets.)
let new_set = to_account_id_set([3, 4, 5]);

assert!(!original_set.is_disjoint(&new_set));

ensure_successful_handover(original_set, sharing_subset, new_set).await;
}

#[tokio::test]
async fn with_different_set_sizes() {
// These parties will hold the original key
let original_set = to_account_id_set([1, 2, 3, 4, 5]);

// A subset of them will contribute their secret shares
let sharing_subset = to_account_id_set([1, 2, 3, 4]);

// These parties will receive the key as the result of key handover.
// (Note that that the two sets have different size.)
let new_set = to_account_id_set([4, 5, 6]);

assert_ne!(original_set.len(), new_set.len());

ensure_successful_handover(original_set, sharing_subset, new_set).await;
}

#[tokio::test]
async fn should_recover_if_agree_on_values_stage0() {
type Scheme = BtcSigning;
type Point = <Scheme as CryptoScheme>::Point;
type Scalar = <Point as ECPoint>::Scalar;

let all_account_ids: Vec<AccountId> =
[1, 2, 3, 4, 5].iter().map(|i| AccountId::new([*i; 32])).collect();
let original_set = to_account_id_set([1, 2, 3, 4]);

// Accounts (1), (2) and (3) will hold the original key
let original_set: BTreeSet<_> = all_account_ids.iter().take(3).cloned().collect();
// NOTE: 3 sharing nodes is the smallest set that where
// recovery is possible during certain stages
let sharing_subset = to_account_id_set([2, 3, 4]);

// Accounts (3), (4) and (5) will receive the key as the result of this ceremony.
// (Note that (3) appears in both sets.)
let new_set: BTreeSet<_> = all_account_ids.iter().skip(2).take(3).cloned().collect();
let receiving_set = to_account_id_set([3, 4, 5]);

// Perform a regular keygen to generate initial keys:
let (_initial_key, mut key_infos) = keygen::generate_key_data::<Scheme>(
original_set.clone().into_iter().collect(),
&mut Rng::from_seed(DEFAULT_KEYGEN_SEED),
);
// This account id will fail to broadcast initial public keys
let bad_account_id = sharing_subset.iter().next().unwrap().clone();

// Only 2 and 3 will contribute their secret shares
let sharing_participants: BTreeSet<AccountId> =
original_set.clone().into_iter().skip(1).collect();
let (mut ceremony, initial_key) = prepare_handover_test::<Scheme>(
original_set,
sharing_subset,
receiving_set.clone(),
HandoverTestOptions::default(),
)
.await;

// Sanity check: we have (just) enough participants to re-share the key
assert_eq!(
key_infos.values().next().as_ref().unwrap().params.threshold + 1,
sharing_participants.len() as u32
);
let messages = ceremony.gather_outgoing_messages::<keygen::PubkeyShares0<Point>, _>().await;

let receiving_participants: BTreeSet<AccountId> = new_set.clone().into_iter().collect();
// Accounts (2), (3), (4) and (5) will participate, with (2) and (3)
// re-sharing their key to (3), (4) and (5)
let all_participants: BTreeSet<_> =
sharing_participants.union(&receiving_participants).cloned().collect();
ceremony.distribute_messages_with_non_sender(messages, &bad_account_id).await;

// Now perform a key hand-over ceremony where one of the participants
// commits to an unexpected secret
let messages = ceremony.gather_outgoing_messages::<keygen::HashComm1, _>().await;

// This account id will commit to an unexpected secret
let bad_account_id = all_participants.iter().next().unwrap().clone();
let messages = run_stages!(
ceremony,
messages,
keygen::VerifyHashComm2,
CoeffComm3,
VerifyCoeffComm4,
SecretShare5,
Complaints6,
VerifyComplaints7
);

let mut ceremony = KeygenCeremonyRunner::<Scheme>::new(
new_nodes(all_participants),
DEFAULT_KEYGEN_CEREMONY_ID,
Rng::from_seed(DEFAULT_KEYGEN_SEED),
ceremony.distribute_messages(messages).await;
let (new_key, new_shares) = ceremony.complete().await;

assert_eq!(new_key, initial_key);

// Ensure that the new key shares can be used for signing:
let mut signing_ceremony = SigningCeremonyRunner::<Scheme>::new_with_all_signers(
new_nodes(receiving_set),
DEFAULT_SIGNING_CEREMONY_ID,
vec![PayloadAndKeyData::new(Scheme::signing_payload_for_test(), new_key, new_shares)],
Rng::from_entropy(),
);
standard_signing(&mut signing_ceremony).await;
}

let ceremony_details = ceremony.keygen_ceremony_details();
// Test that a party who doesn't perform re-sharing correctly
// (commits to an unexpected secret) gets reported
#[tokio::test]
async fn key_handover_with_incorrect_commitment() {
type Scheme = BtcSigning;
type Point = <Scheme as CryptoScheme>::Point;

for (id, node) in &mut ceremony.nodes {
// Give the right context type depending on whether they have keys
let mut context = if sharing_participants.contains(id) {
let key_info = key_infos.remove(id).unwrap();
ResharingContext::from_key(
&key_info,
id,
&sharing_participants,
&receiving_participants,
)
} else {
ResharingContext::without_key(&sharing_participants, &receiving_participants)
};
// Accounts (1), (2) and (3) will hold the original key
let original_set = to_account_id_set([1, 2, 3]);

if id == &bad_account_id {
// Adding a small tweak to the share to make it incorrect
match &mut context.party_status {
ParticipantStatus::Sharing(secret_share, _) => {
*secret_share = &*secret_share + &Scalar::from(1);
},
_ => panic!("Unexpected status"),
}
}
// Only (2) and (3) will contribute their secret shares
let sharing_subset = to_account_id_set([2, 3]);

node.request_key_handover(ceremony_details.clone(), context).await;
}
// Accounts (3), (4) and (5) will receive the key as the result of this ceremony.
// (Note that (3) appears in both sets.)
let receiving_set = to_account_id_set([3, 4, 5]);

// This account id will commit to an unexpected secret
let bad_account_id = sharing_subset.iter().next().unwrap().clone();

let (mut ceremony, _initial_key) = prepare_handover_test::<Scheme>(
original_set,
sharing_subset,
receiving_set.clone(),
HandoverTestOptions {
nodes_sharing_invalid_secret: BTreeSet::from([bad_account_id.clone()]),
},
)
.await;

let messages = ceremony.gather_outgoing_messages::<keygen::PubkeyShares0<Point>, _>().await;

Expand Down