Fault proof part 1 #824
Merged
Fault proof part 1 #824
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This command generates fault_ids.rs which contains the ELF and Image ID of the fault checker. When ever the fault checker guest code changes, this command should be run to re-generate the ELF and Image ID values.
This is a WIP commit that's intended to get bootstrap to simply generate an image ID and elf. The actual guest code will be added in subsequent changes.
This enables risc0-zkvm to build properly. TODO: add executor changes, fault checker (without merklization), and tests
Rather than panicing, return an Error result.
When rrs encounters an error while trying to run the next instruction, this change will run the fault checker. Note: the fault checker guest code will be added in a subsequent change.
this code is temporary and does not compute the post id
expose fault monitor only for std
SchmErik
force-pushed
the
erik/fault-proof-part-1
branch
from
ed0c8be
to
337b622
Compare
Benchmark for Linux-cuda 6747cd1Click to hide benchmark
Benchmark for Linux-defaultBenchmark for macOS-default 6747cd1Click to hide benchmark
Benchmark for macOS-metal |
Benchmark for Linux-cudaBenchmark for Linux-defaultBenchmark for macOS-defaultBenchmark for macOS-metal |
Ensure that the fault proof is enabled and that there are more than one receipts at this point.
Benchmark for Linux-cuda 9fc434dClick to hide benchmark
Benchmark for Linux-default 9fc434dClick to hide benchmark
Benchmark for macOS-defaultBenchmark for macOS-metal 9fc434dClick to hide benchmark
|
It's possible for the fault checker to consume multiple segments. Change the verifier logic to account for a single segment with a pre image id that is the fault checker and only allow the fault checker to exit with ExitCode::Halted(0)
Benchmark for Linux-cuda c0a5a9bClick to hide benchmark
Benchmark for Linux-defaultBenchmark for macOS-defaultBenchmark for macOS-metal c0a5a9bClick to hide benchmark
|
Benchmark for Linux-cuda 4d6d6f4Click to hide benchmark
Benchmark for Linux-default 4d6d6f4Click to hide benchmark
Benchmark for macOS-default 4d6d6f4Click to hide benchmark
Benchmark for macOS-metal |
We want to make it difficult for users to mistakenly prove a faulted session. This change encapsulates faulted sessions as a part of an Err object. If the user wishes to prove a faulted session, they must explicity write code to match and extract the session from an error.
…nd tests This change adds an additional member to the VericiationError enum to represent a successful verification on a fault receipt. We want to return an Err on successful verification to ensure that those who do not intend to verifiy fault proofs do not mistakenly verify fault receipts.
Benchmark for Linux-cuda 4163fc7Click to hide benchmark
Benchmark for Linux-default 4163fc7Click to hide benchmark
Benchmark for macOS-default 4163fc7Click to hide benchmark
Benchmark for macOS-metal 4163fc7Click to hide benchmark
|
Benchmark for Linux-cudaBenchmark for Linux-defaultBenchmark for macOS-default 8cf11acClick to hide benchmark
Benchmark for macOS-metal 8cf11acClick to hide benchmark
|
nategraf
approved these changes
Sep 19, 2023
There was a problem hiding this comment.
Approved! I also have a comment that it seems we will end up replacing some of this code in the near future when user recursion is available. I think with the feature flags, we can go ahead and merge as is, but removing the code from this PR that we expect not to be used later would be cleaner.
There was a problem hiding this comment.
When we were discussing on Monday, we realized that if/when we rebase fault proofs onto user recursion, the changes to this file will be removed. Do you think we should remove these changes from this PR and keep the other parts, such as the guest and memory monitor code changes?
There was a problem hiding this comment.
I need to add the computation of the merkle root in the fault checker in a separate PR so I'll leave it in for ease of testing. I'm definitely OK with dropping all of this once we can use user recursion though!
|
I'm OK with either landing now and removing the verify changes when user recursion lands, or removing it now. |
Co-authored-by: Victor Graf <victor@risczero.com>
Benchmark for Linux-cudaBenchmark for Linux-defaultBenchmark for macOS-defaultBenchmark for macOS-metal |
flaub
added a commit
that referenced
this pull request
Sep 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR consists of part 1 of what appears in #736 This PR implements a fault checker but does not compute the post image ID. I will follow up with a PR that implements the computation of the post image ID within the fault checker guest.