Support new preimage types in proving #1814
Merged
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
cla-bot
bot
added
the
s
Codecov Report
@@ Coverage Diff @@
## master #1814 +/- ##
==========================================
+ Coverage 28.59% 28.69% +0.10%
==========================================
Files 218 219 +1
Lines 32681 32709 +28
==========================================
+ Hits 9345 9386 +41
+ Misses 22275 22264 -11
+ Partials 1061 1059 -2 |
magicxyyz
reviewed
Sep 29, 2023
| if s.config.CheckAlreadyExists { | ||
| _, err = s.syncTo.GetByHash(ctx, hash) | ||
| } | ||
| if err == nil || errors.Is(err, ErrNotFound) { |
There was a problem hiding this comment.
I am a bit confused what is checked here
in case CheckAlreadyExists is set, if err == nil then GetByHash must have succeeded and entry under hash already exists, if the error is ErrNotFound then GetByHash didn't find entry under hash.
Is it something that is required by some StorageServices? What is the logic here?
There was a problem hiding this comment.
Yeah this code doesn't look right but it's mostly unrelated to this PR. I'll open up a separate ticket to fix it.
PlasmaPower
dismissed stale reviews from magicxyyz and joshuacolvin0
via
6ecadc3
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 adds the infrastructure for resolving additional types of preimages in proving, and as a proof of concept, adds sha256 support and uses it in the host io rust proving test. This is fully backwards compatible with older replay binaries.
Pulls in OffchainLabs/nitro-contracts#50