New type: field

[josojo]

Hi,

we would like to merge a new type: field.

This field does not change anything with the logic in the snark. However, this allows to test our snarks much better. In cpp, we define a type: field with the same properties as the underlying prime field ( same modular arithmetic, etc). Now, if we run the tests in cpp, we can simulate much closer how the snark will behave and hence spot errors earlier.

An example of such an testing happens here: https://github.com/gnosis/dex-zksnarks/pull/7

After discussion with @fleupold, we decided to take out the int128, as it is no longer needed.

[maxhowald]

Doesn't this just change the type to behave like an unsigned integer with 254 bits? That's not quite the same as the field defined, for example, here.

For things like inequality operations that work bitwise, increasing the size of the integers might be what you want. Each bit of the integer is constrained on its own, so it's not really important what the size of the underlying field is.

If you want to do arithmetic operations that overflow according to the arithmetic of the underlying field, what you need is a type that the compiler can treat as if it never overflows. This is actually already the default behavior for all integer types unless the --cstdarithtruncate option is passed to the compiler, though, so that's probably why your tests are passing.

I'm not sure it's right to call this a "field" type though, since you are presumably still doing bitwise operations on it. A proper field type wouldn't need to know the width because the compiler would treat it as if it never overflowed (regardless of the cstdarithtruncate flag), and bitwise operations on this type such as inequality comparisons would be compilation errors, unless explicitly cast to, e.g. "int254", which might be a better name for the type defined in this PR.

[josojo]

If you want to do arithmetic operations that overflow according to the arithmetic of the underlying field, what you need is a type that the compiler can treat as if it never overflows. This is actually already the default behavior for all integer types unless the --cstdarithtruncate option is passed to the compiler, though, so that's probably why your tests are passing.

Yes, this is exactly what we want. We are aware that this is the default behavior. However, we submit this pr, as we want to have this behaviour in our tests of the snark code as well. Because of this, we want to have a new type, which is interpreted by pepper as the usual field element(as it is by default) and it is interpreted by our tests as such a field element too.

Just a little bit more details: For testing, we define in our code of the snarks: typedef libff::alt_bn128_pp ppT; typedef libff::Fr<ppT> field;, so that all arithmetic operations behave according to the field. This allows us to test the default behaviour of pepper in our tests and saves us a lot of time. Now, we all we want to have is that this field variable does not throw any errors in pepper. Therefore, we would like to introduce it to pepper as a standard variable type.

[josojo]

Actually, your suggestion for the naming fits well. I modified field => field254, so that every coder is aware about what happens under the hood bitwise.

Or do you think that int254 would be even a bitter fit? We like " field ", as it will overflow differently than a 254 bit int.

[maxhowald]

The way that this type behaves is that it is treated by the compiler either as a vector of 254 bits or as an element in a prime field, depending on what operations it is used in. This seems like it has the potential to be confusing, since arithmetic on 254 bit integers is not the same as arithmetic in a prime field for a prime with 254 bits.

I'll go ahead and merge this change though, since this is actually how the other integer types behave already, and it seems useful to have a larger-width type for use with bitwise operations.

It would still be nice if the compiler would ignore cstdarithtruncate for this type, and it might be useful to make sure that some of your tests fail when this flag is turned on, given the current behavior. I might look in to this myself if I have some time, but that might not be for a while. If you want to submit another PR for that, it would definitely be welcome.