-
Notifications
You must be signed in to change notification settings - Fork 161
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
test: Add Falcon signature tests. #1257
test: Add Falcon signature tests. #1257
Conversation
The sentence describing the column $k_0$ had its order reversed, and the table headers were incorrectly named as $x_i$ and $y_i$ instead of $a_i$ and $b_i$ as described in the introduction.
…std-asm-utils Introduce `std::utils` assembly module
… info This commit changes the `import_info` fields of ProgramAst and ModuleAst to remove the `Option` wrapper, since it does not appear to serve a specific purpose, and both complicates access to the imports, and requires redundant code in a few places to access common information from the imports. It also required fallible checks in a few places where empty module import info would be suitable as a fallback. After this change, one can access the ModuleImports struct directly on both ProgramAst and ModuleAst via the `import_info` function, which returns a reference to the field. Redundant functions in both structs were removed if they are already provided by ModuleImports.
…c-fix docs(mdbook): minor change in the bitwise chiplet
…and-debug Expand capabilities of the `debug` instruction
…orconfig add .editorconfig
…oc-fix Minor doc fixes
…oc-fix-logup Minor doc fix LogUp
…host-event-handler Add `emit` instruction and host event handler
…precommit-sync Chore: pre-commit sync
* ContextId type * propagate MemoryContextId * fix tests * MemoryContextId -> ContextId * move definition * Remove derive-more dependency * fix repl * fmt * docstring * import newline * Remove `Sub` from `ContextId` * blank line * section name change * blank line * use `ContextId::root()` * fmt * memory context -> execution context
…-sig-tests Merge local and next branch.
…-sig-tests Merge local and next branch.
Up to date with next and merge conflicts gone. |
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.
Looks great, thank you @scottdieringer !
Ready for review. |
@bobbinth Merged the latest from miden-crypto 0.9.0 into this PR. Ready for review. |
@scottdieringer thank you! Will review today - sorry for all the delays! |
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.
Looks great! Thank you! (and sorry again for all the delays). I left a few comments inline - but nothing major.
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.
Looks great! Thanks again @scottdieringer for doing this
@bobbinth Addressed PR comments, ready for review. |
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.
Looks good! Thank you! I left one tiny nit inline - but we can merge even without it.
Describe your changes
Addresses 1096 - Adding Falcon signature tests.
Adds following tests:
norm_sq
procedure, both overflow and underflow.diff_mod_q
procedure.powers_of_tau
procedure.load_h_s2_and_product
procedure to theprobabilistic_product
procedure. Both success and failure cases.adv.push_sig.rpo_falcon512
procedure. Both success and failure cases.Checklist before requesting a review
next
according to naming convention.