-
Notifications
You must be signed in to change notification settings - Fork 226
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: Sync from aztec-packages #4630
Closed
Closed
Conversation
This file contains 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
Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore(github): Improve PR template "document later" checkbox description (#4625) chore: Update integers.md to note support for Fields using `from_integer` (#4536) chore: update docs with function names to match version 0.25.0 specifications (#4466) feat: add specific error for attempting `string[x] = ".."` (#4611) fix(ssa): Use accurate type during SSA AsSlice simplficiation (#4610) END_COMMIT_OVERRIDE --------- Co-authored-by: sirasistant <[email protected]>
NOTE: I don't know why this triggered a change in the snapshot. --- This structure lets us easily pass things from the (TS) context to the constructor of the `AvmContext` in Noir, using `calldata` as a vehicle (effectively adding them as public inputs). The choice to add the structure to the function arguments as LAST and not first is because adding them first would break any non-noir-generated bytecode (since they would have to shift their expected calldata by a magic number `N = sizeof(AvmContextInputs)`). Putting it last, makes it transparent for them. A calldatacopy would still work. However, having this makes any external call always have `AvmContextInputs` in the calldata, bloating it (on chain) for non-noir-generated bytecode. This is not an issue now. For the moment, this is temporary, but might be useful long term. (Sean mentioned passing maybe env getters like this). --- Implemented `AvmContext.selector()` and `AvmContext.get_args_hash()` using the above.
NOTE: I don't know why this triggered a change in the snapshot. --- This structure lets us easily pass things from the (TS) context to the constructor of the `AvmContext` in Noir, using `calldata` as a vehicle (effectively adding them as public inputs). The choice to add the structure to the function arguments as LAST and not first is because adding them first would break any non-noir-generated bytecode (since they would have to shift their expected calldata by a magic number `N = sizeof(AvmContextInputs)`). Putting it last, makes it transparent for them. A calldatacopy would still work. However, having this makes any external call always have `AvmContextInputs` in the calldata, bloating it (on chain) for non-noir-generated bytecode. This is not an issue now. For the moment, this is temporary, but might be useful long term. (Sean mentioned passing maybe env getters like this). --- Implemented `AvmContext.selector()` and `AvmContext.get_args_hash()` using the above.
Brillig had implicitely typed memory, where the memory typing was supossedly respected by the compiler (brillig_gen) but was never checked in runtime. Instead, in runtime the values were truncated to the appropiate bit size when used. This hid some bugs in typing that the compiler was outputting and that would have made the AVM crash. Memory typing bugs found and fixed: - to_radix vector length type - to_radix limb type - keccak256 length type (u32 vs u64) - directive quotient not casting arguments - directive invert jump condition on non boolean This PR aligns brillig more with the AVM by having typed memory actually checked in runtime, and removing the truncations that arithmetic.rs was doing.
Brillig had implicitely typed memory, where the memory typing was supossedly respected by the compiler (brillig_gen) but was never checked in runtime. Instead, in runtime the values were truncated to the appropiate bit size when used. This hid some bugs in typing that the compiler was outputting and that would have made the AVM crash. Memory typing bugs found and fixed: - to_radix vector length type - to_radix limb type - keccak256 length type (u32 vs u64) - directive quotient not casting arguments - directive invert jump condition on non boolean This PR aligns brillig more with the AVM by having typed memory actually checked in runtime, and removing the truncations that arithmetic.rs was doing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Automated pull of Noir development from aztec-packages.
BEGIN_COMMIT_OVERRIDE
feat!: Brillig typed memory (AztecProtocol/aztec-packages#5395)
feat(avm): add AvmContextInputs (AztecProtocol/aztec-packages#5396)
feat: Sync from noir (AztecProtocol/aztec-packages#5416)
feat: truncate SHA hashes inside circuits (AztecProtocol/aztec-packages#5160)
END_COMMIT_OVERRIDE