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

feat(protocol): add SP1 verification support #17861

Merged
merged 8 commits into from
Aug 1, 2024
Merged

feat(protocol): add SP1 verification support #17861

merged 8 commits into from
Aug 1, 2024

Conversation

adaki2004
Copy link
Contributor

This PR is aiming to integrate SP1's on-chain verifier (gateway).

Functional wise it is identical to this PR/branch: #17215 the key difference is, that SP1 now is managing the verifier deployment themselves, we just need to call into their contracts. So no need to incorporate the veriying logic into our own contracts.

Veriifer deployments: https://github.com/succinctlabs/sp1-contracts/tree/main/contracts/deployments

If accepted, i'll do a mock VerifierGateway contract for unit tests.

Copy link

openzeppelin-code bot commented Jul 29, 2024

feat(protocol): add SP1 verification support

Generated at commit: 65da232de004f60bbd3fb46b907259b6cd13bf7f

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
2
0
8
42
54
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@adaki2004 adaki2004 requested a review from Brechtpd July 29, 2024 13:45
@CeciliaZ030
Copy link
Contributor

@adaki2004 I think for the testnet, if we are not interacting with Sp1 contracts directly you have two ways of doing this:

  1. Directly route to a deployment of v1.0.5 Sp1 Verifier exported from ZkVM
  2. Deploy their whole Gateway contract to our testnet and register v1.0.5 Sp1 Verifier exported from ZkVM to the gateway. On our end, route to the gateway.

Option 2 is more compatible with their setting on mainnet, option 1 is more convenient.

@CeciliaZ030
Copy link
Contributor

Also if we want to integrate blob verification altogether see taikoxyz/raiko#292 (comment)
Does TaikoData.TierProof has the kzg proof needed to call the point precompile?

@adaki2004
Copy link
Contributor Author

adaki2004 commented Jul 29, 2024

@adaki2004 I think for the testnet, if we are not interacting with Sp1 contracts directly you have two ways of doing this:

  1. Directly route to a deployment of v1.0.5 Sp1 Verifier exported from ZkVM
  2. Deploy their whole Gateway contract to our testnet and register v1.0.5 Sp1 Verifier exported from ZkVM to the gateway. On our end, route to the gateway.

Option 2 is more compatible with their setting on mainnet, option 1 is more convenient.

I think option 1 is more close what setup we will have anyways on mainnet (since we wont be responsible for veritication + deployment anyways if not mistaken) bc. why not interacting with those stateless contracts ? These are just readonly calls.
But that’s up to who deploys these contracts on e.g.: holesky (hekla), simply just sets the SP1 router correctly and that is it. (As said they have it on Holesky Sepolia, etc.).

I was referring “tests” in unit test context for coverage.

@adaki2004
Copy link
Contributor Author

Will add deployment script + unit test Tuesday (plz dont merge till).

packages/protocol/contracts/verifiers/IVerifier.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/verifiers/SP1Verifier.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/verifiers/SP1Verifier.sol Outdated Show resolved Hide resolved
packages/protocol/contracts/verifiers/SP1Verifier.sol Outdated Show resolved Hide resolved
@adaki2004 adaki2004 requested a review from dantaik July 30, 2024 12:08
@dantaik dantaik added this pull request to the merge queue Aug 1, 2024
Merged via the queue into main with commit 2936312 Aug 1, 2024
5 checks passed
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.

4 participants