-
Notifications
You must be signed in to change notification settings - Fork 326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix quotation mark #4159
base: main
Are you sure you want to change the base?
fix quotation mark #4159
Conversation
📝 WalkthroughWalkthroughThe pull request introduces changes to two files: an architectural decision record (ADR) document and a configuration file. The ADR document Changes
Sequence DiagramsequenceDiagram
participant Validator
participant Block
participant Transaction
participant ShareCommitment
Validator->>Block: Recreate share commitments
Block->>Transaction: Extract MsgPayForBlob
Transaction->>ShareCommitment: Verify subtree roots
ShareCommitment-->>Validator: Validate message inclusion
This sequence diagram illustrates the high-level process of message inclusion checks in blockchain transactions, highlighting the validator's role in recreating share commitments and verifying transaction details. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
docs/architecture/adr-006-non-interactive-defaults.md (1)
Line range hint
138-177
: Consider adding error handling for edge cases incalculateSubTreeRootCoordinates
.The function assumes that
end >= start
and thatend
doesn't exceed the tree depth, but it should handle these cases gracefully with proper error messages.def calculateSubTreeRootCoordinates(maxDepth, start, end uint64) []coord { + if end < start { + raise ValueError("end must be greater than or equal to start") + } + if end >= (1 << maxDepth) { + raise ValueError("end exceeds the range of the tree depth") + } ...🧰 Tools
🪛 LanguageTool
[grammar] ~24-~24: Possible subject-verb agreement error detected.
Context: ...ut optional** message layout rules that enables the user to follow the above block vali...(PLURAL_THAT_AGREEMENT)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...nts to messages can consist entirely of sub-tree roots of the data hash, and those sub-t...(EN_COMPOUNDS_SUB_TREE)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...-tree roots of the data hash, and those sub-tree roots are to be generated only from the...(EN_COMPOUNDS_SUB_TREE)
🧹 Nitpick comments (3)
docs/architecture/adr-006-non-interactive-defaults.md (3)
24-24
: Fix grammar and spelling in the documentation.The text has some minor issues:
- Subject-verb agreement: "rules that enables" should be "rules that enable"
- Compound word spelling: "sub-tree" should be "subtree" for consistency with the codebase
-To fix this, the spec outlines the "non-interactive default rules". These involve a few additional **default but optional** message layout rules that enables the user to follow the above block validity rule, while also not interacting with a block producer. Commitments to messages can consist entirely of sub-tree roots of the data hash, and those sub-tree roots are to be generated only from the message itself (so that the user can sign something "non-interactively"). +To fix this, the spec outlines the "non-interactive default rules". These involve a few additional **default but optional** message layout rules that enable the user to follow the above block validity rule, while also not interacting with a block producer. Commitments to messages can consist entirely of subtree roots of the data hash, and those subtree roots are to be generated only from the message itself (so that the user can sign something "non-interactively").🧰 Tools
🪛 LanguageTool
[grammar] ~24-~24: Possible subject-verb agreement error detected.
Context: ...ut optional** message layout rules that enables the user to follow the above block vali...(PLURAL_THAT_AGREEMENT)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...nts to messages can consist entirely of sub-tree roots of the data hash, and those sub-t...(EN_COMPOUNDS_SUB_TREE)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...-tree roots of the data hash, and those sub-tree roots are to be generated only from the...(EN_COMPOUNDS_SUB_TREE)
Line range hint
180-196
: Consider adding thread-safety toEDSSubTreeRootCacher
.The current implementation is not thread-safe due to the shared counter. Consider using sync.Mutex or atomic operations to make it thread-safe.
type EDSSubTreeRootCacher struct { + mu sync.Mutex caches []*subTreeRootCacher squareSize uint64 counter int } func (stc *EDSSubTreeRootCacher) Constructor() rsmt2d.Tree { + stc.mu.Lock() + defer stc.mu.Unlock() ... }🧰 Tools
🪛 LanguageTool
[grammar] ~24-~24: Possible subject-verb agreement error detected.
Context: ...ut optional** message layout rules that enables the user to follow the above block vali...(PLURAL_THAT_AGREEMENT)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...nts to messages can consist entirely of sub-tree roots of the data hash, and those sub-t...(EN_COMPOUNDS_SUB_TREE)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...-tree roots of the data hash, and those sub-tree roots are to be generated only from the...(EN_COMPOUNDS_SUB_TREE)
Line range hint
267-290
: Consider adding metrics for block space utilization.Since non-interactive defaults can lead to wasted square space, it would be valuable to track metrics about block space utilization to help optimize the implementation.
Consider adding Prometheus metrics to track:
- Percentage of wasted space per block
- Average message size vs padded size
- Distribution of message alignments
🧰 Tools
🪛 LanguageTool
[grammar] ~24-~24: Possible subject-verb agreement error detected.
Context: ...ut optional** message layout rules that enables the user to follow the above block vali...(PLURAL_THAT_AGREEMENT)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...nts to messages can consist entirely of sub-tree roots of the data hash, and those sub-t...(EN_COMPOUNDS_SUB_TREE)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...-tree roots of the data hash, and those sub-tree roots are to be generated only from the...(EN_COMPOUNDS_SUB_TREE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/architecture/adr-006-non-interactive-defaults.md
(1 hunks)local_devnet/celestia-app/config.toml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- local_devnet/celestia-app/config.toml
🧰 Additional context used
🪛 LanguageTool
docs/architecture/adr-006-non-interactive-defaults.md
[grammar] ~24-~24: Possible subject-verb agreement error detected.
Context: ...ut optional** message layout rules that enables the user to follow the above block vali...
(PLURAL_THAT_AGREEMENT)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...nts to messages can consist entirely of sub-tree roots of the data hash, and those sub-t...
(EN_COMPOUNDS_SUB_TREE)
[misspelling] ~24-~24: This word is normally spelled as one.
Context: ...-tree roots of the data hash, and those sub-tree roots are to be generated only from the...
(EN_COMPOUNDS_SUB_TREE)
No description provided.