apoelstra / secp256k1-zkp

Personal fork of -zkp. Has some old mimblewimble stuff but mostly I just PR everything upstream now.
MIT License
82 stars 27 forks source link

branch: bullet-rangeonly in rangeproof_verify failing on borromean memcmp #2

Closed FrankC01 closed 6 years ago

FrankC01 commented 6 years ago

I am running into an issue (RC=0) on rangeproof_verify where, at the end of the borromean_verify the memory compare is failing. I am still quite the n00b on this and haven't reached a level of understanding to understand why.

The rangeproof_info runs and returns what appears to be sensible min/max value range. The rangeproof_verify fails (but the min/max range returned are the same as the rangeproof_info)

Here are some of the data points (converted to hex strings for this issue): Pederson commit (RC=1) : commit result = 8186b483d56a33826ae73d88f732985c4ccb1f32ba35f4b4cc47fdcf04aa6eb

rangeproof_sign: (RC=1): proof result = 651 bytes with: 60700000001e71d15da123812f19dbdae65812ac8433ece2d6dbf4fda54ee87a112620c8469ae80bc67945d9a15aea39018195a3b75babce28a683c10688a21c43769388594b417548738a277be6b728b197c659342ca55b2188bd96e3723139cfe19d8b734c5afa7eb9b01fb46f5b40fcfc62ff7864c72a4cab1a543fd6bc695d97b0b83dad4a9fe6f4198b1a97204a12ea298296a9be1c1b1cac28d075a31dfebfedc3cb515a1b97ed96c2fd848795f62295fdaa293bad5427ed2417a59df9a6a85b8ec4dce06d6632d247a8ec2dcaf3f9eb3d071deb71242434c389f355637647afcf67cca57cc64d6735fef67ea977bd556ac3824a77e7402f9825cbe0157d7aa0f8bf2e68e0aa267f0471189b6d0cc242e404abdc34b5d4d1403ad03e4d375e4784fcba7d3066e716fa52761513eaace3c338aeaefefea3ad9ef627521d1ce8cb2608a683e8bf4b36fb01465bc2b5e55dbab981b61c43ef27123d83bdafa0d180797640e7b5ded4cdb396cfd71d9ddaebfb88582ed3d05a2388b47affb4e4b67232c9ce3691552e9bcb7ef7b1b845ffed7fba9116ce62ecbc40f542f94056ab20ffc72af0142fdec0bf5d310be269c25bb5ddaa23737a6422de65274f668529adcc5bc7bb7ab854d47c14e27f2d4f56079b79e7d552e4fbe85527b6e56fc3efc0884cde739ff953b2dfbc61d41ad9f9a61d98d86628563f5f88d3e83576954592ca8eba3ce5acfe6670cb4d5d175997814bf64180fe56a41845c6dd49f47d6cef1c40c59cc054154af6c12a84bbfd9c531526618cd098793675ccf5d5c261669c2fe10484c701f2cf15bb1c74087d1a3893e3b72ca2ba2d18f295af7b5bcaf70877842ba20307a6edd6324414b380c4a99cc8

In rangeproof_verify, in borromean verify: e0 = 8b734c5afa7eb9b01fb46f5b40fcfc62ff7864c72a4cab1a543fd6bc695d97b0 tmp = acd4ce0ac50ba4fe2a7732a821010e8d3247316781370f9bef0561bbcf289e

Any insight would be helpful.

apoelstra commented 6 years ago

rangeproof_sign does not produce a 651 byte output given any input. It sounds like you are using bulletproof_rangeproof_sign, in which case rangeproof_verify will obviously fail.

FrankC01 commented 6 years ago

Hmmm. Even though I build the bullet-rangeonly branch, the calls I am making are:

secp256k1_pedersen_commit secp256k1_rangeproof_sign secp256k1_rangeproof_verify secp256k1_rangeproof_info

As defined in the include/secp256k1_rangeproof.h header

apoelstra commented 6 years ago

What are the arguments to rangeproof_sign?

FrankC01 commented 6 years ago
  1. Context setup for 'both'
  2. Pointer to 5134 byte buffer
  3. Pointer to 5134 buffer length
  4. Min_value (in this case 30)
  5. Pointer to the commitment
  6. Pointer to 32 byte blind array (populated arbitrarily but consistent with commitment)
  7. Pointer to nonce where I use the commitment.data pointer
  8. Exponent (0 in this case)
  9. min_bits (8 in this case)
  10. Original value (65 in this case and used in commitment generation)
  11. Pointer to message[120] containing string
  12. Message len (120)
  13. Passing NULL for extra pointer 14 Passing 0 for extra pointer length
  14. &secp256k1_generator_const_h (for last argument)
apoelstra commented 6 years ago

I'd need to see complete code to see what's going wrong. Does it work if you increase min_bits to 32? I'm not sure what you mean by blind being "populated arbitrarily but consistent with commitment". That parameter is a 32-byte char array that needs to be equal to the blinding factor of the commitment, it is not arbitrary at all.

FYI using commitment.data as a nonce means the rangeproof is not zero-knowledge. It won't hurt correctness but in a real system nonce needs to be secret.

FrankC01 commented 6 years ago

Setting min_bits to 32 increases the proof size to 2572 but still getting 0 on verify RC. What I meant by the blind being arbitrary: It is consistent for both the commitment and the rangeproof_sign as it is the same 32 byte string.

I will factor the code to a smaller set to just reproduce the error and post to this issue:

void test_rangeproof(secp256k1_context *sign, secp256k1_context *verify, secp256k1_context *both) {

    //  Setup commitment

    secp256k1_pedersen_commitment   commitment;
    const unsigned char         blind[32]="01234567890123456789012345678901";
    uint64_t                value = 65;

    int commit_res = secp256k1_pedersen_commit(
        sign,
        &commitment,
        blind,
        value,
        &secp256k1_generator_const_g,
        &secp256k1_generator_const_h);

    printf("Commitment RC = %i\n", commit_res);
    printf("Commitment = ");
    for(uint64_t i=0; i < sizeof(commitment.data); i++)
        printf("%x", commitment.data[i]);

    //  Setup proof

    const unsigned char message[120] = "When I see my own likeness in the depths of someone else's consciousness,  I always experience a moment of panic.";
    size_t          message_len = sizeof(message);
    unsigned char   proof[5134];
    uint64_t        proof_len = sizeof(proof);
    uint64_t        min_value = 35;
    int         min_bits = 32;
    int         exponent = 0;

    int proof_res = secp256k1_rangeproof_sign(
        both,
        proof,
        &proof_len,
        min_value,
        &commitment,
        blind,
        commitment.data,
        exponent,
        min_bits,
        value,
        message,
        message_len,
        NULL,
        0,
        &secp256k1_generator_const_h);

    printf("Proof RC = %i\n", proof_res);
    printf("Proof len = %lu\n", proof_len);
        for(uint64_t i=0; i < proof_len; i++)
          printf("%x", proof[i]);
        printf("\n");

    //  Setup verify
    uint64_t            verify_min=0;
    uint64_t            verify_max=0;

    int verify_res = secp256k1_rangeproof_verify(
        verify,
        &verify_min,
        &verify_max,
        &commitment,
        proof,
        proof_len,
        NULL,
        0,
        &secp256k1_generator_const_h);

   printf("Verify RC = %i\n", verify_res);
    printf("Verify min value = %lu\n", verify_min);
    printf("Verify max value = %lu\n", verify_max);
}

int main(int argc, char argv[]) {
    secp256k1_context *sign_ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
    secp256k1_context *verify_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
    secp256k1_context *both_ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
    int32_t ecount=0;

    secp256k1_context_set_error_callback(sign_ctx, counting_illegal_callback_fn, &ecount);
    secp256k1_context_set_error_callback(verify_ctx, counting_illegal_callback_fn, &ecount);
    secp256k1_context_set_error_callback(both_ctx, counting_illegal_callback_fn, &ecount);
    secp256k1_context_set_illegal_callback(sign_ctx, counting_illegal_callback_fn, &ecount);
    secp256k1_context_set_illegal_callback(verify_ctx, counting_illegal_callback_fn, &ecount);
    secp256k1_context_set_illegal_callback(both_ctx, counting_illegal_callback_fn, &ecount);

    test_rangeproof(sign_ctx, verify_ctx, both_ctx);

  // ...
  return 0;
 }
apoelstra commented 6 years ago

Ah! In secp256k1_pedersen_commit you need to swap the G and H generators. The old rangeproof code always uses the standard G generator for the blinding factor, and the alternate generator for the value.

The reason is that the value is always ≤ 64 bits, while the blinding factor is always 256 bits. We have giant precomp tables for G (this is what context_create(SECP256K1_CONTEXT_SIGN) creates) so better to use that for the larger multiplication.

FrankC01 commented 6 years ago

Wow, I'm staring at my own code too long. Thanks for that, in the meantime I've setup using the bulletproof_xxx calls (which worked without issue).