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

Refactor to Use ZkVm-Specific ProofReceipt #543

Merged
merged 11 commits into from
Dec 19, 2024

Conversation

prajwolrg
Copy link
Contributor

@prajwolrg prajwolrg commented Dec 15, 2024

Description

This PR addresses a bug in the write_proof function of the Risc0 Input Builder and introduces ZkVm Specific Proof Types to avoid such bug in the future. It also updates the error types to catch such bugs easily.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@prajwolrg prajwolrg changed the title Refactor to Use ZKVM-Specific Proof Types Refactor to Use ZkVm-Specific Proof Types Dec 15, 2024
@prajwolrg prajwolrg changed the title Refactor to Use ZkVm-Specific Proof Types Refactor to Use ZkVm-Specific ProofReceipt Dec 15, 2024
@prajwolrg prajwolrg force-pushed the STR-744-use-zkvm-specific-proof-types branch from efbef35 to b89e496 Compare December 15, 2024 09:47
Copy link

codecov bot commented Dec 15, 2024

Codecov Report

Attention: Patch coverage is 32.20339% with 80 lines in your changes missing coverage. Please review.

Project coverage is 56.67%. Comparing base (bb58728) to head (360a113).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
crates/zkvm/adapters/sp1/src/proof.rs 0.00% 31 Missing ⚠️
crates/zkvm/adapters/sp1/src/input.rs 0.00% 18 Missing ⚠️
crates/zkvm/zkvm/src/errors.rs 0.00% 18 Missing ⚠️
crates/zkvm/adapters/sp1/src/host.rs 0.00% 7 Missing ⚠️
crates/zkvm/zkvm/src/lib.rs 0.00% 3 Missing ⚠️
crates/proof-impl/checkpoint/src/lib.rs 0.00% 1 Missing ⚠️
crates/zkvm/adapters/native/src/env.rs 0.00% 1 Missing ⚠️
crates/zkvm/adapters/risc0/src/env.rs 0.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main     #543      +/-   ##
==========================================
- Coverage   56.70%   56.67%   -0.03%     
==========================================
  Files         303      306       +3     
  Lines       31000    31041      +41     
==========================================
+ Hits        17577    17591      +14     
- Misses      13423    13450      +27     
Files with missing lines Coverage Δ
crates/proof-impl/btc-blockspace/src/prover.rs 100.00% <100.00%> (ø)
crates/proof-impl/checkpoint/src/prover.rs 100.00% <100.00%> (ø)
crates/proof-impl/cl-agg/src/prover.rs 100.00% <100.00%> (ø)
crates/proof-impl/cl-stf/src/prover.rs 100.00% <100.00%> (ø)
crates/proof-impl/evm-ee-stf/src/prover.rs 100.00% <100.00%> (ø)
crates/proof-impl/l1-batch/src/prover.rs 96.00% <100.00%> (ø)
crates/zkvm/adapters/native/src/host.rs 100.00% <100.00%> (ø)
crates/zkvm/adapters/native/src/input.rs 100.00% <100.00%> (ø)
crates/zkvm/adapters/native/src/proof.rs 100.00% <100.00%> (ø)
crates/zkvm/adapters/risc0/src/verifier.rs 100.00% <100.00%> (+1.72%) ⬆️
... and 12 more

... and 6 files with indirect coverage changes

@prajwolrg prajwolrg marked this pull request as ready for review December 15, 2024 14:04
@prajwolrg prajwolrg requested a review from a team as a code owner December 15, 2024 14:04
crates/zkvm/zkvm/src/host.rs Outdated Show resolved Hide resolved
@prajwolrg prajwolrg self-assigned this Dec 17, 2024
@prajwolrg prajwolrg added the prover Prover-related, extremely sensntive in production label Dec 17, 2024
@prajwolrg prajwolrg requested review from a team as code owners December 17, 2024 09:06
@prajwolrg prajwolrg requested a review from MdTeach December 17, 2024 09:09
MdTeach
MdTeach previously approved these changes Dec 17, 2024
Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly stylistic comments.

crates/zkvm/adapters/native/src/host.rs Show resolved Hide resolved
crates/zkvm/adapters/risc0/src/proof.rs Outdated Show resolved Hide resolved
crates/zkvm/zkvm/src/errors.rs Outdated Show resolved Hide resolved
crates/zkvm/zkvm/src/errors.rs Show resolved Hide resolved
crates/zkvm/zkvm/src/errors.rs Outdated Show resolved Hide resolved
@prajwolrg prajwolrg requested a review from delbonis December 18, 2024 03:23
@prajwolrg prajwolrg force-pushed the STR-744-use-zkvm-specific-proof-types branch from 0f72c5e to 360a113 Compare December 18, 2024 03:23
@prajwolrg prajwolrg requested a review from MdTeach December 18, 2024 03:23
Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only saw the tests CODEOWNER

ACK 360a113

@prajwolrg prajwolrg enabled auto-merge December 19, 2024 01:59
@prajwolrg prajwolrg added this pull request to the merge queue Dec 19, 2024
Merged via the queue into main with commit 632951d Dec 19, 2024
17 of 18 checks passed
@prajwolrg prajwolrg deleted the STR-744-use-zkvm-specific-proof-types branch December 19, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prover Prover-related, extremely sensntive in production
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants