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.

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

Expose subgroup check for signatures #35

Closed
wants to merge 2 commits into from

Conversation

kirk-baird
Copy link
Contributor

What has been changed

Adds the binding to rust which allows the subgroup check to be made for signatures.

unsafe {
$sig_to_aff(&mut sig, &self.point);
$sig_in_group(&sig)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is a "philosophical" problem. Why would one need such a method? Customarily you'd need to perform the group check on incoming data, i.e. right after deserialization. But then it's affine coordinates... Yes, one can say "this is for completeness or symmetry." I for one have kind of a problem with such approach, because it effectively goes against the spirit of security. I mean we should concentrate on making the least amount of code work as well as possible, as opposed to producing as much [symmetric] code as possible. Now, from the group-check algorithm's perspective input doesn't have to be affine. So solving it the suggested way is a waste, and not insignificant, because inversion is ~1/3-1/4 of the group-check. So how come the interface takes affine? It was argued that since input is always affine (straight after deserialization), providing an affine-only interface is sufficient. To summarize, if there is a legitimate need for group-checking non-affine points, then it's not a proper way to solve this. Proper way would be to add additional C interface...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've decided to do subgroup checking for Signatures/AggregateSignatures at verification time rather than deserialisation time. Our reasoning is we deserialise a large quantity of signatures from the database for slashing protection and the slasher, where a large proportion will not be verified (and/or have already been verified). So it was wasteful to do the subgroup check at deserialisation time.

In terms of converting it to affine I agree this is inefficient and if it is to be kept should use a method with takes projective points.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Straight after deserialization" doesn't have to be literal. I should have used "from" instead of "after," right? I mean question is how come out-of-deserialization affine point becomes non-affine at all, not necessarily the moment next to deserialization.

Again, if there is a legitimate need for group-checking non-affine points, there is better way than suggested. In other words question is is there actual need?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the need is there as most of the signatures we have in our database are AggregateSignatures and when we do eventually use them it will use the aggregate verify functions.

We do deserialise them as Signatures first and then convert them to AggregateSignatures though this is all during the deserialisation process. It would add a fair bit of complexity to store them as Signatures (affine) until the first verification and then convert them to AggregateSignatures (projective) keeping them as projective afterwards for any future aggregations/verifications.

So it would be preferable to have a projective version of the subgroup checks for AggregateSignatures but we can work around it if need be.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Give me some time then:-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :) no rush, whenever you've got time is good.

@dot-asm
Copy link
Collaborator

dot-asm commented Nov 25, 2020

Solution is in #44. Closing this. Thanks!

@dot-asm dot-asm closed this Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants