Skip to content

Commit f6c4d10

Browse files
mrtcnkmathbunnyru
andauthored
fix: resolve zero-value proof failure in tests (#13)
* fix: resolve zero-value proof failure in tests Updated the commitment logic to safely skip empty curve points (Point at Infinity) generated by all-zero bit vectors, rather than treating them as a fatal library error. * refactor: address reviewer comments for helper function and formatting * test: resolve Copilot warning by using dynamic allocation * fix: resolve msm masking and add test safety guards * fix: max-value overflow and explicit fast path for n_pts == 1 * Update src/bulletproof_aggregated.c Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com> * Update src/bulletproof_aggregated.c Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com> --------- Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
1 parent 81e531e commit f6c4d10

File tree

2 files changed

+140
-99
lines changed

2 files changed

+140
-99
lines changed

src/bulletproof_aggregated.c

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,6 +1092,62 @@ int secp256k1_bulletproof_create_commitment(
10921092
return 1;
10931093
}
10941094

1095+
/* Helper for a vector 0 */
1096+
static int scalar_vector_all_zero(const unsigned char* scalars, size_t n) {
1097+
unsigned char zero[32] = {0};
1098+
for (size_t i = 0; i < n; ++i) {
1099+
if (memcmp(scalars + 32*i, zero, 32) != 0)
1100+
return 0; /* found non-zero */
1101+
}
1102+
return 1; /* all zero */
1103+
}
1104+
1105+
/* Helper to calculate commitment terms like A and S */
1106+
static int calculate_commitment_term(
1107+
const secp256k1_context* ctx,
1108+
secp256k1_pubkey* out,
1109+
const secp256k1_pubkey* pk_base,
1110+
const unsigned char* base_scalar,
1111+
const unsigned char* vec_l,
1112+
const unsigned char* vec_r,
1113+
const secp256k1_pubkey* G_vec,
1114+
const secp256k1_pubkey* H_vec,
1115+
size_t n
1116+
) {
1117+
secp256k1_pubkey tG, tH, tB;
1118+
const secp256k1_pubkey* pts[3];
1119+
int n_pts = 0;
1120+
1121+
/* 1. base_scalar * Base */
1122+
tB = *pk_base;
1123+
if (!secp256k1_ec_pubkey_tweak_mul(ctx, &tB, base_scalar)) return 0;
1124+
pts[n_pts++] = &tB;
1125+
1126+
/* 2. <vec_l, G> */
1127+
if (!scalar_vector_all_zero(vec_l, n)) {
1128+
if (!secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, vec_l, n))
1129+
return 0; /* REAL FAILURE */
1130+
pts[n_pts++] = &tG;
1131+
}
1132+
1133+
/* 3. <vec_r, H> */
1134+
if (!scalar_vector_all_zero(vec_r, n)) {
1135+
if (!secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, vec_r, n))
1136+
return 0; /* REAL FAILURE */
1137+
pts[n_pts++] = &tH;
1138+
}
1139+
1140+
if (n_pts == 1) {
1141+
*out = *pts[0];
1142+
return 1;
1143+
}
1144+
1145+
if (!secp256k1_ec_pubkey_combine(ctx, out, pts, n_pts))
1146+
return 0;
1147+
1148+
return 1;
1149+
}
1150+
10951151
/**
10961152
* Generates an aggregated Bulletproof for m values.
10971153
*
@@ -1217,24 +1273,8 @@ int secp256k1_bulletproof_prove_agg(
12171273
* A = alpha*Base + <al,G> + <ar,H>
12181274
* S = rho*Base + <sl,G> + <sr,H>
12191275
*/
1220-
{
1221-
secp256k1_pubkey tG, tH, tB;
1222-
const secp256k1_pubkey* pts[3];
1223-
1224-
if (!secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, al, n)) goto cleanup;
1225-
if (!secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, ar, n)) goto cleanup;
1226-
tB = *pk_base;
1227-
if (!secp256k1_ec_pubkey_tweak_mul(ctx, &tB, alpha)) goto cleanup;
1228-
pts[0] = &tB; pts[1] = &tG; pts[2] = &tH;
1229-
if (!secp256k1_ec_pubkey_combine(ctx, &A, pts, 3)) goto cleanup;
1230-
1231-
if (!secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, sl, n)) goto cleanup;
1232-
if (!secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, sr, n)) goto cleanup;
1233-
tB = *pk_base;
1234-
if (!secp256k1_ec_pubkey_tweak_mul(ctx, &tB, rho)) goto cleanup;
1235-
pts[0] = &tB; pts[1] = &tG; pts[2] = &tH;
1236-
if (!secp256k1_ec_pubkey_combine(ctx, &S, pts, 3)) goto cleanup;
1237-
}
1276+
if (!calculate_commitment_term(ctx, &A, pk_base, alpha, al, ar, G_vec, H_vec, n)) goto cleanup;
1277+
if (!calculate_commitment_term(ctx, &S, pk_base, rho, sl, sr, G_vec, H_vec, n)) goto cleanup;
12381278

12391279
/* ---- 6. Fiat–Shamir y,z ---- */
12401280
{

tests/test_bulletproof_agg.c

Lines changed: 82 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -6,46 +6,33 @@
66
#include "secp256k1_mpt.h"
77
#include "test_utils.h"
88

9-
/* ---- Aggregation parameters ---- */
10-
#define M 2
119
#define BP_VALUE_BITS 64
1210
#define BP_TOTAL_BITS(m) ((size_t)(BP_VALUE_BITS * (m)))
13-
14-
/* ---- Benchmark parameters ---- */
1511
#define VERIFY_RUNS 5
1612

17-
1813
/* ---- Helpers ---- */
19-
2014
static inline double elapsed_ms(struct timespec a, struct timespec b) {
2115
return (b.tv_sec - a.tv_sec) * 1000.0 +
2216
(b.tv_nsec - a.tv_nsec) / 1e6;
2317
}
2418

25-
/* ---- Main ---- */
26-
int main(void) {
27-
printf("[TEST] Aggregated Bulletproof test (m = %d)\n", M);
19+
/* ---- Core Test Logic ---- */
20+
void run_test_case(secp256k1_context* ctx, const char* name, uint64_t* values, size_t num_values, int run_benchmarks) {
21+
EXPECT(num_values > 0);
22+
printf("\n[TEST] %s (num_values = %zu)\n", name, num_values);
2823

29-
/* ---- Context ---- */
30-
secp256k1_context* ctx =
31-
secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
32-
EXPECT(ctx != NULL);
33-
34-
/* ---- Values ---- */
35-
uint64_t values[M] = { 5000, 123456 };
36-
unsigned char blindings[M][32];
37-
secp256k1_pubkey commitments[M];
38-
39-
/* ---- Context Binding ---- */
24+
unsigned char (*blindings)[32] = malloc(num_values * sizeof(*blindings));
25+
secp256k1_pubkey* commitments = malloc(num_values * sizeof(*commitments));
4026
unsigned char context_id[32];
27+
28+
EXPECT(blindings != NULL && commitments != NULL);
4129
EXPECT(RAND_bytes(context_id, 32) == 1);
4230

4331
secp256k1_pubkey pk_base;
44-
/* Use the standard H generator from the library */
4532
EXPECT(secp256k1_mpt_get_h_generator(ctx, &pk_base));
4633

4734
/* ---- Commitments ---- */
48-
for (size_t i = 0; i < M; i++) {
35+
for (size_t i = 0; i < num_values; i++) {
4936
random_scalar(ctx, blindings[i]);
5037
EXPECT(secp256k1_bulletproof_create_commitment(
5138
ctx,
@@ -56,7 +43,7 @@ int main(void) {
5643
}
5744

5845
/* ---- Generator vectors ---- */
59-
const size_t n = BP_TOTAL_BITS(M);
46+
const size_t n = BP_TOTAL_BITS(num_values);
6047
secp256k1_pubkey* G_vec = malloc(n * sizeof(secp256k1_pubkey));
6148
secp256k1_pubkey* H_vec = malloc(n * sizeof(secp256k1_pubkey));
6249
EXPECT(G_vec && H_vec);
@@ -66,35 +53,28 @@ int main(void) {
6653
EXPECT(secp256k1_mpt_get_generator_vector(
6754
ctx, H_vec, n, (const unsigned char*)"H", 1));
6855

69-
/* ---- Prove (timed) ---- */
56+
/* ---- Prove ---- */
7057
unsigned char proof[4096];
7158
size_t proof_len = sizeof(proof);
7259

73-
printf("[TEST] Proving aggregated range proof...\n");
74-
7560
struct timespec t_p_start, t_p_end;
7661
clock_gettime(CLOCK_MONOTONIC, &t_p_start);
7762

78-
/* Note: We cast the 2D array 'blindings' to flat pointer */
7963
EXPECT(secp256k1_bulletproof_prove_agg(
8064
ctx,
8165
proof,
8266
&proof_len,
8367
values,
8468
(const unsigned char*)blindings,
85-
M,
69+
num_values,
8670
&pk_base,
8771
context_id));
8872

8973
clock_gettime(CLOCK_MONOTONIC, &t_p_end);
74+
printf(" Proof size: %zu bytes\n", proof_len);
75+
if (run_benchmarks) printf(" [BENCH] Proving time: %.3f ms\n", elapsed_ms(t_p_start, t_p_end));
9076

91-
printf("[TEST] Proof size = %zu bytes\n", proof_len);
92-
printf("[BENCH] Proving time: %.3f ms\n",
93-
elapsed_ms(t_p_start, t_p_end));
94-
95-
/* ---- Verify (single run, timed) ---- */
96-
printf("[TEST] Verifying aggregated proof...\n");
97-
77+
/* ---- Verify ---- */
9878
struct timespec t_v_start, t_v_end;
9979
clock_gettime(CLOCK_MONOTONIC, &t_v_start);
10080

@@ -105,59 +85,53 @@ int main(void) {
10585
proof,
10686
proof_len,
10787
commitments,
108-
M,
88+
num_values,
10989
&pk_base,
11090
context_id);
11191

11292
clock_gettime(CLOCK_MONOTONIC, &t_v_end);
113-
11493
EXPECT(ok);
115-
116-
printf("PASSED\n");
117-
printf("[BENCH] Verification time (single): %.3f ms\n",
118-
elapsed_ms(t_v_start, t_v_end));
119-
120-
/* ---- Verify benchmark (average) ---- */
121-
double total_ms = 0.0;
122-
123-
for (int i = 0; i < VERIFY_RUNS; i++) {
124-
struct timespec ts, te;
125-
clock_gettime(CLOCK_MONOTONIC, &ts);
126-
127-
ok = secp256k1_bulletproof_verify_agg(
128-
ctx,
129-
G_vec,
130-
H_vec,
131-
proof,
132-
proof_len,
133-
commitments,
134-
M,
135-
&pk_base,
136-
context_id);
137-
138-
clock_gettime(CLOCK_MONOTONIC, &te);
139-
140-
EXPECT(ok);
141-
total_ms += elapsed_ms(ts, te);
94+
printf(" PASSED (Verification)\n");
95+
if (run_benchmarks) printf(" [BENCH] Verification time: %.3f ms\n", elapsed_ms(t_v_start, t_v_end));
96+
97+
/* ---- Benchmark Verify ---- */
98+
if (run_benchmarks) {
99+
double total_ms = 0.0;
100+
for (int i = 0; i < VERIFY_RUNS; i++) {
101+
struct timespec ts, te;
102+
clock_gettime(CLOCK_MONOTONIC, &ts);
103+
ok = secp256k1_bulletproof_verify_agg(
104+
ctx,
105+
G_vec,
106+
H_vec,
107+
proof,
108+
proof_len,
109+
commitments,
110+
num_values,
111+
&pk_base,
112+
context_id);
113+
clock_gettime(CLOCK_MONOTONIC, &te);
114+
EXPECT(ok);
115+
total_ms += elapsed_ms(ts, te);
116+
}
117+
printf(" [BENCH] Verification avg over %d runs: %.3f ms\n", VERIFY_RUNS, total_ms / VERIFY_RUNS);
142118
}
143119

144-
printf("[BENCH] Verification avg over %d runs: %.3f ms\n",
145-
VERIFY_RUNS, total_ms / VERIFY_RUNS);
146-
147-
/* ---- Negative test ---- */
148-
printf("[TEST] Tamper test... ");
149-
150-
secp256k1_pubkey bad_commitments[M];
151-
memcpy(bad_commitments, commitments, sizeof(commitments));
120+
/* ---- Negative Test (Tamper) ---- */
121+
secp256k1_pubkey* bad_commitments = malloc(num_values * sizeof(*bad_commitments));
122+
EXPECT(bad_commitments != NULL);
152123

124+
memcpy(bad_commitments, commitments, sizeof(*commitments) * num_values);
153125
unsigned char bad_blinding[32];
154126
random_scalar(ctx, bad_blinding);
155127

156-
/* Create a fake commitment to (value + 1) to break the sum */
128+
/* Create fake commitment to (value + 1) */
157129
EXPECT(secp256k1_bulletproof_create_commitment(
158130
ctx,
159-
&bad_commitments[1],
160-
values[M - 1] + 1,
131+
&bad_commitments[num_values - 1],
132+
(values[num_values - 1] == UINT64_MAX)
133+
? values[num_values - 1] - 1
134+
: values[num_values - 1] + 1,
161135
bad_blinding,
162136
&pk_base));
163137

@@ -168,22 +142,49 @@ int main(void) {
168142
proof,
169143
proof_len,
170144
bad_commitments,
171-
M,
145+
num_values,
172146
&pk_base,
173147
context_id);
174148

175149
if (ok) {
176150
fprintf(stderr, "FAILED: Accepted invalid proof!\n");
177151
exit(EXIT_FAILURE);
178152
}
153+
printf(" PASSED (Rejected invalid proof)\n");
179154

180-
printf("PASSED (rejected invalid proof)\n");
181-
182-
/* ---- Cleanup ---- */
155+
free(bad_commitments);
156+
free(commitments);
157+
free(blindings);
183158
free(G_vec);
184159
free(H_vec);
185-
secp256k1_context_destroy(ctx);
160+
}
161+
162+
/* ---- Main ---- */
163+
int main(void) {
164+
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
165+
EXPECT(ctx != NULL);
166+
167+
/* 1. Single proof, value 0 (The Bug Fix) */
168+
uint64_t v1[] = {0};
169+
run_test_case(ctx, "Single Proof (Value 0)", v1, 1, 0);
170+
171+
/* 2. Single proof, value 1 */
172+
uint64_t v2[] = {1};
173+
run_test_case(ctx, "Single Proof (Value 1)", v2, 1, 0);
186174

187-
printf("[TEST] Aggregated Bulletproof test completed successfully\n");
175+
/* 3. Single proof, MAX VALUE (Tests opposite vector edge case) */
176+
uint64_t v3[] = {0xFFFFFFFFFFFFFFFF}; // 2^64 - 1
177+
run_test_case(ctx, "Single Proof (MAX Value)", v3, 1, 0);
178+
179+
/* 4. Aggregated proof, {0, 1} */
180+
uint64_t v4[] = {0, 1};
181+
run_test_case(ctx, "Aggregated Proof (0, 1)", v4, 2, 0);
182+
183+
/* 5. Aggregated proof, {0, 0} with Benchmarks */
184+
uint64_t v5[] = {0, 0};
185+
run_test_case(ctx, "Aggregated Proof (0, 0)", v5, 2, 1);
186+
187+
secp256k1_context_destroy(ctx);
188+
printf("\n[TEST] All Bulletproof tests completed successfully\n");
188189
return 0;
189190
}

0 commit comments

Comments
 (0)