-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Precompiled contracts for addition and scalar multiplication on the elliptic curve alt_bn128 #213
Conversation
EIPS/ecopts.md
Outdated
|
||
### Encoding | ||
|
||
Field elements are encoded as 32 byte big-endian numbers. Curve points are encoded as two field elements `(x, y)`, where the point at infinity is encoded as `(0, 0)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also mention how is a scalar encoded.
EIPS/ecopts.md
Outdated
|
||
Invalid input: For both contracts, if any input point does not lie on the curve or any of the field elements (point coordinates or scalar) is equal or larger than the field modulus p, the contract fails. | ||
|
||
ADD: Input: two curve points `(x, y)`. Fail on invalid input. Otherwise, return the curve point `x + y` where `+` is point addition on the elliptic curve `alt_bn128` specified above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to format it as:
#### ADD
Input: two curve points `(x, y)`...
Output: resulting curve point
...description...
EIPS/ecopts.md
Outdated
Current smart contract executions on Ethereum are fully transparent, which makes them unsuitable for several use-cases that involve private information like the location, identity or history of past transactions. The technology of zkSNARKs could be a solution to this problem. While the Ethereum Virtual Machine can make use of zkSNARKs in theory, they are currently too expensive | ||
to fit the block gas limit. Because of that, this EIP proposes to specify certain parameters for some elementary primitives that enable zkSNARKs so that they can be implemented more efficiently and the gas cost be reduced. | ||
|
||
Note that fixing these parameters will in no way limit the use-cases for zkSNARKs, it will even allow for incorporating some advances in zkSNARK research without the need for a further hard fork. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot immediately grasp how fixing parameters makes it easier to incorporate advances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the wording.
EIPS/ecopts.md
Outdated
|
||
### Encoding | ||
|
||
Field elements and scalars are encoded as 32 byte big-endian numbers. Curve points are encoded as two field elements `(x, y)`, where the point at infinity is encoded as `(0, 0)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't zero already used? I guess the elements of F_p are represented by numbers in the range of [0, that prime number - 1]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but (0,0)
should not satisfy the curve equation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Somehow I was thinking about (0, y)
s that might satisfy the equations. Now I realize there (0, y)
s are just interpreted as they are when y
is not zero.
EIPS/ecopts.md
Outdated
|
||
Field elements and scalars are encoded as 32 byte big-endian numbers. Curve points are encoded as two field elements `(x, y)`, where the point at infinity is encoded as `(0, 0)`. | ||
|
||
For both precompiled contracts, if the input is shorter than expected, it is padded with zeros at the end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the padding bit-wise or byte-wise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, does it make sense to pad a big-endian number at the end? 1
and 256
would look the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the wording, please see if you come to the conclusion that these questions are irrelevant :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was reading "if any parameter is less than 2^255, thus any parameter is shorter than expected, the caller should pad the input at the end before calling the contract". Now this interpretation is impossible.
EIPS/ecopts.md
Outdated
Invalid input: For both contracts, if any input point does not lie on the curve or any of the field elements (point coordinates or scalar) is equal or larger than the field modulus p, the contract fails. | ||
|
||
#### ADD | ||
Input: two curve points `(x, y)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean the input has 128 bytes, or are field elements put in RLP or some structure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified above.
EIPS/ecopts.md
Outdated
@@ -0,0 +1,99 @@ | |||
## Preamble | |||
<pre> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the <pre>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was part of the template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be using an old template.
Scalars larger than the field characteristic are allowed because it only makes sense to restrict them to the group order, not the field characteristic and adding the group order as another magic constant here would complicated the specification.
TODO: change semantics to throw an exception both for too short and too long input. |
A comment from my perspective. @chriseth and me are in the process of "implementing" this and #212 EIPs. "Implementing" means to integrate the external library: libff which was a part of bigger libsnark just a month ago. The libff supports only Linux at the moment and nothing indicates that it is activelly developed. After the split the libff does not have tests properly integrated. It also depends on GMP what is another issue. My personal opinion is this piece of code if to be integrated to any Ethereum client needs full code review and good set of unit tests. I don't know why alt_bn128 curve is so special, but if we plan to stick with only this one for a long time, we can modify the code base of libff to be less generic and support only this single curve. Such fork can be joint effort of multiple teams if others are interested. I wander what other libraries do you use. And what is used by ZCash? |
@chfast at least in the latest version, zcash uses libsnark (or some fork of it). Note that geth will use a different library, I think it might be this one here: https://github.com/golang/crypto/tree/master/bn256 and there is yet another implementation in rust: https://github.com/zcash/bn So I think this is a C++-only discussion. We might take libff and swap out GMP for something else. |
EIPS/ecopts.md
Outdated
|
||
Add precompiled contracts for point addition (ADD) and scalar multiplication (MUL) on the elliptic curve "alt_bn128". | ||
|
||
Address of ADD: 0x6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EIP 101 also uses this address.
EIPS/ecopts.md
Outdated
Add precompiled contracts for point addition (ADD) and scalar multiplication (MUL) on the elliptic curve "alt_bn128". | ||
|
||
Address of ADD: 0x6 | ||
Address for MUL: 0x7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EIP 101 uses this address too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make sure the precompiled addresses are continuous in the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. 0x10
and 0x20
are taken in #96 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The available addresses seem to be 0xa
and 0xb
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense changing #96 to use continuous addresses?
EIPS/ecopts.md
Outdated
- Points not on curve (but valid otherwise). | ||
- Multiply point with scalar that lies between the order of the group and the field (should succeed). | ||
- Multiply point with scalar that is larger than the field order (should succeed). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can someone create those tests cases and include them here?
I am 100% too late to the party with this, but it could be actually useful to have the inputs ABI encoded and reside on the same address for this reason: in WASM this precompile can be implemented natively as a contract, but with the current API it would be present in two copies. This of course assumes WASM would come to life in a reasonable timeframe, otherwise the current API is more useful from the implementation perspective. From in-contract usage perspective having it ABI encoded would enable using them without any complicated bindings, a simple interface contract would be enough in Solidity without the need of touching assembly. |
- Truncated input that results in a valid curve point. | ||
- Points not on curve (but valid otherwise). | ||
- Multiply point with scalar that lies between the order of the group and the field (should succeed). | ||
- Multiply point with scalar that is larger than the field order (should succeed). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Multiply point with scalar that is larger than the field order (should succeed).
- YES
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecmul_7827-6598_9935_28000_96Filler.json#L137
- multiplies by scalar (2**256 - 1) 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cdetrio do you think a link to the test filler should be added to the EIP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pirapira I was intending to link the test cases. But on second thought, we don't do that for other EIPs so I guess having the links in the PR/discussion is enough.
- Both contracts should succeed on empty input. | ||
- Truncated input that results in a valid curve point. | ||
- Points not on curve (but valid otherwise). | ||
- Multiply point with scalar that lies between the order of the group and the field (should succeed). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Multiply point with scalar that lies between the order of the group and the field (should succeed).
- YES
- order of the group: curve_order = 21888242871839275222246405745257275088548364400416034343698204186575808495617 (in hex: 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001)
- order of the field (same as modulus of the field): field_modulus = 21888242871839275222246405745257275088696311157297823662689037894645226208583 (in hex: 0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47)
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/pointMulAdd2Filler.json#L692-L693
- uses ECMUL with scalars -1 mod field_order and -2 mod field_order (0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd46 and 0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd45)
- Curve points which would be valid if the numbers were taken mod p (should fail). | ||
- Both contracts should succeed on empty input. | ||
- Truncated input that results in a valid curve point. | ||
- Points not on curve (but valid otherwise). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Points not on curve (but valid otherwise).
- YES
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecadd_0-3_1-2_25000_128Filler.json
- (0,3) is not on curve
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecmul_1-3_1_28000_128Filler.json#L137
- (1,3) is not on the curve
|
||
- Curve points which would be valid if the numbers were taken mod p (should fail). | ||
- Both contracts should succeed on empty input. | ||
- Truncated input that results in a valid curve point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Truncated input that results in a valid curve point.
- NO
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/pointAddTruncFiller.json
- calls ECADD but only truncates at the 64th byte, i.e. after a full point. does not truncate at e.g. 96th byte (after the x coord of the 2nd point) or at bytes in the middle of a coordinate
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecadd_1-3_0-0_25000_80Filler.json
- calls ECADD but truncates at the 80th byte of a zero point
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecmul_7827-6598_1456_28000_80Filler.json
- truncates at the 80th byte, which is the middle of the scalar factor, not the middle of a point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added such cases in the test google spreadsheet
Inputs to test: | ||
|
||
- Curve points which would be valid if the numbers were taken mod p (should fail). | ||
- Both contracts should succeed on empty input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- both contracts should succeed on empty input
- YES
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecadd_0-0_0-0_21000_0Filler.json#L135
- calls ECADD with empty input.
- https://github.com/ethereum/tests/blob/develop/src/GeneralStateTestsFiller/stZeroKnowledge/ecmul_0-0_0_28000_0Filler.json
- calls ECMUL with empty input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Hello @pirapira As important optimizations were merged in 1.8.x and actual processor time for every operation (especially MUL) was significantly reduced, what are the plans to reduce a precompile costs? Sincerely, Alex |
@shamatar thanks for the news. Many people would be happier with the lower gas amounts, so my suggestion is to file an EIP about the newer gas costs (if there is none yet). |
I've opened #1088 to propose this, as I didn't see any other EIPs on the matter. |
Precompiled contracts for elliptic curve operations are required in order to perform zkSNARK verification within the block gas limit.
Replaces #196