refactor: remove invariants #22994
refactor: remove invariants #22994
Conversation
aljo242
requested review from
facundomedica,
testinginprod,
JulianToledano,
ziscky,
a team,
kocubinski,
akhilkumarpilli,
julienrbrt,
sontrinh16 and
alpe
as code owners
|
Warning Rate limit exceeded@aljo242 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThis pull request focuses on removing invariant-related code from various Cosmos SDK modules. The changes involve deprecating and removing invariant types, interfaces, and methods across multiple packages. The primary objective is to eliminate unused invariant functionality, particularly after the removal of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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
|
github-actions
bot
added
C:x/staking
C:x/gov
C:x/distribution
github-prbot
assigned JulianToledano, julienrbrt, kocubinski, sontrinh16, testinginprod and ziscky
julienrbrt
added
the
backport/v0.52.x
|
Could you remove it as well from the |
![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)
types/module/module.go (1)
Invariant deprecation needs additional cleanup
While the code marks invariants as deprecated, there are still several active references and usages throughout the codebase that should be addressed:
types/invariant.gocontains the core invariant types and interfaces that are marked as deprecated but still in use- Several modules still contain invariant-related code:
- x/bank: Contains invariant checks in balance management
- x/staking: Has invariant-dependent logic in slash operations
- x/auth: Contains supply invariant references
- Server configuration still includes invariant check period flag (
FlagInvCheckPeriod)Since invariants are being deprecated, these remaining usages should be refactored to use alternative validation mechanisms or be removed if no longer needed.
🔗 Analysis chain
Line range hint
87-399: Verify complete removal of invariant usageLet's ensure no active usage of invariants remains in the codebase.
Let me gather more context to ensure we have a complete picture.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of invariants in the codebase # Search for any remaining invariant registrations echo "Checking for invariant registrations..." rg -l "RegisterInvariants|HasInvariants" --type go # Search for any remaining invariant implementations echo "Checking for invariant implementations..." ast-grep --pattern 'type $_ struct { $$$ } func ($x $_) RegisterInvariants($y $_) { $$$ }'Length of output: 355
Script:
#!/bin/bash # Search for any remaining invariant-related code or references echo "Checking for any invariant-related code..." rg -A 5 "invariant" --type go # Search for any remaining SDK invariant registry usage echo "Checking for InvariantRegistry usage..." rg -A 5 "InvariantRegistry" --type go # Check for any remaining test files that might be using invariants echo "Checking test files for invariant usage..." rg -l "invariant" --type go --glob "*_test.go"Length of output: 14319
🧹 Nitpick comments (2)
CHANGELOG.md (2)
67-68: Fix inconsistent bullet point formattingThe bullet points under "Deprecated" use inconsistent formatting - some use
*while others use-. Standardize on one format for consistency.
Line range hint
2392-2396: Consider removing redundant previous changelog linkThe link to previous changelog at the end appears redundant since this file already contains comprehensive history. Consider removing or updating the note.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
CHANGELOG.md(1 hunks)testutil/mock/types_invariant.go(0 hunks)testutil/mock/types_mock_appmodule.go(0 hunks)testutil/mock/types_module_module.go(0 hunks)types/invariant.go(1 hunks)types/module/mock_appmodule_test.go(1 hunks)types/module/module.go(2 hunks)types/module/module_test.go(0 hunks)x/bank/keeper/invariants.go(0 hunks)x/bank/module.go(0 hunks)x/distribution/keeper/invariants.go(0 hunks)x/distribution/module.go(0 hunks)x/gov/keeper/invariants.go(0 hunks)x/gov/module.go(0 hunks)x/group/keeper/invariants.go(0 hunks)x/group/keeper/invariants_test.go(0 hunks)x/group/module/module.go(0 hunks)x/staking/keeper/invariants.go(0 hunks)x/staking/module.go(0 hunks)
💤 Files with no reviewable changes (15)
- x/gov/keeper/invariants.go
- x/group/keeper/invariants_test.go
- x/distribution/module.go
- testutil/mock/types_mock_appmodule.go
- x/gov/module.go
- x/staking/module.go
- x/bank/keeper/invariants.go
- x/distribution/keeper/invariants.go
- x/bank/module.go
- testutil/mock/types_invariant.go
- types/module/module_test.go
- x/group/keeper/invariants.go
- x/staking/keeper/invariants.go
- x/group/module/module.go
- testutil/mock/types_module_module.go
✅ Files skipped from review due to trivial changes (1)
- types/invariant.go
🧰 Additional context used
📓 Path-based instructions (3)
types/module/module.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
types/module/mock_appmodule_test.go (2)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🔇 Additional comments (4)
types/module/mock_appmodule_test.go (1)
Line range hint 1-24: LGTM: Clean removal of invariant-related code
The removal of HasInvariants from the test interfaces is consistent with the deprecation of invariants across the codebase.
types/module/module.go (2)
Line range hint 87-92: LGTM: Clear deprecation notice for HasInvariants interface
The deprecation notice clearly communicates that invariants are no longer used from modules.
Line range hint 393-399: LGTM: Consistent deprecation notice for RegisterInvariants method
The deprecation notice for the method aligns with the interface deprecation.
CHANGELOG.md (1)
Line range hint 1-2396: Documentation structure and organization looks good
The CHANGELOG follows standard practices with:
- Clear version sections
- Categorized changes (Features, Improvements, Bug Fixes etc.)
- PR references
- Breaking changes clearly marked
Co-authored-by: Marko <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
Line range hint
1-1: Add missing title to changelogThe changelog should start with a clear title like "# Changelog" at the top of the file.
+ # Changelog
Line range hint
15-25: Consider adding version comparison tableFor better readability, consider adding a version comparison table at the top showing key differences between major versions.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)types/invariant.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- types/invariant.go
🧰 Additional context used
📓 Path-based instructions (1)
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🔇 Additional comments (1)
CHANGELOG.md (1)
Line range hint 1-1200: LGTM on overall changelog structure and content
The changelog follows good practices:
- Clear version sections with dates
- Categorized changes (Features, Improvements, Bug Fixes etc.)
- Links to PRs/issues
- Breaking changes clearly marked
- Detailed descriptions of changes
|
@aljo242 you need #22994 (review) otherwise it won't merged. |
Co-authored-by: Marko <[email protected]> (cherry picked from commit 71ddfbd) # Conflicts: # CHANGELOG.md # testutil/mock/types_mock_appmodule.go # x/distribution/keeper/invariants.go # x/group/go.mod
Description
Closes: #22993
InvariantandInvariantRegistrytypes and associated methods (can be removed in future)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
New Features
versioncommand to return v2 server information.Improvements
Bug Fixes
Deprecations
Invariant,Invariants, andInvariantRegistry.Tests