witnet / vrf-rs

Verifiable Random Function (VRF) library written in Rust
MIT License
91 stars 37 forks source link

Different results #17

Closed kroggen closed 4 years ago

kroggen commented 4 years ago

Hi Mario and others,

Congrats for the great ECVRF implementation!

I made an implementation in C using the secp256k1 curve as you can check here

As there is no specification of suite string for this curve on IETF draft yet, I used the same number (0xFE) that you used here for compatibility.

Unfortunately the proofs are not completely the same: the first 2 parts, Gamma and c, match on our implementations. But the last part, s, differs.

Interestingly, the proof is verified correctly by the verify function. And the proofs you are using here do not pass the verification there.

I was wondering if this could be due to some curve parameter, but as the other parts are OK, it may not be. It could be just the last calculation of s. But then the proof would not pass the verification.

If you have any idea that could help, it would be very appreciated.

Cheers,

girazoki commented 4 years ago

Hi Bernardo!

Thanks for taking a look at our library. If c and Gamma are identical, the only thing I can think about for failing the verification is that you might be doing s = (k - c*x) mod q while we are doing s = (k + c*x) mod q. There is no difference in doing that as long as in the verification you are adding U = sB + cY. In our case, we are substracting in the verification U = sB - cY.

In any case we can take a look at your implementation and address this issue better.

Cheers,

Gorka

kroggen commented 4 years ago

Hi Gorka,

Thank you for your time!

I found the problem:

We are dealing with the c scalar in a different way. I put the 16 bytes at the beginning and put zeros at the end, as done by the Algorand code here

When I changed it to the other way around, the result was the same as yours.

As this is done so in the prove as in the verify functions, it passes the verification.

Now we need to check the specification.

kroggen commented 4 years ago

Your code is right according to the IETF draft.

I would not like to say that Algorand's implementation is "wrong", because Silvio Micali was one of the inventors of the VRFs... And also won a Turing Award. Maybe they have a reason to do that in that way?

Interestingly, their implementation mentions the ietfdraft03. And nothing is mentioned about the difference on the README

girazoki commented 4 years ago

Hi again! Glad you found the issue.

I would say it does not really matter as longs as you truncate it to the appropriate length and on the verification side you truncate identically. That being said, we have tried to follow the IETF draft as much as possible. Apparently ietfdraft03 also truncates the same way as the latest version of the draft.

kroggen commented 4 years ago

I contacted them and I am waiting for the answer. If there is no special reason for that, then I will make my implementation compatible with yours for standardization.

Thank you for the help!