fix: resolve zero-value proof failure in tests

mrtcnk · mrtcnk · commit 63e41d6e7059 · 2026-02-19T16:20:51.000-05:00
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.
diff --git a/src/bulletproof_aggregated.c b/src/bulletproof_aggregated.c
@@ -1220,20 +1220,49 @@ int secp256k1_bulletproof_prove_agg(
     {
         secp256k1_pubkey tG, tH, tB;
         const secp256k1_pubkey* pts[3];
+        int n_pts = 0;
 
-        if (!secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, al, n)) goto cleanup;
-        if (!secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, ar, n)) goto cleanup;
+        /* A Calculation */
+        /* 1. alpha * Base (Always exists) */
         tB = *pk_base;
         if (!secp256k1_ec_pubkey_tweak_mul(ctx, &tB, alpha)) goto cleanup;
-        pts[0] = &tB; pts[1] = &tG; pts[2] = &tH;
-        if (!secp256k1_ec_pubkey_combine(ctx, &A, pts, 3)) goto cleanup;
+        pts[n_pts++] = &tB;
 
-        if (!secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, sl, n)) goto cleanup;
-        if (!secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, sr, n)) goto cleanup;
+        /* 2. <al, G> (Only add if al is non-zero) */
+        /* Note: ipa_msm returns 0 if result is infinity (all scalars 0). This is valid for value=0. */
+        if (secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, al, n)) {
+            pts[n_pts++] = &tG;
+        }
+
+        /* 3. <ar, H> (Only add if ar is non-zero) */
+        if (secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, ar, n)) {
+            pts[n_pts++] = &tH;
+        }
+
+        /* Combine valid points for A */
+        if (!secp256k1_ec_pubkey_combine(ctx, &A, pts, n_pts)) goto cleanup;
+
+
+        /* S Calculation (Follows same logic for consistency) */
+        n_pts = 0;
+
+        /* 1. rho * Base */
         tB = *pk_base;
         if (!secp256k1_ec_pubkey_tweak_mul(ctx, &tB, rho)) goto cleanup;
-        pts[0] = &tB; pts[1] = &tG; pts[2] = &tH;
-        if (!secp256k1_ec_pubkey_combine(ctx, &S, pts, 3)) goto cleanup;
+        pts[n_pts++] = &tB;
+
+        /* 2. <sl, G> */
+        if (secp256k1_bulletproof_ipa_msm(ctx, &tG, G_vec, sl, n)) {
+            pts[n_pts++] = &tG;
+        }
+
+        /* 3. <sr, H> */
+        if (secp256k1_bulletproof_ipa_msm(ctx, &tH, H_vec, sr, n)) {
+            pts[n_pts++] = &tH;
+        }
+
+        /* Combine valid points for S */
+        if (!secp256k1_ec_pubkey_combine(ctx, &S, pts, n_pts)) goto cleanup;
     }
 
     /* ---- 6. Fiat–Shamir y,z ---- */
diff --git a/tests/test_bulletproof_agg.c b/tests/test_bulletproof_agg.c
@@ -6,184 +6,134 @@
 #include "secp256k1_mpt.h"
 #include "test_utils.h"
 
-/* ---- Aggregation parameters ---- */
-#define M 2
 #define BP_VALUE_BITS 64
 #define BP_TOTAL_BITS(m) ((size_t)(BP_VALUE_BITS * (m)))
-
-/* ---- Benchmark parameters ---- */
 #define VERIFY_RUNS 5
 
-
 /* ---- Helpers ---- */
-
 static inline double elapsed_ms(struct timespec a, struct timespec b) {
     return (b.tv_sec - a.tv_sec) * 1000.0 +
            (b.tv_nsec - a.tv_nsec) / 1e6;
 }
 
-/* ---- Main ---- */
-int main(void) {
-    printf("[TEST] Aggregated Bulletproof test (m = %d)\n", M);
+/* ---- Core Test Logic ---- */
+void run_test_case(secp256k1_context* ctx, const char* name, uint64_t* values, size_t m, int run_benchmarks) {
+    printf("\n[TEST] %s (m = %zu)\n", name, m);
 
-    /* ---- Context ---- */
-    secp256k1_context* ctx =
-            secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
-    EXPECT(ctx != NULL);
-
-    /* ---- Values ---- */
-    uint64_t values[M] = { 5000, 123456 };
-    unsigned char blindings[M][32];
-    secp256k1_pubkey commitments[M];
-
-    /* ---- Context Binding ---- */
+    unsigned char blindings[m][32];
+    secp256k1_pubkey commitments[m];
     unsigned char context_id[32];
     EXPECT(RAND_bytes(context_id, 32) == 1);
 
     secp256k1_pubkey pk_base;
-    /* Use the standard H generator from the library */
     EXPECT(secp256k1_mpt_get_h_generator(ctx, &pk_base));
 
     /* ---- Commitments ---- */
-    for (size_t i = 0; i < M; i++) {
+    for (size_t i = 0; i < m; i++) {
         random_scalar(ctx, blindings[i]);
         EXPECT(secp256k1_bulletproof_create_commitment(
-                ctx,
-                &commitments[i],
-                values[i],
-                blindings[i],
-                &pk_base));
+                ctx, &commitments[i], values[i], blindings[i], &pk_base));
     }
 
     /* ---- Generator vectors ---- */
-    const size_t n = BP_TOTAL_BITS(M);
+    const size_t n = BP_TOTAL_BITS(m);
     secp256k1_pubkey* G_vec = malloc(n * sizeof(secp256k1_pubkey));
     secp256k1_pubkey* H_vec = malloc(n * sizeof(secp256k1_pubkey));
     EXPECT(G_vec && H_vec);
 
-    EXPECT(secp256k1_mpt_get_generator_vector(
-            ctx, G_vec, n, (const unsigned char*)"G", 1));
-    EXPECT(secp256k1_mpt_get_generator_vector(
-            ctx, H_vec, n, (const unsigned char*)"H", 1));
+    EXPECT(secp256k1_mpt_get_generator_vector(ctx, G_vec, n, (const unsigned char*)"G", 1));
+    EXPECT(secp256k1_mpt_get_generator_vector(ctx, H_vec, n, (const unsigned char*)"H", 1));
 
-    /* ---- Prove (timed) ---- */
+    /* ---- Prove ---- */
     unsigned char proof[4096];
     size_t proof_len = sizeof(proof);
 
-    printf("[TEST] Proving aggregated range proof...\n");
-
     struct timespec t_p_start, t_p_end;
     clock_gettime(CLOCK_MONOTONIC, &t_p_start);
 
-    /* Note: We cast the 2D array 'blindings' to flat pointer */
     EXPECT(secp256k1_bulletproof_prove_agg(
-            ctx,
-            proof,
-            &proof_len,
-            values,
-            (const unsigned char*)blindings,
-            M,
-            &pk_base,
-            context_id));
+            ctx, proof, &proof_len, values, (const unsigned char*)blindings, m, &pk_base, context_id));
 
     clock_gettime(CLOCK_MONOTONIC, &t_p_end);
+    printf("  Proof size: %zu bytes\n", proof_len);
+    if (run_benchmarks) printf("  [BENCH] Proving time: %.3f ms\n", elapsed_ms(t_p_start, t_p_end));
 
-    printf("[TEST] Proof size = %zu bytes\n", proof_len);
-    printf("[BENCH] Proving time: %.3f ms\n",
-           elapsed_ms(t_p_start, t_p_end));
-
-    /* ---- Verify (single run, timed) ---- */
-    printf("[TEST] Verifying aggregated proof...\n");
-
+    /* ---- Verify ---- */
     struct timespec t_v_start, t_v_end;
     clock_gettime(CLOCK_MONOTONIC, &t_v_start);
 
     int ok = secp256k1_bulletproof_verify_agg(
-            ctx,
-            G_vec,
-            H_vec,
-            proof,
-            proof_len,
-            commitments,
-            M,
-            &pk_base,
-            context_id);
+            ctx, G_vec, H_vec, proof, proof_len, commitments, m, &pk_base, context_id);
 
     clock_gettime(CLOCK_MONOTONIC, &t_v_end);
-
     EXPECT(ok);
-
-    printf("PASSED\n");
-    printf("[BENCH] Verification time (single): %.3f ms\n",
-           elapsed_ms(t_v_start, t_v_end));
-
-    /* ---- Verify benchmark (average) ---- */
-    double total_ms = 0.0;
-
-    for (int i = 0; i < VERIFY_RUNS; i++) {
-        struct timespec ts, te;
-        clock_gettime(CLOCK_MONOTONIC, &ts);
-
-        ok = secp256k1_bulletproof_verify_agg(
-                ctx,
-                G_vec,
-                H_vec,
-                proof,
-                proof_len,
-                commitments,
-                M,
-                &pk_base,
-                context_id);
-
-        clock_gettime(CLOCK_MONOTONIC, &te);
-
-        EXPECT(ok);
-        total_ms += elapsed_ms(ts, te);
+    printf("  PASSED (Verification)\n");
+    if (run_benchmarks) printf("  [BENCH] Verification time: %.3f ms\n", elapsed_ms(t_v_start, t_v_end));
+
+    /* ---- Benchmark Verify ---- */
+    if (run_benchmarks) {
+        double total_ms = 0.0;
+        for (int i = 0; i < VERIFY_RUNS; i++) {
+            struct timespec ts, te;
+            clock_gettime(CLOCK_MONOTONIC, &ts);
+            ok = secp256k1_bulletproof_verify_agg(
+                    ctx, G_vec, H_vec, proof, proof_len, commitments, m, &pk_base, context_id);
+            clock_gettime(CLOCK_MONOTONIC, &te);
+            EXPECT(ok);
+            total_ms += elapsed_ms(ts, te);
+        }
+        printf("  [BENCH] Verification avg over %d runs: %.3f ms\n", VERIFY_RUNS, total_ms / VERIFY_RUNS);
     }
 
-    printf("[BENCH] Verification avg over %d runs: %.3f ms\n",
-           VERIFY_RUNS, total_ms / VERIFY_RUNS);
-
-    /* ---- Negative test ---- */
-    printf("[TEST] Tamper test... ");
-
-    secp256k1_pubkey bad_commitments[M];
-    memcpy(bad_commitments, commitments, sizeof(commitments));
-
+    /* ---- Negative Test (Tamper) ---- */
+    secp256k1_pubkey bad_commitments[m];
+    memcpy(bad_commitments, commitments, sizeof(secp256k1_pubkey) * m);
     unsigned char bad_blinding[32];
     random_scalar(ctx, bad_blinding);
 
-    /* Create a fake commitment to (value + 1) to break the sum */
+    /* Create fake commitment to (value + 1) */
     EXPECT(secp256k1_bulletproof_create_commitment(
-            ctx,
-            &bad_commitments[1],
-            values[M - 1] + 1,
-            bad_blinding,
-            &pk_base));
+            ctx, &bad_commitments[m - 1], values[m - 1] + 1, bad_blinding, &pk_base));
 
     ok = secp256k1_bulletproof_verify_agg(
-            ctx,
-            G_vec,
-            H_vec,
-            proof,
-            proof_len,
-            bad_commitments,
-            M,
-            &pk_base,
-            context_id);
+            ctx, G_vec, H_vec, proof, proof_len, bad_commitments, m, &pk_base, context_id);
 
     if (ok) {
         fprintf(stderr, "FAILED: Accepted invalid proof!\n");
         exit(EXIT_FAILURE);
     }
+    printf("  PASSED (Rejected invalid proof)\n");
 
-    printf("PASSED (rejected invalid proof)\n");
-
-    /* ---- Cleanup ---- */
     free(G_vec);
     free(H_vec);
-    secp256k1_context_destroy(ctx);
+}
 
-    printf("[TEST] Aggregated Bulletproof test completed successfully\n");
+/* ---- Main ---- */
+int main(void) {
+    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
+    EXPECT(ctx != NULL);
+
+    /* 1. Single proof, value 0 (The Bug Fix) */
+    uint64_t v1[] = {0};
+    run_test_case(ctx, "Single Proof (Value 0)", v1, 1, 0);
+
+    /* 2. Single proof, value 1 */
+    uint64_t v2[] = {1};
+    run_test_case(ctx, "Single Proof (Value 1)", v2, 1, 0);
+
+    /* 3. Single proof, MAX VALUE (Tests opposite vector edge case) */
+    uint64_t v3[] = {0xFFFFFFFFFFFFFFFF}; // 2^64 - 1
+    run_test_case(ctx, "Single Proof (MAX Value)", v3, 1, 0);
+
+    /* 4. Aggregated proof, {0, 1} */
+    uint64_t v4[] = {0, 1};
+    run_test_case(ctx, "Aggregated Proof (0, 1)", v4, 2, 0);
+
+    /* 5. Aggregated proof, {0, 0} with Benchmarks */
+    uint64_t v5[] = {0, 0};
+    run_test_case(ctx, "Aggregated Proof (0, 0)", v5, 2, 1);
+
+    secp256k1_context_destroy(ctx);
+    printf("\n[TEST] All Bulletproof tests completed successfully\n");
     return 0;
 }
