[Storehouse] 002 - Payloadless forest#8573
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
these tests are to verify payloadless implementation with the existing full trie implementation.
| } | ||
|
|
||
| // look up for non existing paths | ||
| exists, err := f.HasPaths(r) |
There was a problem hiding this comment.
The full trie forest uses ValueSize() method to look up for non existing paths, but since payloadless forest does not store value size anymore, instead, a HasPaths() method is added to be used here.
|
|
||
| // TestForestEquivalence_HasPathsVsValueSizes verifies that for every path, | ||
| // payloadless.HasPaths reports true iff the full forest's ValueSizes is > 0. | ||
| func TestForestEquivalence_HasPathsVsValueSizes(t *testing.T) { |
There was a problem hiding this comment.
verify HasPaths is equivalent to ValueSize in checking the path exists.
| // Under the hood it uses a circular buffer | ||
| // of mtrie pointers and a map of rootHash to cache index for fast lookup | ||
| type TrieCache struct { | ||
| tries []*MTrie |
There was a problem hiding this comment.
same implementation as ledger/complete/mtrie/trieCache.go, except the tries is a slice of payloadless trie
zhangchiqing
changed the title
zhangchiqing
changed the title
zhangchiqing
changed the title
|
This is a bit difficult to review, because its copy and modify. Do you think you could split this into a copy PR + modify PR? |
zhangchiqing
changed the base branch from
leo/payloadless-tests
to
leo/payloadless-forest-base
It's hard to do. I tried to add a commit with the original forest file copied to payloadless package to be used as a comparison base, and changed the base branch. But it doesn't work, because github would complain about merge conflict. Is that ok if you use local diff tool to show the actual diff? |
zhangchiqing
changed the base branch from
leo/payloadless-forest-base
to
leo/payloadless-tests
|
That is ok, thank you for checking. I'll continue diffing locally |
| return nil, err | ||
| } | ||
|
|
||
| return trie.ReadSingleLeafHash(r.Path), nil |
There was a problem hiding this comment.
we previously copied the value to prevent bugs/problems where downstream code would modify the value. Do we want to return a copy here as well, for the same reason.
There was a problem hiding this comment.
yeah, make sense, updated
zhangchiqing
force-pushed
the
leo/payloadless-tests
branch
from
a5b9162 to
b213f48
Compare
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- switch package to payloadless and drop import of mtrie/trie - Forest now stores leaf hashes per register, not payloads - replace ValueSizes with HasPaths (returns existence, not byte sizes) - replace Read/ReadSingleValue with ReadLeafHashes/ReadSingleLeafHash - Update/NewTrie extract value bytes from u.Payloads and discard keys - Proofs returns *ledger.PayloadlessTrieBatchProof - drop RegSize metrics and payload-size accounting Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- switch package to payloadless and drop external imports of mtrie/{trie,node}, ptrie, prf
- read assertions now compare HashLeaf(path, value) against returned *hash.Hash
- ReadSingleValue tests become ReadSingleLeafHash
- ValueSizes tests become HasPaths (existence-only flags)
- drop tests that depend on full-payload proof verification or partial-trie reconstruction:
TestNonExistingInvalidProof, TestRandomUpdateReadProofValueSizes, TestProofGenerationInclusion
- randomMTrie uses NewNode and NewMTrie from the payloadless package
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zhangchiqing
force-pushed
the
leo/payloadless-forest
branch
from
b10a8f7 to
ba46fa2
Compare
2 participants
This PR adds Payloadless forest.
Strategy: same copy-then-modify pattern as Spec 001 (mtrie/forest.go → payloadless/forest.go).
See spec