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

Group Order() function signature oddity #214

Closed
chris-wood opened this issue Feb 15, 2021 · 4 comments · Fixed by #356
Closed

Group Order() function signature oddity #214

chris-wood opened this issue Feb 15, 2021 · 4 comments · Fixed by #356
Labels
question Further information is requested

Comments

@chris-wood
Copy link
Contributor

chris-wood commented Feb 15, 2021

@bwesterb points out that this prototype is somewhat strange:

type Group interface {
...
	Order() Scalar
...
}

Since Scalars are integers between 0 and q-1, the output of this function should be zero! Perhaps this just needs to be a big.Int, instead?

@armfazh, what do you think?

@armfazh
Copy link
Contributor

armfazh commented Feb 16, 2021

About returning big.Int: I think returning this type motivates users to perform scalar operations using big.Int, which is not desirable, instead we are already providing Scalar arithmetic operations.

I think the q = Order() function is merely for informational purposes, and not to perform any operation modulo q.
or maybe returning another type be the option.

@armfazh armfazh added the question Further information is requested label Feb 16, 2021
@chris-wood
Copy link
Contributor Author

I think the q = Order() function is merely for informational purposes, and not to perform any operation modulo q.
or maybe returning another type be the option.

I think you may have misunderstood the comment. In the example snippet you wrote -- q = Order() -- q would be 0. That's not very useful, or informational. If we're going to expose an interface that returns the order of the group, then I think we should actually return the order of the group.

@bwesterb
Copy link
Member

I do agree with @armfazh that big.Int isn't great for cryptographic purposes due to timing attacks.

@chris-wood
Copy link
Contributor Author

I do agree with @armfazh that big.Int isn't great for cryptographic purposes due to timing attacks.

That's fair. I think a reasonable thing to do here is to just remove this function from the interface altogether. It's not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants