feat: add query to retrieve parent tree information#368
feat: add query to retrieve parent tree information#368QuantumExplorer merged 2 commits intodevelopfrom
Conversation
WalkthroughThe changes introduce the ability for GroveDB proof verification to return information about the parent tree's feature type during query verification. This includes new methods, internal refactoring to propagate the parent tree info, updates to version tracking, and expanded test coverage to validate the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test
participant GroveDb as GroveDb
participant ProofVerifier as ProofVerifier (internal)
participant Element as Element
Test->>GroveDb: verify_query_get_parent_tree_info(proof, query, version)
GroveDb->>ProofVerifier: verify_proof_internal(proof, query, options, version)
ProofVerifier->>Element: tree_feature_type()
Element-->>ProofVerifier: TreeFeatureType
ProofVerifier-->>GroveDb: (root_hash, TreeFeatureType, results)
GroveDb-->>Test: (root_hash, TreeFeatureType, results)
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
grovedb/src/operations/proof/mod.rs (1)
73-82: Verify verification method documentation for new return value structure.The method comments for verification methods should be updated to reflect that internally these methods now return a triple containing parent tree information, even though the public API continues to return just root hash and results.
/// Verifies a query with options using the proof and returns the root hash /// and the query result. +/// Note: Internally, this also computes parent tree information which is exposed +/// through the dedicated `verify_query_get_parent_tree_info_with_options` method. pub fn verify_with_options(grovedb/src/tests/sum_tree_tests.rs (2)
109-122: Consider extracting a helper for the repeated proof-verification assertionsThe 11-line block that:
- calls
verify_query_get_parent_tree_info,- asserts
root_hashequality,- checks
result_set.len(),- asserts on
parent,is duplicated several times in this file (and will likely appear in future tests). A small helper such as
assert_parent_tree_info(...)would keep individual tests focused on intent and reduce maintenance overhead.This is purely a readability / DRY improvement – feel free to ignore if you prefer the current explicit style.
1741-1809: Huge raw proof literal hampers readability and slows down compile times
test_verify_query_get_parent_tree_infoembeds a ~3 KB hex string directly in the source. Consider moving it to:
- a compressed test fixture file loaded at runtime, or
- a
const &[u8]in a separate module reused by multiple tests.This makes the test file easier to scan and prevents accidental whitespace edits from breaking the hex payload.
grovedb/src/operations/proof/verify.rs (1)
70-122: Duplicate pre-condition logic – extract or reuse existing validator
verify_query_get_parent_tree_info_with_optionsre-implements all the limit/offset/subquery checks that already exist inverify_query_with_options. Duplicating this logic invites divergence (e.g. the double word in the error message “for for root tree”). Consider factoring the common validation into a small private helper and call it from both entry points.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
grovedb-version/src/version/grovedb_versions.rs(1 hunks)grovedb-version/src/version/v1.rs(1 hunks)grovedb-version/src/version/v2.rs(1 hunks)grovedb/src/element/helpers.rs(1 hunks)grovedb/src/operations/proof/mod.rs(6 hunks)grovedb/src/operations/proof/verify.rs(11 hunks)grovedb/src/tests/sum_tree_tests.rs(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
grovedb/src/operations/proof/mod.rs (2)
grovedb/src/lib.rs (1)
root_hash(463-478)merk/src/merk/mod.rs (1)
root_hash(313-319)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Code Coverage
- GitHub Check: Compilation errors
- GitHub Check: Linting
- GitHub Check: Tests
🔇 Additional comments (5)
grovedb-version/src/version/v2.rs (1)
147-147: Versioning for new parent tree info retrieval feature correctly added.The addition of
verify_query_get_parent_tree_info_with_optionswith an initial version value of 0 properly tracks the new verification method in the versioning system. This versioning change is consistent with the PR's objective to add query functionality that retrieves parent tree information.grovedb-version/src/version/grovedb_versions.rs (1)
81-81: Added proper struct field for version tracking of new feature.This field declaration for
verify_query_get_parent_tree_info_with_optionsin theGroveDBOperationsProofVersionsstruct correctly enables version tracking for the new verification method that returns parent tree information. The change is consistent with the PR's goals and the versioning pattern used throughout the project.grovedb-version/src/version/v1.rs (1)
147-147: Consistently added versioning for feature across all GroveDB versions.The addition of
verify_query_get_parent_tree_info_with_optionswith a version value of 0 in theGROVE_V1constant ensures the new verification capability is properly tracked across all supported GroveDB versions. This consistency is important for maintaining backward compatibility.grovedb/src/operations/proof/mod.rs (1)
81-81: Maintained API compatibility while extending internal verification results.All public verification methods have been updated to handle the new triple return format from internal verification methods, which now include parent tree information. The
.map(|(root_hash, _, results)| (root_hash, results))transformation elegantly maintains backward compatibility by discarding the middle element (parent tree info) for existing API methods, while allowing new specialized methods to access this information.This approach ensures existing code continues to work while new functionality is made available through dedicated methods.
Also applies to: 101-101, 121-121, 141-141, 161-161, 181-181
grovedb/src/operations/proof/verify.rs (1)
392-401: Last parent tree feature type may be non-deterministic for multi-key queries
last_parent_tree_typeis overwritten every time the recursion descends into a child tree.
If the query contains several keys at the same level (e.g.{k1,k2}) the last visited key will win, leaving the returned parent type dependent on iteration order, not on the caller’s intent.Please confirm that:
- the API is only meant for single-key path queries (then add an explicit guard), or
- the “first” tree should be retained (then update logic to assign only once).
|
Self reviewed. |
Issue being fixed or feature implemented
Add functionality to retrieve parent tree information when querying in a subtree.
What was done?
verify_query_get_parent_tree_info_with_optionsandverify_query_get_parent_tree_infomethods to retrieve parent tree information based on the provided proof and query.GroveDBProofstructure to return the parent tree type along with the root hash and results.How Has This Been Tested?
Unit tests were added to verify the correctness of the new query methods, including checks for expected outputs and handling of edge cases.
Breaking Changes
None
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests