chore(serialization): Disable the generation of C++ code for serialising ACIR

[aakoshh]

Description

Problem

@ludamad asked that we completely stop generating the msgpack_pack methods.

Summary

Adds the following configuration options to the C++ codegen:

• NOIR_CODEGEN_PACK_COMPACT instructs it to use ARRAY to serialise structs, otherwise it uses MAP; default true
• NOIR_CODEGEN_NO_PACK instructs it to not emit code for msgpack_pack, which means bb will only be able to read the data, not write it; default true

Additional Context

bb doesn't normally write ACIR, the only use for serialization was in a test. Adam thought that if somebody wants to generate ACIR for a test, they can 1) do it in memory and never write it to disk, or 2) use nargo. I suppose to avoid having to audit code they don't plan on using, they think it's better if it doesn't even exist, and this way there is no need for the Python script that existed in AztecProtocol/aztec-packages#19047 to wrangle the code.

I left the option to emit this code on the Rust side in tact because I thought it served as a good reference about what the data is expected to look like (I do have some unit tests for this in Rust as well), and who knows, the use case might come back. I know, I know: we could always recover it from Git history as well. Maybe it is emotional attachment, having spent so much time coming up with it in the first place. Let me know if you insist on removing it.

I did not remove the ability to read the non-compact format, which is easy to switch on in nargo for extra backwards compatibility, and should be easy to test as well using an env var. Ostensibly we can add another flag to disable even that. It just feels wrong that on the Rust side it's just a serializer library switch to go from one format to the other, and the reader handles both transparently, but bb should only be able to read one of them.

User Documentation

Check one:

• No user documentation needed.
• Changes in docs/ included in this PR.
• [For Experimental Features] Changes in docs/ to be submitted in a separate PR.

PR Checklist

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 3c7c9ec	Previous: 78a5445	Ratio
rollup-tx-merge	0.003 s	0.002 s	1.50

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[TomAFrench]

LGTM.

I'm happy to remove the non-compact msgpack format if we're not going to have anything which reads it.

[aakoshh]

I wouldn't be so hasty to remove it because it's just more readable. For example we don't have a JSON format at the moment (although we could, and JsonSchema would potentially be a good way to help others bootstrap the DTOs), but we can export to msgpack and use the incantation you found to transform it into JSON. That is much more legit if structs use field names, than if they are just arrays with numbers. Keeping it requires virtually no effort on our part at this point.

Another benefit would be as an alternative format if we get into some kind of backwards incompatibility pickle. For example today I thought there was an optional field on Program that we don't use (as it turns out we do), and I thought: if we used msgpack we could just remove it, and bb would still be able to read the data, but if we use msgpack-compact we cannot do it, it has to stay. If, for some reason, we really wanted to remove such a field, we'd still have the option to fall back to msgpack until the C++ code is updated, because it can read whichever version we produce.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: bd4d83a	Previous: 833c0e3	Ratio
test_report_zkpassport_noir-ecdsa_	3 s	2 s	1.50

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench