Skip to content

test: introduce group order byte-array constant for deduplication #1745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+13 −25
Open

test: introduce group order byte-array constant for deduplication #1745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions src/modules/ecdh/tests_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,7 @@ static void test_ecdh_generator_basepoint(void) {

static void test_bad_scalar(void) {
unsigned char s_zero[32] = { 0 };
unsigned char s_overflow[32] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe,
0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b,
0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41
};
unsigned char s_overflow[32] = { 0 };
unsigned char s_rand[32] = { 0 };
unsigned char output[32];
secp256k1_scalar rand;
Expand All @@ -107,6 +102,7 @@ static void test_bad_scalar(void) {
CHECK(secp256k1_ec_pubkey_create(CTX, &point, s_rand) == 1);

/* Try to multiply it by bad values */
memcpy(s_overflow, secp256k1_group_order_bytes, 32);
CHECK(secp256k1_ecdh(CTX, output, &point, s_zero, NULL, NULL) == 0);
CHECK(secp256k1_ecdh(CTX, output, &point, s_overflow, NULL, NULL) == 0);
/* ...and a good one */
Expand Down
22 changes: 3 additions & 19 deletions src/tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6036,12 +6036,7 @@ static void run_ec_pubkey_parse_test(void) {
}

static void run_eckey_edge_case_test(void) {
const unsigned char orderc[32] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe,
0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b,
0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41
};
const unsigned char *orderc = secp256k1_group_order_bytes;
const unsigned char zeros[sizeof(secp256k1_pubkey)] = {0x00};
unsigned char ctmp[33];
unsigned char ctmp2[33];
Expand Down Expand Up @@ -6355,13 +6350,7 @@ static int nonce_function_test_retry(unsigned char *nonce32, const unsigned char
return 1;
}
if (counter < 5) {
static const unsigned char order[] = {
0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,
0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFE,
0xBA,0xAE,0xDC,0xE6,0xAF,0x48,0xA0,0x3B,
0xBF,0xD2,0x5E,0x8C,0xD0,0x36,0x41,0x41
};
memcpy(nonce32, order, 32);
memcpy(nonce32, secp256k1_group_order_bytes, 32);
if (counter == 4) {
nonce32[31]++;
}
Expand Down Expand Up @@ -7379,12 +7368,7 @@ static void test_ecdsa_edge_cases(void) {
/* Privkey export where pubkey is the point at infinity. */
{
unsigned char privkey[300];
unsigned char seckey[32] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe,
0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b,
0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41,
};
const unsigned char *seckey = secp256k1_group_order_bytes;
size_t outlen = 300;
CHECK(!ec_privkey_export_der(CTX, privkey, &outlen, seckey, 0));
outlen = 300;
Expand Down
8 changes: 8 additions & 0 deletions src/testutil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
#include "testrand.h"
#include "util.h"

/* group order of the secp256k1 curve in 32-byte big endian representation */
static const unsigned char secp256k1_group_order_bytes[32] = {
Copy link
Member

@furszy furszy Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny q: why not use capital letters for a constant?
It’s usually the convention to distinguish functions vs constants so we don't have to search for its definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair question. It seems that we use capital letters mainly for constants and macros defined with the preprocessor (i.e. #define ...), where as actual global constant objects are still in lowercase, see e.g.

$ git grep "^static const"

vs.

$ git grep "^#define"

Maybe it's worth it to add that to the style conventions section in CONTRIBUTING.md (and fix the few exceptions that don't follow this conventions, e.g. SECP256K1_SIGNED{30,62}_ONE).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don’t you feel this is one of those things so widely accepted that it’s strange to have to formally state it? It’s like seeing someone write class members in capital letters.

In any case, it was a tiny question anyway. The upside of this change is that we can script any future change, since everything uses the same variable now.

0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe,
0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b,
0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41
};

static void testutil_random_fe(secp256k1_fe *x) {
unsigned char bin[32];
do {
Expand Down