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

Jump to bottom

Is full PublicKeySet really needed to combine signature/decryption shares? #103

Closed
madadam opened this issue May 20, 2020 · 5 comments · May be fixed by #119
Closed

Is full PublicKeySet really needed to combine signature/decryption shares? #103

madadam opened this issue May 20, 2020 · 5 comments · May be fixed by #119

Comments

@madadam
Copy link
Contributor

madadam commented May 20, 2020

It seems the only piece of info used is the threshold: https://github.com/poanetwork/threshold_crypto/blob/master/src/lib.rs#L606, https://github.com/poanetwork/threshold_crypto/blob/master/src/lib.rs#L616

Would an alternative API that only takes the threshold make sense?

fn combine_signatures<'a, T, I>(threshold: usize, shares: I) -> Result<Signature>
    where I: IntoIterator<Item = (T, &'a SignatureShare)>, ... 
{}

fn decrypt<'a, T, I>(threshold: usize, shares: I) -> Result<Vec<u8>>
    where I: IntoIterator<Item = (T, &'a DecryptionShare)>, ... 
{}

This way people wouldn't have to store the full PublicKeySet which is kind of expensive.

Would you guys accept a PR implementing this?
@dirvine dirvine mentioned this issue May 25, 2020
Merged
@afck
Copy link
Collaborator

afck commented May 25, 2020 •
edited
Loading

That makes sense. I'm happy to review it.

But note that to validate signature shares you still need the PublicKeySet. And if you don't validate them and a combined signature doesn't match, you don't know which of the shares was wrong.
(Well… the public key shares also suffice to validate the signature shares, of course, so I guess you don't need the PublicKeySet.)

@madadam
Copy link
Contributor Author

madadam commented May 26, 2020

That's a good point which I didn't consider and which makes this whole idea significantly less useful for our needs. Closing the issue for now.

@madadam madadam closed this as completed May 26, 2020
@niko-ng
Copy link

niko-ng commented Aug 17, 2024 •
edited
Loading

Hello,
It seems that eventually @afck backtracked on the uselessness of this proposal.
I am also considering that only the threshold (degree of the commitment) is needed in order to combine_signatures.

It seems no change in the API was made eventually.
Would you consider adding the APIs that @madadam proposed?

@niko-ng niko-ng mentioned this issue Aug 18, 2024
Open
@afck
Copy link
Collaborator

afck commented Aug 18, 2024

(Sorry, I'm not on this project anymore. Not sure if it's currently maintained.)

@niko-ng
Copy link

niko-ng commented Aug 19, 2024

Thanks for your feedback! I will create a fork and publish it to crates.io, as the maintainer seems to have disappeared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

added combine_signatures_with_threshold nextgraph-org/threshold_crypto
3 participants
@madadam @afck @niko-ng