remove Groth16VerifierExtensions.sol

[RyanRHall]

I'd like to move ownership of this contract from MR-P2 repo to the contracts repo. This contract acts as a template when generating new verifiers and is integral to the deployment process. I'd like to make some tweaks to reduce our deployment complexity and to increase our test-ability. But moving ownership of this contract to the contracts repo is a blocker.

As far as I can tell, this contract is not used anywhere in this repo, including tests (not sure why the CI failure, but it seems unrelated).

I've left the two other contracts in this repo alone, the verifier.sol and TestGroth16Verifier.sol. These seem like they are actually used in tests. In the future, I might push us to migrate these also, but for now I don't think there's an urgent need.

[nikkolasg]

@RyanRHall I think this is a good idea directionally but we need to be careful about the implications. For example if we move it out, then we can't test any upgrades "easily". For example say we need to change the public inputs of our proof, then that file groth16verifierextension.sol needs to change as well and we can't test that easily in the contracts repo since we don't have a "test framework for proofs".

That means we would need to move some of our current test framework to the contracts repo.
What do you think of that @silathdiir ? what would we need to port to contract repo ? gnark-utils? groth16-framework? a mix ?

[silathdiir]

what would we need to port to contract repo ? gnark-utils? groth16-framework? a mix ?

We have some discussion today. If we want to test Groth16VerifierExtensions.sol in lagrange-lpn-contracts, seems we need to make lagrange-lpn-contracts depends on mp2 (verifiable-db including the revelation public inputs and groth16-framework including the proof generation). For simple test, we may just use a generated proof data (to avoid depending on mp2).

But we need to make Groth16VerifierExtensions.sol consistent with a specific mp2 version:

• verifier.sol should be committed to lagrange-lpn-contracts directly when it's generated in S3. It's committed to mp2 for now.
• I should send a lagrange-lpn-contracts PR to update Groth16VerifierExtensions.sol when a mp2 PR (which updates that contract) is merged into mp2 main branch (this is handled by the lagrange-lpn-contracts script automatically for now).

What do you think?

[nikkolasg]

verifier.sol should be committed to lagrange-lpn-contracts directly when it's generated in S3. It's committed to mp2 for now.

Which verifier.sol ?

[RyanRHall]

(X-posted from the slack thread) my concerns with our current cross-repo contract setup are:

• The build pipeline feels overly complex. To build our contracts we have to pull one dependency from S3, pull another dependency from from a branch on a sister repo, then use a bash script to manually edit several lines in the contract. We've also seen instances of the script running differently on different people's machines.
• We have two different versions of these contracts: the mp2 repo version and the contracts repo version. The mp2 repo version is used for integration tests, but the contracts repo version is what gets deployed. I think we would ideally deploy the same version we test with!
• The mp2 contracts are excluded from testing and analysis tools currently running in the contracts repo. This means bugs introduced in mp2 contracts have a greater likelihood of going undetected.

I should also say, I think 1 & 2 above can be addressed without moving the contracts out of mp2.

[silathdiir]

Which verifier.sol ?

The PRs to update verifier.sol as #361.

[RyanRHall]

Closing for now, pending larger discussion on dependency management and E2E testing