akosba / jsnark

A Java library for zk-SNARK circuits
MIT License
207 stars 85 forks source link

A Trivial Issue: setWireValue(Wire w, long v) #4

Closed yylluu closed 5 years ago

yylluu commented 7 years ago
public void setWireValue(Wire wire, long v) {
    setWireValue(wire, new BigInteger(v + ""));
}

For the above method, I found a trivial issue when I occasionally gave a "negative" v. In this case, "circuitName.in" will have negative signs shown, and the libsnark interface cannot understand negative signs correctly. The prover will fail to create proof.

A temporary fix is as following:

public void setWireValue(Wire wire, int v) {
    setWireValue(wire, v & 0xffffffffL);
}

public void setWireValue (Wire wire, Long v) {
    setWireValue(wire, new BigInteger(Long.toUnsignedString(v)));
}
akosba commented 7 years ago

Actually, the implementation does not support negative values on the wires currently. The implementation of negative values is up to the programmer at this point. In the future, there will hopefully be classes that support this on top.

The modifications you suggested will only help to make the program run, but they won't make it run consistently as the programmer expects. For example, if the comparison methods of the Wire class are used (e.g. isGreaterThan) while supporting your modifications, they won't always give consistent results depending on the usage, because they assume unsigned values at this point (in other words, a method may output that a negative number is greater than a positive number if we let the programmer assign negative values on the wires your way). Therefore, I preferred to make the negative values explicitly not supported on the wires in this basic version, until consistent support is added in another version. I will add a clarifying comment later in the code that the setWireValue and other methods only assume unsigned values within 0 and p-1 at this point.

yylluu commented 7 years ago

Thanks for kind reply.

I agree Jsnark does not support negative and the "negative" is up to the programmer.

The problem happened when the first binary bit is one (long or int type), even if I know I just need to set 32bit or 64bit binary strings (unsigned) to some wires. It is kind a issue of Java, since we know the lack of unsigned types there.

As for my circuit, the purpose is to make unsigned arithmetic operations for 2048bit large integers. So all values to set are unsigned in my case. When I gave some 32 bit or 64 bit unsigned values, if some of them have the first binary bit as 1 (I made quotations to negative), the Snark interface will not work. The temporary fix is just for my case above.

It seems possible for us to represent real negative by using 2's complementary representation, as long as we take care of the trivial complementary calculation in the implementation of circuits, although I did not make trails. However, I used similar idea when implementing comparison of two unsigned wires.

A comment or new method will be more than great! Thanks so much for following up.

Might be we can hope Jsnark can support more types such as signed values, such that a lot of trivial work of designing circuit can be avoided. Thanks again for the great contribution!