Feat/add zk snark support using alt bn 128 precompiles

[bisbist]

This PR is a part of following CPS grant.
https://cps.icon.community/proposals/bafybeih5njxsqe7nxfvw4b2lms7mg7cxy6jeeeaveg2q5sh24cptjucegm

Here's also a forum post that adds some context.
https://forum.icon.community/t/bringing-zero-knowledge-proofs-to-icon-platform-with-zksnark/3293

NOTE: We've changed the name of the exposed function to altBN128 from bn256, as specified in above post.

We're adding pre-compiles for alt-bn128 curve from ethereum crypto package, bn256. We've not yet added test cases and need a feedback from goloop core devs on how to proceed. Also, please suggest any changes necessary for the PR to be accepted.

We've already deployed a separate testnet that is open to public. You can try out the pre-compiles exposed at:
Blockchain: https://node.iconzkp.io/api/v3
Tracker: https://tracker.iconzkp.io
Faucet: https://faucet.iconosphere.io/ (iconzkp)

We've also released a fork of iden3/snarkjs, which is used to generate a java verification key for a circom circuit. All the necessary details are in the README.md.

We'll soon release a demo app with relevant circom circuits, corresponding java verifier contracts along with a frontend. We'll keed adding various circuits for example use cases.

[bbist]

As we're using the go-ethereum/crypto/bn256 package, it already inherits the test cases they have on the go implementation. We've only created a wrapper around those core functions, and exposed via "chainscore" system contract. We've created a Java Context API to call the altBN128 pre-compile. As the java API makes a chainscore contract call, I think it requires blockchain node to be running in background to interact with the go based chainscore API.

Can someone please guide us through a process on how we can implement a test coverage for the newly added code?

[sink772]

Hello @bisbist,

First of all, I think it would have been better if you had discussed your goals and design decision with us before creating this PR.
Adding a new API to the engine is a very delicate and important decision and should be approached with care.
Also, as I said earlier in the Discord DM, adding a new JavaEE API should not be like that, i.e., chainscore is not the subject for adding a new crypto API.

By the way, I have a fundamental question. Why should we adopt another curve for zk-SNARK, where we already have a more efficient and secure pairing curve, i.e., BLS12-381?

[1] https://electriccoin.co/blog/new-snark-curve/
[2] https://0xparc.org/blog/zk-pairing-1
[3] https://eips.ethereum.org/EIPS/eip-2537
[4] https://ethereum-magicians.org/t/eip-2537-bls12-precompile-discussion-thread/4187

According to [3], it clearly states that

Motivation of this precompile is to add a cryptographic primitive that allows to get 120+ bits of security for operations over pairing friendly curve compared to the existing BN254 precompile that only provides 80 bits of security.

Therefore, I don't think it's necessary to add the legacy and less secure BN pairing curve to the engine at this moment.
If you need a pairing curve, please use BLS12-381 curve, which is available via the following APIs.

• Context.aggregate()
• Context.verifySignature()

[bbist]

Hi @sink772,

Thanks for your response.

We definitely agree with most of your points, and the reasoning behind them. We understand that these kinds of changes are delicate and should be approached with great care. We're planning to add BLS12-381 curve operations in the future: Point addition, Scalar multiplication, and Pairing checks, just like BN254 in this PR. But it might have to be through pre-compiles on go code, rather than a native Java implementation. This is so that we use an already audited code from go-ethereum crypto library.
Also, the aggregate and verifySignature methods are for BLS signature aggregation and verification which are insufficient for this particular use case. We need aforementioned curve operations for zkSNARK proof verification. In this case, looks like the java implementation for existing Java BLS12-381 is from supranational's BLST package, we might be able to refactor current goloop code to expose the curve operations mentioned before. We need to ensure that the Java BLST package is properly audited.

The reason that we started with BN254 curve is that a lot of existing applications use BN254 on ethereum for circom based ZK circuits. Circom/SnarkJS has been supporting BN254 (or alt-bn128) curve since the beginning. It is a simple curve, which is obviously less secure than BLS12-381, but it's a familiar curve and easier to get started with available tooling in the existing ZK ecosystem. There are a lot of applications that have already been built that we can easily port to ICON, including frontends if we support BN254 curve. We can make a general recommendation for developers to use BLS12-381 whenever they can, but don't want to limit the support to just BLS12-381. This PR is part of a general improvements in support of better crypto primitives on-chain. We're starting with groth16 proof system, but later plan to add PLONK, fflonk, and even recursive proofs via venture23-zkp/icon-snarkjs. Therefore, at this point we'd appreciate if BN254 can be added to goloop. Example of large projects using BN254 curve are, Dark Forest, Tornado Cash, Railgun Privacy, etc.

The primary reason we submitted this PR is to get a suggestion on how we could expose go-ethereum pre-compiles. We added them to the existing chainscore API, as it was the only way we knew to add it and expose it via context API. But we'd like to hear your thoughts on whether there's some other way, without having to rewrite the code in native Java. This is the way we'll likely add the BLS12-381 pre-compiles as well (if Java BLST is not audited), so your suggestions would be greatly appreciated.

Besides, we also want to hear your thoughts on how we could add a proper testing for the added pre-compiles, as well as on deciding appropriate gas costs for these operations.

[sink772]

1. The current BLS12-381 implementation we are using is from https://github.com/supranational/blst, which is used by many production-grade consensus clients including ether2 beacon chain. Thus I totally disagree with your intention to add the go version from go-ethereum code for BLS12-381.
   We don't need to manage crypto code in our own repo, and even it should be avoided.
   Even the ethereum community has voices calling for replacing its custom go impl with blst, but they just gave it up for cost and policy reason (https://ethereum-magicians.org/t/eip-2537-bls12-precompile-discussion-thread/4187/21).

2. If you need additional curve operations for zk-SNARK on BLS12-381, we can start discussion how the required APIs should be provided from the current blst integration. API design should come first.

3. It seems the snarkjs supports both bn128 and bls12-381 (https://github.com/venture23-zkp/icon-snarkjs#1-start-a-new-powers-of-tau-ceremony). So you may experiment with bls12-381 curve instead of bn128.

   The first parameter after new refers to the type of curve you wish to use. At the moment, we support both bn128 and bls12-381.

4. Again, we do not intend to add new crypto APIs via chainscore. It seems market trend is moving to use BLS curve. Once the EIP-2537 activated, there is no reason not to use BLS over BN. All the application infra will rapidly move to use BLS. It's a just matter of time I think.

[bbist]

1. My point might not have been clear in previous statement, but I agree with you on the first point. If we go with Java BLST package, we don't have to expose BLS pre-compiles via go chainscore. We can simply expose them on Context API that uses the suprarational/blst java library which is already used by goloop, as you mentioned in 2.
2. Yeah, SnarkJS already supports BLS12-381 curve. For all the new projects, we should use BLS12-381 if the proving cost is acceptable. But for large scale projects, if we want to port an entire application we might have to modify a lot of the frontend code to make it work with BLS12-381 verifiers. Anyway, if there's no chance that BN254 gets added to goloop, we can add BLS12-381 first.

[bbist]

Let's discuss about how we should expose BLS12-381 curve operations. Is it okay, if we expose multiple functions for BLS curve operations? As done for aggregation, and verification via aggregate and verifySignature methods.

We'll add following methods in particular:

1. bls12381Add(BigInteger x1, y1, x2, y2) -> (BigInteger, BigInteger)
2. bls12381Mul(BigInteger x, y, scalar) -> (BigInteger, BigInteger)
3. bls12381Pairing(BigInteger[] flattenedArgs) -> boolean

[sink772]

I think it's better to open another issue ticket for the additional BLS curve operations APIs.

And please note that you need to design a draft APIs by referencing EIP-2537 and others, in order to make them sufficiently general to use.

Also the draft APIs format should follows the existing conventions in score.Context.

[bbist]

Thanks for the suggestions.

We'll open a new issue to add BLS12-381 curve operations. We may not have to add all operations as specified in the given EIP at the moment, as we're mostly focusing on supporting ZKSNARK for the CPS proposal. But, we can add them gradually with multiple PRs.

One question I have about the Context AIP spec is whether we can return an array of BigInteger. I don't see any function doing that. If we can add that, it would be very easy to use vs encoding/decoding these large integers into bytearray.

[sink772]

One question I have about the Context AIP spec is whether we can return an array of BigInteger. I don't see any function doing that. If we can add that, it would be very easy to use vs encoding/decoding these large integers into bytearray.

I would recommend to use byte[] instead of BigInteger as we've done in aggregate, hash, etc., since we need to give the blst library input as byte[] anyway. Such a BigInteger to byte[] conversion should be done in user space to calculate the step (gas) cost more accurately.