address issues raised by Nitin#105
Conversation
| @@ -1 +1 @@ | |||
| {"parties":"3", "threshold":"1"} | |||
| {"parties":"3", "threshold":"2"} | |||
There was a problem hiding this comment.
Any specific reason to change the threshold to 2 here ?
There was a problem hiding this comment.
No, I changed it while testing to be consistent with the assumption. Though I don't thing the threshold value is meaningful
There was a problem hiding this comment.
threshold value is meaningful but not for this demo. So can leave it as is
| let aad: [u8; 0] = []; | ||
| let mut gcm = AesGcm::new(KeySize256, key, &nonce[..], &aad); | ||
| gcm.decrypt(&aead_pack.ciphertext[..], &mut out, &aead_pack.tag[..]); | ||
| let successful_decrypt = gcm.decrypt(&aead_pack.ciphertext[..], &mut out, &aead_pack.tag[..]); |
There was a problem hiding this comment.
This probably can just be written as:
if !gcm.decrypt(&aead_pack.ciphertext[..], &mut out, &aead_pack.tag[..]) {
panic!("Error: Could not decrypt AES ciphertext");
}
There was a problem hiding this comment.
It's a matter of style, but I can change it as you suggested
| } | ||
|
|
||
| assert_eq!(signers_vec.len(), (THRESHOLD + 1) as usize); | ||
| if signers_vec.len()> (THRESHOLD + 1) as usize{ |
There was a problem hiding this comment.
I think the assert_eq! can just be removed and the if can be changed to:
if signers_vec.len() != (THRESHOLD + 1) as usize{
panic!("For the demo we always sign with minimum number of parties(THRESHOLD + 1). This should not happen.");
}
There was a problem hiding this comment.
Why minimum number of parties, shouldn't it be exactly THRESHOLD + 1?
There was a problem hiding this comment.
that's what I meant by minimum number of parties. You need minimum THRESHOLD + 1 signers to generate a usable signature.
Also this comment is just for style. We don't need an assert and an if statement. Could just use an if statement only.
But that should not stop you from merging this as is.
2 participants
No description provided.