doc: clean up lingering ECMULT_WINDOW_SIZE comment #1946

pull Yudis-bit wants to merge 2 commits into bitcoin-core:master from Yudis-bit:doc-ecmult-window-size-comment changing 2 files +10 −11

Yudis-bit commented at 4:42 PM on September 27, 2026: contributor

Move the ECMULT_WINDOW_SIZE comment from under WINDOW_A in src/ecmult_impl.h to the window-size setting in src/ecmult.h, and replace the stale WINDOW_G references.

The second commit corrects the table-size formula to ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) * 64 bytes. A comment above it points to the existing STATIC_ASSERT in group_impl.h that guarantees the storage size. The macro can only be used inside functions.

Tested on Windows x64 with GCC 16.2.0, C90, -Werror -pedantic-errors, and all modules enabled. All 237 CTest tests passed, including exhaustive tests and examples.

Closes #1766.

doc: clean up lingering ECMULT_WINDOW_SIZE comment

The comment documenting the costs and precomputed table size of ECMULT_WINDOW_SIZE in src/ecmult_impl.h was placed under the definition of WINDOW_A and referenced WINDOW_G, which became out of context since commit 6815761cf5500f1a619965c5b4bbc8918b334a35. Move the documentation comment to src/ecmult.h right above the definition of ECMULT_WINDOW_SIZE, format the table size using ECMULT_TABLE_SIZE, and update references to WINDOW_G in the boundary checks to ECMULT_WINDOW_SIZE. Closes #1766.

cc0e03ffd4

Yudis-bit requested review from Copilot on Sep 27, 2026
Copilot commented at 4:42 PM on September 27, 2026: none

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
in src/ecmult.h:19 in cc0e03ffd4
```
  10 | @@ -11,6 +11,15 @@
  11 |  #include "scalar.h"
  12 |  #include "scratch.h"
  13 |  
  14 | +/** Larger values for ECMULT_WINDOW_SIZE result in possibly better
  15 | + *  performance at the cost of an exponentially larger precomputed
  16 | + *  table. The exact table size is
  17 | + *      ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) * sizeof(secp256k1_ge_storage) bytes,
  18 | + *  where sizeof(secp256k1_ge_storage) is typically 64 bytes but can
  19 | + *  be larger due to platform-specific padding and alignment.
```
real-or-random commented at 8:58 AM on October 1, 2026:

You could also do this change in another commit (see #1480 for background).

* table. The exact table size is * ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) * 64 bytes.

I'd suggest copying the STATIC_ASSERT above the comment to help a reader understand where the guarantee comes from and to document that this comment is another code location that relies on it.
Yudis-bit commented at 11:58 AM on October 1, 2026:

Added the 64-byte fix in 685247a as a separate commit. STATIC_ASSERT only works inside functions, so I put a reference to the existing assertion in group_impl.h above the comment.

Built on Windows x64 with GCC 16.2.0 and -Werror -pedantic-errors; all 237 CTest tests passed locally.

real-or-random commented at 3:42 PM on October 1, 2026:

Ah yes, I always found this annoying. See #1952 but this shouldn't hold up this PR here.
real-or-random approved
real-or-random commented at 8:58 AM on October 1, 2026: contributor

ACK cc0e03ffd4ddfce3663dc807a10cc4e3d0132d79

Though I'd prefer to have the other fix in the same PR.
real-or-random added the label tweak/refactor on Oct 1, 2026
real-or-random added the label meta/development on Oct 1, 2026

doc: correct ecmult table storage size

The storage type is required to be exactly 64 bytes, as enforced by
STATIC_ASSERT in group_impl.h. Drop the outdated padding caveat and
point to that assertion above the table-size comment.

685247a381

real-or-random approved
real-or-random commented at 3:42 PM on October 1, 2026: contributor

ACK 685247a3817cdb932fa21c785c64a60bc0e9f135
theStack approved
theStack commented at 4:38 PM on October 1, 2026: contributor

ACK 685247a3817cdb932fa21c785c64a60bc0e9f135
theStack merged this on Oct 1, 2026
theStack closed this on Oct 1, 2026

Contributors

Labels

tweak/refactor meta/development

Linked (view graph)

#1766 Lingering comment documenting `ECMULT_WINDOW_SIZE` large values' costs