Merge bitcoin-core/secp256k1#1946: doc: clean up lingering ECMULT_WINDOW_SIZE comment
What the developer wrote
Authored by merge-script
Merge bitcoin-core/secp256k1#1946: doc: clean up lingering ECMULT_WINDOW_SIZE comment
685247a3817cdb932fa21c785c64a60bc0e9f135 doc: correct ecmult table storage size (Yudistira Putra)
cc0e03ffd4ddfce3663dc807a10cc4e3d0132d79 doc: clean up lingering ECMULT_WINDOW_SIZE comment (Yudistira Putra)
Pull request description:
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.
ACKs for top commit:
real-or-random:
ACK 685247a3817cdb932fa21c785c64a60bc0e9f135
theStack:
ACK 685247a3817cdb932fa21c785c64a60bc0e9f135
Tree-SHA512: e54dc1cd9f27da491d3ab60a1b1265f4e30397528e7146c04e608616badc6ad3c3b88f1e67f37d42d124feea8f36a3ed8bcf9f199d0289b5dd75bbbcec42eccf
What changed, and why it matters
This is a documentation-only cleanup. It moves a comment about the ECMULT_WINDOW_SIZE setting to a better location and fixes an outdated formula in the comment. No code behavior changes.
No security action needed. This is a non-functional documentation cleanup.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit moves a comment block from src/ecmult_impl.h under WINDOW_A to src/ecmult.h near the ECMULT_WINDOW_SIZE definition. It also updates stale references from WINDOW_G to ECMULT_WINDOW_SIZE and corrects the table-size formula in the comment to ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) * 64 bytes, referencing the existing STATIC_ASSERT that secp256k1_ge_storage is 64 bytes. No functional code is modified.
Changed components
src/ecmult.hsrc/ecmult_impl.hInspect captured patch +10 / −11
### src/ecmult.h
@@ -11,6 +11,14 @@
#include "scalar.h"
#include "scratch.h"
+/* See STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64) in group_impl.h. */
+/** Larger values for ECMULT_WINDOW_SIZE result in possibly better
+ * performance at the cost of an exponentially larger precomputed
+ * table. The exact table size is
+ * ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) * 64 bytes.
+ * Two tables of this size are used (due to the endomorphism
+ * optimization).
+ */
#ifndef ECMULT_WINDOW_SIZE
# define ECMULT_WINDOW_SIZE 15
# ifdef DEBUG_CONFIG
@@ -27,10 +35,10 @@
* tested.
*
* The following limitations are known, and there are probably more:
- * If WINDOW_G > 27 and size_t has 32 bits, then the code is incorrect
+ * If ECMULT_WINDOW_SIZE > 27 and size_t has 32 bits, then the code is incorrect
* because the size of the memory object that we allocate (in bytes)
* will not fit in a size_t.
- * If WINDOW_G > 31 and int has 32 bits, then the code is incorrect
+ * If ECMULT_WINDOW_SIZE > 31 and int has 32 bits, then the code is incorrect
* because certain expressions will overflow.
*/
#if ECMULT_WINDOW_SIZE < 2 || ECMULT_WINDOW_SIZE > 24
### src/ecmult_impl.h
@@ -30,15 +30,6 @@
#else
/* optimal for 128-bit and 256-bit exponents. */
# define WINDOW_A 5
-/** Larger values for ECMULT_WINDOW_SIZE result in possibly better
- * performance at the cost of an exponentially larger precomputed
- * table. The exact table size is
- * (1 << (WINDOW_G - 2)) * sizeof(secp256k1_ge_storage) bytes,
- * where sizeof(secp256k1_ge_storage) is typically 64 bytes but can
- * be larger due to platform-specific padding and alignment.
- * Two tables of this size are used (due to the endomorphism
- * optimization).
- */
#endif
#define WNAF_BITS 128Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.