feat: msgpack encoding for Program and WitnessStack

[aakoshh]

Adds msgpack serialisation to the generated Acir and Witness C++ code.

I moved the alterations described in dsl/README.md into the code generation itself, so no manual work is required. The PR is running against a feature branch with the same name in Noir, here's the upstream PR: noir-lang/noir#7716

With this PR is merged, bb should be able to handle msgpack or bincode. Once that's released we can switch to using msgpack in Noir in both native and wasm by merging noir-lang/noir#7810. And then we can remove the msgpack format detection and the fallback to bincode.

TODO:

• Get it to compile
• Change nargo to allow compiling contracts with msgpack format: added NOIR_SERIALIZATION_FORMAT env var
• Add a first byte to the data to say which serialization format it is. There is a chance that it would clash with existing bincode data though, so a fallback would anyway be necessary. (Or we should ascertain that bincode never starts with some specific bit sequence).
• Change the bb code so it tries bincode, then falls back to msgpack - this way the currently used format stays fast, but we can feed it new data. This looks problematic, as exceptions in the wasm build is disabled in arch.cmake and throw_or_abort aborts in wasm. Instead we run msgpack::parse first to check if the data looks like msgpack; if not, we use bincode.
• Run integration tests with msgpack on both sides in test(DO NOT MERGE): Try running integration tests with msgpack #13021
• Ignore the Brillig opcodes in Barretenberg
• Change the serialization of WitnessStack and WitnessMap to use the env var, add fallbacks in bb for them
• Revert the change to noir-repo-ref before merging

Use of MSGPACK_FIELDS

The generated code is using MSGPACK_FIELDS for structs, to keep it more terse.

At some point during debugging the memory issue below I changed it so that I can have more direct control by generating code for individual fields. That needed some helper functions which I looted from the msgpack-c library and injected into the namespaces as a Helpers struct. This approach might be useful if we wanted to have extra checks, for example rejecting the data if there are extra fields, indicating a type has been extended with things we don't recognise, or if we wanted handle renamed fields. I left it out so there is less code to maintain, but if we need it we can recover it from the commit history.

I'm not actually sure if the code that handles structs can deal with missing optional fields, which could be another reason to bypass the macro.

Compile nargo with the msgpack feature

echo af/msgpack-codegen > noir/noir-repo-ref
noir/bootstrap.sh

Generate and compile C++ code

cd noir/noir-repo && NOIR_CODEGEN_OVERWRITE=1 cargo test -p acir cpp_codegen && cd -
cp noir/noir-repo/acvm-repo/acir/codegen/acir.cpp barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp
cp noir/noir-repo/acvm-repo/acir/codegen/witness.cpp barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/witness_stack.hpp
cd barretenberg/cpp && ./format.sh changed && cd -

barretenberg/cpp/bootstrap.sh

Try the following for a faster bb build:

NATIVE_PRESET=default barretenberg/cpp/bootstrap.sh build_native

Test nargo with bb

One example of an integration test that uses bb and noir in the Noir repo is https://github.com/noir-lang/noir/actions/runs/13631231158/job/38099477964

We can call it like this:

cd noir/noir-repo && cargo install --path tooling/nargo_cli && cd -
./barretenberg/cpp/bootstrap.sh
export BACKEND=$(pwd)/barretenberg/cpp/build/bin/bb
export NOIR_SERIALIZATION_FORMAT=msgpack
noir/noir-repo/examples/prove_and_verify/test.sh

If it works, it should print this:

% unset NOIR_SERIALIZATION_FORMAT                       
% noir/noir-repo/examples/prove_and_verify/test.sh      
[hello_world] Circuit witness successfully solved
[hello_world] Witness saved to /mnt/user-data/akosh/aztec-packages/noir/noir-repo/examples/prove_and_verify/target/witness.gz
Finalized circuit size: 18
Proof saved to "./proofs/proof"
Finalized circuit size: 18
VK saved to "./target/vk"
Proof verified successfully

Whereas if it doesn't:

% export NOIR_SERIALIZATION_FORMAT=msgpack                                                                                                                                                            
% noir/noir-repo/examples/prove_and_verify/test.sh             
[hello_world] Circuit witness successfully solved
[hello_world] Witness saved to /mnt/user-data/akosh/aztec-packages/noir/noir-repo/examples/prove_and_verify/target/witness.gz
Length is too large

I attached the final artefacts to the PR so it's easier to test with just bb.
hello_world.json
witness.gz

Further testing

With the noir-repo-ref pointing at the feature af/msgpack-codegen feature branch, we can run all the contract compilations and tests with msgpack as follows:

export NOIR_SERIALIZATION_FORMAT=msgpack       
./bootstrap.sh ci

This is tested in #13021

Peek into artefacts

We can inspect the file in JSON format using this msgpack CLI tool.

jq -r '.bytecode' ./target/program.json | base64 --decode | gunzip | tail -c +2 | mpk --to-json | jq

Thanks Tom for the spell 🙏

False bug

At some point I thought had to make some fixes in msgpack-c itself to make this work: AztecProtocol/msgpack-c#5
A similar blocking bug was encountered when running the entire ci build with msgpack format.

It turned out it was a dangling pointer issue, fixed in 5810e3b

Much of the comments below are related to my struggles that came from this mistake; you can ignore them.

[aakoshh]

For the record here's the msgpack bytecode without the format marker encoded as base64:

bytecode.msgpack.b64.txt

It can be pasted into https://ref45638.github.io/msgpack-converter/ and parsed into correct looking JSON.

[aakoshh]

Instead of trying to implement void msgpack_unpack(auto o) {...} for each type, another pattern that could work suggested by claude.ai is this:

namespace msgpack {
namespace adaptor {
// For Opcode
template <> struct msgpack::adaptor::convert<Acir::Opcode> {
    msgpack::object const& operator()(msgpack::object const& o, Acir::Opcode& v) const
    {
        if (o.type != msgpack::type::MAP || o.via.map.size != 1) {
            throw_or_abort("expecteed signle element map for 'Opcode'");
        }

        auto& kv = o.via.map.ptr[0];
        std::string key = kv.key.as<std::string>();

        if (key == "BrilligCall") {
            Acir::Opcode::BrilligCall bc = kv.val.as<Acir::Opcode::BrilligCall>();
            v.value = bc;
        } else if (key == "AssertZero") {
            Acir::Opcode::AssertZero az = kv.val.as<Acir::Opcode::AssertZero>();
            v.value = az;
        } else {
            throw_or_abort("unknown tag for 'Opcode': " + key);
        }

        return o;
    }
};
} // namespace adaptor
} // namespace msgpack

This looks like an extension method that will make o.convert(v) work for Opcode. struct_map_impl.hpp also has a thing like this for anything that uses MSGPACK_FIELDS.

The problem is that msgpack_unpack gets a msgpack::object::implicit_type, so we can't invoke it on that.

[aakoshh]

Unfortunately all of the following variants result in Opcode getting null in o:

with templating:

template <> struct msgpack::adaptor::convert<Acir::Opcode> {
    msgpack::object const& operator()(msgpack::object const& o, Acir::Opcode& v) const {
    // o is null

taking an implicit object and casting:

void msgpack_unpack(auto o) {
  mspack::object obj = o;
  // obj is null
  ...
}

taking an object reference:

void msgpack_unpack(msgpack::object const& o) {
  // o is null
}

😞

[aakoshh]

My hypotheses is null is being passed here

I modified that file at barretenberg/cpp/build/_deps/msgpack-c/src/msgpack-c/include/msgpack/v1/adaptor/vector.hpp like so:

template <typename T, typename Alloc>
struct convert<std::vector<T, Alloc> > {
    msgpack::object const& operator()(msgpack::object const& o, std::vector<T, Alloc>& v) const {
        std::cerr << "reading array: " << o << std::endl;
        if (o.type != msgpack::type::ARRAY) { THROW msgpack::type_error(); }
        v.resize(o.via.array.size);
        if (o.via.array.size > 0) {
            msgpack::object* p = o.via.array.ptr;
            msgpack::object* const pend = o.via.array.ptr + o.via.array.size;
            typename std::vector<T, Alloc>::iterator it = v.begin();
            do {
                std::cerr << "item: " << *p << std::endl;
                p->convert(*it);
                ++p;
                ++it;
            } while(p < pend);
        }
        return o;
    }
};

It prints this during deserialisation:

reading array: [{"BrilligCall":{"id":0,"inputs":[{"Single":{"mul_terms":[],"linear_combinations":[["0000000000000000000000000000000000000000000000000000000000000001",0],["30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000",1]],"q_c":"0000000000000000000000000000000000000000000000000000000000000000"}}],"outputs":[{"Simple":2}],"predicate":null}},{"AssertZero":{"mul_terms":[["0000000000000000000000000000000000000000000000000000000000000001",0,2],["30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000",1,2]],"linear_combinations":[],"q_c":"30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000"}}]
item: null
reading into 'Opcode': null
item: null
reading into 'Opcode': null

Not sure what's wrong with it.

To be clear, the object is not null as in what should be a NullPointerException, rather, its object_type is NIL, which is printed as null.

What I don't understand is how can it print the whole array, and then when looping through the items see them as NIL.

I bit further investigation reveals that the type of the objects in the array changes between the printing and the processing:

opcodes: [starting array ptr: 0x55a1aa246c00 type: 8
{"BrilligCall":{"id":0,"inputs":[starting array ptr: 0x55a1aa246d38 type: 8
...
item ptr: 0x55a1aa246c00 type: 0 obj: null

So the pointer address is the same 0x55a1aa246c00, but the type goes from 8 to 0 🤯

[aakoshh]

Further weirdness:

template <typename T, typename Alloc>
struct convert<std::vector<T, Alloc> > {
    msgpack::object const& operator()(msgpack::object const& o, std::vector<T, Alloc>& v) const {
        if (o.type != msgpack::type::ARRAY) { THROW msgpack::type_error(); }
        std::cerr << "unpacking array 1: " << o << std::endl;
        std::cerr << "unpacking array 2: " << o << std::endl;
        v.resize(o.via.array.size);
        std::cerr << "unpacking array 3: " << o << std::endl;
        if (o.via.array.size > 0) {
            std::cerr << "unpacking array 4: " << o << std::endl;
            msgpack::object* p = o.via.array.ptr;
            msgpack::object* const pend = o.via.array.ptr + o.via.array.size;
            typename std::vector<T, Alloc>::iterator it = v.begin();
            do {
                p->convert(*it);
                ++p;
                ++it;
            } while(p < pend);
        }
        return o;
    }
};

Prints:

unpacking array 1: [{"BrilligCall":{"id":0,"inputs":[{"Single":{"mul_terms":[],"linear_combinations":[["0000000000000000000000000000000000000000000000000000000000000001",0],["30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000",1]],"q_c":"0000000000000000000000000000000000000000000000000000000000000000"}}],"outputs":[{"Simple":2}],"predicate":null}},{"AssertZero":{"mul_terms":[["0000000000000000000000000000000000000000000000000000000000000001",0,2],["30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000",1,2]],"linear_combinations":[],"q_c":"30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000"}}]
unpacking array 2: [{"BrilligCall":{"id":0,"inputs":[{"Single":{"mul_terms":[],"linear_combinations":[["0000000000000000000000000000000000000000000000000000000000000001",0],["30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000",1]],"q_c":"0000000000000000000000000000000000000000000000000000000000000000"}}],"outputs":[{"Simple":2}],"predicate":null}},{"AssertZero":{"mul_terms":[["0000000000000000000000000000000000000000000000000000000000000001",0,2],["30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000",1,2]],"linear_combinations":[],"q_c":"30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000"}}]
unpacking array 3: [null,null]
unpacking array 4: [null,null]

It's almost as if resizing v, the target vector, overwrites the input array itself.

[aakoshh]

This version works:

template <typename T, typename Alloc>
struct convert<std::vector<T, Alloc> > {
    msgpack::object const& operator()(msgpack::object const& o, std::vector<T, Alloc>& v) const {
        if (o.type != msgpack::type::ARRAY) { THROW msgpack::type_error(); }
        bool target_size = v.size();
        if (target_size <= o.via.array.size) {
            v.reserve(o.via.array.size);
        } else {
            v.resize(o.via.array.size);
        }
        if (o.via.array.size > 0) {
            msgpack::object* p = o.via.array.ptr;
            msgpack::object* const pend = o.via.array.ptr + o.via.array.size;
            size_t i = 0;
            do {
                if (i >= target_size) {
                    T item;
                    p->convert(item);
                    v.push_back(item);
                } else {
                    p->convert(v[i]);
                }
                ++i;
                ++p;
            } while(p < pend);
        }
        return o;
    }
};

[aakoshh]

@ludamad suggested that I should have used the Address Sanitizer when I hit weird behaviour.

I changed the code back to auto o = msgpack::unpack(buffer, size).get(); and ran the sanitiser like this:

cmake --preset asan && cmake --build --preset asan

This did not work, seemingly for the same reason as here:

% cmake --preset asan && cmake --build --preset asan                                                                                                                              8s ~/aztec-packages/barretenberg/cpp af/msgpack-codegen+ akosh-box
Preset CMake variables:

  DISABLE_ASM="ON"
  ENABLE_ASAN="ON"
  TARGET_ARCH="skylake"

...
-- Build type: Debug
...
-- Build files have been written to: /mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan
[1/5] Linking CXX executable bin/ultra_vanilla_client_ivc_tests
FAILED: bin/ultra_vanilla_client_ivc_tests src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc_tests[1]_tests.cmake /mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan/src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc_tests[1]_tests.cmake 
: && /usr/bin/clang++-16 -gdwarf-4 -g -gdwarf-4    -fsanitize=address -pthread src/barretenberg/ultra_vanilla_client_ivc/CMakeFiles/ultra_vanilla_client_ivc_test_objects.dir/ultra_vanilla_client_ivc.test.cpp.o -o bin/ultra_vanilla_client_ivc_tests  lib/libultra_vanilla_client_ivc.a  lib/libstdlib_honk_verifier.a  lib/libgtest.a  lib/libgtest_main.a  lib/libgmock_main.a  lib/libstdlib_transcript.a  lib/libstdlib_poseidon2.a  lib/libstdlib_circuit_builders.a  lib/libtrace_to_polynomials.a  lib/libstdlib_primitives.a  lib/libplonk_honk_shared.a  lib/libcircuit_checker.a  lib/libultra_honk.a  lib/libsumcheck.a  lib/libstdlib_keccak.a  lib/libstdlib_sha256.a  lib/libstdlib_circuit_builders.a  lib/libtrace_to_polynomials.a  lib/libstdlib_primitives.a  lib/libplonk_honk_shared.a  lib/libcircuit_checker.a  lib/libultra_honk.a  lib/libsumcheck.a  lib/libstdlib_keccak.a  lib/libstdlib_sha256.a  lib/librelations.a  lib/libcrypto_pedersen_hash.a  lib/libcrypto_pedersen_commitment.a  lib/libcommitment_schemes.a  lib/libsrs.a  lib/libpolynomials.a  lib/libtranscript.a  lib/libcrypto_poseidon2.a  lib/libecc.a  lib/libnumeric.a  lib/libcommon.a  lib/libenv.a  lib/libcrypto_sha256.a  lib/libcrypto_keccak.a  lib/libgmock.a  lib/libgtest.a && cd /mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan/src/barretenberg/ultra_vanilla_client_ivc && /usr/bin/cmake -D TEST_TARGET=ultra_vanilla_client_ivc_tests -D TEST_EXECUTABLE=/mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan/bin/ultra_vanilla_client_ivc_tests -D TEST_EXECUTOR= -D TEST_WORKING_DIR=/mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D "TEST_FILTER=-*_SKIP_CI*" -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=ultra_vanilla_client_ivc_tests_TESTS -D CTEST_FILE=/mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan/src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc_tests[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=5 -D TEST_XML_OUTPUT_DIR= -P /usr/share/cmake-3.28/Modules/GoogleTestAddTests.cmake
/usr/bin/ld: src/barretenberg/ultra_vanilla_client_ivc/CMakeFiles/ultra_vanilla_client_ivc_test_objects.dir/ultra_vanilla_client_ivc.test.cpp.o:(.data.rel.ro._ZTVN2bb13CircuitSourceINS_11UltraFlavorENS_20UltraCircuitBuilder_INS_25UltraExecutionTraceBlocksEEENS1_15VerificationKeyEEE[_ZTVN2bb13CircuitSourceINS_11UltraFlavorENS_20UltraCircuitBuilder_INS_25UltraExecutionTraceBlocksEEENS1_15VerificationKeyEEE]+0x10): undefined reference to `bb::CircuitSource<bb::UltraFlavor, bb::UltraCircuitBuilder_<bb::UltraExecutionTraceBlocks>, bb::UltraFlavor::VerificationKey>::next()'
/usr/bin/ld: src/barretenberg/ultra_vanilla_client_ivc/CMakeFiles/ultra_vanilla_client_ivc_test_objects.dir/ultra_vanilla_client_ivc.test.cpp.o:(.data.rel.ro._ZTVN2bb13CircuitSourceINS_11UltraFlavorENS_20UltraCircuitBuilder_INS_25UltraExecutionTraceBlocksEEENS1_15VerificationKeyEEE[_ZTVN2bb13CircuitSourceINS_11UltraFlavorENS_20UltraCircuitBuilder_INS_25UltraExecutionTraceBlocksEEENS1_15VerificationKeyEEE]+0x18): undefined reference to `bb::CircuitSource<bb::UltraFlavor, bb::UltraCircuitBuilder_<bb::UltraExecutionTraceBlocks>, bb::UltraFlavor::VerificationKey>::num_circuits() const'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
[4/5] Linking CXX executable bin/dsl_tests
ninja: build stopped: subcommand failed.

Thinking that the level of optimisation needs to be higher, I changed barretenberg/cpp/build-asan/CMakeCache.txt like so:

//Flags used by the CXX compiler during DEBUG builds.
CMAKE_CXX_FLAGS_DEBUG:STRING=-O3 -g

With this change the above error disappeared, but after a 45 minute build, there was nothing to point out the errors of my ways:

% cmake --preset asan && cmake --build --preset asan       
...

-- Configuring done (1.7s)
-- Generating done (0.3s)
-- Build files have been written to: /mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan
[846/859] Building CXX object src/barretenberg/vm2/CMakeFiles/relations_acc_bench_objects.dir/constraining/benchmark/relations_acc.bench.cpp.o
[859/859] Linking CXX executable bin/bb
%                                                                                                                                                                            43m 20s ~/aztec-packages/barretenberg/cpp af/msgpack-codegen+ akosh-box
% cmake --preset asan && cmake --build --preset asan                                                                                                                                 ~/aztec-packages/barretenberg/cpp af/msgpack-codegen+ akosh-box
...
-- Build files have been written to: /mnt/user-data/akosh/aztec-packages/barretenberg/cpp/build-asan
ninja: no work to do.

Not sure if I'm doing it right.

[TomAFrench]

We've got some merge conflicts which have appeared. LGTM once addressed

[fcarreiro]

Some initial comments.

It seems like you guys want to merge quickly so I won't block. If there's any problem down the line at least it seems quite localized.