[Storehouse] 011 - Add checkpoint iterate nodes function and util - #8587
[Storehouse] 011 - Add checkpoint iterate nodes function and util#8587zhangchiqing wants to merge 4 commits into
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 Plus 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 |
zhangchiqing
changed the title
zhangchiqing
changed the title
534061d to
b826c88
Compare
b826c88 to
39f9dcb
Compare
39f9dcb to
61ed3ef
Compare
61ed3ef to
a4335f8
Compare
| if isV7 { | ||
| subtrieChecksums, _, err = readCheckpointHeaderV7(headerPath, logger) | ||
| } else { | ||
| subtrieChecksums, _, err = readCheckpointHeader(headerPath, logger) |
There was a problem hiding this comment.
The header's top-trie checksum is discarded here, so the top-trie part file never gets the header-vs-footer cross-check that subtries get via processCheckpointSubTrie. The godoc claim "matching the regular checkpoint readers" is inaccurate for the top-trie file.
| } | ||
| } | ||
|
|
||
| isDef := meta.hash == ledger.GetDefaultHashForHeight(int(meta.height)) |
There was a problem hiding this comment.
meta.height is an unvalidated uint16 from disk, and GetDefaultHashForHeight is defaultHashes[height] with 257 entries and no bounds check (ledger/trie.go:63). A corrupt checkpoint with height > 256 panics instead of returning an error.
| if !it.referenced.get(idx) { | ||
| return fmt.Errorf("%w: node at global index %d is not referenced by any parent or trie root (orphan node)", | ||
| ErrCheckpointIntegrity, idx) | ||
| } | ||
| } |
There was a problem hiding this comment.
This orphan-node pass, the documented "callback error aborts iteration" contract, and the trie-root rootIndex > total rejection in iterateTopTrie have no test coverage.
| // | ||
| // No error returns are expected during normal operation. | ||
| func readCheckpointHeaderVersion(headerPath string) (uint16, error) { | ||
| f, err := os.Open(headerPath) |
There was a problem hiding this comment.
nit: two pattern divergences in this version read: the header file ends up opened and parsed twice (here, then readCheckpointHeader/readCheckpointHeaderV7 re-opens it), and this uses plain os.Open + defer Close without the evictFileFromLinuxPageCache + closeAndMergeError pattern (or withFile) used by every other checkpoint file read.
| res.interimNodes++ | ||
|
|
||
| // An interim node with exactly one nil child is legitimate in a compactified | ||
| // trie (the present child is itself an interim node). Both-nil cannot occur, |
There was a problem hiding this comment.
nit: "Both-nil cannot occur" is true for writer-produced compactified tries, but emit does not reject a both-nil interim node in a corrupt checkpoint. Reword (e.g. "does not occur in a valid checkpoint").
|
|
||
| it := &checkpointIterator{ | ||
| fn: fn, | ||
| isDefault: newBitset(total + 1), |
There was a problem hiding this comment.
nit: the bitsets are sized by footer node counts, which decodeNodeCount accepts without any bound. A corrupt footer declaring e.g. 2^40 nodes makes this allocation panic (OOM) instead of returning an error. We could add a cheap bounds check.
a4335f8 to
de8435f
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
de8435f to
14fedc3
Compare
14fedc3 to
af08206
Compare
af08206 to
758bce9
Compare
758bce9 to
09cf0c9
Compare
09cf0c9 to
b4dd69a
Compare
f6ca6ac to
2810330
Compare
2810330 to
a5c5025
Compare
This comment has been minimized.
This comment has been minimized.
Co-authored-by: zhangchiqing <811374+zhangchiqing@users.noreply.github.com>
a5c5025 to
f0aa72c
Compare
This PR introduces a utility that streams a v6 or v7 checkpoint file and reports the total number of leaf and interim nodes. To minimize memory usage, it processes the checkpoint incrementally instead of loading the entire checkpoint into memory and reconstructing the trie.
While the initial use case is fairly specific, the implementation provides a generic checkpoint trie node iterator (
ledger/complete/wal/checkpoint_node_iterator.go#IterateCheckpointNodes) that can be reused for other purposes. For example, it can be used to collect payload size statistics, group nodes by owner key, or support other checkpoint analysis tasks.