-
Notifications
You must be signed in to change notification settings - Fork 581
feat: add comprehensive bb_rs compatibility tests for barretenberg-rs #19461
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
base: merge-train/barretenberg
Are you sure you want to change the base?
feat: add comprehensive bb_rs compatibility tests for barretenberg-rs #19461
Conversation
1509b0e to
c1e425e
Compare
…berg (#19464) ## Summary - Fixes a bug where the `ci-barretenberg-full` label was being ignored for PRs targeting `merge-train/barretenberg` - The branch-based check for `ci-barretenberg` was evaluated first, causing the explicit label to be skipped - This reorders the conditions so explicit labels take precedence over branch defaults ## Context Discovered on PR #19461 where setting `ci-barretenberg-full` label resulted in only 13 seconds of CI because the target branch (`merge-train/barretenberg`) matched `ci-barretenberg` mode first, causing a cache hit.
|
Looking good :) and yeah the goal is to show a proof of concept of bb-rs being fully moved over, and adapt their tests to the new format at that time (tho not strictly necessary for those redundant here, but some sanity checks are good) |
I thought if we can port their tests, it already shows that our FfiBackend can replace bb-rs? Or is a wrapper that resembles the original bb-rs preferrable? |
8218e9c to
56ee54e
Compare
710539f to
512a859
Compare
57043db to
d045197
Compare
4e7fe0a to
1afc054
Compare
Add test suites ported from zkpassport/aztec-packages bb_rs tests: - aes.rs: AES-128-CBC encrypt/decrypt tests (6 tests) - bn254.rs: BN254 field sqrt operations (9 tests) - ecdsa.rs: ECDSA secp256k1/secp256r1 sign/verify (16 tests) - grumpkin.rs: Grumpkin curve operations (12 tests) - schnorr.rs: Schnorr signature sign/verify (11 tests) - secp256k1.rs: secp256k1 curve operations (14 tests) Enhanced existing test suites: - pedersen.rs: Added JS compatibility tests with exact expected values (12 new tests) - poseidon.rs: Added poseidon2_permutation JS compatibility tests (2 new tests) All 94 tests pass (with 3 ignored performance tests).
- Add 56 FFI tests covering AES, Blake2s, BN254, ECDSA, Grumpkin, Pedersen, Poseidon, Schnorr, and secp256k1 - Fix build.rs FFI linking to handle libbarretenberg.a, libenv.a, libvm2.a, libdsl.a, and libchonk.a - Add ffi feature flag to tests crate (disabled by default) - Update tests Cargo.toml to use default-features = false to avoid automatic FFI linking FFI tests require RUSTFLAGS due to duplicate symbols between libbarretenberg.a and libdsl.a: RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" cargo test --features ffi
…odegen The Rust codegen was not generating proper serde attributes for fixed-size arrays of bytes (e.g., [Vec<u8>; 4] for Poseidon2 state). This caused msgpack serialization to fail because the bytes were not being serialized as binary. Added needsSerdeArray4Bytes check and serde_array4_bytes module to handle [Vec<u8>; 4] types used by Poseidon2Permutation and Poseidon2PermutationResponse.
d045197 to
8e84eaf
Compare
|
For reviewers: The original bb_rs tests from Obsidion can be found at https://github.com/zkpassport/aztec-packages/tree/pr/obsidion-mobile-devnet/barretenberg/bb_rs/src/barretenberg_api/tests for comparison. |
Summary
rust_codegen.ts) to properly serialize[Vec<u8>; 4]types used by Poseidon2Permutation stateOriginal bb_rs tests for comparison: https://github.com/zkpassport/aztec-packages/tree/pr/obsidion-mobile-devnet/barretenberg/bb_rs/src/barretenberg_api/tests
Changes
TypeScript Code Generator (
barretenberg/ts/src/cbind/rust_codegen.ts)needsSerdeArray4Bytes()method to detect[Vec<u8>; 4]field typesserde_array4_byteshelper module togenerated_types.rsfor proper msgpack serialization of fixed-size byte array tuplesposeidon2_permutationAPI which uses 4-element state arraysFFI Tests (
barretenberg/rust/tests/src/ffi/)--features ffiand linker flags:Not Ported
The ACIR tests from the Obsidion bb_rs (
acir.rs) were not ported because:acir::get_circuit_sizes,acir::acir_vk_as_fields_mega_honk), while barretenberg-rs uses a pipe-based msgpack protocol with different method signaturesinclude_str!()(vk_in_args.json,vk_serialized.json) don't exist in the Obsidion repositorycircuit_stats,mega_vk_as_fields) but requires structuredCircuitInputtypes rather than raw bytesTest plan