chore(auth): fix typos and improve comments #25164
Conversation
📝 WalkthroughWalkthroughPrimarily comment and documentation typo/grammar fixes across auth packages and tests. One method in x/auth/tx/builder.go was renamed from GetTimeoutTimeStamp to GetTimeoutTimestamp; implementation unchanged. Several test expected error strings were adjusted to match corrected spelling/capitalization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ 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/Issue comments)Type 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 (2)
x/auth/types/params.go (1)
76-84: Fix typo in SECP256k1 error message across code and testsThe error string “SECK256k1” is misspelled in both the implementation and its test. Please update both occurrences to “SECP256k1”:
• x/auth/types/params.go (line 83)
• x/auth/types/params_test.go (line 32)Proposed diff:
--- a/x/auth/types/params.go +++ b/x/auth/types/params.go @@ -80,7 +80,7 @@ func validateSigVerifyCostSecp256k1(i any) error { if v == 0 { - return fmt.Errorf("invalid SECK256k1 signature verification cost: %d", v) + return fmt.Errorf("invalid SECP256k1 signature verification cost: %d", v) } return nil }--- a/x/auth/types/params_test.go +++ b/x/auth/types/params_test.go @@ -29,7 +29,7 @@ func TestValidateSigVerifyCostSecp256k1(t *testing.T) { {"invalid parameter type", &Params{...}, fmt.Errorf("invalid parameter type: %T", "foo")}, {"invalid SigVerifyCostSecp256k1 cost", &Params{...}, fmt.Errorf("invalid SigVerifyCostSecp256k1 cost: %d", 0)}, - {"invalid SECK256k1 signature verification cost", types.NewParams(...), nil}, + {"invalid SECP256k1 signature verification cost", types.NewParams(...), nil}, }x/auth/tx/builder.go (1)
98-104: Ensure backward-compatibility for the renamed GetTimeoutTimestampThe
wrapperinx/auth/tx/builder.gonow only providesGetTimeoutTimestamp(), but:
- The
TxWithTimeoutTimeStamp/TxWithUnorderedinterfaces intypes/tx_msg.gostill declareGetTimeoutTimeStamp().- Numerous call sites (
x/auth/ante/…,types/mempool/…,baseapp/…, docs, tests) invoke the oldGetTimeoutTimeStamp().Without an alias or interface update, the code will no longer compile.
Recommended fixes:
- In x/auth/tx/builder.go, add a deprecated alias for the old name:
// Deprecated: use GetTimeoutTimestamp. func (w *wrapper) GetTimeoutTimeStamp() time.Time { return w.GetTimeoutTimestamp() }- Add a compile-time assertion to catch drift early (e.g., in builder.go):
var _ sdk.TxWithTimeoutTimeStamp = (*wrapper)(nil)- Either update the interface in types/tx_msg.go to
GetTimeoutTimestamp()(and rename callers accordingly) or keep the alias until a later major bump.Pinpoint locations needing attention:
- x/auth/tx/builder.go (add alias + assertion)
- types/tx_msg.go (interface declaration)
- All call sites still referencing
GetTimeoutTimeStamp()(rg results above)
🧹 Nitpick comments (7)
x/auth/signing/sign_mode_handler.go (1)
18-19: Nit: “supporting” → “supported”.Minor grammar tweak for clarity.
Apply this diff:
- // Modes is the list of modes supporting by this handler + // Modes is the list of modes supported by this handlerx/auth/keeper/deterministic_test.go (1)
91-92: Optional: possessive form for readability.“keeper store” → “keeper’s store”.
-// createAndSetAccounts creates random accounts and sets them to the keeper store. +// createAndSetAccounts creates random accounts and sets them to the keeper's store.x/auth/types/account.go (1)
162-162: Clarify comment: be explicit about the input being a module name“from a string” is vague. Suggest clarifying it’s the module name.
-// NewEmptyModuleAccount creates an empty ModuleAccount from a string +// NewEmptyModuleAccount creates an empty ModuleAccount from the module namex/auth/types/credentials_test.go (1)
16-16: Strengthen the assertion to validate the error message contentCurrently, the third parameter is just the failure message, not a check on the error text. Use ErrorContains (or EqualError) to assert the actual error string.
- require.Error(t, err, "derivation keys must be non-empty") + require.ErrorContains(t, err, "derivation keys must be non-empty")x/auth/ante/validator_tx_fee.go (1)
53-54: Tiny grammar improvement“could not be prioritized” reads a bit absolute. “might not be prioritized” better conveys possibility.
-// where txs with multiple coins could not be prioritized as expected. +// where txs with multiple coins might not be prioritized as expected.x/auth/types/params_legacy.go (1)
2-4: Polish the header comment for grammar and readabilityAdd a hyphen for “x/gov-controlled” and tighten phrasing.
-NOTE: Usage of x/params to manage parameters is deprecated in favor of x/gov -controlled execution of MsgUpdateParams messages. These types remain solely -for migration purposes and will be removed in a future release. +NOTE: Usage of x/params to manage parameters is deprecated in favor of the x/gov-controlled +execution of MsgUpdateParams messages. These types remain solely for migration purposes +and will be removed in a future release.x/auth/types/params_test.go (1)
32-32: Fix typo in test case name: SECK256k1 → SECP256k1.The test case label still contains the misspelling. Align it with the corrected terminology.
- {"invalid SECK256k1 signature verification cost", types.NewParams(types.DefaultMaxMemoCharacters, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte, + {"invalid SECP256k1 signature verification cost", types.NewParams(types.DefaultMaxMemoCharacters, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte,
📜 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 (16)
x/auth/ante/sigverify.go(2 hunks)x/auth/ante/validator_tx_fee.go(1 hunks)x/auth/client/tx.go(1 hunks)x/auth/keeper/deterministic_test.go(1 hunks)x/auth/keeper/msg_server_test.go(1 hunks)x/auth/signing/sign_mode_handler.go(1 hunks)x/auth/tx/README.md(2 hunks)x/auth/tx/builder.go(1 hunks)x/auth/tx/service.go(1 hunks)x/auth/types/account.go(1 hunks)x/auth/types/codec.go(1 hunks)x/auth/types/credentials_test.go(1 hunks)x/auth/types/genesis.go(1 hunks)x/auth/types/params.go(1 hunks)x/auth/types/params_legacy.go(1 hunks)x/auth/types/params_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
x/auth/types/params_test.go (1)
x/auth/types/params.go (1)
DefaultSigVerifyCostED25519(12-12)
⏰ 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 (13)
x/auth/signing/sign_mode_handler.go (1)
11-13: Good grammar fix: “an interface”.No functional changes; comment reads clearly.
x/auth/keeper/deterministic_test.go (1)
91-92: Doc comment improvement looks good.Accurately reflects that multiple accounts are created.
x/auth/client/tx.go (1)
108-110: LGTM: comment now correctly says “multiple txs”.Matches behavior (returns a scanner and does not decode).
x/auth/types/codec.go (1)
29-30: LGTM: “its concrete implementations”.Correct possessive form; no behavioral impact.
x/auth/types/params.go (1)
44-46: LGTM: corrected comparative phrasing in the comment.Accurate and clearer wording.
x/auth/tx/service.go (1)
24-24: Doc capitalization fix LGTMThe BaseApp capitalization in the comment is correct and improves clarity. No functional changes.
x/auth/types/genesis.go (1)
94-99: LGTM: comment clarificationGrammar fix improves clarity; no functional changes.
x/auth/ante/sigverify.go (2)
38-42: LGTM: typo fix in commentComment now correctly references secp256k1; no logic changes.
103-106: LGTM: punctuation/grammar fixComment reads better; no functional impact.
x/auth/tx/README.md (2)
119-119: LGTM: grammar fix in encode description“The transaction is serialized to Protobuf...” reads correctly. No content changes beyond wording.
131-131: LGTM: grammar fix in decode descriptionSingular “command decodes” is correct here. No functional changes.
x/auth/keeper/msg_server_test.go (1)
95-96: LGTM: standardized SECP256k1 capitalization in expected errorMatches the corrected wording across the codebase.
x/auth/types/params_test.go (1)
33-33: LGTM: expected error message corrected to SECP256k1.The asserted error string now matches the Validate() output and the standardized naming.
|
this doesnt build |
Pull request was closed
2 participants
Corrected spelling mistakes, fixed grammar, and improved comment clarity across the
x/authpackage.Also standardized naming conventions (e.g., SECK256k1 → SECP256k1, multiples → multiple).
Summary by CodeRabbit
Refactor
Documentation
Tests