Merkle proofs 3.0 #1729
Merged
Merkle proofs 3.0 #1729
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
Ngoguey42
force-pushed
the
merkle-proofs-3.0
branch
14 times, most recently
from
f2ae1d1
to
9750ae6
Compare
Ngoguey42
force-pushed
the
merkle-proofs-3.0
branch
from
9750ae6
to
58b8a98
Compare
craigfe
reviewed
Feb 9, 2022
|
Is there anything significant remaining to be done with this one? |
|
Ngoguey42
force-pushed
the
merkle-proofs-3.0
branch
from
58b8a98
to
b121e8b
Compare
|
It seems that the MP tests fail on codecov (but not on the CI) |
|
See aantron/bisect_ppx#398 for the coverage issue. I'll replace some |
|
|
The streamed proofs rely on the result of There is no need to retain de laziness in |
Codecov Report
@@ Coverage Diff @@
## main #1729 +/- ##
==========================================
+ Coverage 64.56% 67.44% +2.87%
==========================================
Files 98 100 +2
Lines 12294 12914 +620
==========================================
+ Hits 7938 8710 +772
+ Misses 4356 4204 -152
Continue to review full report at Codecov.
|
Ngoguey42
commented
Feb 10, 2022
Ngoguey42
commented
Feb 10, 2022
Comment on lines
+1548
to
+1594
| (* Trigger certain paths in [S.Tree] during "verify" *) | ||
| let portable = | ||
| (* During "verify" [portable] is [Pruned] with [portable] in env *) | ||
| t0 | ||
| in | ||
| let portable_dirty = t in | ||
| let trigger_node_to_map t = | ||
| S.Tree.fold ~depth:(`Eq 1) ~order:`Sorted ~force:`True t () | ||
| in | ||
| let* () = trigger_node_to_map portable in | ||
| let* () = trigger_node_to_map portable_dirty in | ||
| let trigger_node_length t = | ||
| let+ (_ : int) = S.Tree.length t [] in | ||
| () | ||
| in | ||
| let* () = trigger_node_length portable in | ||
| let* () = trigger_node_length portable_dirty in | ||
| let trigger_node_fold_undefined t = | ||
| S.Tree.fold ~depth:(`Eq 1) ~order:`Undefined ~force:`True t () | ||
| in | ||
| let* () = trigger_node_fold_undefined portable in | ||
| let* () = trigger_node_fold_undefined portable_dirty in | ||
| let (_ : bool) = S.Tree.is_empty portable in | ||
| let trigger_node_to_backend_portable t = | ||
| match S.Tree.destruct t with | ||
| | `Contents _ -> assert false | ||
| | `Node n -> | ||
| let+ _ = S.to_backend_portable_node n in | ||
| () | ||
| in | ||
| let* () = trigger_node_to_backend_portable portable_dirty in |
There was a problem hiding this comment.
These tests are new. They cover all the Portable* cases when using Scan in tree
|
This is now ready for review! Most of the lines brought by this PR already exist on 2.10. See https://github.com/mirage/irmin/compare/2.10..Ngoguey42:merkle-proofs-3.0 for the full diff between 2.10 and this PR |
craigfe
approved these changes
Feb 11, 2022
Forward ported from many PRs spanning from 2.9 to 2.10
When computing the segments of an extender node in a stream proof, the current implementation returns them in reverse order (i.e. deepest segment first, progressing up the tree). This commit exercises this bug by modifying the existing stream extender test to use a non-palindrome test path.
The segment list contained in a stream proof extender node is computed in reverse order, and previously was not being properly reversed at the end of this process. This commit fixes the bug and introduces a `Reversed_list.t` type to make it impossible to forget to reverse the list before use in future.
Ngoguey42
force-pushed
the
merkle-proofs-3.0
branch
from
96c3a39
to
efe5dc4
Compare
|
I'm not writing anything in the changelog as this should already be part of the 2.10 changelog Let's merge when green :) |
|
Thanks for this huge work! Merging now. |
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.
No description provided.