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

chore: remove knowledge of aztec function types from compiler #4975

Conversation

TomAFrench
Copy link
Member

This PR removes the concept of a function being internal or "open" from the Noir compiler as these are now concepts which are aztec specific. We instead attach all of a contract functions custom attributes to it in the build artifact so aztec can parse these to determine whether a function is open or secret, etc.

@spalladino This follows on from #4967 but I've made this as a separate PR into yours as I don't want to hijack it.

@TomAFrench
Copy link
Member Author

I'm assuming that all the contract artifacts get fed through the AVM transpiler so perhaps this could parse the custom attributes and set the function type, etc (although this could just be done on the fly).

Copy link
Collaborator

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @TomAFrench! I just left a question to double check, but if it's fine, I'll merge with my PR. Next step (which I can tackle) would be to tweak yarn-project/types/src/abi/contract_artifact.ts#loadContractArtifact to understand the attributes for populating the fields we expect in ts-land.

match ty {
"Private" => func.def.return_distinctness = Distinctness::Distinct,
"Public" => func.def.is_open = true,
"Public" => func.def.is_unconstrained = true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

For compilation purposes (ie emitting ACIR or Brillig or AVM bytecode), is it equivalent to flag the function as unconstrained instead of open? @TomAFrench @Maddiaa0 @sirasistant

Copy link
Member Author

Choose a reason for hiding this comment

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

"open" implies AVM, no? We should compile to brillig in this case so we should tell the Noir compiler that it should compile the function as if it were unconstrained.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't output AVM bytecode so we can only output ACIR or brillig.

@spalladino spalladino merged commit cff565e into palla/remove-open-keyword Mar 6, 2024
14 of 15 checks passed
@spalladino spalladino deleted the tf/remove-aztec-leakage-into-function-types branch March 6, 2024 18:59
spalladino pushed a commit that referenced this pull request Mar 6, 2024
This PR removes the concept of a function being internal or "open" from
the Noir compiler as these are now concepts which are aztec specific. We
instead attach all of a contract functions custom attributes to it in
the build artifact so aztec can parse these to determine whether a
function is open or secret, etc.

@spalladino This follows on from #4967 but I've made this as a separate
PR into yours as I don't want to hijack it.
spalladino pushed a commit that referenced this pull request Mar 7, 2024
This PR removes the concept of a function being internal or "open" from
the Noir compiler as these are now concepts which are aztec specific. We
instead attach all of a contract functions custom attributes to it in
the build artifact so aztec can parse these to determine whether a
function is open or secret, etc.

@spalladino This follows on from #4967 but I've made this as a separate
PR into yours as I don't want to hijack it.
spalladino pushed a commit that referenced this pull request Mar 11, 2024
This PR removes the concept of a function being internal or "open" from
the Noir compiler as these are now concepts which are aztec specific. We
instead attach all of a contract functions custom attributes to it in
the build artifact so aztec can parse these to determine whether a
function is open or secret, etc.

@spalladino This follows on from #4967 but I've made this as a separate
PR into yours as I don't want to hijack it.
spalladino pushed a commit that referenced this pull request Mar 11, 2024
This PR removes the concept of a function being internal or "open" from
the Noir compiler as these are now concepts which are aztec specific. We
instead attach all of a contract functions custom attributes to it in
the build artifact so aztec can parse these to determine whether a
function is open or secret, etc.

@spalladino This follows on from #4967 but I've made this as a separate
PR into yours as I don't want to hijack it.
spalladino pushed a commit that referenced this pull request Mar 11, 2024
This PR removes the concept of a function being internal or "open" from
the Noir compiler as these are now concepts which are aztec specific. We
instead attach all of a contract functions custom attributes to it in
the build artifact so aztec can parse these to determine whether a
function is open or secret, etc.

@spalladino This follows on from #4967 but I've made this as a separate
PR into yours as I don't want to hijack 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.

None yet

2 participants