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

bug fix: e2e missed proof validity check #854

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hero78119
Copy link
Collaborator

@hero78119 hero78119 commented Mar 6, 2025

previously end-to-end (e2e) tests were missing an assertion check for verify_proof, meaning that failed proofs due to the absence of a halt instruction were not properly detected.

Since the absence of a halt instruction can be a valid scenario (as execution can be controlled via max_step), this PR:

  • Adds the missing verification check in e2e tests
  • Makes the halt instruction optional

Note: its doen't affect previous e2e testing result

@hero78119 hero78119 changed the title bug fix: e2e halt check bug fix: e2e missed proof validity check Mar 6, 2025
@hero78119 hero78119 added the bug Something isn't working label Mar 6, 2025
@@ -148,7 +148,11 @@ fn main() {
// do statistics
let stat_recorder = StatisticRecorder::default();
let transcript = TranscriptWithStat::new(&stat_recorder, b"riscv");
verifier.verify_proof(zkvm_proof.clone(), transcript).ok();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this check was missing before fix

@hero78119 hero78119 requested a review from lispc March 6, 2025 10:14
@hero78119 hero78119 enabled auto-merge March 6, 2025 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant