Skip to content
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

BLS12-381 curve operations #2537

Merged
merged 29 commits into from
Jun 1, 2020
Merged

Conversation

shamatar
Copy link
Contributor

This PR adds a draft of the EIP that adds a set of precompiles for efficient implementation of operations on BLS12-381 curve

@vbuterin
Copy link
Contributor

vbuterin commented Mar 1, 2020

Thanks for the ongoing hard work!

A few questions:

  1. Are we sure that doing pairing checks over points that have not been subgroup-checked is safe? I have not thought about this deeply; I added mandatory subgroup checks to the bn128 EIP back then because I wasn't sure whether or not different pairing implementations might give different answers on adversarially provided inputs, but would be good to get more expert feedback on this.
  2. Do we need an explicit point decompression precompile? Seems like the existing MODEXP precompile can accomplish the heavy lifting here just fine.
  3. Why have three sets of precompiles for three different curves instead of a single set of precompiles that takes as input some kind of curve ID?
  4. Should "multiexponentiation" perhaps be renamed to "linear combination"? The other vocabulary ("addition", "multiplication") is expressed additively, so seems more correct to be consistent.
  5. If a subgroup check is not mandatory, costs 55000 gas and users can do it anyway outside the precompile, what's the value in including the option inside the precompile?
  6. Shouldn't the ABIs follow the usual ABI 32-byte-packing format? Perhaps packing together 48 and 48 bytes to make 96 is ok, but eg. boolean flags seem much cleaner to just put as a separate 32 byte chunk.
  7. Why bother with a multiplication precompile when a linear combination with one element suffices to do the same thing?

Strictly following the spec will eliminate security implications or consensus implications in a contrast to the previous BN254 precompile.

What does this mean? Was the spec not "strictly followed" last time? Because of the greater flexibility of this EIP (eg. it requires on-chain representations of G2 points and not just G1), it seems like there's more stuff that has to be strictly followed, and so more risk that there will be consensus implications. This sentence in plain English reads like "we'll be fine if people try harder", which only works in practice if you assume that (i) normally people don't try hard and (ii) they can be motivated to change this with just a few words. So I feel like trying to cut down the number of special cases within the precompiles would still be valuable to increase security further.

I do think the focus on a few curves instead of trying to support every possible curve is a great improvement.

@shamatar
Copy link
Contributor Author

shamatar commented Mar 2, 2020

Hey Vitalik. Here are answers and rationale in the same sequence:

  1. This part is being worked on. Currently this part is a legacy form EIP1962 where there were much stricter conventions on how particular operations must be implemented, e.g. Miller loop was required to be "double-and-add" in homogenius projective coordinates. This guaranteed that numerically there would be no consensus problems between different implementations. So right now I'm working on two cases:
  • Fuzzy test two completely independent implementations (by taking "pairing" crate for BLS12-381)
  • Test if there are discrepancies in e.g. Miller loop is implemented in wNAF form
  • Worst case - speedup the multiplication by using wNAF (for multiplication there is no question about discrepancy in different ways of implementing it). It can bring down subgroup check costs by roughly 40% in a worst case when it would be strictly required to run it.
    Then I'll make an adjustment if necessary to the spec.
  1. Point decompression precompile is optional, but it can not be that easily degraded to MODEXP cause we also allow decompression for G2 points (over Fp2), and while the square root algorithm there can be implemented using EVM instructions, it would also require one to first implement 384 bit arithmetic (that can be done, but is another layer of work and runtime expenses). Also this functionality was asked for by Eth2.0 people.
  2. This is a hard one. Originally EIP1962 did cover all those, but as a result of the last developers call it was considered to be "too hard", but it would be "easy" to add separate precompiles for separate curves. So I'm just satisfying this requirement. Alternatively one would just make "whitelist" for EIP1962, but if core devs will never accept EIP1962 in full generic form, it's most likely not the best option (curve specific ones can allow larger degree of optimization at the end of the day).
  3. I've used a term that is usually used in literature for such operation, but sure, it can be renamed. Although I hope that for people that would use this function the is no problem with how it is named.
  4. This option was simply to allow to use single precompile call in common cases. Consider that Kate commitment opening. Polynomial commitment and opening proof are just G1 points, and "verification key" that is just two G2 points is fixed. In this case verification could be done with a single precompile call where caller would require subgroup check for G1 points only cause verification key could have been checked separately on deployment. Same is for Groth16 proof where one would have to require one G2 subgroup check. (Obviously all this may change after optionality of subgroup checks is better investigated).
  5. This is also part of the EIP1962 legacy (that has tight packing ABI). It's not difficult to use 32 byte aligned ABI, but originally I've hoped that points will be encoded as e.g. uint192 and booleans to be uint8, and then users would just call abi.encodePacked(). I'll think more about it.
  6. I'm not 100% sure that Peppinger algorithm (for a current MULTIEXPs) for a single point is as efficient as multiplication, and one can use other optimizations for a single multiplications, such as wNAF.

Remark about BN254 was about the time when subgroup checks were not implemented in one of the clients and could potentially lead to consensus issues. For this reason I try to put more emphasis on it, whether it will end up being optional or mandatory.

About the separate curves: BLS12-381 is a "standard" and required by many parties, but BLS12-377 offers more options for developers and difference between them is just change of parameters. For the third currently posted proposal (wrapping curve for BLS12-377) all the special cases resolved here will still be applicable, so we just need to resolve them once and then use such a "guidebook" for all other.

EIPS/eip-2537.md Outdated Show resolved Hide resolved
EIPS/eip-2537.md Outdated Show resolved Hide resolved
EIPS/eip-2537.md Outdated Show resolved Hide resolved
@Souptacular
Copy link
Contributor

Waiting to merge this until @holiman's concerns are addressed by @shamatar.

@shamatar
Copy link
Contributor Author

Mapping spec is now included and gas cost was measured. Should be good for inclusion.

@shamatar
Copy link
Contributor Author

Tests for expected negative outcomes are being added to the same folder.

@ralexstokes
Copy link
Member

just an FYI, i've sketched out what a Solidity smart contract consuming some of the precompiles in this EIP would look like:

https://github.com/ralexstokes/deposit-contract-verifying-proxy/blob/master/deposit_contract_proxy.sol

it hasn't been tested end-to-end yet but even just skimming the types and looking at one use case (the eth2 deposit verifier) could be helpful in the above conversations on API, etc.

@shamatar
Copy link
Contributor Author

Following general sentiment form reviews I'm splitting mapping precompile to two separate ones.

@shamatar
Copy link
Contributor Author

Test vectors for mapping functions will be updated when IETF spec v7 is finalized (expected soon).

@shamatar
Copy link
Contributor Author

Reference implementation now conforms to v7 mapping draft and has ABI according to the latest draft.

@kilic
Copy link

kilic commented Apr 29, 2020

Go implementation now also follows v7 draft.

EIPS/eip-2537.md Outdated Show resolved Hide resolved
@Souptacular
Copy link
Contributor

Is this okay to be merged? @shamatar?

@shamatar
Copy link
Contributor Author

shamatar commented Jun 1, 2020

Yes

@Souptacular Souptacular merged commit 5edff4a into ethereum:master Jun 1, 2020
pizzarob pushed a commit to pizzarob/EIPs that referenced this pull request Jun 12, 2020
* draft

* Update eip-x.md

* add ABI

* also mention encoding of boolean vars in pairing

* add point decompression

* fix decompression, add gas price

* expand on square root extraction and checks

* Cleanup title

* Fix typo and remove optional header fields

* set EIP number

* hm, html verifier is not satisfied...

* update on ABI, costs and subgroup checks

* add more information about field-to-curve

* add links to implementations

* spellcheck

* mapping costs are underetmined yet

* add mapping operations cost

* whoops, spellcheck!

* updates: test vectors, prefixes

* more explicit pairing output

* gas consumption in case of error

* fix multiplication -> multiexp

* split mapping into two separate ones

* spellcheck

* update link to Go implementation

* add addresses

Co-authored-by: Kobi Gurkan <kobigurk@gmail.com>
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
* draft

* Update eip-x.md

* add ABI

* also mention encoding of boolean vars in pairing

* add point decompression

* fix decompression, add gas price

* expand on square root extraction and checks

* Cleanup title

* Fix typo and remove optional header fields

* set EIP number

* hm, html verifier is not satisfied...

* update on ABI, costs and subgroup checks

* add more information about field-to-curve

* add links to implementations

* spellcheck

* mapping costs are underetmined yet

* add mapping operations cost

* whoops, spellcheck!

* updates: test vectors, prefixes

* more explicit pairing output

* gas consumption in case of error

* fix multiplication -> multiexp

* split mapping into two separate ones

* spellcheck

* update link to Go implementation

* add addresses

Co-authored-by: Kobi Gurkan <kobigurk@gmail.com>
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
* draft

* Update eip-x.md

* add ABI

* also mention encoding of boolean vars in pairing

* add point decompression

* fix decompression, add gas price

* expand on square root extraction and checks

* Cleanup title

* Fix typo and remove optional header fields

* set EIP number

* hm, html verifier is not satisfied...

* update on ABI, costs and subgroup checks

* add more information about field-to-curve

* add links to implementations

* spellcheck

* mapping costs are underetmined yet

* add mapping operations cost

* whoops, spellcheck!

* updates: test vectors, prefixes

* more explicit pairing output

* gas consumption in case of error

* fix multiplication -> multiexp

* split mapping into two separate ones

* spellcheck

* update link to Go implementation

* add addresses

Co-authored-by: Kobi Gurkan <kobigurk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.