pepper-project / pequin

A system for verifying outsourced computations, and applying SNARKs. Simplified release of the main Pepper codebase.
Other
122 stars 46 forks source link

Printf is broken #36

Closed jimouris closed 5 years ago

jimouris commented 5 years ago

Hello,

It seems that printf does not work as expected in the latest commits. I did not trace the origin of the problem, however in commit 4a441d3 it works as expected. Below are the same printings from two different commits (master and 4a441d3) for printing input hash and preimage ( from PR #27 ):

master:

bin/pepper_prover_prover_knows_hash_preimage prove prover_knows_hash_preimage.pkey prover_knows_hash_preimage.inputs prover_knows_hash_preimage.outputs prover_knows_hash_preimage.proof
NUMBER OF CONSTRAINTS:  35491
PRINTF in computation_p 0:
"Preimage\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 0:
"Input Hash\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
PRINTF in computation_p 1:
"1106433712\n" 
reading proving key from file...
Reset time counters for profiling
....

commit 4a441d3:

bin/pepper_prover_prover_knows_hash_preimage prove prover_knows_hash_preimage.pkey prover_knows_hash_preimage.inputs prover_knows_hash_preimage.outputs prover_knows_hash_preimage.proof
NUMBER OF CONSTRAINTS:  35491
PRINTF in computation_p 0:
"Preimage\n" 
PRINTF in computation_p 1:
"97\n" 
PRINTF in computation_p 1:
"98\n" 
PRINTF in computation_p 1:
"99\n" 
PRINTF in computation_p 1:
"100\n" 
PRINTF in computation_p 0:
"Input Hash\n" 
PRINTF in computation_p 1:
"136\n" 
PRINTF in computation_p 1:
"212\n" 
PRINTF in computation_p 1:
"38\n" 
PRINTF in computation_p 1:
"111\n" 
PRINTF in computation_p 1:
"212\n" 
PRINTF in computation_p 1:
"230\n" 
PRINTF in computation_p 1:
"51\n" 
PRINTF in computation_p 1:
"141\n" 
PRINTF in computation_p 1:
"19\n" 
PRINTF in computation_p 1:
"184\n" 
PRINTF in computation_p 1:
"69\n" 
PRINTF in computation_p 1:
"252\n" 
PRINTF in computation_p 1:
"242\n" 
PRINTF in computation_p 1:
"137\n" 
PRINTF in computation_p 1:
"87\n" 
PRINTF in computation_p 1:
"157\n" 
PRINTF in computation_p 1:
"32\n" 
PRINTF in computation_p 1:
"156\n" 
PRINTF in computation_p 1:
"137\n" 
PRINTF in computation_p 1:
"120\n" 
PRINTF in computation_p 1:
"35\n" 
PRINTF in computation_p 1:
"185\n" 
PRINTF in computation_p 1:
"33\n" 
PRINTF in computation_p 1:
"125\n" 
PRINTF in computation_p 1:
"163\n" 
PRINTF in computation_p 1:
"225\n" 
PRINTF in computation_p 1:
"97\n" 
PRINTF in computation_p 1:
"147\n" 
PRINTF in computation_p 1:
"111\n" 
PRINTF in computation_p 1:
"3\n" 
PRINTF in computation_p 1:
"21\n" 
PRINTF in computation_p 1:
"137\n" 
reading proving key from file...
Reset time counters for profiling
...

The second is the correct - expected output. Below I have also posted the printfs that I used for the above printings (file prover_knows_hash_preimage.c):

...
    exo_compute(input_hash, lens, preimage, 0);     /* fill preimage variable with prover's private preimage */

    printf("Preimage\n");
    for (i = 0 ; i < 4 ; i++) {
        printf("%d\n", preimage[0].preimage[i]);
    }

    printf("Input Hash\n");
    for (i = 0 ; i < SHA256_BLOCK_SIZE ; i++) {     /* check each byte of hash */
        printf("%d\n", input_hash[0][i]);
    }

    /* prover_hash = sha256(preimage) */
    SHA256_CTX ctx;
...

Dimitris Mouris

maxhowald commented 5 years ago

Thanks for reporting this! It looks like this commit:

https://github.com/pepper-project/pequin/commit/bef43f2e445e567a222bfcd3a308920fba1bb3dd

Is the only change that could have introduced the problem, but it doesn't look like it is doing anything obviously wrong; the only thing it does is change the type of the arguments passed to gmp_printf() from an int to mpz_t. What happens if you change the format flag from %d to %Zd?

jimouris commented 5 years ago

Yes, you are right, %Zd is the correct way to print integers in GMP and works fine. Probably should update readme or revert the change with mpz_set_q to mpz_get_si in order for %d to work as it used to.

maxhowald commented 5 years ago

I'm hesitant to revert this, since some other recent contributions depend on it, and mpz_t is at least a bit more general than restricting printf to signed ints.

Long term, it would be nice to pass type information to the prover's printf function, but I think this would require a significant rewrite. I'll add a note somewhere when I get the chance.

jimouris commented 5 years ago

Yes, I agree. Passing type information to printf would be a good solution.