codex-storage / codex-contracts-eth

Ethereum smart contracts for Codex
Other
6 stars 9 forks source link

Refactor of the solidity Groth16 verifier #83

Closed markspanbroek closed 6 months ago

markspanbroek commented 8 months ago

Refactors the Groth16 verifier to:

markspanbroek commented 8 months ago

Honestly I have very limited understanding of what is going on here 😅 Maybe @bkomuves could do proper review? If not then I will dive in, but it will take me quite some time to understand it what is going on and what has changed.

That's ok, nobody in our team knows everything about zero knowledge proofs and solidity. That's why I asked multiple people to review. Balazs can review the crypto part, and you and Eric can look for solidity mistakes.

Things that you could focus a review on:

For reference, this is what I used to look up the precompiled contracts: https://www.evm.codes/precompiled

And this is what I used as a reference for the STATICCALL opcode: https://www.evm.codes/#fa?fork=shanghai

markspanbroek commented 6 months ago

I would add a comment explaining that the 3 precompiles are all supposed to check for the validity of the group elements, with a link to a definitive source.

Good idea, I added the comments.

I can also imagine that with many different EVM implementations, some may forget to actually do the checks, but I guess that's not our problem? :)

That would lead to a consensus failure, and seems unlikely. The EIPs state that these checks should be done, and looking at the Geth code, it seems to do these checks.

markspanbroek commented 6 months ago

I think that this is ready to be merged. @AuHau or @emizzle, could you do a final review to check for any Solidity shenanigans? Then I can merge this :pray: