Skip to content

address issues raised by Nitin #105

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

address issues raised by Nitin #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f9aa08d
adressing issues
doronz2 Dec 10, 2020
ec30669
adressing issues
doronz2 Dec 10, 2020
f6fc519
adressing issues
doronz2 Dec 10, 2020
508438c
adressing issues
doronz2 Dec 10, 2020
9acaf03
adressing issues
doronz2 Dec 10, 2020
4727fa8
adressing issues
doronz2 Dec 10, 2020
ba4fbc1
adressing issues
doronz2 Dec 10, 2020
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
5 changes: 4 additions & 1 deletion examples/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ pub fn aes_decrypt(key: &[u8], aead_pack: AEAD) -> Vec<u8> {
let nonce: Vec<u8> = repeat(3).take(12).collect();
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[..]);
Copy link
Contributor

Choose a reason for hiding this comment

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

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");
        }

Copy link
Author

@doronz2 doronz2 Dec 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It's a matter of style, but I can change it as you suggested

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed.

if !successful_decrypt {
panic!("Error: Could not decrypt AES ciphertext");
}
out
}

Expand Down
5 changes: 5 additions & 0 deletions examples/gg20_sign_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ fn main() {
j += 1;
}
}

assert_eq!(signers_vec.len(), (THRESHOLD + 1) as usize);
if signers_vec.len()> (THRESHOLD + 1) as usize{
Copy link
Contributor

Choose a reason for hiding this comment

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

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.");
}

Copy link
Author

Choose a reason for hiding this comment

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

Why minimum number of parties, shouldn't it be exactly THRESHOLD + 1?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

panic!("The assumption is that number of shares = threshold + 1");
}
let mut enc_key: Vec<Vec<u8>> = vec![];
for (i, k) in signers_vec.iter().enumerate() {
if *k != signers_vec[party_num_int as usize - 1] as usize {
Expand Down
2 changes: 1 addition & 1 deletion params.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"parties":"3", "threshold":"1"}
{"parties":"3", "threshold":"2"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason to change the threshold to 2 here ?

Copy link
Author

Choose a reason for hiding this comment

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

No, I changed it while testing to be consistent with the assumption. Though I don't thing the threshold value is meaningful

Copy link
Contributor

Choose a reason for hiding this comment

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

threshold value is meaningful but not for this demo. So can leave it as is