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

#461 Prover: Add constraints version validation #470

Merged
merged 13 commits into from
Dec 19, 2024

Conversation

srinathln7
Copy link
Contributor

This PR implements issue(s) #

  • Adds a command in the MakeFile under zkevm/arithmetization/zkevm.bin to extract the version from the git branch in the constraints repo and creates a constraints-versions.txt file. This approach relies on branch naming conventions as the linea-constraints repo does not support tags for every release. Tags with version names for every release are recommended to reliably extract versions from tags instead of branches.
  • Adds version validation in the prover to ensure the version from the conflated trace file matches the trace engine version and the ones listed in the constraints-versions.txt file.
  • Code changes and validation checks were successfully tested manually with correct and incorrect trace files in the prover dev mode.

@srinathln7 srinathln7 linked an issue Dec 19, 2024 that may be closed by this pull request
@srinathln7 srinathln7 marked this pull request as draft December 19, 2024 09:00
@srinathln7 srinathln7 self-assigned this Dec 19, 2024
prover/Makefile Outdated Show resolved Hide resolved
prover/run.sh Outdated Show resolved Hide resolved
@AlexandreBelling AlexandreBelling marked this pull request as ready for review December 19, 2024 09:47
@gusiri
Copy link
Contributor

gusiri commented Dec 19, 2024

I think you can exclude *.config.toml files and run.sh

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.18%. Comparing base (29a8d19) to head (ac6a3b6).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #470   +/-   ##
=========================================
  Coverage     68.17%   68.18%           
  Complexity     1125     1125           
=========================================
  Files           319      319           
  Lines         12789    12789           
  Branches       1275     1275           
=========================================
+ Hits           8719     8720    +1     
+ Misses         3542     3541    -1     
  Partials        528      528           
Flag Coverage Δ *Carryforward flag
hardhat 98.70% <ø> (+0.10%) ⬆️
kotlin 65.80% <ø> (ø) Carriedforward from 359b02d

*This pull request uses carry forward flags. Click here to find out more.

see 1 file with indirect coverage changes

@gusiri gusiri added the Prover Tag to use for all work impacting the prover label Dec 19, 2024
@gusiri
Copy link
Contributor

gusiri commented Dec 19, 2024

You can change the title of this PR to something like
Prover: Add constraints version validation, as many different parts of the zkEVMs are merging into main.

@srinathln7 srinathln7 changed the title 461 add version validation in prover #461 Prover: Add constraints version validation Dec 19, 2024
@srinathln7 srinathln7 temporarily deployed to docker-build-and-e2e December 19, 2024 13:02 — with GitHub Actions Inactive
gusiri
gusiri previously approved these changes Dec 19, 2024
Copy link
Contributor

@gusiri gusiri left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@srinathln7
Copy link
Contributor Author

UPDATE: @gusiri @AlexandreBelling Reworking this PR as e2e tests are failing due to the absence of constraints-versions.txt file in the environment. The plan now is to use go embed plugin and moving this file into backend/execution.

gusiri
gusiri previously approved these changes Dec 19, 2024
@srinathln7 srinathln7 dismissed stale reviews from gusiri and AlexandreBelling via ac6a3b6 December 19, 2024 15:56
@srinathln7 srinathln7 temporarily deployed to docker-build-and-e2e December 19, 2024 15:59 — with GitHub Actions Inactive
@srinathln7
Copy link
Contributor Author

Ok, previous e2e test was failing because apparently the conflated trace file can contain optional dir path For eg: /data/traces/v2/conflated/33-33.conflated.v0.8.0-rc8.ltas opposed to just 7036668-7036683.conflated.v0.8.0-rc8.lt that we see from proof requests. Have fixed this by updating the regex pattern to accommodate optional dir prefix so that works for both the file patterns. @gusiri @AlexandreBelling Requesting for review and approval.

@srinathln7 srinathln7 merged commit a83d22c into main Dec 19, 2024
25 checks passed
@srinathln7 srinathln7 deleted the 461-add-version-validation-in-prover branch December 19, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prover Tag to use for all work impacting the prover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add version validation in prover
4 participants