fix: update BuildTestVectors call to adjust batch size based on output flag

[PastaPastaPasta]

Issue being fixed or feature implemented

see #7011

What was done?

How Has This Been Tested?

runs locally; but I didn't reproduce the failure, likely due to different compilation flags. Even with undefined sanitizer, I couldn't reproduce.

Breaking Changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

Two benchmark function calls in src/bench/bls.cpp have been modified to conditionally adjust the invalidCount parameter passed to BuildTestVectors. In both BLS_Verify_Batched and BLS_Verify_BatchedParallel, the invalidCount is now set to 10 when benchmark output is enabled (via bench.output()), and 1 when disabled. All other behavior remains unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

• Single file modified with two identical, homogeneous conditional parameter adjustments
• No logic changes, control flow modifications, or public API alterations
• Simple ternary operator applied consistently across both call sites

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	❓ Inconclusive	The description references an issue but lacks substantive details about what was done, making it difficult to assess completeness.	Provide implementation details explaining why the batch size adjustment was necessary and how it addresses the referenced issue.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: updating BuildTestVectors calls to adjust batch size based on an output flag, which directly corresponds to the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b9c29d and b220dec.

📒 Files selected for processing (1)

• src/bench/bls.cpp (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

• src/bench/bls.cpp

src/bench/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Performance benchmarks in src/bench/ must use the nanobench library

Files:

• src/bench/bls.cpp

🧠 Learnings (2)

📓 Common learnings

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

• src/bench/bls.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: Lint / Run linters
• GitHub Check: x86_64-w64-mingw32 / Build depends
• GitHub Check: x86_64-apple-darwin / Build depends
• GitHub Check: x86_64-pc-linux-gnu / Build depends
• GitHub Check: arm-linux-gnueabihf / Build depends
• GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends

🔇 Additional comments (2)

src/bench/bls.cpp (2)

251-259: Fixes invalidCount > count UB in BLS_Verify_Batched.

Tying both count and invalidCount to bench.output() ensures that when bench.output() is false you no longer pass (1, 10) into BuildTestVectors, which previously caused out-of-bounds writes to invalid[i]. With (1, 1) you still exercise an invalid vector path without invoking UB, and behavior for the verbose case (1000, 10) is preserved.

---

307-315: Parallel batched benchmark receives the same UB fix and stays consistent.

Applying the same (bench.output() ? 1000 : 1, bench.output() ? 10 : 1) pattern here removes the same potential invalidCount > count issue in the parallel benchmark and keeps the test-vector mix consistent with the non‑parallel version. The duplication between the two call sites looks fine given the small expression.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[UdjinM6]

utACK b220dec