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

fix: cleaner sign share request implementation #114

Merged
merged 4 commits into from
Jan 14, 2025
Merged

fix: cleaner sign share request implementation #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b86fad3
Better sign share function
djordon Dec 24, 2024
1ee8680
update the test
djordon Dec 24, 2024
f31898d
remove extraneous assert; fix boolean cache name; use short name when…
xoloki Jan 14, 2025
d70f355
test SignatureShareRequest with no NonceResponse and out of range sig…
xoloki Jan 14, 2025
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
53 changes: 46 additions & 7 deletions src/state_machine/coordinator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1117,19 +1117,58 @@ pub mod test {
}
}

// add NonceResponses so there's more than the number of signers
let messages = outbound_messages.clone();
let result = feedback_messages_with_errors(&mut coordinators, &mut signers, &messages);
assert!(result.is_ok());

// test request with no NonceResponses
let mut packet = outbound_messages[0].clone();
if let Message::SignatureShareRequest(ref mut request) = packet.msg {
for _ in 0..1000 {
request
.nonce_responses
.push(request.nonce_responses.first().unwrap().clone());
}
request.nonce_responses.clear();
} else {
panic!("failed to match message");
}

// Send the SignatureShareRequest message to all signers and share their responses with the coordinator and signers
// Send the SignatureShareRequest message to all signers and share
// their responses with the coordinator and signers
let result = feedback_messages_with_errors(&mut coordinators, &mut signers, &[packet]);
if !matches!(
result,
Err(StateMachineError::Signer(SignerError::InvalidNonceResponse))
) {
panic!("Should have received signer invalid nonce response error, got {result:?}");
}

// test request with a duplicate NonceResponse
let mut packet = outbound_messages[0].clone();
if let Message::SignatureShareRequest(ref mut request) = packet.msg {
request
.nonce_responses
.push(request.nonce_responses[0].clone());
} else {
panic!("failed to match message");
}

// Send the SignatureShareRequest message to all signers and share
// their responses with the coordinator and signers
let result = feedback_messages_with_errors(&mut coordinators, &mut signers, &[packet]);
if !matches!(
result,
Err(StateMachineError::Signer(SignerError::InvalidNonceResponse))
) {
panic!("Should have received signer invalid nonce response error, got {result:?}");
}

// test request with an out of range signer_id
let mut packet = outbound_messages[0].clone();
if let Message::SignatureShareRequest(ref mut request) = packet.msg {
request.nonce_responses[0].signer_id = num_signers;
} else {
panic!("failed to match message");
}

// Send the SignatureShareRequest message to all signers and share
// their responses with the coordinator and signers
let result = feedback_messages_with_errors(&mut coordinators, &mut signers, &[packet]);
if !matches!(
result,
Expand Down
128 changes: 56 additions & 72 deletions src/state_machine/signer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,91 +563,75 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
sign_request: &SignatureShareRequest,
rng: &mut R,
) -> Result<Vec<Message>, Error> {
let mut msgs = vec![];

let signer_ids = sign_request
.nonce_responses
.iter()
.map(|nr| nr.signer_id)
.collect::<Vec<u32>>();

let signer_id_set = sign_request
.nonce_responses
.iter()
.map(|nr| nr.signer_id)
.collect::<BTreeSet<u32>>();

if signer_ids.len() != signer_id_set.len()
// The expected usage is that Signer IDs start at zero and
// increment by one until self.total_signers - 1. So the checks
// here should be sufficient for catching empty signer ID sets,
// duplicate signer IDs, or unknown signer IDs.
let is_invalid_request = sign_request.nonce_responses.len() != signer_id_set.len()
|| signer_id_set.is_empty()
|| signer_id_set.len() > self.total_signers.try_into().unwrap()
|| signer_id_set.last().unwrap() > &self.total_signers
{
warn!(
?signer_ids,
"Got SignatureShareRequest with invalid NonceResponse"
);
|| signer_id_set.last() >= Some(&self.total_signers);

if is_invalid_request {
warn!("received an invalid SignatureShareRequest");
return Err(Error::InvalidNonceResponse);
} else {
debug!(?signer_ids, "Got SignatureShareRequest");
}

for signer_id in &signer_ids {
if *signer_id == self.signer_id {
let key_ids: Vec<u32> = sign_request
.nonce_responses
.iter()
.flat_map(|nr| nr.key_ids.iter().copied())
.collect::<Vec<u32>>();
let nonces = sign_request
.nonce_responses
.iter()
.flat_map(|nr| nr.nonces.clone())
.collect::<Vec<PublicNonce>>();
let signature_shares = match sign_request.signature_type {
SignatureType::Taproot(m) => self.signer.sign_taproot(
&sign_request.message,
&signer_ids,
&key_ids,
&nonces,
m,
),
SignatureType::Schnorr => self.signer.sign_schnorr(
&sign_request.message,
&signer_ids,
&key_ids,
&nonces,
),
SignatureType::Frost => {
self.signer
.sign(&sign_request.message, &signer_ids, &key_ids, &nonces)
}
};
debug!(signer_id = %self.signer_id, "received a valid SignatureShareRequest");

if signer_id_set.contains(&self.signer_id) {
let key_ids: Vec<u32> = sign_request
.nonce_responses
.iter()
.flat_map(|nr| nr.key_ids.iter().copied())
.collect::<Vec<u32>>();
let nonces = sign_request
.nonce_responses
.iter()
.flat_map(|nr| nr.nonces.clone())
.collect::<Vec<PublicNonce>>();

let signer_ids = signer_id_set.into_iter().collect::<Vec<_>>();
let msg = &sign_request.message;
let signature_shares = match sign_request.signature_type {
SignatureType::Taproot(merkle_root) => {
self.signer
.sign_taproot(msg, &signer_ids, &key_ids, &nonces, merkle_root)
}
SignatureType::Schnorr => {
self.signer
.sign_schnorr(msg, &signer_ids, &key_ids, &nonces)
}
SignatureType::Frost => self.signer.sign(msg, &signer_ids, &key_ids, &nonces),
};

self.signer.gen_nonces(rng);
self.signer.gen_nonces(rng);

let response = SignatureShareResponse {
dkg_id: sign_request.dkg_id,
sign_id: sign_request.sign_id,
sign_iter_id: sign_request.sign_iter_id,
signer_id: *signer_id,
signature_shares,
};
info!(
%signer_id,
dkg_id = %sign_request.dkg_id,
sign_id = %sign_request.sign_id,
sign_iter_id = %sign_request.sign_iter_id,
"sending SignatureShareResponse"
);

let response = Message::SignatureShareResponse(response);

msgs.push(response);
} else {
debug!("SignatureShareRequest for {} dropped.", signer_id);
}
let response = SignatureShareResponse {
dkg_id: sign_request.dkg_id,
sign_id: sign_request.sign_id,
sign_iter_id: sign_request.sign_iter_id,
signer_id: self.signer_id,
signature_shares,
};
info!(
signer_id = %self.signer_id,
dkg_id = %sign_request.dkg_id,
sign_id = %sign_request.sign_id,
sign_iter_id = %sign_request.sign_iter_id,
"sending SignatureShareResponse"
);

Ok(vec![Message::SignatureShareResponse(response)])
} else {
debug!(signer_id = %self.signer_id, "signer not included in SignatureShareRequest");
Ok(Vec::new())
}
Ok(msgs)
}

fn dkg_begin<R: RngCore + CryptoRng>(
Expand Down
Loading