fix: correct typos and grammar in architecture documentation #25124
Conversation
📝 WalkthroughWalkthroughThis set of changes consists solely of minor textual corrections and grammatical improvements across several architectural decision record (ADR) documentation files. No technical content, logic, code examples, or control flow are altered. All modifications are limited to enhancing clarity, fixing typographical errors, and improving readability in documentation comments and prose. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches🧪 Generate unit tests
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. 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
🔭 Outside diff range comments (1)
docs/architecture/adr-033-protobuf-inter-module-comm.md (1)
147-152: Fix example: missing ctx and inconsistent receiver usage
- bank.QueryClient methods require ctx as the first argument.
- Use the receiver name consistently for moduleKey.
Apply the following diff to the snippet:
- balance, err := foo.bankQuery.Balance(&bank.QueryBalanceRequest{Address: foo.moduleKey.Address(), Denom: "foo"}) + balance, err := foo.bankQuery.Balance(ctx, &bank.QueryBalanceRequest{Address: foo.moduleKey.Address(), Denom: "foo"}) - res, err := foo.bankMsg.Send(ctx, &bank.MsgSendRequest{FromAddress: fooMsgServer.moduleKey.Address(), ...}) + res, err := foo.bankMsg.Send(ctx, &bank.MsgSendRequest{FromAddress: foo.moduleKey.Address(), ...})
🧹 Nitpick comments (19)
docs/architecture/adr-033-protobuf-inter-module-comm.md (1)
113-118: Use plural method name: GetSignersIn the Cosmos SDK, messages implement GetSigners() []sdk.AccAddress (plural). Consider updating “Msg.GetSigner” to “Msg.GetSigners” for accuracy.
docs/architecture/adr-035-rosetta-api-support.md (1)
39-41: Tighten phrasing for readabilityConsider simplifying to: “We will achieve these principles by:” to remove repetition.
docs/architecture/adr-036-arbitrary-signature.md (6)
20-23: Tighten phrasing; remove redundancy and improve flow.Recommend the following edits for clarity:
-Currently, in the Cosmos SDK, there is no convention to sign arbitrary messages like in Ethereum. We propose with this specification, for Cosmos SDK ecosystem, a way to sign and validate off-chain arbitrary messages. +Currently, the Cosmos SDK has no convention for signing arbitrary messages (as in Ethereum). This specification proposes a way for the Cosmos SDK ecosystem to sign and validate arbitrary off-chain messages. - -This specification serves the purpose of covering every use case; this means that Cosmos SDK application developers decide how to serialize and represent `Data` to users. +This specification aims to cover a wide range of use cases; application developers decide how to serialize and represent `Data` to users.
26-28: Fix grammar and complete the sentence.Current text contains agreement errors and a sentence fragment. Suggested patch:
-Having the ability to sign messages off-chain has proven to be a fundamental aspect of nearly any blockchain. The notion of signing messages off-chain has many added benefits such as saving on computational costs and reducing transaction throughput and overhead. Within the context of the Cosmos, some of the major applications of signing such data include, but is not limited to, providing a cryptographic secure and verifiable means of proving validator identity and possibly associating it with some other framework or organization. In addition, having the ability to sign Cosmos messages with a Ledger or similar HSM device. +Having the ability to sign messages off‑chain is fundamental to most blockchains. It offers benefits such as saving computational costs and reducing transaction throughput and overhead. In the Cosmos context, major applications include, but are not limited to, providing a cryptographically secure and verifiable means of proving validator identity and associating it with other frameworks or organizations. In addition, the ability to sign Cosmos messages with a Ledger or similar HSM device is desirable.
32-35: Minor style: infinitive form and modal usage.-The aim is being able to sign arbitrary messages, even using Ledger or similar HSM devices. +The aim is to enable signing arbitrary messages, including via Ledger or similar HSM devices. -As a result, signed messages should look roughly like Cosmos SDK messages but **must not** be a valid on-chain transaction. `chain-id`, `account_number` and `sequence` can all be assigned invalid values. +As a result, signed messages should resemble Cosmos SDK messages but **must not** be valid on-chain transactions. `chain-id`, `account_number`, and `sequence` may be assigned invalid values.
40-41: Consistency: off-chain hyphenation and singular “functionality.”-To create the `offchain` proto definitions, we extend the auth module with `offchain` package to offer functionalities to verify and sign offline messages. +To create the `offchain` proto definitions, we extend the auth module with an `offchain` package to offer functionality to verify and sign off‑chain messages.
54-57: Fix developer phrasing and object examples.-`MsgSignData` allows developers to sign arbitrary bytes validatable offchain only. `Signer` is the account address of the signer. `Data` is arbitrary bytes which can represent `text`, `files`, `object`s. It's applications developers decision how `Data` should be deserialized, serialized and the object it can represent in their context. -It's applications developers decision how `Data` should be treated, by treated we mean the serialization and deserialization process and the Object `Data` should represent. +`MsgSignData` allows developers to sign arbitrary bytes that are verifiable off‑chain only. `Signer` is the account address of the signer. `Data` is arbitrary bytes that can represent text, files, or objects. It is up to application developers to decide how `Data` is serialized/deserialized and what object it represents in their context. +It is up to application developers to decide how `Data` is handled, i.e., its serialization/deserialization process and the object `Data` should represent.
124-126: Formalize responsibility wording and tighten the sentence.-* Regarding security in `MsgSignData`, the developer using `MsgSignData` is in charge of making the content contained in `Data` non-replayable when, and if, needed. -* The offchain package will be further extended with extra messages that target specific use cases such as, but not limited to, authentication in applications, payment channels, L2 solutions in general. +* Regarding security in `MsgSignData`, developers are responsible for ensuring the content contained in `Data` is non‑replayable, when needed. +* The `offchain` package will be further extended with messages that target specific use cases such as, but not limited to, application authentication, payment channels, and general L2 solutions.docs/architecture/adr-034-account-rekeying.md (2)
45-46: Replace informal “anyways” and streamline sentence.-... we do not automatically prune any accounts anyways, but we would like to keep this option open down the road ... +... we do not automatically prune any accounts anyway, but we would like to keep this option open down the road ...
64-67: Minor clarity in consequences bullet.Consider tightening the first sentence and ensuring present tense consistency:
-Breaks the current assumed relationship between address and pubkey as H(pubkey) = address. This has a couple of consequences. +This breaks the current assumed relationship between address and pubkey (H(pubkey) = address). This has several consequences:docs/architecture/adr-038-state-listening.md (3)
10-10: Fix sub-list indentation to satisfy markdownlint (MD007).Reduce indentation to two spaces for this sub-bullet to match the preceding items and typical markdownlint settings.
- * Remove `HaltAppOnDeliveryError()`, the errors are propagated by default, the implementations should return nil if they don't want to propagate errors. + * Remove `HaltAppOnDeliveryError()`. Errors are propagated by default; implementations should return `nil` if they don't want to propagate errors.If markdownlint is enforced, verify locally that MD007 is resolved after this change.
43-44: Good catch — “accumulates” is correct. Add article for completeness.-// NewMemoryListener creates a listener that accumulates the state writes in memory. +// NewMemoryListener creates a listener that accumulates state writes in memory.
228-229: Align comment with terminology used elsewhere.-// ListenCommit updates the streaming service with the latest Commit messages and state changes +// ListenCommit updates the streaming service with the latest Commit messages and state changes.docs/architecture/adr-039-epoched-staking.md (6)
32-35: Consistent terminology and punctuation.The updates read well. Consider adding a reference link formatting for “tendermint/spec#217” if available, e.g., full URL, to improve portability.
71-71: Smoother phrasing for the trade-off sentence.-When choosing the epoch length, there is a trade-off between queued state/computation buildup, and countering the previously discussed limitations of immediate execution if they apply to a given chain. +When choosing the epoch length, there is a trade‑off between queued state and computation build‑up and mitigating the previously discussed limitations of immediate execution (as applicable to a given chain).
82-85: Use “return” (avoid “return back”) and tighten sentences.-* **MsgCreateValidator**: Move user's self-bond to `EpochDelegationPool` immediately. Queue a message for the epoch boundary to handle the self-bond, taking the funds from the `EpochDelegationPool`. If Epoch execution fails, return back funds from `EpochDelegationPool` to user's account. +* **MsgCreateValidator**: Move the user's self‑bond to `EpochDelegationPool` immediately. Queue a message for the epoch boundary to handle the self‑bond, taking the funds from `EpochDelegationPool`. If epoch execution fails, return the funds from `EpochDelegationPool` to the user's account. @@ -* **MsgDelegate**: Move user's funds to `EpochDelegationPool` immediately. Queue a message for the epoch boundary to handle the delegation, taking the funds from the `EpochDelegationPool`. If Epoch execution fails, return back funds from `EpochDelegationPool` to user's account. +* **MsgDelegate**: Move the user's funds to `EpochDelegationPool` immediately. Queue a message for the epoch boundary to handle the delegation, taking the funds from `EpochDelegationPool`. If epoch execution fails, return the funds from `EpochDelegationPool` to the user's account.
92-92: Clarify apparent contradiction in slash queuing vs immediate effect.The bullet says the slash event “gets queued ... to apply at the end of the epoch” and “should be set up such that this slash applies immediately.” Please clarify whether:
- jailing applies immediately while weight reduction is applied at the epoch boundary, or
- both jailing and weight updates are deferred.
If it is the former, suggest:
-* **Slash Event**: Whenever a slash event is created, it gets queued in the slashing module to apply at the end of the epoch. The queues should be set up such that this slash applies immediately. +* **Slash Event**: Whenever a slash event is created, jailing is applied immediately, while the associated weight reduction is queued to apply at the end of the epoch.
104-104: Minor term: “hash map” vs “hashmap.”-... by maintaining an auxiliary hashmap for indexing upcoming staking events by address) +... by maintaining an auxiliary hash map for indexing upcoming staking events by address)
111-111: Slightly crisper phrasing.-We leave it out of scope for how to weight future computation versus current computation in gas pricing, and have it set such that they are weighted equally for now. +How to weight future versus current computation in gas pricing is out of scope; for now, treat them as weighted equally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (9)
docs/architecture/adr-031-msg-service.md(3 hunks)docs/architecture/adr-032-typed-events.md(1 hunks)docs/architecture/adr-033-protobuf-inter-module-comm.md(5 hunks)docs/architecture/adr-034-account-rekeying.md(3 hunks)docs/architecture/adr-035-rosetta-api-support.md(5 hunks)docs/architecture/adr-036-arbitrary-signature.md(3 hunks)docs/architecture/adr-037-gov-split-vote.md(2 hunks)docs/architecture/adr-038-state-listening.md(5 hunks)docs/architecture/adr-039-epoched-staking.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/adr-036-arbitrary-signature.md
[style] ~124-~124: Consider using more formal and concise wording here.
Context: ...ata, the developer using MsgSignDatais in charge of making the content contained inData` ...
(RESPONSIBLE_FOR)
docs/architecture/adr-034-account-rekeying.md
[style] ~46-~46: The word ‘anyways’ is informal American English. Did you mean “anyway”?
Context: ...do not automatically prune any accounts anyways, but we would like to keep this option ...
(ANYWAYS)
docs/architecture/adr-039-epoched-staking.md
[style] ~82-~82: Consider using just “return”.
Context: ...egationPool. If Epoch execution fails, return back funds from EpochDelegationPool` to use...
(RETURN_BACK)
[style] ~84-~84: Consider using just “return”.
Context: ...egationPool. If Epoch execution fails, return back funds from EpochDelegationPool` to use...
(RETURN_BACK)
[style] ~107-~107: ‘Merging together’ might be wordy. Consider a shorter alternative.
Context: ...transaction when its done immediately. (Merging together costs of p2p overhead, state access ove...
(EN_WORDINESS_PREMIUM_MERGING_TOGETHER)
🪛 markdownlint-cli2 (0.17.2)
docs/architecture/adr-038-state-listening.md
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
228-228: Hard tabs
Column: 1
(MD010, no-hard-tabs)
docs/architecture/adr-039-epoched-staking.md
29-29: Hard tabs
Column: 1
(MD010, no-hard-tabs)
30-30: Hard tabs
Column: 1
(MD010, no-hard-tabs)
31-31: Hard tabs
Column: 1
(MD010, no-hard-tabs)
32-32: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (23)
docs/architecture/adr-031-msg-service.md (5)
14-15: Grammar improvement LGTMClearer phrasing without affecting technical meaning.
19-21: Clarity improvement LGTMEditorial cleanup; no change in substance.
108-110: Encoding section wording LGTMAccurate and clearer wording for Any packing.
113-114: Decoding sentence LGTMCorrect subject-verb agreement and readability.
160-161: Method reference wording LGTM“from the context.Context parameter” reads correctly now.
docs/architecture/adr-033-protobuf-inter-module-comm.md (4)
45-46: Word choice fix LGTM“simply by calling” reads better; no technical change.
151-152: AI summary inconsistency: receiver field name not updated hereThe AI summary mentions correcting fooMsgServer.moduleKey to foo.moduleKey, but this line still uses fooMsgServer.moduleKey. The diff above addresses it.
Likely an incorrect or invalid review comment.
273-281: Dependency declaration wording LGTMPossessive “its” is correct; example remains clear.
333-335: Authorization sentence clarity LGTM“authorize specific module accounts” reads correctly.
docs/architecture/adr-035-rosetta-api-support.md (5)
12-12: Changelog spacing/wording LGTMEditorial; no technical change.
36-36: Wording simplification LGTMLess redundancy in “release series” phrasing.
56-57: Section intro wording LGTMReads more naturally; content unchanged.
123-125: Comment label fix LGTM“ByHeight” comment now matches method name.
193-193: Past participle correction LGTM“spun in offline mode” is correct.
docs/architecture/adr-032-typed-events.md (1)
156-157: Grammar fix LGTMThe sentence now reads correctly and clearly.
docs/architecture/adr-037-gov-split-vote.md (3)
17-18: Singular “option” LGTMImproves precision without changing meaning.
19-20: “Oftentimes” usage LGTMStandardized form; clearer prose.
56-57: Lowercase “rates” LGTMConsistent with sentence casing and style.
docs/architecture/adr-036-arbitrary-signature.md (1)
119-121: LGTM — clear articulation of current limitations.The bullets accurately capture the constraints (fixed address–pubkey relationship; multisig unsupported).
docs/architecture/adr-034-account-rekeying.md (1)
17-20: LGTM — context is clearer and uses correct hyphenation.Edits improve readability (cannot, three‑week, on‑chain). No further changes needed here.
docs/architecture/adr-038-state-listening.md (2)
117-129: Interface docstring alignment.Wording aligns with the later use in rootmulti. LGTM.
723-724: Em dashes are a readability improvement; keep them.The neutral bullet reads well with em dashes and matches house style. LGTM.
docs/architecture/adr-039-epoched-staking.md (1)
22-25: LGTM — clearer framing of the motivation and alternative design.Edits improve precision (“simplest from an implementation perspective”, epoched guarantee). No additional changes needed.
2 participants
Description
Fix various typos, grammatical errors, and stylistic issues across
ADR (Architecture Decision Record) documents:
Summary by CodeRabbit