Verify subtree inclusion proofs#227
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
===========================================
- Coverage 89.33% 58.57% -30.76%
===========================================
Files 7 8 +1
Lines 497 618 +121
===========================================
- Hits 444 362 -82
- Misses 48 251 +203
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlCutter
left a comment
AlCutter
left a comment
There was a problem hiding this comment.
I'll admit I've not looked at the millions of lines of json in here...
| } | ||
| if !isSubtreeValid(start, end) { | ||
| return Nodes{}, fmt.Errorf("start %d not a multiple of bit_ceil(end - start) = %d", start, end-start) | ||
| return Nodes{}, fmt.Errorf("start %d not a multiple of bit_ceil(end - start)", start) |
There was a problem hiding this comment.
I quite liked having the thing it wasn't a multiple of in there (although it was wrong, but that makes me think that perhaps isSubtreeValid should return an error explaining why it isn't - technically this error would be wrong/misleading if e.g. a very large l was passed in).
There was a problem hiding this comment.
Hehe, it was actually returning an error, but I removed it thinking this is what you were asking for yesterday, I'll put it back!
| // VerifySubtreeInclusion verifies the correctness of the subtree inclusion | ||
| // proof for the leaf with the specified hash and index, relatively to the | ||
| // [start, end) subtree with a given subtree root hash. |
There was a problem hiding this comment.
| // VerifySubtreeInclusion verifies the correctness of the subtree inclusion | |
| // proof for the leaf with the specified hash and index, relatively to the | |
| // [start, end) subtree with a given subtree root hash. | |
| // VerifySubtreeInclusion verifies the correctness of the subtree inclusion | |
| // proof for the leaf with the specified hash and index, relative to the | |
| // provided subtree [start, end) and root hash. |
| return fmt.Errorf("index %d out of bounds for subtree [%d, %d)", index, start, end) | ||
| } | ||
| if !isSubtreeValid(start, end) { | ||
| return fmt.Errorf("start %d not a multiple of bit_ceil(end - start)", start) |
There was a problem hiding this comment.
(same comment about errors here)
| return nil | ||
| } | ||
|
|
||
| if filepath.Ext(d.Name()) != ".json" { |
There was a problem hiding this comment.
Maybe with strings.ToLower just in case?
There was a problem hiding this comment.
Sure thing - this is just a copy of TestVerifyInclusionProbes, so let's maybe do that in a followup PR?
| err := VerifySubtreeInclusion(rfc6962.DefaultHasher, p.LeafIdx, p.Start, p.End, p.LeafHash, p.Proof, p.Root) | ||
| if p.WantError && err == nil { | ||
| wrong = append(wrong, fmt.Sprintf("expected error but didn't get one: %s", p.Desc)) | ||
| continue | ||
| } | ||
|
|
||
| if !p.WantError && err != nil { | ||
| wrong = append(wrong, fmt.Sprintf("unexpected error: %s, %s", p.Desc, err)) | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| if len(wrong) > 0 { | ||
| t.Errorf("errors verifying subtree inclusion probes: \n%d out of %d failures \nError messages: \n%s", len(wrong), len(probes), strings.Join(wrong, "\n")) | ||
| } | ||
| } |
There was a problem hiding this comment.
Would this be more "modern" as a t.Run(p.Desc, func(t *testing.T) { ...?
Can ditch the continues and len(wrong) stuff then, and just let testing do the right reporting.
There was a problem hiding this comment.
Sure thing - this is just a copy of TestVerifyInclusionProbes, so let's maybe do that in a followup PR?
Reviewing individual commits will help to understand what's happening under the hood. |
| if start%bitCeil(l) != 0 { | ||
| return fmt.Errorf("start %d not a multiple of bit_ceil(end - start) = %d", start, l) | ||
| } |
There was a problem hiding this comment.
| if start%bitCeil(l) != 0 { | |
| return fmt.Errorf("start %d not a multiple of bit_ceil(end - start) = %d", start, l) | |
| } | |
| if bc := bitCeil(l); start%bc != 0 { | |
| return fmt.Errorf("start %d not a multiple of bit_ceil(end - start) = %d", start, bc) | |
| } |
2 participants
Towards #225.
This PR implements
VerifySubtreeInclusionand adds relevant tests:LeafIdxwas set to 2^64-1)/errorsLike in the previous PR, there's little benefit to factoring
VerifyInclusionandVerifySubtreeInclusioninto each other, but we can always do that later if we want to.While I'm there, fix a error message in
SubtreeInclusion.