feat: drop unused public functions from contract artifact

[TomAFrench]

I'm not sure if this is done later in the pipeline but there's no reason for aztec to track any functions other than public_dispatch so we can drop these to reduce the size of the artifact.

[fcarreiro]

The AVM only handles the public_dispatch function exposed by Aztec contracts.
This is not strictly true, actually the AVM can execute any function but Aztec only deploys public_dispatch.

1. This will break avm_simulator.test.ts. We test some functions directly there, since otherwise we need to emulate the AztecNr calling convention (via public dispatch) which is sth the AVM in principle doesn't need to know of.**

2. My only concern with removing these is down to tooling/debugging. Are we sure the debugging info for these functions will never be needed? Also not by the PXE? I'm not sure to what extent the artifact information is used.

**however, we do test some functions via public_dispatch so this is negotiable.

[TomAFrench]

This is not strictly true, actually the AVM can execute any function but Aztec only deploys public_dispatch.

This will break avm_simulator.test.ts. We test some functions directly there, since otherwise we need to emulate the AztecNr calling convention (via public dispatch) which is sth the AVM in principle doesn't need to know of.**

Perhaps we can move this to whichever service is storing all of the contract artifacts and serving them to users when requested? I'm not sure if this is done "intelligently" so that only the relevant functions are transmitted or whether users get the full json object thrown at them. I'm not sure who's responsible for building that service though.

My only concern with removing these is down to tooling/debugging. Are we sure the debugging info for these functions will never be needed? Also not by the PXE? I'm not sure to what extent the artifact information is used.

All the relevant debug information should be in the debug info attached to public_dispatch.

[fcarreiro]

What's the problem that you are trying to solve? :) if it's raw artifact size, this PR makes sense. I didn't understand very well what you mention about transferring functions, that seems to point to some other serving problem?

Let me briefly discuss with @dbanks12 about these changes. I see two options forward
(1) The changes stay as they are, and you fix the breaking tests (I can provide a pointer): the downside is that our avm simulator tests wouldn't be as independent as they are now, since they would be testing all the public_dispatch preamble.
(2) You add some flag to the transpiler like --keep-all-functions and change the scripts so that the needed test contracts keep all functions.

[TomAFrench]

I didn't understand very well what you mention about transferring functions, that seems to point to some other serving problem?

If having access to the underlying functions are useful for the AVM team then I'm all in favour of keeping them. The existence of these functions in the artifact after transpilation isn't a problem for me.

My concern is whether or not the aztec nodes filter out these functions when it comes time for a user to download it. Bytecode sizes of contracts are consistently raised as a problem and while I'm not up to date on how contract artifacts are ingested by the aztec node and made available for download, I'm just wanting to check that we're not asking users to download a number of functions which are irrelevant.

If aztec nodes strip out these functions so they don't store/serve them, then this PR can be closed.

[fcarreiro]

IDK what Zac means in that thread by bandwidth/downloading contracts. AFAIK the only thing that will be deployed is public_dispatch but maybe someone knows better than me how the rest of the contract is handled from a users PoV. If someone wants to call a contract do they need to (or usually do) download the whole artifact from somewhere? @spalladino @just-mitch

[spalladino]

If someone wants to call a contract do they need to (or usually do) download the whole artifact from somewhere?

Yes, that's the expected flow. They need to at least download the ABI and bytcode for the private functions they're interacting with. We expect that dapps will bundle these artifacts, so the app frontend would provide these artifacts to the user's connected PXE. Alternatively, we can have some kind of public off-chain repository that stores source and artifact given an address, but that should not be a requirement.

[fcarreiro]

Ok so IIUC the most likely scenario is that the whole artifacts will be served somehow.

There's another option that I was thinking: add an option to the transpiler --strip-functions that does exactly what Tom intended here. Then we modify the aztec-nargo wrapper (that currently compiles and transpiles) to use that option. In that way, we keep the full artifacts in our dev environment but external app developers would have slimmer ones.

Alternatively, make this a default and have --no-strip-functions.

WDYT?

[fcarreiro]

The only risk I see with the above is that (1) we might forget and regard the artifact we see as bigger than it actually is, (2) if WE(/automation) deploy something, we should remember to transpile with --strip-functions and not just grab the artifact.

[fcarreiro]

@TomAFrench @nventuro Are we sure we don't expect any testing problems if we strip these functions? For example a #test written in the contract, or running a TXE test, or running an AztecJS test. I'd expect at least the latter to fail if the functions are not there and only "public_dispatch" is there?

[nventuro]

I don't think we do any special handling of publci fns, we interact with them via the simulator, and we try to keep our flows as similar as the actual thing whenever possible. As far as we know David was the last person to touch that, so if things are fine on his end then we should be good to go.

[fcarreiro]

Ok but how does nargo test work. For example I assume the test functions themselves are not included in the artifact. IIRC you interact with public functions via the simulator but only when an external call happens right (via the interfaces). If you are testing a public function directly, what would happen? Tagging @Thunkar maybe as well.

[TomAFrench]

nargo test doesn't generate any artifact files, it's all self contained where nargo compiles the test function into a circuit and immediately executes the test. Us filtering these functions from artifacts after they're built can't affect nargo test.

[fcarreiro]

Hmm ok but can you remind me then how the interaction w/the simulator works? Because the simulator can only work with AVM bytecode, and nargo test would output Brillig.

[Thunkar]

Ok but how does nargo test work.

@fcarreiro This is the main reason why TXE implements the avmOpcodeCall pseudooracle. If you call a public function "directly" from a test, you're actually not running the contract code directly, but rather your test code is expected to have called testEnv.deploy first, which would "deploy" the artifact in TXE so the transpiled bytecode is available for execution.

At this point, TXE behaves much like an e2e does, just much, much faster

[fcarreiro]

Ok I see, it eventually calls cheatcodes::deploy which IIUC ends up using the contract artifact to deploy the contract. This should be ok I assume if it uses the same deployment path as the real thing, since public calls are going to call via public_dispatch.

[fcarreiro]

I think everything would work ok if

• The default is to strip
• Add a --no-strip-functions to avoid it
• Modify the repo's bootstrap or whatever part that transpiles to add --no-strip-functions when transpiling avm_test_contract and avm_initializer_test_contract

[fcarreiro]

I just saw @Thunkar 's app where the Token artifact is selected and a list of private AND public functions appear, which you can call. How is this supposed to work if only public_dispatch is in the artifact?

[spalladino]

How is this supposed to work if only public_dispatch is in the artifact?

We'd need to keep the ABI (but remove the bytecode) for all functions but public_dispatch

[fcarreiro]

Yeah my point all along, that's not what this PR is doing. Then we'd have to keep the function but remove (presumably) bytecode, debug symbols, what else? Partial hydration is error prone. For example, if #11193 went in, then partial artifacts would start failing on load, because the very reasonable assumption of a non-empty (compressed in that case) bytecode would be broken. The same might happen with debug symbols since they are decompressed as well (but maybe just on access).

[fcarreiro]

Maybe @TomAFrench @nventuro @spalladino @Thunkar we should have a more general discussion about this during the offsite. Keeping the functions that are not deployed for the sake of their ABI (with or without bytecode etc) might be unavoidable but leaks some knowledge to the user of the artifact: e.g., you shouldn't use the bytecode, nor the debug symbols, and you can call them but always using the aztec-nr convention [via public_dispatch] etc).

[spalladino]

Maybe we should have a more general discussion about this during the offsite

Sounds good. Do you want to add an item to the spreadsheet?