refactor: cleaning up macros::functions::utils#17840
Merged
Conversation
Contributor
Author
benesjan
force-pushed
the
10-20-refactor_introducing_external_._
branch
from
e0c28ff to
c55306a
Compare
benesjan
force-pushed
the
10-21-refactor_cleaning_up_macros_functions_utils
branch
3 times, most recently
from
670931d to
f3f7fbd
Compare
benesjan
commented
Oct 21, 2025
noir-projects/aztec-nr/aztec/src/macros/functions/call_interface_stubs.nr
Outdated
Show resolved
Hide resolved
benesjan
commented
Oct 21, 2025
benesjan
commented
Oct 21, 2025
noir-projects/aztec-nr/aztec/src/macros/functions/call_interface_stubs.nr
Show resolved
Hide resolved
benesjan
commented
Oct 21, 2025
noir-projects/aztec-nr/aztec/src/macros/functions/call_interface_stubs.nr
Show resolved
Hide resolved
benesjan
force-pushed
the
10-20-refactor_introducing_external_._
branch
from
c977176 to
1c35ebe
Compare
benesjan
force-pushed
the
10-21-refactor_cleaning_up_macros_functions_utils
branch
2 times, most recently
from
95f7457 to
17acac1
Compare
nventuro
approved these changes
Oct 22, 2025
Contributor
nventuro
left a comment
nventuro
left a comment
There was a problem hiding this comment.
Agree with your comments re. how this simplifies things. Also greatly appreciate how in the description you went over your thinking process, it made reading this much easier. Thanks!
noir-projects/aztec-nr/aztec/src/macros/functions/call_interface_stubs.nr
Outdated
Show resolved
Hide resolved
noir-projects/aztec-nr/aztec/src/macros/functions/call_interface_stubs.nr
Outdated
Show resolved
Hide resolved
noir-projects/aztec-nr/aztec/src/macros/functions/stub_registry.nr
Outdated
Show resolved
Hide resolved
benesjan
force-pushed
the
10-20-refactor_introducing_external_._
branch
from
b2aacae to
f77b39a
Compare
benesjan
force-pushed
the
10-21-refactor_cleaning_up_macros_functions_utils
branch
3 times, most recently
from
8a7cbc8 to
abe4d5c
Compare
benesjan
force-pushed
the
10-20-refactor_introducing_external_._
branch
from
7ecca22 to
24854f2
Compare
AztecBot
pushed a commit
that referenced
this pull request
Oct 23, 2025
Closes https://linear.app/aztec-labs/issue/F-19/rename-privateorpublicorutility-to-externalprivateorpublicorutility Having `#[private]` and `#[public]` macros was misleading because in Solidity these define function visibility while in Aztec's case they define the environment in which the functions are executed. For this reason I introduce `#[external(...)]` macro while the argument of the attribute defines the execution environment: `private`, `public` or `utility`. ## Notes for reviewer - I did a refactor in a [PR up the stack](#17840) which allowed me to drop a lot of the complexity introduced in this PR. For this reason I encourage you to check that one first to have context on what in this PR is a problem and what is not. It might make sense to just merge these 2 PRs into 1 but I think that would make it all harder to review. - This PR touches a lot of files but all the juicy bits are in the `noir-projects/aztec-nr/aztec/src/macros/` directory so I encourage you to just check that first. The `noir-contracts` dir can be completely ignored as there I just update the imports and the macros. - The new naming is now in conflict with the `#[internal]` macro as having both external and internal applied to a function is valid: <img width="874" height="182" alt="image" src="https://github.com/user-attachments/assets/4582a51a-225c-4a4c-9b9a-d2c2d413d927" /> This issue will be tackled in the near future. Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
AztecBot
force-pushed
the
10-20-refactor_introducing_external_._
branch
from
24854f2 to
711bbee
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 24, 2025
Closes https://linear.app/aztec-labs/issue/F-19/rename-privateorpublicorutility-to-externalprivateorpublicorutility Having `#[private]` and `#[public]` macros was misleading because in Solidity these define function visibility while in Aztec's case they define the environment in which the functions are executed. For this reason I introduce `#[external(...)]` macro while the argument of the attribute defines the execution environment: `private`, `public` or `utility`. ## Notes for reviewer - I did a refactor in a [PR up the stack](#17840) which allowed me to drop a lot of the complexity introduced in this PR. For this reason I encourage you to check that one first to have context on what in this PR is a problem and what is not. It might make sense to just merge these 2 PRs into 1 but I think that would make it all harder to review. - This PR touches a lot of files but all the juicy bits are in the `noir-projects/aztec-nr/aztec/src/macros/` directory so I encourage you to just check that first. The `noir-contracts` dir can be completely ignored as there I just update the imports and the macros. - The new naming is now in conflict with the `#[internal]` macro as having both external and internal applied to a function is valid: <img width="874" height="182" alt="image" src="https://github.com/user-attachments/assets/4582a51a-225c-4a4c-9b9a-d2c2d413d927" /> This issue will be tackled in the near future.
Base automatically changed from
10-20-refactor_introducing_external_._
to
next
October 24, 2025 00:21
benesjan
force-pushed
the
10-21-refactor_cleaning_up_macros_functions_utils
branch
from
abe4d5c to
caa7589
Compare
This PR was quite a journey: ## Step 1 When trying to figure out how to rename the incorrectly named `get_fn_visibility` in a PR down the stack I realized that it's better to just drop it because: - it was only used by `create_assert_correct_initializer_args`, `create_mark_as_initialized` and `create_init_check` functions which in turn where only called in `transfrom_private` and `transform_public`, - these functions contained only 2 lines of code. Because of this ^ it's better to just directly inline contents of these funcs into the `transform_*` functions. When I finished this I checked where are the is `is_fn_private` and `is_fn_public` utils used which led me to step 2: ## Step 2 When checking when are the above mentioned functions used I realized that we use them in `create_authorize_once_check` and in the `stub_fn`. In both of these cases the code was just terrible because these functions are called from `transform_private` and `transform_public` functions hence the flow was: `transform_private` --> `stub_fn` --> are we in private? --> if yes call `create_private_stub` which I could replace with: `transform_private` --> `create_private_stub` So I dropped the `stub_fn` and just called the `create_private_stub` directly. The similar was the case for `create_authorize_once_check` where I could simply pass in an `is_private` arg to the function and get rid of the `is_fn_private` dependency. ## Step 3 Once I did this I realized that I don't need the `EXTERNAL_REGISTRY` I introduced in the PR down the stack at all, removing a lot of the sad ugly complexity I needed to introduce there 🎉 ## Future work - The whole `noir-projects/aztec-nr/aztec/src/macros/functions` directory is very messy and I plan on move the code around into more logical places in the dir once this stack of PRs is merged. - I think the `stub` name is non-descriptive enough so will try to come up with something better. Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
AztecBot
force-pushed
the
10-21-refactor_cleaning_up_macros_functions_utils
branch
from
caa7589 to
5250359
Compare
benesjan
deleted the
10-21-refactor_cleaning_up_macros_functions_utils
branch
This was referenced Oct 24, 2025
ludamad
pushed a commit
that referenced
this pull request
Dec 16, 2025
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
3 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.
This PR was quite a journey:
Step 1
When trying to figure out how to rename the incorrectly named
get_fn_visibilityin a PR down the stack I realized that it's better to just drop it because:create_assert_correct_initializer_args,create_mark_as_initializedandcreate_init_checkfunctions which in turn where only called intransfrom_privateandtransform_public,Because of this ^ it's better to just directly inline contents of these funcs into the
transform_*functions.When I finished this I checked where are the is
is_fn_privateandis_fn_publicutils used which led me to step 2:Step 2
When checking when are the above mentioned functions used I realized that we use them in
create_authorize_once_checkand in thestub_fn. In both of these cases the code was just terrible because these functions are called fromtransform_privateandtransform_publicfunctions hence the flow was:transform_private-->stub_fn--> are we in private? --> if yes callcreate_private_stubwhich I could replace with:
transform_private-->create_private_stubSo I dropped the
stub_fnand just called thecreate_private_stubdirectly.The similar was the case for
create_authorize_once_checkwhere I could simply pass in anis_privatearg to the function and get rid of theis_fn_privatedependency.Step 3
Once I did this I realized that I don't need the
EXTERNAL_REGISTRYI introduced in the PR down the stack at all, removing a lot of the sad ugly complexity I needed to introduce there 🎉Future work
noir-projects/aztec-nr/aztec/src/macros/functionsdirectory is very messy and I plan on move the code around into more logical places in the dir once this stack of PRs is merged.stubname is non-descriptive enough so will try to come up with something better.