Skip to content

Conversation

@johnathan79717
Copy link
Contributor

@johnathan79717 johnathan79717 commented Jan 9, 2026

Summary

  • Add FFI backend tests ported from zkpassport/aztec-packages bb_rs tests
  • 56 FFI tests covering AES, Blake2s, BN254, ECDSA, Grumpkin, Pedersen, Poseidon, Schnorr, and secp256k1
  • Fixed TypeScript code generator (rust_codegen.ts) to properly serialize [Vec<u8>; 4] types used by Poseidon2Permutation state

Original 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)

  • Added needsSerdeArray4Bytes() method to detect [Vec<u8>; 4] field types
  • Added serde_array4_bytes helper module to generated_types.rs for proper msgpack serialization of fixed-size byte array tuples
  • This fixes the poseidon2_permutation API which uses 4-element state arrays

FFI Tests (barretenberg/rust/tests/src/ffi/)

  • aes.rs, blake2s.rs, bn254.rs, ecdsa.rs, grumpkin.rs, pedersen.rs, poseidon.rs, schnorr.rs, secp256k1.rs
  • Require --features ffi and linker flags:
    RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" cargo test --features ffi

Not Ported

The ACIR tests from the Obsidion bb_rs (acir.rs) were not ported because:

  1. The Obsidion bb_rs uses direct FFI bindings (acir::get_circuit_sizes, acir::acir_vk_as_fields_mega_honk), while barretenberg-rs uses a pipe-based msgpack protocol with different method signatures
  2. The JSON fixture files referenced by include_str!() (vk_in_args.json, vk_serialized.json) don't exist in the Obsidion repository
  3. The equivalent functionality exists in barretenberg-rs (circuit_stats, mega_vk_as_fields) but requires structured CircuitInput types rather than raw bytes

Test plan

  • All 56 FFI tests pass locally with RUSTFLAGS
  • CI passes

@johnathan79717 johnathan79717 added the ci-barretenberg-full Run all barretenberg checks. label Jan 9, 2026
@johnathan79717 johnathan79717 force-pushed the jh/bb-rs-tests-and-poseidon-fix branch from 1509b0e to c1e425e Compare January 9, 2026 16:03
@johnathan79717 johnathan79717 changed the base branch from merge-train/barretenberg to next January 9, 2026 16:04
@johnathan79717 johnathan79717 added ci-barretenberg-full Run all barretenberg checks. ci-full Run all master checks. and removed ci-barretenberg-full Run all barretenberg checks. labels Jan 9, 2026
github-merge-queue bot pushed a commit that referenced this pull request Jan 9, 2026
…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.
@ludamad
Copy link
Collaborator

ludamad commented Jan 9, 2026

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)

@johnathan79717
Copy link
Contributor Author

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?

@johnathan79717 johnathan79717 force-pushed the jh/bb-rs-tests-and-poseidon-fix branch from 8218e9c to 56ee54e Compare January 13, 2026 12:47
@johnathan79717 johnathan79717 changed the base branch from next to jh/fix-bb-rs-vm2-library January 13, 2026 12:48
@johnathan79717 johnathan79717 added ci-barretenberg-full Run all barretenberg checks. and removed ci-full Run all master checks. labels Jan 13, 2026
@johnathan79717 johnathan79717 force-pushed the jh/fix-bb-rs-vm2-library branch from 710539f to 512a859 Compare January 13, 2026 12:55
@johnathan79717 johnathan79717 force-pushed the jh/bb-rs-tests-and-poseidon-fix branch 4 times, most recently from 57043db to d045197 Compare January 13, 2026 14:16
@johnathan79717 johnathan79717 force-pushed the jh/fix-bb-rs-vm2-library branch from 4e7fe0a to 1afc054 Compare January 13, 2026 15:54
Base automatically changed from jh/fix-bb-rs-vm2-library to merge-train/barretenberg January 13, 2026 16:02
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.
@johnathan79717 johnathan79717 force-pushed the jh/bb-rs-tests-and-poseidon-fix branch from d045197 to 8e84eaf Compare January 13, 2026 18:17
@johnathan79717
Copy link
Contributor Author

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.

@johnathan79717 johnathan79717 requested review from ledwards2225 and removed request for charlielye January 14, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-barretenberg-full Run all barretenberg checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants