fix: small fix on finalizing non-regular trees #358
Conversation
ogabrielides
requested review from
QuantumExplorer and
fominok
as code owners
WalkthroughThis pull request involves changes in two areas. In the restoration module ( Changes
Sequence Diagram(s)Restoration Finalization FlowsequenceDiagram
participant U as User
participant R as Restorer
participant M as Merk
U->>R: Call finalize(grove_version)
R->>M: verify(tree_type != NormalTree, grove_version)
alt Tree type is NormalTree
M-->>R: Return error indication
R-->>U: Error returned, restoration invalid
else Tree type is not NormalTree
M-->>R: Verification success
R-->>U: Proceed with finalization
end
Database Population Flow in ReplicationsequenceDiagram
participant M as Main (populate_db)
participant I as insert_empty_count_tree_db
participant DB as Database
participant Q as Query Handler
M->>I: Call to insert empty count tree
I->>DB: Create empty count tree (KEY_INT_COUNT)
M->>DB: Insert range of values into count tree
M->>Q: Query KEY_INT_COUNT in db_checkpoint_0 and db_destination
Q-->>M: Return query results
Poem
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tutorials/src/bin/replication.rs (2)
19-20: Consider adding a comment explaining the new keyThis constant name is consistent with the existing naming convention. Adding a short explanation or doc comment about how
KEY_INT_COUNTis used (e.g. a “count tree for integer values”) would be helpful for clarity.
221-228: Add a doc comment for new helperThis new helper function parallels the existing “insert_empty_tree_db” for count trees. Adding a brief doc comment describing its purpose would improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
merk/src/merk/restore.rs(1 hunks)tutorials/src/bin/replication.rs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Tests
- GitHub Check: Compilation errors
- GitHub Check: Linting
- GitHub Check: Code Coverage
🔇 Additional comments (3)
merk/src/merk/restore.rs (1)
458-458: Confirm condition correctnessThis line now rejects normal trees during the finalization step. If your intention is to exclude
NormalTreeduring restoration, then this is correct. Otherwise, confirm whether finalizing a normal tree should also be allowed.tutorials/src/bin/replication.rs (2)
76-81: Changes look goodThe insertion of the empty count tree followed by a transaction-based population matches the existing pattern from the other keys and trees. No issues found here.
139-145: Query block is consistentQuerying the
KEY_INT_COUNTrange is consistent with how other keys are verified. This helps confirm that inserts and retrievals function as expected.
Issue being fixed or feature implemented
When restoring non-regular trees, verify tree was failing.
What was done?
Correct condition
How Has This Been Tested?
Added sync of new non-regular trees and tested queries on them (replication tutorial)
Breaking Changes
no
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Bug Fixes
New Features