-
Notifications
You must be signed in to change notification settings - Fork 168
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
Fix IBE in the drand merge #514
Conversation
* Fix a typo * BugFix: use array index instead of node index
group/mod/int.go
Outdated
|
||
// GroupOrder returns the order of the underlying group | ||
func (i *Int) GroupOrder() *big.Int { | ||
return big.NewInt(0).SetBytes(i.M.Bytes()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not returning a clone of i.M
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you mean:
return big.NewInt(0).SetBytes(i.M.Bytes()) | |
return i.Clone().(*Int).M |
but Clone() doesn't do a deep copy of M
, this may be a problem if one were to accidently modify it, for instance the Add
method of big.Int modify the underlying big number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, Clone should probably be doing a deep copy, that might be a bug.
but then
func (z *Int) Set(x *Int) *Int
is probably what to use instead of SetBytes, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
much more concise, sounds good to me, I can look into fixing Clone() as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do in another PR
Quality Gate passedIssues Measures |
* Added circl and kilic implementation of bls12381 + benchmarks * Added deserialization tests compressed vectors +more comments * Fix IBE in the drand merge (#514) * Add ByteOrder() function + constant from int.go * Update circl_bls12381 and edwards25519 scalars * Add Order() to the scalar interface * Adapt IBE to make it general * Fix dkg bug (#515) * BugFix: use array index instead of node index * Fix endianess bool * Add tests for endianess * Fixed deserialization tests + circl issue * Removed groupchecker iface * Added back circl deserialization test (cloudflare/circl#499) * Since Go 1.21 go mod tidy require the go directive to match the highest of our dependencies --------- Co-authored-by: Kilian <79536516+K1li4nL@users.noreply.github.com> Co-authored-by: Yolan Romailler <anomalroil@users.noreply.github.com>
This pull request aims to make the IBE implementation support different pairing suite. To that end it is necessary to add a couple functions to the
Scalar
interface.Edit: cherry-picked the dkg bug-fix commit to make the CI happy.