Conversation
…tion (#21189) This is the first part of my review of domain separator usage and hashing. I created two standard functions for computing note hashes and note nullifiers in aztecnr, and added some comments to the separators themselves. In a future PR we'll move them to aztec-nr (not now as it'd be api breakage). I also fixed `SingleUseClaim` by having it use a new dedicated domain separator, as should've always been the case given that it is not a note nullifier - it is its own thing. I added comments to the protocol contracts explaining why the lack of separators there is ok, and found an issue in our Orderbook contract in which we miss domain separation, which is wrong - left a todo. Finally, there's also a todo to fix how partial notes compute both of their note hashes, as they dont' follow the current domain separation. Given we're about to rework them I chose not to mess with it at this time. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: benesjan <benesjan@users.noreply.github.com> Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
In this PR I ensure that contracts get automatically recompiled when `aztec test` is run. To avoid unnecessary compilations I use `make` command inspired approach of checking timestamp of source files (*.toml and *.nr files) against the timestamp of the oldest artifact in the `/target` dir (as artifact I consider any `*.json` file). **Note:** By using the oldest timestamp we would get constant recompilation on each run in case the dev placed there some random `*.json` file. **Question**: Does the reviewer think this ^ is fine? Alternatively I could nuke the `target` dir whenever a recompilation is being run. This feels a bit dangerous though. Another approach would be to predict the artifact name of individual crates and just check against those. This would be the most efficient but it introduces complexity and brittleness in case `nargo` artifact names change. I think I would keep it as it is for now and come up with solution if someone complains about constant recompilations. I initially thought I will implement this in the `aztec test` command but that command is fully implemented in bash. When I had AI implement this the resulting bash was impossible to comprehend so I told it to rewrite in typescript. Given that the `aztec test` command is calling `aztec compile` that is implemented in typescript it made sense for this PR to modify only that command. This has the added advantage that it will make bugs in this code way more visible because when a dev runs `aztec compile` they will either see that the code is getting re-compiled or they will get "No source changes detected, skipping compilation." log. If they modify a contract, run `aztec compile` and see that nothing got compiled it will be obvious that the thing is broken. Closes https://linear.app/aztec-labs/issue/F-197/auto-recompile-contracts-on-tests --------- Co-authored-by: benesjan <benesjan@users.noreply.github.com> Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Just a small clarification.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BEGIN_COMMIT_OVERRIDE
feat: add note hash and nullifier helper functions with domain separation (#21189)
feat(cli): auto-recompiling when aztec test is run (#20729)
docs: small delayedpubmut update (#21229)
END_COMMIT_OVERRIDE