Ensure nodes that are not part of the compact merkle tree state #180
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
for given tree size are set to nil when new leaves are added. Check using an invariant after each AddLeaf in the tests. Fix test data to have nils in the appropriate places.
sizes. Test data generated with the code to write nil in 'nodes []trillian.Hash' disabled, so that it is known to be good with the existing CompactMerkleTree. With the new 'write nil' code enabled this acts as a regression check.
AlCutter
reviewed
Oct 4, 2016
| @@ -17,6 +18,40 @@ func getTree() *CompactMerkleTree { | |||
| return NewCompactMerkleTree(NewRFC6962TreeHasher(trillian.NewSHA256())) | |||
| } | |||
|
|
|||
| func (c *CompactMerkleTree) checkUnusedNodesInvariant() error { | |||
There was a problem hiding this comment.
Since the test is in the same package you should have access to the non-exported contents of structs, so this func could just take a *CompactMerkleTree and reach in, rather than adding this testing method to CMT.
| for i, n := range c.nodes { | ||
| expectNil := i != sizeBits-1 | ||
| if expectNil && n != nil { | ||
| return fmt.Errorf("Perfect Tree size %d has non-nil node at index %d, wanted nil", size, i) |
There was a problem hiding this comment.
no initial-caps on these errors
There was a problem hiding this comment.
Is that for chaining-of-errors purposes? (so you get approximately readable sentences with no jarring mid-sentence caps?)
| {4095, testonly.MustDecodeBase64("cWRFdQhPcjn9WyBXE/r1f04ejxIm5lvg40DEpRBVS0w=")}, | ||
| {4096, testonly.MustDecodeBase64("6uU/phfHg1n/GksYT6TO9aN8EauMCCJRl3dIK0HDs2M=")}, | ||
| {10000, testonly.MustDecodeBase64("VZcav65F9haHVRk3wre2axFoBXRNeUh/1d9d5FQfxIg=")}, | ||
| {65535, testonly.MustDecodeBase64("iPuVYJhP6SEE4gUFp8qbafd2rYv9YTCDYqAxCj8HdLM=")}, |
phad
force-pushed
the
ph_compact_merkle_tree_nil_nodes
branch
2 times, most recently
from
86b7d92
to
42240d8
Compare
AlCutter
approved these changes
Oct 4, 2016
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.
for given tree size are set to nil when new leaves are added.
Check using an invariant after each AddLeaf in the tests.
Fix test data to have nils in the appropriate places.