tests: generate scalars_near_split_bounds instead of hardcoding #1935

pull Anshikakalpana wants to merge 1 commits into bitcoin-core:master from Anshikakalpana:fix-scalar-split-bound-vectors changing 1 files +29 −23

Anshikakalpana commented at 7:49 PM on September 9, 2026: none

tests: generate scalars_near_split_bounds instead of hardcoding

The hardcoded scalars_near_split_bounds array didn't match its documented formula (a*LAMBDA + (ORDER+b)/2) % ORDER. All 20 entries were off in the same 32-bit window, likely a transcription error. As a result the vectors only reached ~126-bit outputs through secp256k1_scalar_split_lambda instead of the intended ~127-128-bit boundary, so the case they exist to test was never exercised.

Generate the values at runtime from the formula via fill_scalars_near_split_bounds(), which writes into a caller-provided array rather than a file-scope static. It is called at each of the three sites that use the array, so the values can no longer drift from their formula.

Verified the generated values reach within 1-2 units of the true k1_bound/k2_bound from the split_lambda_verify proof, confirming theexisting a/b ranges are sufficient.

Fixes #1920.

in src/tests.c:4650 in d45d1a0eee

4667 | -    SECP256K1_SCALAR_CONST(0x26c75a99, 0x80b861c1, 0x4a4c3805, 0x1024c8b4, 0x704d760e, 0xe95e7cd3, 0xde1bfdb1, 0xce2c5a42),
4668 | -    SECP256K1_SCALAR_CONST(0x26c75a99, 0x80b861c1, 0x4a4c3805, 0x1024c8b4, 0x704d760e, 0xe95e7cd3, 0xde1bfdb1, 0xce2c5a43),
4669 | -    SECP256K1_SCALAR_CONST(0x26c75a99, 0x80b861c1, 0x4a4c3805, 0x1024c8b4, 0x704d760e, 0xe95e7cd3, 0xde1bfdb1, 0xce2c5a44),
4670 | -    SECP256K1_SCALAR_CONST(0x26c75a99, 0x80b861c1, 0x4a4c3805, 0x1024c8b4, 0x704d760e, 0xe95e7cd3, 0xde1bfdb1, 0xce2c5a45)
4671 | -};
4672 | +static secp256k1_scalar scalars_near_split_bounds[20];

real-or-random commented at 6:44 AM on September 10, 2026:

This should not be static at the file level. The function should take an out param scalars20

in src/tests.c:4648 in d45d1a0eee
```
4643 | @@ -4644,29 +4644,32 @@ static void test_point_times_order(const secp256k1_gej *point) {
4644 |   * - For a in [-2, -1, 0, 1, 2]:
4645 |   *   - For b in [-3, -1, 1, 3]:
4646 |   *     - Output (a*LAMBDA + (ORDER+b)/2) % ORDER
4647 | + *
4648 | + * Generated at runtime by fill_scalars_near_split_bounds() rather than hardcoded.
```
real-or-random commented at 6:45 AM on September 10, 2026:

This comment is a typical AI comment that should be dropped. It will confuse any reader who isn't aware of the history will wonder what it's supposed to convey.

Anshikakalpana commented at 8:36 AM on September 10, 2026:

Fixed both ,switched to an out param and dropped the comment.
real-or-random commented at 6:46 AM on September 10, 2026: contributor

Thanks for picking that work up!
real-or-random added the label assurance on Sep 10, 2026
real-or-random added the label tweak/refactor on Sep 10, 2026
real-or-random removed the label tweak/refactor on Sep 10, 2026
real-or-random added the label bug on Sep 10, 2026
real-or-random commented at 8:37 AM on September 10, 2026: contributor

ACK but can you squash this into a single commit? Ideally, the commit message would be the same as PR summary in your first comment here (and possibly updated)

tests: generate scalars_near_split_bounds instead of hardcoding

The hardcoded scalars_near_split_bounds array didn't match its
documented formula (a*LAMBDA + (ORDER+b)/2) % ORDER. All 20 entries
were off in the same 32-bit window, likely a transcription error.
As a result the vectors only reached ~126-bit outputs through
secp256k1_scalar_split_lambda instead of the intended ~127-128-bit
boundary, so the case they exist to test was never exercised.

Generate the values at runtime from the formula via
fill_scalars_near_split_bounds(), which writes into a caller-provided
array rather than a file-scope static. It is called at each of the
three sites that use the array, so the values can no longer drift
from their formula.

Verified the generated values reach within 1-2 units of the true
k1_bound/k2_bound from the split_lambda_verify proof, confirming the
existing a/b ranges are sufficient.

Fixes #1920.

014825f0a3

Anshikakalpana force-pushed on Sep 10, 2026
Anshikakalpana commented at 1:01 PM on September 10, 2026: none

updated!
real-or-random requested review from Copilot on Sep 10, 2026
?
copilot_work_started real-or-random
Copilot commented at 1:55 PM on September 10, 2026: none
🟢 Approval recommended

The generated vectors correctly implement the documented formula and are consistently used by all affected tests.

<details> <summary>Pull request overview</summary>

Replaces incorrect hardcoded split-bound test vectors with runtime-generated scalars matching their documented formula.

Changes:
- Adds fill_scalars_near_split_bounds().
- Generates vectors at all three test sites.
  </details>
<details> <summary>File summaries</summary>

File Description

src/tests.c Generates and uses corrected split-bound test scalars.

</details>

<details> <summary>Review details</summary>
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
  </details>
💡 <a href="/bitcoin-core/secp256k1/new/master?filename=.github/skills/code-review/SKILL.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add a code-review agent skill</a> or configure MCP servers for context-aware, tailored reviews. <a href="https://docs.github.com/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review?tool=webui#mcp-servers-and-agent-skills" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn more in the docs.</a>

File	Description
`src/tests.c`	Generates and uses corrected split-bound test scalars.

Contributors

Labels

Linked (view graph)

#1920 scalars_near_split_bounds in tests.c don't match their documented construction

tests: generate scalars_near_split_bounds instead of hardcoding #1935

🟢 Approval recommended