arnaucube / go-snark-study

zkSNARK library implementation in Go from scratch (compiler, setup, prover, verifier)
GNU General Public License v3.0
255 stars 57 forks source link

Changing Public Signals on Proof does not effect verification #1

Closed drbh closed 5 years ago

drbh commented 5 years ago

No matter the specified Public Signal the proof will return verified. This should create a different output and cause the verification function to return false somewhere around here

https://github.com/arnaucube/go-snark/blob/master/snark.go#L274

pubsigfail

In the attached screenshot you can see the reproduced bugs and some extra output printed. You can see that in both cases (the correct public signal, and the bad one)

Utils.Bn.G1.MulScalar(setup.Vk.A[i+1], proof.PublicSignals[i]) returns 0,0,0 and then we add this to setup.Vk.A[0] and we get the value of setup.Vk.A[0] back again... as we can see here the function will never return false for a bad public signal...

arnaucube commented 5 years ago

thank you very much, that's right I'll try to have time to fix this (and other stuff that I just discovered that is wrong) next week

drbh commented 5 years ago

Looking forward to the updates. Is there anyway I can help?

I did some digging and the function GenerateTrustedSetup seems to always sets setup.VK.A[1] as [0,0,0].

Not sure what to change around: https://github.com/arnaucube/go-snark/blob/master/snark.go#L171

whiteJadeSoup commented 5 years ago

The problem still exists。setup.VK.A[1] set to [0,0,0] as QAPs' A[1] always set to [0,0,0,0]. And I notice your code is Go version of Vitalik Buterin's code in article https://medium.com/@VitalikButerin/quadratic-arithmetic-programs-from-zero-to-hero-f6d558cea649.

Maybe he's wrong in the process converting R1CS to QAP?

arnaucube commented 5 years ago

yes,

2 days ago I opened the issue #3 , currently I'm fixing it and have some code changes doing it, but I'm not having as much free time as I desire. Hope to find some more free time and fix this all

whiteJadeSoup commented 5 years ago

After days of research, I think I know where the problem lies.

Looking at the following code, we can find the reason.

    // step 1: cals vkX
    vkX := vk.vkIC[0]
    for i := 0; i < vk.nPublic; i++ {
        vkX = G1.Add(vkX, G1.MulScalar(vk.vkIC[i+1], publicSignals[i]))
    }

It seems VkX doesn't affect whether PublicSignals is correct or not.

After hours of debugging, I found vkIC[1] (equal to VK.A[1]) set to [0,0,0,0] as QAPs' A[1] always set to [0,0,0,0].

And QAP is the transformation of r1cs. So. what's wrong with r1cs?

The following value is R1CS'a which will be converted to A in QAP.

[0, 0, 1,  0, 0, 0]
[0, 0, 0,  1, 0, 0]
[0, 0, 1,  0, 1, 0]
[5, 0, 0,  0, 0, 1]

According to the method of transformation, we can get the following equation: (r1, r2, r3…) is the distinct number for the ith equation.

A1[r1] = a[0][1] = 0 A1[r2] = a[1][1] = 0 A1[r3] = a[2][1] = 0 A1[r4] = a[3][1] = 0

so the A1(x) = 0

In your code, publicSignals are same as outputs, and output never presents in the r1cs' a. so in the column representing outputs alway zero in a. That is exactly the problem.

I did some minor change. in my code. flat code should be like this.

    flatCode := 
    func test(1n, x):
        aux = x * x
        y = aux * x
        z = x + y
        sym = 1n * 1
        out = z + 5

1n is the same as out. but with the help of 1n, the publicInputs now show up in the a, Verify takes as input publicInputs which you have to pass manually.

different publicInputs, different outcome. Not elegant, but works.

arnaucube commented 5 years ago

That's right! I'll start a branch to update circuit compiler to add something like:

func test(private x, public r):
    aux = x*x
    y = aux*x
    z = x+y
    res = z+5
    equals(r, res)
    out = 1

In order to be able to specify private and public inputs, and also fix the current error.

Thanks again @dzshubin :)

arnaucube commented 5 years ago

code updated and working now.

If you find anything broken let me know, I've been doing multiple tests and seems that works properly now. And thanks again for your points :) were very helpful

arnaucube commented 5 years ago

I'll close this issue as it seems to be resolved now, ping me and reopen it if you found something related