chore(docs): update store v2 adr #22794
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe document Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
julienrbrt
added
the
backport/v0.52.x
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
docs/architecture/adr-065-store-v2.md (3)
84-86: Consider adding migration impact detailsWhile the removal of store types and branching logic aligns with the simplification goal, it would be helpful to add:
- The rationale behind removing CacheKVStore
- How existing applications should handle caching after this change
- Migration steps for applications currently using these features
96-101: Consider adding performance metricsTo help developers make informed decisions about IAVL versions:
- Add quantitative performance comparisons between IAVL v1, v2, and MemIAVL
- Include estimated state migration times based on state size
- Document any breaking changes between versions
135-135: Clarify architectural implicationsThe simplified method description "Manage commitment of state" removes explicit mention of SS and SC. Consider:
- Documenting whether this is purely a documentation change or reflects architectural changes
- Explaining how SS and SC responsibilities are now handled
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
docs/architecture/adr-065-store-v2.md(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/architecture/adr-065-store-v2.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/architecture/adr-065-store-v2.md
[grammar] ~153-~153: Two determiners in a row. Choose either “the” or “a”.
Context: ... Retrieving proofs will be done through the a RootStore, which will internally rout...
(DT_DT)
| So providing inclusion and exclusion proofs, via a `CommitmentOp` type, will be | ||
| the responsibility of the SC backend. Retrieving proofs will be done through the | ||
| a `RootStore`, which will internally route the request to the SC backend. | ||
| Providing a `CommitmentOp` type, will be the responsibility of the SC backend. Retrieving proofs will be done through the a `RootStore`, which will internally route the request to the SC backend. |
There was a problem hiding this comment.
Fix grammatical error
The sentence contains a grammatical error and could be clearer.
-Providing a `CommitmentOp` type, will be the responsibility of the SC backend. Retrieving proofs will be done through the a `RootStore`, which will internally route the request to the SC backend.
+Providing a `CommitmentOp` type will be the responsibility of the SC backend. Retrieving proofs will be done through a `RootStore`, which will internally route the request to the SC backend.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Providing a `CommitmentOp` type, will be the responsibility of the SC backend. Retrieving proofs will be done through the a `RootStore`, which will internally route the request to the SC backend. | |
| Providing a `CommitmentOp` type will be the responsibility of the SC backend. Retrieving proofs will be done through a `RootStore`, which will internally route the request to the SC backend. |
🧰 Tools
🪛 LanguageTool
[grammar] ~153-~153: Two determiners in a row. Choose either “the” or “a”.
Context: ... Retrieving proofs will be done through the a RootStore, which will internally rout...
(DT_DT)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
store/v2/README.md (2)
10-10: Fix grammatical issuesPlease address the following grammatical issues:
- Line 10: Replace "it's" with "its" (possessive form)
- Line 51: Add a hyphen in "implementation-specific"
-abstraction layer around it's primary constituent components - state commitment (SC). It acts as the main entry point into storage for an +abstraction layer around its primary constituent components - state commitment (SC). It acts as the main entry point into storage for an -of the underlying commitment layer. This means pruning can be implementation specific, +of the underlying commitment layer. This means pruning can be implementation-specific,Also applies to: 51-51
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Did you mean the possessive pronoun “its”?
Context: ...d to act as an abstraction layer around it's primary constituent components - state ...(IT_S_ITS)
62-62: Enhance test coverage documentationThe test coverage section appears incomplete. Consider adding:
- List of specific logical components that require coverage
- Instructions for measuring coverage
- Guidelines for maintaining or improving coverage
Would you like me to help draft a more detailed test coverage section?
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
store/v2/README.md(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
store/v2/README.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
store/v2/README.md
[grammar] ~10-~10: Did you mean the possessive pronoun “its”?
Context: ...d to act as an abstraction layer around it's primary constituent components - state ...
(IT_S_ITS)
[uncategorized] ~51-~51: This expression is ususally spelled with a hyphen
Context: ...itment layer. This means pruning can be implementation specific, such as being synchronous or asynchron...
(SPECIFIC_HYPHEN)
🔇 Additional comments (1)
store/v2/README.md (1)
10-18: Technical content accurately reflects architectural changes
The documentation updates correctly reflect the simplified storage design where:
- State Commitment (SC) is now the primary focus
- State Storage (SS) references have been appropriately removed
- Pruning responsibility is clearly defined as part of the underlying commitment layer
Also applies to: 39-41
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Did you mean the possessive pronoun “its”?
Context: ...d to act as an abstraction layer around it's primary constituent components - state ...
(IT_S_ITS)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
store/v2/pruning/README.md (2)
4-4: LGTM with suggestions for documentation improvementsThe updated description correctly reflects the focus on state commitment (SC). However, consider adding:
- A brief explanation of why SS was removed from pruning responsibilities
- A link to ADR-65 for more context about this architectural decision
Line range hint
1-50: Consider clarifying architectural implicationsThe document would benefit from explaining:
- The impact of this change on existing implementations
- How this relates to the broader store v2 architecture
- Whether users need to make any adjustments to their pruning configurations
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
store/v2/pruning/README.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
store/v2/pruning/README.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
(cherry picked from commit f995d0a) # Conflicts: # store/v2/README.md # store/v2/pruning/README.md
Co-authored-by: Marko <[email protected]> Co-authored-by: Julien Robert <[email protected]>
Description
Update store v2 adr. The design of storage is a large simplification and focus on communicating to disk. Previously storage handled cachekv and gas kv which now have been moved to the stf layer
closes #22741
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
RootStoreinterface for improved management of state commitments and storage backends.storeandpruningdocumentation to reflect the streamlined focus on state commitment.