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

feat(noir): better NoteGetterOptions. #1695

Merged
merged 9 commits into from
Aug 22, 2023
Merged

Conversation

LeilaWang
Copy link
Contributor

@LeilaWang LeilaWang commented Aug 21, 2023

  • Allow to select notes from db that match the given values.
  • Better NoteGetterOptions syntax: NoteGetterOptions::new().select(1, val).sort(0, SortOrder.DESC).set_limit(2).set_offset(10).
  • Add unconstrained view_notes api to Set.
  • Add unconstrained view_note api to singletons.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

NADA: u8,
DESC: u2,
ASC: u2,
NADA: u2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sticking with 'nada'? haha

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 don't have strong opinion on this. Any suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, haha.

@@ -64,13 +66,20 @@ impl<Note, N> Set<Note, N> {
destroy_note(context, self.storage_slot, note, self.note_interface);
}

fn get_notes<S, P>(
fn get_notes<FILTER_ARGS>(
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably add a comment to say where to read more info about FILTER_ARGS - perhaps a link to the NoteGetterOptions file and a link to the docs?
I'm just thinking, if someone reads this Set method without having seen examples, they'll not know how to construct this template argument without help.

@LeilaWang LeilaWang changed the title Refactor NoteGetterOptions. feat(noir): better NoteGetterOptions. Aug 21, 2023
@LeilaWang LeilaWang merged commit 2f78293 into master Aug 22, 2023
2 checks passed
@LeilaWang LeilaWang deleted the lw/note_getter_options branch August 22, 2023 10:58
codygunton pushed a commit that referenced this pull request Jan 23, 2024
* Account bug fix (#1490)

* fix account bug - remove y coordinate for public key nullifier

* fix account bug in all relevant places in barretenberg

* fix note_algorithms test

* account circuit doc update for nullifier Y coord removal (account bug)

* Added account nullifier content in notes_and_nullifiers.md. Fixed up some formatting in account_circuit.md and possibly fixed a typo

* fixed up formatting in account circuit and notes/nullifiers docs

* updated Verification Key since circuits were updated

* fix: Build verification keys

* fix: update circuit constants

* reverting the root rollup constants changes

* fix: update vk_hash in UpgradeV2 + fmt

* Bigfield formula fix for single division (#1610)

* Added division formula bug test

* Fixed the single div bug by adding an unreduced zero to the formula

* Trigger rebuild

* Updated root rollup size

* Update rollup names and add tests to more jobs

* Updated some hashes and 1 test

* Updated hashes and gate counts

* Updated rollup gates

* Revert adding extra information to log

* Remove the cosmetic change

* Updated hashes

* fix: generate vks

* fix: update `PROD_VK_HASH` in UpgradeV2

* check signing key on curve, and rename grumpkin.one (#1781)

* check signing key on curve, and rename grumpkin.one

* Rm commented-out line

* update gate count & vkhash of js circuit

* update vkhashes of all rollup circuits

* Add `on_curve` check in stdlib point.

* Update circuit keys.

* Change root verifier circuit vk hash.

* fix: generate vk contracts + upgrade prod_key_hash

Co-authored-by: Suyash Bagad <suyashnbagad1997@gmail.com>
Co-authored-by: LHerskind <lasse.herskind@gmail.com>

Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: arijitdutta67 <arijit.dutta67@gmail.com>
Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com>
Co-authored-by: Michael Connor <mike@aztecprotocol.com>
Co-authored-by: Suyash Bagad <suyashnbagad1997@gmail.com>
codygunton pushed a commit that referenced this pull request Jan 23, 2024
codygunton pushed a commit that referenced this pull request Jan 23, 2024
codygunton pushed a commit that referenced this pull request Jan 23, 2024
Revert "Revert "Verification Key changes (#1695)""
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants