Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Non-canonical infinity point & bad flags in BLS12-381 serialization should fail #176

Merged
merged 13 commits into from
Sep 11, 2023

Conversation

mmagician
Copy link
Member

Description

Follow-up to #157, the original is stale.

Thanks @ArnaudBrousseau for providing a test for bad flags (lmk if you'd like me to add you as co-author on the commit here)

closes: #157


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

Copy link

@ArnaudBrousseau ArnaudBrousseau left a comment

Choose a reason for hiding this comment

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

The new flag logic and tests look good to me! Thanks for taking this on so quickly @mmagician!

@Pratyush
Copy link
Member

Pratyush commented Sep 8, 2023

Thanks for the great PR @mmagician! Could you add a CHANGELOG entry as this is an important fix? Thanks!

@mmagician
Copy link
Member Author

@Pratyush It's actually a breaking change no?

@Pratyush
Copy link
Member

In Rust breaking changes are only when the API changes, not necessarily behaviour

@mmagician
Copy link
Member Author

@Pratyush Thanks, I moved it to Bugfixes.

@Pratyush Pratyush merged commit 7e4b4bf into arkworks-rs:master Sep 11, 2023
33 checks passed
Shigoto-dev19 pushed a commit to Shigoto-dev19/ark-babyjub that referenced this pull request Sep 23, 2023
…hould fail (arkworks-rs#176)

Co-authored-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants