Skip to content

chore: add some macro checks#16149

Merged
benesjan merged 2 commits intonextfrom
nv/macro-cehcks
Aug 1, 2025
Merged

chore: add some macro checks#16149
benesjan merged 2 commits intonextfrom
nv/macro-cehcks

Conversation

@nventuro
Copy link
Contributor

These are just some extra checks to try to prevent incorrect usage of these.

@nventuro nventuro requested a review from benesjan July 31, 2025 20:48

#[utility]
pub(crate) unconstrained fn private_balance_of(owner: AztecAddress) -> u128 {
unconstrained fn private_balance_of(owner: AztecAddress) -> u128 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pulled a Jan here: we no longer need for these functions to be pub as we no longer invoke them directly. Indeed we want to use noir-lang/noir#9363 to prevent that, but we don't yet have this feature because of the noir sync issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

😠

@benesjan
Copy link
Contributor

benesjan commented Aug 1, 2025

@nventuro The l2_to_l1 test was failing and this fixed it.

In the function transformations we inject things like PrivateContextInputs into function args and that didn't play well with create_fn_abi_export. This should have been documented.

@benesjan benesjan enabled auto-merge August 1, 2025 07:30
Copy link
Contributor

@benesjan benesjan left a comment

Choose a reason for hiding this comment

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

Very nice!

Reason is returning to macros. Having the checks be in the transform functions was disgusting


#[utility]
pub(crate) unconstrained fn private_balance_of(owner: AztecAddress) -> u128 {
unconstrained fn private_balance_of(owner: AztecAddress) -> u128 {
Copy link
Contributor

Choose a reason for hiding this comment

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

😠

@benesjan benesjan added this pull request to the merge queue Aug 1, 2025
Merged via the queue into next with commit 894442c Aug 1, 2025
4 checks passed
@benesjan benesjan deleted the nv/macro-cehcks branch August 1, 2025 08:21
github-merge-queue bot pushed a commit that referenced this pull request Aug 8, 2025
Documenting this function as it should have been documented in the first
place.

[This
bug](#16149 (comment))
motivated me to do it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants