From 9bf8c92325a3364ba85737892577a8cf6e06a550 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 10 Oct 2023 17:08:09 -0400 Subject: [PATCH] Correct the coordinator tests They assumed processor 0 had keys `i = 1`. Under the new validator-set code, the first key is the one with the highest amount, In case of tie, the key (or as of the last commit, a Blake hash) decides order. This commit kludges in a mapping from processor index to assigned key index, no longer assuming its value. --- tests/coordinator/src/tests/batch.rs | 48 +++++++++---------- tests/coordinator/src/tests/key_gen.rs | 65 ++++++++++++++++++++------ tests/coordinator/src/tests/sign.rs | 56 +++++++++++----------- 3 files changed, 99 insertions(+), 70 deletions(-) diff --git a/tests/coordinator/src/tests/batch.rs b/tests/coordinator/src/tests/batch.rs index ea41807c..caa56733 100644 --- a/tests/coordinator/src/tests/batch.rs +++ b/tests/coordinator/src/tests/batch.rs @@ -29,6 +29,7 @@ use crate::{*, tests::*}; pub async fn batch( processors: &mut [Processor], + processor_is: &[u8], substrate_key: &Zeroizing<::F>, batch: Batch, ) -> u64 { @@ -59,7 +60,7 @@ pub async fn batch( .send_message(messages::coordinator::ProcessorMessage::BatchPreprocess { id: id.clone(), block: batch.block, - preprocess: [u8::try_from(i).unwrap(); 64].to_vec(), + preprocess: [processor_is[i]; 64].to_vec(), }) .await; } @@ -73,7 +74,7 @@ pub async fn batch( .send_message(messages::coordinator::ProcessorMessage::BatchPreprocess { id: id.clone(), block: batch.block, - preprocess: [u8::try_from(excluded_signer).unwrap(); 64].to_vec(), + preprocess: [processor_is[excluded_signer]; 64].to_vec(), }) .await; @@ -86,36 +87,32 @@ pub async fn batch( ) => { assert_eq!(&id, &this_id); assert_eq!(preprocesses.len(), THRESHOLD - 1); - assert!(!preprocesses - .contains_key(&Participant::new(u16::try_from(known_signer).unwrap() + 1).unwrap())); + let known_signer_i = Participant::new(u16::from(processor_is[known_signer])).unwrap(); + assert!(!preprocesses.contains_key(&known_signer_i)); - let mut participants = - preprocesses.keys().map(|p| usize::from(u16::from(*p)) - 1).collect::>(); + let mut participants = preprocesses.keys().cloned().collect::>(); for (p, preprocess) in preprocesses { - assert_eq!(preprocess, vec![u8::try_from(u16::from(p)).unwrap() - 1; 64]); + assert_eq!(preprocess, vec![u8::try_from(u16::from(p)).unwrap(); 64]); } - participants.insert(known_signer); + participants.insert(known_signer_i); participants } _ => panic!("coordinator didn't send back BatchPreprocesses"), }; for i in participants.clone() { - if i == known_signer { + if u16::from(i) == u16::from(processor_is[known_signer]) { continue; } - let processor = &mut processors[i]; + + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; let mut preprocesses = participants .clone() .into_iter() - .map(|i| { - ( - Participant::new(u16::try_from(i + 1).unwrap()).unwrap(), - [u8::try_from(i).unwrap(); 64].to_vec(), - ) - }) + .map(|i| (i, [u8::try_from(u16::from(i)).unwrap(); 64].to_vec())) .collect::>(); - preprocesses.remove(&Participant::new(u16::try_from(i + 1).unwrap()).unwrap()); + preprocesses.remove(&i); assert_eq!( processor.recv_message().await, @@ -129,25 +126,25 @@ pub async fn batch( } for i in participants.clone() { - let processor = &mut processors[i]; + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; processor .send_message(messages::coordinator::ProcessorMessage::BatchShare { id: id.clone(), - share: [u8::try_from(i).unwrap(); 32], + share: [u8::try_from(u16::from(i)).unwrap(); 32], }) .await; } wait_for_tributary().await; for i in participants.clone() { - let processor = &mut processors[i]; + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; let mut shares = participants .clone() .into_iter() - .map(|i| { - (Participant::new(u16::try_from(i + 1).unwrap()).unwrap(), [u8::try_from(i).unwrap(); 32]) - }) + .map(|i| (i, [u8::try_from(u16::from(i)).unwrap(); 32])) .collect::>(); - shares.remove(&Participant::new(u16::try_from(i + 1).unwrap()).unwrap()); + shares.remove(&i); assert_eq!( processor.recv_message().await, @@ -271,9 +268,10 @@ async fn batch_test() { } let mut processors = new_processors; - let (substrate_key, _) = key_gen::(&mut processors).await; + let (processor_is, substrate_key, _) = key_gen::(&mut processors).await; batch( &mut processors, + &processor_is, &substrate_key, Batch { network: NetworkId::Bitcoin, diff --git a/tests/coordinator/src/tests/key_gen.rs b/tests/coordinator/src/tests/key_gen.rs index a13a9dd0..81a6aa15 100644 --- a/tests/coordinator/src/tests/key_gen.rs +++ b/tests/coordinator/src/tests/key_gen.rs @@ -11,7 +11,7 @@ use ciphersuite::{ group::{ff::Field, GroupEncoding}, Ciphersuite, Ristretto, Secp256k1, }; -use dkg::{Participant, ThresholdParams}; +use dkg::ThresholdParams; use serai_client::{ primitives::NetworkId, @@ -24,21 +24,32 @@ use crate::{*, tests::*}; pub async fn key_gen( processors: &mut [Processor], -) -> (Zeroizing<::F>, Zeroizing) { - let participant_from_i = |i: usize| Participant::new(u16::try_from(i + 1).unwrap()).unwrap(); +) -> (Vec, Zeroizing<::F>, Zeroizing) { + let mut participant_is = vec![]; let set = ValidatorSet { session: Session(0), network: NetworkId::Bitcoin }; let id = KeyGenId { set, attempt: 0 }; for (i, processor) in processors.iter_mut().enumerate() { + let msg = processor.recv_message().await; + match &msg { + CoordinatorMessage::KeyGen(messages::key_gen::CoordinatorMessage::GenerateKey { + params, + .. + }) => { + participant_is.push(params.i()); + } + _ => panic!("unexpected message: {msg:?}"), + } + assert_eq!( - processor.recv_message().await, + msg, CoordinatorMessage::KeyGen(messages::key_gen::CoordinatorMessage::GenerateKey { id, params: ThresholdParams::new( u16::try_from(((COORDINATORS * 2) / 3) + 1).unwrap(), u16::try_from(COORDINATORS).unwrap(), - participant_from_i(i), + participant_is[i], ) .unwrap() }) @@ -47,7 +58,7 @@ pub async fn key_gen( processor .send_message(messages::key_gen::ProcessorMessage::Commitments { id, - commitments: vec![u8::try_from(i).unwrap()], + commitments: vec![u8::try_from(u16::from(participant_is[i])).unwrap()], }) .await; } @@ -55,9 +66,14 @@ pub async fn key_gen( wait_for_tributary().await; for (i, processor) in processors.iter_mut().enumerate() { let mut commitments = (0 .. u8::try_from(COORDINATORS).unwrap()) - .map(|l| (participant_from_i(l.into()), vec![l])) + .map(|l| { + ( + participant_is[usize::from(l)], + vec![u8::try_from(u16::from(participant_is[usize::from(l)])).unwrap()], + ) + }) .collect::>(); - commitments.remove(&participant_from_i(i)); + commitments.remove(&participant_is[i]); assert_eq!( processor.recv_message().await, CoordinatorMessage::KeyGen(messages::key_gen::CoordinatorMessage::Commitments { @@ -66,13 +82,20 @@ pub async fn key_gen( }) ); - // from (0 .. n), to (1 ..= n) + // Recipient it's for -> (Sender i, Recipient i) let mut shares = (0 .. u8::try_from(COORDINATORS).unwrap()) - .map(|l| (participant_from_i(l.into()), vec![u8::try_from(i).unwrap(), l + 1])) + .map(|l| { + ( + participant_is[usize::from(l)], + vec![ + u8::try_from(u16::try_from(participant_is[i]).unwrap()).unwrap(), + u8::try_from(u16::from(participant_is[usize::from(l)])).unwrap(), + ], + ) + }) .collect::>(); - let i = participant_from_i(i); - shares.remove(&i); + shares.remove(&participant_is[i]); processor.send_message(messages::key_gen::ProcessorMessage::Shares { id, shares }).await; } @@ -87,14 +110,22 @@ pub async fn key_gen( wait_for_tributary().await; for (i, processor) in processors.iter_mut().enumerate() { - let i = participant_from_i(i); + let i = participant_is[i]; assert_eq!( processor.recv_message().await, CoordinatorMessage::KeyGen(messages::key_gen::CoordinatorMessage::Shares { id, shares: { let mut shares = (0 .. u8::try_from(COORDINATORS).unwrap()) - .map(|l| (participant_from_i(l.into()), vec![l, u8::try_from(u16::from(i)).unwrap()])) + .map(|l| { + ( + participant_is[usize::from(l)], + vec![ + u8::try_from(u16::from(participant_is[usize::from(l)])).unwrap(), + u8::try_from(u16::from(i)).unwrap(), + ], + ) + }) .collect::>(); shares.remove(&i); shares @@ -172,7 +203,11 @@ pub async fn key_gen( (Public(substrate_key), network_key.try_into().unwrap()) ); - (substrate_priv_key, network_priv_key) + ( + participant_is.into_iter().map(|i| u8::try_from(u16::from(i)).unwrap()).collect(), + substrate_priv_key, + network_priv_key, + ) } #[tokio::test] diff --git a/tests/coordinator/src/tests/sign.rs b/tests/coordinator/src/tests/sign.rs index 1a835b20..4c990338 100644 --- a/tests/coordinator/src/tests/sign.rs +++ b/tests/coordinator/src/tests/sign.rs @@ -29,6 +29,7 @@ use crate::{*, tests::*}; pub async fn sign( processors: &mut [Processor], + processor_is: &[u8], network_key: &Zeroizing, plan_id: [u8; 32], ) { @@ -50,7 +51,7 @@ pub async fn sign( processor .send_message(messages::sign::ProcessorMessage::Preprocess { id: id.clone(), - preprocess: [u8::try_from(i).unwrap(); 64].to_vec(), + preprocess: [processor_is[i]; 64].to_vec(), }) .await; } @@ -63,7 +64,7 @@ pub async fn sign( processors[excluded_signer] .send_message(messages::sign::ProcessorMessage::Preprocess { id: id.clone(), - preprocess: [u8::try_from(excluded_signer).unwrap(); 64].to_vec(), + preprocess: [processor_is[excluded_signer]; 64].to_vec(), }) .await; @@ -76,37 +77,32 @@ pub async fn sign( }) => { assert_eq!(&id, &this_id); assert_eq!(preprocesses.len(), THRESHOLD - 1); - assert!(!preprocesses - .contains_key(&Participant::new(u16::try_from(known_signer).unwrap() + 1).unwrap())); + let known_signer_i = Participant::new(u16::from(processor_is[known_signer])).unwrap(); + assert!(!preprocesses.contains_key(&known_signer_i)); - let mut participants = - preprocesses.keys().map(|p| usize::from(u16::from(*p)) - 1).collect::>(); + let mut participants = preprocesses.keys().cloned().collect::>(); for (p, preprocess) in preprocesses { - assert_eq!(preprocess, vec![u8::try_from(u16::from(p)).unwrap() - 1; 64]); + assert_eq!(preprocess, vec![u8::try_from(u16::from(p)).unwrap(); 64]); } - participants.insert(known_signer); + participants.insert(known_signer_i); participants } _ => panic!("coordinator didn't send back Preprocesses"), }; for i in participants.clone() { - if i == known_signer { + if u16::from(i) == u16::from(processor_is[known_signer]) { continue; } - let processor = &mut processors[i]; + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; let mut preprocesses = participants .clone() .into_iter() - .map(|i| { - ( - Participant::new(u16::try_from(i + 1).unwrap()).unwrap(), - [u8::try_from(i).unwrap(); 64].to_vec(), - ) - }) + .map(|i| (i, [u8::try_from(u16::from(i)).unwrap(); 64].to_vec())) .collect::>(); - preprocesses.remove(&Participant::new(u16::try_from(i + 1).unwrap()).unwrap()); + preprocesses.remove(&i); assert_eq!( processor.recv_message().await, @@ -118,28 +114,25 @@ pub async fn sign( } for i in participants.clone() { - let processor = &mut processors[i]; + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; processor .send_message(messages::sign::ProcessorMessage::Share { id: id.clone(), - share: vec![u8::try_from(i).unwrap(); 32], + share: vec![u8::try_from(u16::from(i)).unwrap(); 32], }) .await; } wait_for_tributary().await; for i in participants.clone() { - let processor = &mut processors[i]; + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; let mut shares = participants .clone() .into_iter() - .map(|i| { - ( - Participant::new(u16::try_from(i + 1).unwrap()).unwrap(), - vec![u8::try_from(i).unwrap(); 32], - ) - }) + .map(|i| (i, vec![u8::try_from(u16::from(i)).unwrap(); 32])) .collect::>(); - shares.remove(&Participant::new(u16::try_from(i + 1).unwrap()).unwrap()); + shares.remove(&i); assert_eq!( processor.recv_message().await, @@ -152,7 +145,8 @@ pub async fn sign( // Send Completed for i in participants.clone() { - let processor = &mut processors[i]; + let processor = + &mut processors[processor_is.iter().position(|p_i| u16::from(*p_i) == u16::from(i)).unwrap()]; processor .send_message(messages::sign::ProcessorMessage::Completed { key: id.key.clone(), @@ -199,7 +193,8 @@ async fn sign_test() { } let mut processors = new_processors; - let (substrate_key, network_key) = key_gen::(&mut processors).await; + let (participant_is, substrate_key, network_key) = + key_gen::(&mut processors).await; // 'Send' external coins into Serai let serai = processors[0].serai().await; @@ -234,6 +229,7 @@ async fn sign_test() { let coin_block = BlockHash([0x33; 32]); let block_included_in = batch( &mut processors, + &participant_is, &substrate_key, Batch { network: NetworkId::Bitcoin, @@ -366,7 +362,7 @@ async fn sign_test() { .await; } - sign::(&mut processors, &network_key, plan_id).await; + sign::(&mut processors, &participant_is, &network_key, plan_id).await; }) .await; }