Skip to content

Fix/chunk test suite stale num pubs - #20

Open
yeziR4 wants to merge 85 commits into
GOATNetwork:goat-testfrom
yeziR4:fix/chunk-test-suite-stale-num-pubs
Open

Fix/chunk test suite stale num pubs#20
yeziR4 wants to merge 85 commits into
GOATNetwork:goat-testfrom
yeziR4:fix/chunk-test-suite-stale-num-pubs

Conversation

@yeziR4

@yeziR4 yeziR4 commented Aug 25, 2026

Copy link
Copy Markdown

Summary

cargo test -p bitvm --release chunk:: on this branch currently fails to compile before any test runs:

error[E0425]: cannot find function `generate_segments_using_mock_vk_and_mock_proof` in this scope
    --> bitvm/src/chunk/api.rs:1477:25

generate_segments_using_mock_vk_and_mock_proof was moved to api_compiletime_utils in an earlier refactor; the #[cfg(test)] mod test in chunk/api.rs was never updated to import it from its new location. This one-line fix restores the import and unblocks compilation.

What this uncovers (not fixed in this PR)

With the import restored, the suite compiles and 7 tests fail:

chunk::api::test::full_e2e_execution
chunk::api::test::test_largest_chunks
chunk::api_runtime_utils::test::test_runtime_execution_looped
chunk::g16_runner_core::test::test_groth16
chunk::g16_runner_core::test::test_verify_pairing
chunk::taps_msm::test::test_tap_msm_invalid_inputs_scalar_not_fr
chunk::taps_msm::test::test_tap_msm_valid_inputs

All 7 trace to the same root cause: a hardcoded test fixture (vk_bytes / proof_bytes / a single scalar) shared across five test functions was built from a verifying key with 1 public input. NUM_PUBS (bitvm/src/chunk/api.rs) has been 2 since the merge-conflict-resolution commit 7b31dfd (Nov 21, 2025) — roughly 9 months. Representative panics:

thread 'chunk::g16_runner_core::test::test_verify_pairing' panicked at bitvm/src/chunk/g16_runner_core.rs:1128:25:
index out of bounds: the len is 1 but the index is 1

thread 'chunk::api::test::full_e2e_execution' panicked at bitvm/src/chunk/api_compiletime_utils.rs:44:5:
assertion failed: vk.gamma_abc_g1.len() == NUM_PUBS + 1

I don't believe this is exploitable in production — goat::disprove_scripts::test_verify_guest_pubin_ziren, which deserializes a real 2-element public-input array from ziren/public_inputs.bin, passes cleanly, so the live Ziren integration matches NUM_PUBS = 2 correctly. This also isn't the Zellic-audited MSM finding (3.19, commit 9f2a42a) — that was a Script stack-drop bug in taps_msm.rs with no test-fixture changes; this is a separate, distinct gap.

The real-world impact is coverage risk: the regression suite for the chunked Groth16 pairing verifier — the code that ultimately decides whether a cheating operator gets caught on-chain — has evidently not produced a passing cargo test -p bitvm run in ~9 months, meaning a genuine regression introduced anywhere in chunk:: during that window would not have been caught.

This PR intentionally fixes only the compile break. Regenerating the shared fixture with a real 2-public-input proof (or parameterizing the affected tests to read NUM_PUBS from the VK's own gamma_abc_g1.len() - 1 instead of assuming a constant) needs a decision from the team on the right fixture/approach, so I've filed that separately rather than guessing at a replacement proof.

Testing

cd bitvm && cargo test -p bitvm --release chunk::

Before: compile error. After this PR: compiles, 35 passed; 7 failed; 8 ignored (see above — expected given the stale fixture).

hunterfarq and others added 30 commits April 3, 2025 03:47
* chore: fix array out-of-bounds error with index validation

* Update mmr_guest.rs

* Update header-chain/src/mmr_guest.rs

Co-authored-by: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com>

---------

Co-authored-by: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com>
* WIP: Bigint configuration

* feat: bigint transforms fuzzing

* test: bigint limb transforms fuzzing
-add: macro for cleaning, adjusted test parameter, and idiomatic cleanup

* clean: separate files and remove unncessary struct param

* chore: cargo fmt

* Revert "chore: cargo fmt"

This reverts commit 0ea384a.

* chore: cargo formatting error, remove single line enforcement

* chore: move stack element verify script to fuzz library
BitVM#282)

* Fix connector_b_leaf_2_script test for proper Taproot script verification

* Update connector_b.rs

* Update connector_b.rs

* fix cargofmt
* chore: remove paste macro

I simply transform `paste! { X }` into `X`. If the diff for this commit
is large, then git is having an issue. The `paste!` macro is used
elsewhere in the codebase, so I won't remove the dependency.

* chore: clippy

cargo clippy --fix --lib -p bitvm

* chore: replace static with const

* chore: Inline

* refactor: Replace macros with functions

Functions are easier to read and tools like `rustfmt` and `clippy` have
an easier time parsing them.

* refactor: clarify endianness

* doc: winternitz

* refactor: Rename "blocks" to "digits"

This commit renames "blocks" to "digits" in function signatures and
struct members. I used the opportunity to clearly separate lengths
in units of digits from lengths in units of bits.

This commit touches a lot of code. The Parameter struct is technically
public and changed by this commit. However, I argue that no one should
manually set up his parameters. Instead, everyone should use the
high-level methods for wots-hash, wots-blake and wots256, which are not
affected by this commit.

I plan to make the Parameter struct private in a follow-up PR.

* doc: address comments

Only changes to docstrings and parameter names.
No changes to public API.

* fix: documentation
* chore: Mark potential footgun

* feat: New WOTS API

* test: Add WOTS test vectors

The test vectors are generated directly in Rust, so we can add new
vectors or modify existing vectors in the future.

* refactor: Adapt bitvm crate to new WOTS API

Removes the dependency on the modules wots_api and signing_winternitz.

* refactor: adapt bridge to new wots api

Removes the dependency on wots_api.
Removing signing_winternitz turned out to be too much work, as it
touches a lot of code. In particular, the connector outputs handle
public keys for different message lengths in the same vector.
The bridge code can be refactored in the future if needed.

* refactor: Move winternitz_hash into signing_winternitz

signing_winternitz is the only place that calls winternitz_hash,
so why not combine the modules?

BLAKE3 switched from 20-byte WOTS signatures to Wots16 a while ago,
so I use the opportunity to remove the code for 20-byte WOTS.

* chore: Remove wots_api.rs

No more code depends on this, so it is safe to remove.

* fix: Address comments

* refactor: WinternitzSecret constructors

Deprecate from_string constructors. Add from_bytes as a safer
alternative.

* fix: Address comments

* fix: address comments

* fix: cargo fmt
* feat: add tmul for fp2 with 4 window stack len

* feat: add fp2_w4 in ss_mul to reduce script size

* feat: add w4 version of lc4_keep_elements

* chore: specify vec capactiy

* doc: add docs to new functions
sevkett12 and others added 30 commits October 9, 2025 11:04
* zellic g1 fixes

* test for read from stack

* deleted  hinted_x_from_eval_point and  hinted_y_from_eval_point functions

* fmt

* used fq::is_zero instead of comparing bytes to zero

* changed & to &&
* removed unnecessary log asserts

* removed unnecessary if branch in tonelli shanks

* removed unused imports
* fix fq6_hinted_mul_keep_elements

* removed unused import

* fmt
…put if NUM_PUBS > 1 (BitVM#369)

* fixed num_pub>1 case

* fixed num_pubs>1 case
…itVM#371)

* drop c left in the altstack

* remove if branch in utils_fq12_square since it doesnt occur
* fix BitVM#365 Blake3 script inscorrect

fix BitVM#365 Blake3 script inscorrect for more than 1,024 bytes of input or more than 32 bytes of output

* add assert to blake3 compress to check final_rounds is 8

---------

Co-authored-by: lynndell | bitlayer <lynndell.zhong@bitlayer.ltd>
* fix checksum length

* add warning for repeated hashes

* fix pushing digits

* fmt

* add hash length checks and comment verify_test_vector's temporarily since its not compatible with the new checksum length

* fix secret key generation

* remove BinarysearchVerifier bound check and add test to verify that it doesn't cause a vulnerability

* fix raw_witness_to_signature digit conversion
* add validation for hint being a correctly formed T type value to tmul

* check that hint y is valid in Fp254Impl::hinted_inv

* add check_validity to places where hints are used

* add Fq::check_validity and use it in necessary place

* add hint validity checks to chunk::taps_points:ops::utils_point_add_eval

* chore: rename check validity functions for cherry-picking optimized version

* feat: add check_validity function to Fp254

* chore: incorperate cherry-picked version

* opt: optimize copies for hint verifications with the cherry-picked verification

* fmt

---------

Co-authored-by: just-erray <c.e.aslan37@gmail.com>
* fix fp254impl issues

* fmt

* fmt

* add back necessary conflicted functions
* clear part1 report

* add assertion to push_u32_le

* fmt

* remove debug prints and improve test for limb_shr1_carry

* fmt

* limb_to_bits functions assert num_bits>0

* change G1Affine::roll, G2Affine::roll, G2Affine::copy parameters to in terms of Fq, and fix tests for bigint change

* fmt

* assert LIMB_SIZE < 31 in BigIntImpl

* restore bigint/mul.rs because it is fixed in another pr

* fmt

* apply parameter change to double_prevent_overflow_keep_element too
* feat: add check_validity function to Fp254

* feat: add validity checks to some functions in taps_ext_miller.rs as samples

* add validity check to taps_mul

* feat: add is_valid function to Fp254

* fix-feat: fix validity checks for precompute_p and complete validity checks for taps_ext_miller.rs

* feat: fix and add validity checks in taps_msm.rs

* fix: a validity check in taps_points_ops.rs

* fix-feat: correct and add validity checks

* fix: correct tests for test_point_ops_and_multiply_line_evals_step_1 tests according to the changed form of the input

* fix: add validity checks for G1Acc in chunk_msm

* opt: remove unnecessary q4 validity check

* fix: validity checks of taps_ext_miller::chunk_precompute_p_from_hash

* feat: add functions to analyze max stack usage of chunks

* fix: some typos and delete leftovers

---------

Co-authored-by: Hakkush-07 <hakan.karakus@std.bogazici.edu.tr>
* create a version of G1Affine::hinted_check_add that the caller must use with points that are not t==q nor t==-q

* optimize G1Affine::hinted_check_add and add TODO comments for later

* rearrange wrong if checks in G1Affine::hinted_check_add

* add validity checks to hints of new G1Affine::hinted_check_add

* fix hint validity check and roll in G1Affine::hinted_check_add
…etwork#12)

* remove length-prefix when get groth16 pubin

* fix guest inputs verify script
generate_segments_using_mock_vk_and_mock_proof moved to
api_compiletime_utils in an earlier refactor; this test module's import
was never updated, so `cargo test -p bitvm chunk::` has been failing to
compile outright.

With this fix applied, the suite compiles and reveals 7 further test
failures (chunk::api::test::full_e2e_execution, test_largest_chunks,
chunk::api_runtime_utils::test::test_runtime_execution_looped,
chunk::g16_runner_core::test::{test_groth16,test_verify_pairing},
chunk::taps_msm::test::{test_tap_msm_valid_inputs,test_tap_msm_invalid_inputs_scalar_not_fr}),
all tracing to one shared hardcoded test fixture (vk_bytes/proof_bytes/
scalar) built for a 1-public-input verifying key, while NUM_PUBS has been
2 since the Nov 21 2025 merge-conflict-resolution commit 7b31dfd. That
mismatch is a separate, still-open issue (not fixed here — needs a
regenerated 2-public-input fixture) filed alongside this PR.

Not believed to be exploitable in production: the live Ziren integration
test in goat::disprove_scripts (test_verify_guest_pubin_ziren, using a
real 2-element ziren/public_inputs.bin) passes cleanly. The risk is pure
test-suite/CI coverage: this module's regression suite for the chunked
Groth16 pairing verifier has evidently not produced a passing run in
~9 months.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.