Make files mostly self-contained

real-or-random commented at 11:16 am on August 14, 2025: contributor

This is a set of commits, each one trivial, that reorganizes code so that all C files are mostly self-contained.

These changes are merely fixes that make the files adhere to our current implicit style, which I’d describe as:

Every C file (library, tests.c, …) is a single translation unit (“unit build”), except that the libsepc256k1_precomputed is separate to speed up compilation.
Internal “modules” are organized in headers (.h) and implementation files (_impl.h). Symbols not present in the header are private to that module.
Internal “modules” should include the headers of all other modules they use.
secp256k1.c includes all _impl.h files. Other C files include simply secp256k1.c, or, if that isn’t possible for some reason (e.g., in the precompute_*.c files), the individual necessary _impl.h files.

So, none of these changes here should be controversial. In particular, I refrain from changing the above rules (e.g., by renaming the _impl.h to .c as suggested in #1039).

Why “mostly” self-contained?

After this PR, some errors remain when I run clang src/*.c and clang src/*.h on my machine. But all of them are special cases:

ctime_tests.c refuses to be compiled without valgrind or msan enabled.
src/field_10x26_impl.h, src/int128_struct_impl.h, and src/scalar_low_impl.h are confused because the autodetection logic in the preprocessor picks field_5x64, native int 128, and scalar_4x64 on my machine.

0> clang src/*.c
1src/ctime_tests.c:16:4: error: "This tool cannot be compiled without memory-checking interface (valgrind or msan)"
2   16 | #  error "This tool cannot be compiled without memory-checking interface (valgrind or msan)"
3      |    ^
41 error generated.

  0> clang src/*.h -ferror-limit=100
  1src/field_10x26_impl.h:265:21: warning: incompatible pointer types initializing 'const uint32_t *' (aka 'const unsigned int *') with an expression of type 'const uint64_t[5]' (aka 'const unsigned long[5]') [-Wincompatible-pointer-types]
  2  265 |     const uint32_t *t = a->n;
  3      |                     ^   ~~~~
  4src/field_10x26_impl.h:1007:28: warning: incompatible pointer types passing 'uint64_t[5]' (aka 'unsigned long[5]') to parameter of type 'uint32_t *' (aka 'unsigned int *') [-Wincompatible-pointer-types]
  5 1007 |     secp256k1_fe_mul_inner(r->n, a->n, b->n);
  6      |                            ^~~~
  7src/field_10x26_impl.h:401:63: note: passing argument to parameter 'r' here
  8  401 | SECP256K1_INLINE static void secp256k1_fe_mul_inner(uint32_t *r, const uint32_t *a, const uint32_t * SECP256K1_RESTRICT b) {
  9      |                                                               ^
 10src/field_10x26_impl.h:1007:34: warning: incompatible pointer types passing 'const uint64_t[5]' (aka 'const unsigned long[5]') to parameter of type 'const uint32_t *' (aka 'const unsigned int *') [-Wincompatible-pointer-types]
 11 1007 |     secp256k1_fe_mul_inner(r->n, a->n, b->n);
 12      |                                  ^~~~
 13src/field_10x26_impl.h:401:82: note: passing argument to parameter 'a' here
 14  401 | SECP256K1_INLINE static void secp256k1_fe_mul_inner(uint32_t *r, const uint32_t *a, const uint32_t * SECP256K1_RESTRICT b) {
 15      |                                                                                  ^
 16src/field_10x26_impl.h:1007:40: warning: incompatible pointer types passing 'const uint64_t[5]' (aka 'const unsigned long[5]') to parameter of type 'const uint32_t *' (aka 'const unsigned int *') [-Wincompatible-pointer-types]
 17 1007 |     secp256k1_fe_mul_inner(r->n, a->n, b->n);
 18      |                                        ^~~~
 19src/field_10x26_impl.h:401:121: note: passing argument to parameter 'b' here
 20  401 | SECP256K1_INLINE static void secp256k1_fe_mul_inner(uint32_t *r, const uint32_t *a, const uint32_t * SECP256K1_RESTRICT b) {
 21      |                                                                                                                         ^
 22src/field_10x26_impl.h:1011:28: warning: incompatible pointer types passing 'uint64_t[5]' (aka 'unsigned long[5]') to parameter of type 'uint32_t *' (aka 'unsigned int *') [-Wincompatible-pointer-types]
 23 1011 |     secp256k1_fe_sqr_inner(r->n, a->n);
 24      |                            ^~~~
 25src/field_10x26_impl.h:731:63: note: passing argument to parameter 'r' here
 26  731 | SECP256K1_INLINE static void secp256k1_fe_sqr_inner(uint32_t *r, const uint32_t *a) {
 27      |                                                               ^
 28src/field_10x26_impl.h:1011:34: warning: incompatible pointer types passing 'const uint64_t[5]' (aka 'const unsigned long[5]') to parameter of type 'const uint32_t *' (aka 'const unsigned int *') [-Wincompatible-pointer-types]
 29 1011 |     secp256k1_fe_sqr_inner(r->n, a->n);
 30      |                                  ^~~~
 31src/field_10x26_impl.h:731:82: note: passing argument to parameter 'a' here
 32  731 | SECP256K1_INLINE static void secp256k1_fe_sqr_inner(uint32_t *r, const uint32_t *a) {
 33      |                                                                                  ^
 346 warnings generated.
 35src/int128_struct_impl.h:54:6: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 36   54 |     r->hi = hi;
 37      |     ~^ ~~
 38src/int128_struct_impl.h:55:6: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 39   55 |     r->lo = lo;
 40      |     ~^ ~~
 41src/int128_struct_impl.h:59:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 42   59 |    r->lo = secp256k1_umul128(a, b, &r->hi);
 43      |    ~^ ~~
 44src/int128_struct_impl.h:59:38: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 45   59 |    r->lo = secp256k1_umul128(a, b, &r->hi);
 46      |                                     ~^ ~~
 47src/int128_struct_impl.h:65:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 48   65 |    r->lo += lo;
 49      |    ~^ ~~
 50src/int128_struct_impl.h:66:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 51   66 |    r->hi += hi + (r->lo < lo);
 52      |    ~^ ~~
 53src/int128_struct_impl.h:66:20: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 54   66 |    r->hi += hi + (r->lo < lo);
 55      |                   ~^ ~~
 56src/int128_struct_impl.h:70:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 57   70 |    r->lo += a;
 58      |    ~^ ~~
 59src/int128_struct_impl.h:71:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 60   71 |    r->hi += r->lo < a;
 61      |    ~^ ~~
 62src/int128_struct_impl.h:71:14: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 63   71 |    r->hi += r->lo < a;
 64      |             ~^ ~~
 65src/int128_struct_impl.h:80:7: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 66   80 |      r->lo = r->hi >> (n-64);
 67      |      ~^ ~~
 68src/int128_struct_impl.h:80:15: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 69   80 |      r->lo = r->hi >> (n-64);
 70      |              ~^ ~~
 71src/int128_struct_impl.h:81:7: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 72   81 |      r->hi = 0;
 73      |      ~^ ~~
 74src/int128_struct_impl.h:87:7: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 75   87 |      r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n;
 76      |      ~^ ~~
 77src/int128_struct_impl.h:87:22: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 78   87 |      r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n;
 79      |                     ~^ ~~
 80src/int128_struct_impl.h:87:42: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 81   87 |      r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n;
 82      |                                         ~^ ~~
 83src/int128_struct_impl.h:89:7: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 84   89 |      r->hi >>= n;
 85      |      ~^ ~~
 86src/int128_struct_impl.h:94:12: error: member reference base type 'const secp256k1_uint128' (aka 'const unsigned __int128') is not a structure or union
 87   94 |    return a->lo;
 88      |           ~^ ~~
 89src/int128_struct_impl.h:98:12: error: member reference base type 'const secp256k1_uint128' (aka 'const unsigned __int128') is not a structure or union
 90   98 |    return a->hi;
 91      |           ~^ ~~
 92src/int128_struct_impl.h:102:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 93  102 |    r->hi = 0;
 94      |    ~^ ~~
 95src/int128_struct_impl.h:103:5: error: member reference base type 'secp256k1_uint128' (aka 'unsigned __int128') is not a structure or union
 96  103 |    r->lo = a;
 97      |    ~^ ~~
 98src/int128_struct_impl.h:108:22: error: member reference base type 'const secp256k1_uint128' (aka 'const unsigned __int128') is not a structure or union
 99  108 |    return n >= 64 ? r->hi >> (n - 64) == 0
100      |                     ~^ ~~
101src/int128_struct_impl.h:109:22: error: member reference base type 'const secp256k1_uint128' (aka 'const unsigned __int128') is not a structure or union
102  109 |                   : r->hi == 0 && r->lo >> n == 0;
103      |                     ~^ ~~
104src/int128_struct_impl.h:109:36: error: member reference base type 'const secp256k1_uint128' (aka 'const unsigned __int128') is not a structure or union
105  109 |                   : r->hi == 0 && r->lo >> n == 0;
106      |                                   ~^ ~~
107src/int128_struct_impl.h:113:6: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
108  113 |     r->hi = hi;
109      |     ~^ ~~
110src/int128_struct_impl.h:114:6: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
111  114 |     r->lo = lo;
112      |     ~^ ~~
113src/int128_struct_impl.h:119:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
114  119 |    r->lo = (uint64_t)secp256k1_mul128(a, b, &hi);
115      |    ~^ ~~
116src/int128_struct_impl.h:120:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
117  120 |    r->hi = (uint64_t)hi;
118      |    ~^ ~~
119src/int128_struct_impl.h:126:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
120  126 |    r->lo += lo;
121      |    ~^ ~~
122src/int128_struct_impl.h:127:11: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
123  127 |    hi += r->lo < lo;
124      |          ~^ ~~
125src/int128_struct_impl.h:139:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
126  139 |    r->hi += hi;
127      |    ~^ ~~
128src/int128_struct_impl.h:145:11: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
129  145 |    hi += r->lo < lo;
130      |          ~^ ~~
131src/int128_struct_impl.h:156:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
132  156 |    r->hi -= hi;
133      |    ~^ ~~
134src/int128_struct_impl.h:157:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
135  157 |    r->lo -= lo;
136      |    ~^ ~~
137src/int128_struct_impl.h:171:7: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
138  171 |      r->lo = (uint64_t)((int64_t)(r->hi) >> (n-64));
139      |      ~^ ~~
140src/int128_struct_impl.h:171:36: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
141  171 |      r->lo = (uint64_t)((int64_t)(r->hi) >> (n-64));
142      |                                   ~^ ~~
143src/int128_struct_impl.h:172:7: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
144  172 |      r->hi = (uint64_t)((int64_t)(r->hi) >> 63);
145      |      ~^ ~~
146src/int128_struct_impl.h:172:36: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
147  172 |      r->hi = (uint64_t)((int64_t)(r->hi) >> 63);
148      |                                   ~^ ~~
149src/int128_struct_impl.h:174:7: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
150  174 |      r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n;
151      |      ~^ ~~
152src/int128_struct_impl.h:174:22: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
153  174 |      r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n;
154      |                     ~^ ~~
155src/int128_struct_impl.h:174:42: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
156  174 |      r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n;
157      |                                         ~^ ~~
158src/int128_struct_impl.h:175:7: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
159  175 |      r->hi = (uint64_t)((int64_t)(r->hi) >> n);
160      |      ~^ ~~
161src/int128_struct_impl.h:175:36: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
162  175 |      r->hi = (uint64_t)((int64_t)(r->hi) >> n);
163      |                                   ~^ ~~
164src/int128_struct_impl.h:180:12: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
165  180 |    return a->lo;
166      |           ~^ ~~
167src/int128_struct_impl.h:190:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
168  190 |    r->hi = (uint64_t)(a >> 63);
169      |    ~^ ~~
170src/int128_struct_impl.h:191:5: error: member reference base type 'secp256k1_int128' (aka '__int128') is not a structure or union
171  191 |    r->lo = (uint64_t)a;
172      |    ~^ ~~
173src/int128_struct_impl.h:195:12: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
174  195 |    return a->hi == b->hi && a->lo == b->lo;
175      |           ~^ ~~
176src/int128_struct_impl.h:195:21: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
177  195 |    return a->hi == b->hi && a->lo == b->lo;
178      |                    ~^ ~~
179src/int128_struct_impl.h:195:30: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
180  195 |    return a->hi == b->hi && a->lo == b->lo;
181      |                             ~^ ~~
182src/int128_struct_impl.h:195:39: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
183  195 |    return a->hi == b->hi && a->lo == b->lo;
184      |                                      ~^ ~~
185src/int128_struct_impl.h:201:23: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
186  201 |     return n >= 64 ? r->hi == (uint64_t)sign << (n - 64) && r->lo == 0
187      |                      ~^ ~~
188src/int128_struct_impl.h:201:62: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
189  201 |     return n >= 64 ? r->hi == (uint64_t)sign << (n - 64) && r->lo == 0
190      |                                                             ~^ ~~
191src/int128_struct_impl.h:202:23: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
192  202 |                    : r->hi == (uint64_t)(sign >> 1) && r->lo == (uint64_t)sign << n;
193      |                      ~^ ~~
194src/int128_struct_impl.h:202:57: error: member reference base type 'const secp256k1_int128' (aka 'const __int128') is not a structure or union
195  202 |                    : r->hi == (uint64_t)(sign >> 1) && r->lo == (uint64_t)sign << n;
196      |                                                        ~^ ~~
19754 errors generated.
198src/scalar_low_impl.h:19:17: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
199   19 |     return !(*a & 1);
200      |              ~~ ^ ~
201src/scalar_low_impl.h:23:14: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
202   23 |     *r = v % EXHAUSTIVE_TEST_ORDER;
203      |              ^
204src/scalar_low_impl.h:33:20: error: invalid operands to binary expression ('const secp256k1_scalar' and 'unsigned int')
205   33 |         return (*a >> offset) & (0xFFFFFFFF >> (32 - count));
206      |                 ~~ ^  ~~~~~~
207src/scalar_low_impl.h:45:103: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
208   45 | SECP256K1_INLINE static int secp256k1_scalar_check_overflow(const secp256k1_scalar *a) { return *a >= EXHAUSTIVE_TEST_ORDER; }
209      |                                                                                                       ^
210src/scalar_low_impl.h:51:14: error: invalid operands to binary expression ('const secp256k1_scalar' and 'const secp256k1_scalar')
211   51 |     *r = (*a + *b) % EXHAUSTIVE_TEST_ORDER;
212      |           ~~ ^ ~~
213src/scalar_low_impl.h:51:22: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
214   51 |     *r = (*a + *b) % EXHAUSTIVE_TEST_ORDER;
215      |                      ^
216src/scalar_low_impl.h:54:15: error: invalid operands to binary expression ('secp256k1_scalar' and 'const secp256k1_scalar')
217   54 |     return *r < *b;
218      |            ~~ ^ ~~
219src/scalar_low_impl.h:61:12: error: invalid operands to binary expression ('secp256k1_scalar' and 'uint32_t' (aka 'unsigned int'))
220   61 |         *r += ((uint32_t)1 << bit);
221      |         ~~ ^  ~~~~~~~~~~~~~~~~~~~~
222src/scalar_low_impl.h:72:8: error: assigning to 'secp256k1_scalar' from incompatible type 'int'
223   72 |     *r = 0;
224      |        ^ ~
225src/scalar_low_impl.h:74:18: error: invalid operands to binary expression ('secp256k1_scalar' and 'int')
226   74 |         *r = (*r * 0x100) + b32[i];
227      |               ~~ ^ ~~~~~
228src/scalar_low_impl.h:75:19: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
229   75 |         if (*r >= EXHAUSTIVE_TEST_ORDER) {
230      |                   ^
231src/scalar_low_impl.h:77:19: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
232   77 |             *r %= EXHAUSTIVE_TEST_ORDER;
233      |                   ^
234src/scalar_low_impl.h:89:18: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
235   89 |     bin[28] = *a >> 24; bin[29] = *a >> 16; bin[30] = *a >> 8; bin[31] = *a;
236      |               ~~ ^  ~~
237src/scalar_low_impl.h:89:38: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
238   89 |     bin[28] = *a >> 24; bin[29] = *a >> 16; bin[30] = *a >> 8; bin[31] = *a;
239      |                                   ~~ ^  ~~
240src/scalar_low_impl.h:89:58: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
241   89 |     bin[28] = *a >> 24; bin[29] = *a >> 16; bin[30] = *a >> 8; bin[31] = *a;
242      |                                                       ~~ ^  ~
243src/scalar_low_impl.h:89:72: error: assigning to 'unsigned char' from incompatible type 'const secp256k1_scalar'
244   89 |     bin[28] = *a >> 24; bin[29] = *a >> 16; bin[30] = *a >> 8; bin[31] = *a;
245      |                                                                        ^ ~~
246src/scalar_low_impl.h:95:15: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
247   95 |     return *a == 0;
248      |            ~~ ^  ~
249src/scalar_low_impl.h:101:12: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
250  101 |     if (*a == 0) {
251      |         ~~ ^  ~
252src/scalar_low_impl.h:102:12: error: assigning to 'secp256k1_scalar' from incompatible type 'int'
253  102 |         *r = 0;
254      |            ^ ~
255src/scalar_low_impl.h:104:14: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
256  104 |         *r = EXHAUSTIVE_TEST_ORDER - *a;
257      |              ^
258src/scalar_low_impl.h:113:15: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
259  113 |     return *a == 1;
260      |            ~~ ^  ~
261src/scalar_low_impl.h:119:17: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
262  119 |     return *a > EXHAUSTIVE_TEST_ORDER / 2;
263      |                 ^
264src/scalar_low_impl.h:135:14: error: invalid operands to binary expression ('const secp256k1_scalar' and 'const secp256k1_scalar')
265  135 |     *r = (*a * *b) % EXHAUSTIVE_TEST_ORDER;
266      |           ~~ ^ ~~
267src/scalar_low_impl.h:135:22: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
268  135 |     *r = (*a * *b) % EXHAUSTIVE_TEST_ORDER;
269      |                      ^
270src/scalar_low_impl.h:144:9: error: assigning to 'secp256k1_scalar' from incompatible type 'int'
271  144 |     *r2 = 0;
272      |         ^ ~
273src/scalar_low_impl.h:154:15: error: invalid operands to binary expression ('const secp256k1_scalar' and 'const secp256k1_scalar')
274  154 |     return *a == *b;
275      |            ~~ ^  ~~
276src/scalar_low_impl.h:165:14: error: invalid operands to binary expression ('secp256k1_scalar' and 'uint32_t' (aka 'unsigned int'))
277  165 |     *r = (*r & mask0) | (*a & mask1);
278      |           ~~ ^ ~~~~~
279src/scalar_low_impl.h:165:29: error: invalid operands to binary expression ('const secp256k1_scalar' and 'uint32_t' (aka 'unsigned int'))
280  165 |     *r = (*r & mask0) | (*a & mask1);
281      |                          ~~ ^ ~~~~~
282src/scalar_low_impl.h:175:21: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
283  175 |     for (i = 0; i < EXHAUSTIVE_TEST_ORDER; i++) {
284      |                     ^
285src/scalar_low_impl.h:176:16: error: invalid operands to binary expression ('int' and 'const secp256k1_scalar')
286  176 |         if ((i * *x) % EXHAUSTIVE_TEST_ORDER == 1) {
287      |              ~ ^ ~~
288src/scalar_low_impl.h:176:24: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
289  176 |         if ((i * *x) % EXHAUSTIVE_TEST_ORDER == 1) {
290      |                        ^
291src/scalar_low_impl.h:185:8: error: assigning to 'secp256k1_scalar' from incompatible type 'uint32_t' (aka 'unsigned int')
292  185 |     *r = res;
293      |        ^ ~~~
294src/scalar_low_impl.h:201:33: error: invalid operands to binary expression ('const secp256k1_scalar' and 'int')
295  201 |     *r = (*a + ((-(uint32_t)(*a & 1)) & EXHAUSTIVE_TEST_ORDER)) >> 1;
296      |                              ~~ ^ ~
297src/scalar_low_impl.h:201:41: error: use of undeclared identifier 'EXHAUSTIVE_TEST_ORDER'
298  201 |     *r = (*a + ((-(uint32_t)(*a & 1)) & EXHAUSTIVE_TEST_ORDER)) >> 1;
299      |                                         ^
30034 errors generated.

Perhaps some of these cases can be improved further, but these improvements should go to a separate PR.

What is the motivation for these changes?

Self-contained files work much better with tooling. For example, the [clangd](https://clangd.llvm.org/installation) language server works great in my editor after this PR (ignoring the four files mentioned above) and gives me much better completion, symbol lookup, etc. In fact, what made me create this PR is that I got annoyed by not being able to jump to symbols when trying to review silent payments.

Here’s my ~/.config/clangd/config.yaml if you’re interested:

 0# libsecp256k1
 1If:
 2  PathMatch: .*/secp256k1/.*
 3CompileFlags:
 4    # clangd compiles even _impl.h files individually, so it won't
 5    # find definitions of static functions in other files.
 6    Add: [-Wno-undefined-internal]
 7Diagnostics:
 8  # Disable hints about unused and missing includes. The assumptions that clangd
 9  # make about inclusion style are too far off from what libsecp256k1 does.
10  UnusedIncludes: None
11  MissingIncludes: None

You’ll also need to run make clean and bear -- make once to create a compile_commands.json file in the repo root that contains the compiler flags so that clangd knows the configured -D flags etc. Get bear first.

edit: CMake can also generate the json file:

0$ mkdir build
1$ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -B build
2$ cmake --build build

Then the file will be stored in the build subdirectory, whose name is special-cased in clangd so that it the file will be found automatically.

Or the flags can be added above in CompileFlags manually.

real-or-random added the label tweak/refactor on Aug 14, 2025

real-or-random added the label meta/development on Aug 14, 2025

real-or-random force-pushed on Aug 14, 2025

real-or-random commented at 12:27 pm on August 14, 2025: contributor

Remaining pain points that I noticed

There’s no way to enforce that we keep this in the future. I consider adding a check to CI that just compiles all files individually (with a list of exceptions for now).
It’s still annoying to include the right header (=non-impl) .h files. CI can tell us if we forget a file but not if we have too many files. Our inclusion structure is too complicated for tools to figure out automatically (e.g., you should include scalar.h even if some defs you use are in scalar_4x64.h, and I’m not convinced that adding special code comments to explain our structure to tools is the right approach. I’d tend to think that we should just do the opposite thing and create an “umbrella” header internal.h that includes all the internal headers. This makes it simpler for the developer, and the only cost is perhaps a slightly increased compilation time but only when individual files are compiled, i.e., in your IDE if you choose to use clangd or some other language server). But the project is not huge, and I doubt that this will be a problem.
util.h is a bit special. It has mostly preprocessor stuff and some small functions, so I guess it’s acceptable not to have a util_impl.h. But util.h currently contains also the preprocessor logic that picks the scalar, field, and int128 implementations based on the availability of widemul implementations. And this logic needs to happen before any of the resulting macros are used. So I think that part serves a different purpose than what the name “util” suggests and should be moved to a separate file.
We should also have headers for module implementation files, e.g., the extrakeys module “exports” secp256k1_extrakeys_ge_even_y which is used in the musig module.

in src/secp256k1_internal.h:1 in 925070e332

real-or-random commented at 12:29 pm on August 14, 2025:

I think what I picked is fine but I’m happy to take suggestions for the name of this file.

in src/secp256k1.c:43 in 925070e332 outdated

44 #include "int128_impl.h"
45+#include "modinv32_impl.h"
46+#ifdef SECP256K1_WIDEMUL_INT128
47+/* modinv64 is only available if we have an int128 implementation. */
48+#include "modinv64_impl.h"
49+#endif

real-or-random commented at 12:40 pm on August 14, 2025:

That part is a bit ugly, in particular because it needs to be duplicated in multiple C files.

Alternatives that I didn’t find better:

Make including modinv64_impl.h a no-op ifndef SECP256K1_WIDEMUL_INT128. If anything, that violates the principle of least surprise.
Create a modinv_impl.h that selects between the two files. That’s possible, but conceptually not really the right thing to do. modinv64 and modinv32 are two separate modules with different interface. They’re not just two different implementations of the same interfaces (as is the case for the different field, scalar, int128 implementations).

real-or-random force-pushed on Aug 14, 2025

tests: Make ecmult tests use internal scratch API

This has the advantage of keeping the ecmult tests focused on the ecmult
code, and so this change might be desirable anyway. That's why it is
done in a separate commit, so it can be kept if we ever bring back the
public scratch space API.

76ce607df4

Remove ununsed scratch space functions

These were in the public API at some point but they have been made
static in 712e7f8722eba5dec2bc6b37d75aadeb6f6e633b.

bb37ad8fa0

scalar: Move definitions of constants to header d72f3613cc

scalar: Move EXHAUSTIVE_TEST_LAMBDA to header 29e82e7649

Create header for internal API in secp256k1.c 3304ce0c99

gitignore: Add precompiled headers (.gch and .pch)

We don't use precompiled headers in our builds but those files are what
you get if you invoke gcc or clang, respectively, on a .h file, e.g., to check
if it's self-contained.

f181af00d7

group: Add missing declarations to internal header 151a2da0ae

Tidy includes to make many headers self-contained 250c61450f

field: Inline fe_verify fields in repr headers

Otherwise, the repr headers field_AxB.h would need to include field.h to
be self-contained but this recursive inclusion leaves clangd confused,
see https://github.com/clangd/clangd/issues/337 .

8969deb942

bench_internal: Remove unnecessary includes

secp256k1.c is included anyway, so there's no need to include the
individual _impl.h files.

0e17403bb6

group: Move definitions of constants to header

 SECP256K1_B and secp256k1_ge_const_g are used outside of
 group_impl.h, e.g., in ecmult_const_impl.h and ecmult_gen_impl.h,
 respectively.

b7840be6dc

Include modinv32/64 like other internal modules

And also sort the _impl.h includes.

5ac6929f54

selftest: Split into .h and _impl.h a2f118e6a9

ecmult_gen: Export _ecmult_gen_context_is_built() 8cce3922b4

real-or-random force-pushed on Aug 18, 2025

real-or-random commented at 3:48 pm on August 18, 2025: contributor

Added another commit that fixes a missing declaration in ecmult_gen.h

Make files mostly self-contained #1724

Why “mostly” self-contained?

What is the motivation for these changes?

Remaining pain points that I noticed