After the latest changes in removing the internal and open keywords from Noir, we now have the following situation when it comes to function types:
#[aztec(private)]: Private constrained functions
#[aztec(public)]: Public constrained functions
#[aztec(internal)]: Functions only callable via an aztec function call from the same contract (used with either of the two above)
#[contract-library-method]: Constrained functions that are inlined
unconstrained (no attribute): Unconstrained query functions
no attribute: Defaults to a private function, where the inputs and public inputs are set manually. Should not be used.
The last three are the most confusing. We have âexternalâ functions that do not need an attribute (unconstrained query), we have an attribute that few people know exists (contract-library-method), and the default setting of not writing anything is basically a mistake.
I propose the following changes:
All functions with no aztec(*) attributes act as inlined helpers (ie what was contract-library-method). This means that, by default, functions are not callable via an Aztec contract function call.
We introduce the #[aztec(query)] macro to signal query methods. We force that these are annotated with unconstrained so itâs clear to devs. Flagging a non-unconstrained with aztec(query) is a compilation error.
So the result would be the following, with changes from original in bold:
#[aztec(private)]: Private constrained functions
#[aztec(public)]: Public constrained functions
#[aztec(internal)]: Functions only callable via an aztec function call from the same contract (used with either of the two above)
#[aztec(query)]: Unconstrained query function (requires unconstrained)
unconstrained (no attribute): Unconstrained helper function that is inlined
no attribute: Constrained helper function that is inlined
Eventually we may be able to remove aztec(query) altogether, in favor of new simulation and calling methods. But for now, I think this setup is clearer. Thoughts?
There was a recent suggestion that the #[aztec(query)] function (in your proposed wording) could be called #[aztec(offchain)]. Itâs not always going to be a query (e.g. compute_note_hash_and_nullify is such a function, but it isnât making a âqueryâ). Perhaps this then wouldnât need to be accompanied by unconstrained, because it might be clear that if you execute something offchain, itâs not going to be constrained by the network?
One of the criticisms with fallback functions (which relates to this discussion), is that youâd need to declare a private, public and query version. Some people raised that maybe a query function isnât needed in such a context, and that perhaps the private (or public) function could be called by aztec.js in such a way that itâs understood to be an offchain query, and that the private (or public) function would just implicitly get executed as brillig or acir, and the return_values extracted from the PrivateCircuitPublicInputs. Tl;dr, thereâs a suggestion that maybe the #[aztec(query)] functions are superfluous.
I, personally, like the explicitness and the separation, but others donât.
Edit: On the whole, though, I like your suggestions. I didnât even know about the contract-library-method, and so Iâm happy youâre getting rid of that in favour of a much more intuitive approach.
All functions with no aztec(*) attributes act as inlined helpers (ie what was contract-library-method). This means that, by default, functions are not callable via an Aztec contract function call.
I love this. Am in favour of removing it completely
#[aztec(query)]: Unconstrained query function (requires unconstrained)
Should this be called #[aztec(view)] to keep in line with solidity and AztecJS and to minimise burden on new devs? Ofc one argument against calling them view() is unlike solidity, these methods canât be called by other contracts.
If we stick to query(), we should update AztecJS too.
unconstrained (no attribute): Unconstrained helper function that is inlined
This is exactly how aztecnr helper methods (like pop_capsule() or view_notes()) work today so thatâs nice and it is closer to how noir works.
Out of curiousity, is there a reason a contract developer would write such a method in their contract?
#[aztec(private)]: Private constrained functions
#[aztec(public)]: Public constrained functions
Do we allow view() on AztecJS side in such functions?
Should this be called #[aztec(view)] to keep in line with solidity and AztecJS and to minimise
No! A solidity view is still âconstrainedâ, unconstrained is explicitly not. So they are not really the same.
Do we allow view() on AztecJS side in such functions?
I think we should allow calling âviewâ on those as well, and have the view run simulate and return the values that way . Then for some cases, you might not even need to have an explicitly âunconstrainedâ getter (having multiple getters of the same things because it need to be in public as well pisses me off )
Nevertheless. I think having something like a view would be nice as it is a way for you to signal that this function donât have any state changes . With a better way to call functions it could be used to tell you that you are to make a static_call instead of a normal call.
Related to @Mikeâs comments: As long as there is a difference in how much can be returned by the functions they are also very important to have. Not sure speed-wise how quickly each is executed, but you could have that the query would simply be quicker than the other if it is always running âconstrainedâ for simulations etc.
Wholeheartedly agree, familiarity would be great to have but this is a fundamentally different concept. An Aztec developer must understand what an unconstrained function is, just like Solidity developers must understand to what extent external contracts can be trusted.
@alvaro proposed how this could be done here. I discussed this again with him recently and he still thinks it would not be too hard to pull this off.
We sort of have this already in that only functions that are passed &mut context can cause side effects, including function calls, right?
We sort of have this already in that only functions that are passed &mut context can cause side effects, including function calls, right?
It was as a way to convey to the caller how he should call it. If you think about having something where we have nicer call syntax like contract.function(arguments) the view marker on the implementation would indicate to the macro that the underlying call should be a static_call instead of fiddling with it yourself
Also want it enforced in the protocol so we could make sure that it is not up to the caller to decide
Yeah, Iâm not enamoured (gotta love the English language borrowing latinisms) with query. However, naming it offchain opens another discussion: should we allow txs to call these unconstrained âqueryâ functions? After all, if a tx can call an oracle which is unconstrained, why not call unconstrained get_something in another contract?
Big +1!
Agree, but I wanted to leave this out of the discussion for now, in order to focus on whatâs more easily attainable now.
As a nitpick, Iâd keep the goal of moving as much as possible out of the protocol and say we can do this check in the app contract, just like weâre now doing with internal checks. But agree in the goal of not leaving it up to the caller to decide.
All in all, it seems we are all in agreement about the changes proposed in the 1st post, with the exception of the name for aztec(query). Iâll convert this into an issue so it can be actioned:
I think we need a visualisation (a nice pretty graph of arrows) of what kinds of functions can make what kinds of calls to other kinds of functions, because itâs becoming quite unwieldy
Having some contract call another contractâs unconstrained function is a discussion we often come back to. I have concerns that it could lead to the called contract leaking its private data to the calling contract. Maybe the pxe can be configurable to authorise access to certain calling contracts, but it feels dangerous. It might also be a mess, if at runtime, the user is called-back with auth messages âwould you like to allow contract 0x34765 to access the private state of contract 0x86787, via an unconstrained âaztec query/offchainâ function? This is dangerous. Think carefully before granting access.â.
I canât find the GitHub issue, where I commented a big justification for not allowing this.
To be fair, weâll already need to have some sort of authorization for the dapp whoâs making these âoffchainâ calls. So we could simply extend this permission to all txs initiated by the dapp, so if a contract in a tx calls an unconstrained function in another one, we have the PXE apply the same permission rule as if it had been the dapp making the query.
Still, this doesnât work on every case. If the dapp initiates a tx, and the tx ends up calling an unconstrained function in a contract unexpected by the dapp (which may happen, yay composability), we need to accommodate for that. I get the impression that, from a UX perspective, this can be tackled using the simulating simulations approach: we make an initial simulated run of the tx, we show the user a dialog explaining 1) what permissions is the tx requiring and 2) what stuff is the tx accessing, and only then we execute and return control to the calling dapp.
But weâre getting derailed. Iâd suggest we try picking a name for âoffchainâ functions that is not offchain, so we can name it independently of this decision. I donât have good proposal though.
. Then for some cases, you might not even need to have an explicitly âunconstrainedâ getter (having multiple getters of the same things because it need to be in public as well pisses me off 
)
. With a better way to call functions it could be used to tell you that you are to make a 