Skip to content

fix: remove unused hash_challenge variable in batch_merge.test.cpp#22906

Merged
federicobarbacovi merged 1 commit into
merge-train/barretenbergfrom
claudebox/fix-merge-train-bb-ci
May 1, 2026
Merged

fix: remove unused hash_challenge variable in batch_merge.test.cpp#22906
federicobarbacovi merged 1 commit into
merge-train/barretenbergfrom
claudebox/fix-merge-train-bb-ci

Conversation

@AztecBot

@AztecBot AztecBot commented May 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes the merge-train/barretenberg merge-queue failure: http://ci.aztec-labs.com/1617122ce095114b

Summary

compute_running_hash in batch_merge.test.cpp (added by #22775) declared bb::fr hash_challenge(0) but never used it. Under -Werror -Wall, the ARM build fails with [-Wunused-variable]. The fix removes the dead declaration.

Why ci-full on #22775 didn't catch it

Clang's -Wunused-variable is suppressed when a variable's constructor has visible side effects. On x86 (DISABLE_ASM=OFF), field<T> constructors call self_to_montgomery_form()operator*=asm_self_mul_with_coarse_reduction, which contains an __asm__() block with a "memory" clobber. Clang treats inline asm as a side effect and silences the warning.

On ARM, CMakeLists.txt auto-sets DISABLE_ASM=ON (line 50–57), so bb::fr(0) resolves to a pure constexpr arithmetic path with no observable side effect — clang then issues the warning.

ci-full (label) runs only on amd64 (see ci.sh full target — single bootstrap_ec2 ci-full on x86), so the asm side-effect masked the warning. The merge queue (ci.sh merge-queue) is what adds the arm64 leg (a1-fast arm64 ci-fast), which is why the failure surfaced only at merge time.

ClaudeBox log: https://claudebox.work/s/e0102d5ad296fd89?run=1

@AztecBot AztecBot added ci-draft Run CI on draft PRs. claudebox Owned by claudebox. it can push to this PR. labels May 1, 2026
@federicobarbacovi federicobarbacovi marked this pull request as ready for review May 1, 2026 21:02
@federicobarbacovi federicobarbacovi enabled auto-merge (squash) May 1, 2026 21:02
@federicobarbacovi federicobarbacovi merged commit 3952163 into merge-train/barretenberg May 1, 2026
47 of 55 checks passed
@federicobarbacovi federicobarbacovi deleted the claudebox/fix-merge-train-bb-ci branch May 1, 2026 21:13
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request May 6, 2026
BEGIN_COMMIT_OVERRIDE
fix(ci): default S3_BUILD_CACHE_AWS_PARAMS in cache_s3_transfer{,_to}
(AztecProtocol#22898)
chore: low-hanging chonk prover fixes from profiling (AztecProtocol#22855)
chore: fuse N `add_scaled` into one `parallel_for` (AztecProtocol#22893)
feat: Delayed merge implementation (AztecProtocol#22775)
chore: numeric audit response (AztecProtocol#22856)
fix: harden BN254 G2 SRS ingress (AztecProtocol#22858)
fix: remove unused hash_challenge variable in batch_merge.test.cpp
(AztecProtocol#22906)
fix(bbup): remove jq dependency (AztecProtocol#22912)
chore: fix g2 test failing on merge-train (AztecProtocol#22920)
fix(ci): error on disabled-cache in CI hash calculation (AztecProtocol#22904)
END_COMMIT_OVERRIDE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-draft Run CI on draft PRs. claudebox Owned by claudebox. it can push to this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants