refactor!: remove x/params
#22995
refactor!: remove x/params
#22995
Conversation
aljo242
requested review from
auricom,
julienrbrt,
tac0turtle,
a team,
hieuvubk,
kocubinski,
facundomedica,
aaronc and
testinginprod
as code owners
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request involves the comprehensive removal of the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
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:CLI
C:x/params
Type: CI
C:server/v2
github-prbot
assigned aaronc, auricom, facundomedica, hieuvubk, julienrbrt, kocubinski, tac0turtle and testinginprod
There was a problem hiding this comment.
got more code to delete here. https://github.com/cosmos/cosmos-sdk/tree/main/api/cosmos/params. Its going to be a large pr of deletions
![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
🧹 Nitpick comments (5)
UPGRADING.md (2)
47-52: Ensure clear migration path for x/params removalThe migration instructions for x/params removal should be more detailed since this is a critical breaking change. Consider adding:
- Step-by-step migration guide
- Example of how to migrate an existing module's parameters
- Link to a reference implementation
🧰 Tools
🪛 Markdownlint (0.37.0)
47-47: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
1286-1286: Improve clarity in migration instructionsReplace "Have a look at" with "Take a look at" for better clarity and consistency with American English.
- Have a look at `simapp.RegisterUpgradeHandlers()` for an example. + Take a look at `simapp.RegisterUpgradeHandlers()` for an example.🧰 Tools
🪛 LanguageTool
[locale-violation] ~1286-~1286: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look atsimapp.RegisterUpgradeHandlers()f...(HAVE_A_LOOK)
CHANGELOG.md (3)
Line range hint
1-1: Changelog follows standard format but could benefit from more consistent versioning schemeThe changelog follows the Keep a Changelog format which is good practice. However, consider adding a clear specification of the versioning scheme (SemVer) at the top of the file.
+ # Changelog + + All notable changes to this project will be documented in this file. + + The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), + and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
Line range hint
13-15: Improve release date formatting consistencyRelease dates should follow a consistent format. Consider using ISO 8601 (YYYY-MM-DD) for all dates.
Line range hint
17-25: Add deprecation notices sectionConsider adding a dedicated "Deprecated" section at the top level for each release to make it easier to track deprecated features.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
x/params/go.sumis excluded by!**/*.sumx/params/proto/buf.lockis excluded by!**/*.lockx/params/types/proposal/params.pb.gois excluded by!**/*.pb.gox/params/types/proposal/query.pb.gois excluded by!**/*.pb.gox/params/types/proposal/query.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (56)
.github/CODEOWNERS(0 hunks).github/dependabot.yml(0 hunks).github/pr_labeler.yml(0 hunks).github/workflows/test.yml(0 hunks)CHANGELOG.md(1 hunks)README.md(3 hunks)UPGRADING.md(2 hunks)baseapp/params_legacy.go(0 hunks)buf.work.yaml(0 hunks)go.work.example(0 hunks)server/v2/testdata/app.toml(0 hunks)simapp/go.mod(0 hunks)simapp/v2/go.mod(0 hunks)tests/go.mod(0 hunks)x/params/CHANGELOG.md(0 hunks)x/params/README.md(0 hunks)x/params/autocli.go(0 hunks)x/params/client/cli/tx.go(0 hunks)x/params/client/cli/tx_test.go(0 hunks)x/params/client/proposal_handler.go(0 hunks)x/params/client/utils/utils.go(0 hunks)x/params/client/utils/utils_test.go(0 hunks)x/params/depinject.go(0 hunks)x/params/doc.go(0 hunks)x/params/go.mod(0 hunks)x/params/keeper/common_test.go(0 hunks)x/params/keeper/grpc_query.go(0 hunks)x/params/keeper/grpc_query_test.go(0 hunks)x/params/keeper/keeper.go(0 hunks)x/params/keeper/keeper_test.go(0 hunks)x/params/module.go(0 hunks)x/params/proposal_handler.go(0 hunks)x/params/proto/buf.gen.gogo.yaml(0 hunks)x/params/proto/buf.gen.pulsar.yaml(0 hunks)x/params/proto/buf.yaml(0 hunks)x/params/proto/cosmos/params/module/v1/module.proto(0 hunks)x/params/proto/cosmos/params/v1beta1/params.proto(0 hunks)x/params/proto/cosmos/params/v1beta1/query.proto(0 hunks)x/params/sonar-project.properties(0 hunks)x/params/testutil/staking_keeper_mock.go(0 hunks)x/params/types/common_test.go(0 hunks)x/params/types/consensus_params_legacy.go(0 hunks)x/params/types/deref_test.go(0 hunks)x/params/types/doc.go(0 hunks)x/params/types/keys.go(0 hunks)x/params/types/paramset.go(0 hunks)x/params/types/proposal/codec.go(0 hunks)x/params/types/proposal/errors.go(0 hunks)x/params/types/proposal/keys.go(0 hunks)x/params/types/proposal/proposal.go(0 hunks)x/params/types/proposal/proposal_test.go(0 hunks)x/params/types/querier.go(0 hunks)x/params/types/subspace.go(0 hunks)x/params/types/subspace_test.go(0 hunks)x/params/types/table.go(0 hunks)x/params/types/table_test.go(0 hunks)
💤 Files with no reviewable changes (53)
- buf.work.yaml
- go.work.example
- server/v2/testdata/app.toml
- .github/pr_labeler.yml
- x/params/types/proposal/keys.go
- x/params/client/proposal_handler.go
- x/params/sonar-project.properties
- x/params/README.md
- x/params/CHANGELOG.md
- x/params/types/table_test.go
- x/params/types/keys.go
- x/params/proto/buf.gen.gogo.yaml
- x/params/proto/buf.yaml
- x/params/types/proposal/errors.go
- x/params/types/doc.go
- x/params/proto/cosmos/params/module/v1/module.proto
- x/params/types/proposal/codec.go
- x/params/proto/buf.gen.pulsar.yaml
- x/params/types/proposal/proposal_test.go
- x/params/proto/cosmos/params/v1beta1/params.proto
- .github/workflows/test.yml
- x/params/keeper/keeper_test.go
- .github/CODEOWNERS
- x/params/client/utils/utils_test.go
- x/params/types/consensus_params_legacy.go
- x/params/autocli.go
- x/params/keeper/common_test.go
- x/params/types/querier.go
- x/params/types/paramset.go
- x/params/client/cli/tx.go
- x/params/doc.go
- x/params/keeper/grpc_query_test.go
- x/params/types/deref_test.go
- x/params/client/cli/tx_test.go
- x/params/proposal_handler.go
- x/params/keeper/keeper.go
- .github/dependabot.yml
- simapp/v2/go.mod
- x/params/go.mod
- x/params/types/common_test.go
- x/params/keeper/grpc_query.go
- simapp/go.mod
- x/params/types/table.go
- tests/go.mod
- x/params/client/utils/utils.go
- x/params/depinject.go
- x/params/proto/cosmos/params/v1beta1/query.proto
- x/params/types/proposal/proposal.go
- x/params/testutil/staking_keeper_mock.go
- baseapp/params_legacy.go
- x/params/types/subspace_test.go
- x/params/module.go
- x/params/types/subspace.go
🧰 Additional context used
📓 Path-based instructions (3)
README.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
UPGRADING.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 Markdownlint (0.37.0)
UPGRADING.md
47-47: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
UPGRADING.md
[locale-violation] ~1286-~1286: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look at simapp.RegisterUpgradeHandlers() f...
(HAVE_A_LOOK)
🔇 Additional comments (2)
README.md (1)
109-109: Verify x/params module deprecation notice
The x/params module is listed as N/A for v2 and X for 0.50.z, but it's being completely removed in v0.50. Consider adding a note in the table or documentation to make this clearer to users.
CHANGELOG.md (1)
Line range hint 27-1000: LGTM on changelog content organization
The changelog entries are well organized into logical categories (Features, Improvements, Bug Fixes, etc.) with clear references to PRs and detailed descriptions. This makes it easy for users to understand the changes and their impact.
README.md
Outdated
| @@ -106,6 +106,7 @@ Module Dependencies are the modules that an application may depend on and which | |||
| | cosmossdk.io/x/slashing | 0.2.z | 0.2.z | X | | |||
| | cosmossdk.io/x/staking | 0.2.z | 0.2.z | X | | |||
| | cosmossdk.io/x/upgrade | 0.2.z | 0.2.z | 0.1.z | | |||
| | cosmossdk.io/x/params | N/A | 0.2.z | X | | |||
There was a problem hiding this comment.
nit, let's keep the alphabetical order and let's pu it before protocolpool
We shouldn't do that, we cannot have any breaking changes in cosmossdk.io/api for the whole duration of Olympus LTS. |
|
I have just disabled the required job |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
UPGRADING.md (1)
47-52: Consider adding a code example for x/params migrationWhile the documentation clearly explains the deprecation of x/params module, it would be helpful to include a concrete code example showing how to migrate from x/params to module-managed parameters. This would make it easier for developers to understand the migration process.
Consider adding an example like this:
// Before: Using x/params type Params struct { MaxValidators uint32 } // After: Module-managed params type Keeper struct { paramStore paramtypes.Subspace } // Migrate to type Keeper struct { storeKey storetypes.StoreKey params Params } func (k Keeper) GetParams(ctx sdk.Context) Params { return k.params } func (k Keeper) SetParams(ctx sdk.Context, params Params) error { // Add validation if needed k.params = params return nil }🧰 Tools
🪛 Markdownlint (0.37.0)
47-47: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
CHANGELOG.md (3)
Line range hint
1-1: Add title heading to CHANGELOGThe file should start with a top-level heading # CHANGELOG for better documentation structure.
+ # CHANGELOG
62-65: Fix inconsistent heading levelsThe heading levels are inconsistent in this section. "Client Breaking Changes" and "API Breaking Changes" should be at the same level as "Features" since they are major categories.
- ### Client Breaking Changes - ### API Breaking Changes + ## Client Breaking Changes + ## API Breaking Changes
Line range hint
1-2000: Fix formatting inconsistenciesThere are some formatting inconsistencies throughout the document:
- Some bullet points use
*while others use-- Inconsistent spacing after bullet points
- Mixed usage of backticks for code references
Standardize on:
- Using
-for all bullet points- Single space after bullet points
- Backticks for all code references
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
x/params/go.sumis excluded by!**/*.sumx/params/proto/buf.lockis excluded by!**/*.lockx/params/types/proposal/params.pb.gois excluded by!**/*.pb.gox/params/types/proposal/query.pb.gois excluded by!**/*.pb.gox/params/types/proposal/query.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (56)
.github/CODEOWNERS(0 hunks).github/dependabot.yml(0 hunks).github/pr_labeler.yml(0 hunks).github/workflows/test.yml(0 hunks)CHANGELOG.md(1 hunks)README.md(3 hunks)UPGRADING.md(2 hunks)baseapp/params_legacy.go(0 hunks)buf.work.yaml(0 hunks)go.work.example(0 hunks)server/v2/testdata/app.toml(0 hunks)simapp/go.mod(0 hunks)simapp/v2/go.mod(0 hunks)tests/go.mod(0 hunks)x/params/CHANGELOG.md(0 hunks)x/params/README.md(0 hunks)x/params/autocli.go(0 hunks)x/params/client/cli/tx.go(0 hunks)x/params/client/cli/tx_test.go(0 hunks)x/params/client/proposal_handler.go(0 hunks)x/params/client/utils/utils.go(0 hunks)x/params/client/utils/utils_test.go(0 hunks)x/params/depinject.go(0 hunks)x/params/doc.go(0 hunks)x/params/go.mod(0 hunks)x/params/keeper/common_test.go(0 hunks)x/params/keeper/grpc_query.go(0 hunks)x/params/keeper/grpc_query_test.go(0 hunks)x/params/keeper/keeper.go(0 hunks)x/params/keeper/keeper_test.go(0 hunks)x/params/module.go(0 hunks)x/params/proposal_handler.go(0 hunks)x/params/proto/buf.gen.gogo.yaml(0 hunks)x/params/proto/buf.gen.pulsar.yaml(0 hunks)x/params/proto/buf.yaml(0 hunks)x/params/proto/cosmos/params/module/v1/module.proto(0 hunks)x/params/proto/cosmos/params/v1beta1/params.proto(0 hunks)x/params/proto/cosmos/params/v1beta1/query.proto(0 hunks)x/params/sonar-project.properties(0 hunks)x/params/testutil/staking_keeper_mock.go(0 hunks)x/params/types/common_test.go(0 hunks)x/params/types/consensus_params_legacy.go(0 hunks)x/params/types/deref_test.go(0 hunks)x/params/types/doc.go(0 hunks)x/params/types/keys.go(0 hunks)x/params/types/paramset.go(0 hunks)x/params/types/proposal/codec.go(0 hunks)x/params/types/proposal/errors.go(0 hunks)x/params/types/proposal/keys.go(0 hunks)x/params/types/proposal/proposal.go(0 hunks)x/params/types/proposal/proposal_test.go(0 hunks)x/params/types/querier.go(0 hunks)x/params/types/subspace.go(0 hunks)x/params/types/subspace_test.go(0 hunks)x/params/types/table.go(0 hunks)x/params/types/table_test.go(0 hunks)
💤 Files with no reviewable changes (53)
- buf.work.yaml
- go.work.example
- .github/pr_labeler.yml
- server/v2/testdata/app.toml
- x/params/types/keys.go
- x/params/proto/buf.yaml
- x/params/proto/buf.gen.pulsar.yaml
- x/params/client/proposal_handler.go
- x/params/proto/buf.gen.gogo.yaml
- x/params/types/consensus_params_legacy.go
- x/params/types/doc.go
- .github/CODEOWNERS
- .github/workflows/test.yml
- x/params/types/table_test.go
- x/params/proto/cosmos/params/v1beta1/params.proto
- x/params/types/proposal/codec.go
- x/params/types/proposal/keys.go
- x/params/client/utils/utils_test.go
- x/params/README.md
- x/params/go.mod
- x/params/types/paramset.go
- x/params/types/deref_test.go
- x/params/sonar-project.properties
- x/params/proto/cosmos/params/module/v1/module.proto
- x/params/CHANGELOG.md
- simapp/v2/go.mod
- x/params/keeper/common_test.go
- x/params/types/proposal/proposal_test.go
- x/params/client/cli/tx.go
- x/params/types/proposal/errors.go
- x/params/keeper/grpc_query_test.go
- x/params/proposal_handler.go
- x/params/autocli.go
- x/params/depinject.go
- x/params/keeper/keeper_test.go
- x/params/types/querier.go
- x/params/types/subspace_test.go
- .github/dependabot.yml
- x/params/types/table.go
- x/params/keeper/grpc_query.go
- x/params/testutil/staking_keeper_mock.go
- x/params/doc.go
- x/params/client/cli/tx_test.go
- x/params/keeper/keeper.go
- x/params/proto/cosmos/params/v1beta1/query.proto
- baseapp/params_legacy.go
- x/params/types/common_test.go
- x/params/client/utils/utils.go
- x/params/module.go
- x/params/types/proposal/proposal.go
- tests/go.mod
- simapp/go.mod
- x/params/types/subspace.go
🧰 Additional context used
📓 Path-based instructions (3)
README.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
UPGRADING.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 Markdownlint (0.37.0)
UPGRADING.md
47-47: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
UPGRADING.md
[locale-violation] ~1286-~1286: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look at simapp.RegisterUpgradeHandlers() f...
(HAVE_A_LOOK)
🔇 Additional comments (2)
README.md (1)
75-75: LGTM! Version matrix and module dependencies are well documented
The changes accurately reflect:
- Core dependencies compatibility across different versions
- Module dependencies with their respective versions, including the new state of x/params module
Also applies to: 92-109
CHANGELOG.md (1)
Line range hint 1-2000: Verify all PR/Issue links are valid
The changelog contains numerous links to PRs and issues. We should verify these are all valid and accessible.
✅ Verification successful
Based on the output, I can see that most PR/issue links return 200 status codes, indicating they are accessible. However, there are a few 301/302 redirects and some failed requests. Let me verify those specific cases.
All PR and issue links in the changelog are valid and accessible
After checking all URLs in the changelog, including those that initially showed redirects, all links are valid and accessible. The redirects (301/302) we saw in the first check were properly followed to their destinations, resulting in successful 200 status codes. This includes both pull requests and issues referenced throughout the changelog.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Extract and verify all PR/issue links
grep -o 'https://github.com/cosmos/cosmos-sdk/\(pull\|issues\)/[0-9]*' CHANGELOG.md | while read -r url; do
curl -s -o /dev/null -w "%{http_code}" "$url"
done
Length of output: 70701
Script:
#!/bin/bash
# Check only the URLs that returned non-200 status codes
urls=(
"https://github.com/cosmos/cosmos-sdk/issues/17216"
"https://github.com/cosmos/cosmos-sdk/pull/13834"
"https://github.com/cosmos/cosmos-sdk/pull/"
"https://github.com/cosmos/cosmos-sdk/pull/15023"
"https://github.com/cosmos/cosmos-sdk/issues/14019"
"https://github.com/cosmos/cosmos-sdk/pull/3129"
"https://github.com/cosmos/cosmos-sdk/issues/15962"
"https://github.com/cosmos/cosmos-sdk/issues/15948"
"https://github.com/cosmos/cosmos-sdk/issues/15891"
"https://github.com/cosmos/cosmos-sdk/issues/15818"
)
for url in "${urls[@]}"; do
echo "Checking $url"
status=$(curl -sL -w "%{http_code}" "$url" -o /dev/null)
echo "Status: $status"
done
Length of output: 3392
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
UPGRADING.md (1)
1288-1288: Consider using "take a look" instead of "have a look"For consistency with American English conventions, consider rephrasing to "take a look at".
-Have a look at `simapp.RegisterUpgradeHandlers()` for an example. +Take a look at `simapp.RegisterUpgradeHandlers()` for an example.🧰 Tools
🪛 LanguageTool
[locale-violation] ~1288-~1288: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look atsimapp.RegisterUpgradeHandlers()f...(HAVE_A_LOOK)
CHANGELOG.md (2)
Line range hint
1-1: Add a title to the CHANGELOG fileThe file should start with a # CHANGELOG title for better documentation structure.
+ # CHANGELOG
54-56: Fix formatting inconsistency in breaking changes sectionThe breaking changes bullet points have inconsistent formatting - some use
_while others use-. Standardize on using-for consistency.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/test.yml(0 hunks)CHANGELOG.md(1 hunks)README.md(3 hunks)UPGRADING.md(2 hunks)go.work.example(0 hunks)simapp/v2/go.mod(0 hunks)tests/go.mod(0 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/test.yml
- simapp/v2/go.mod
- tests/go.mod
- go.work.example
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
📓 Path-based instructions (2)
UPGRADING.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
UPGRADING.md
[locale-violation] ~1288-~1288: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look at simapp.RegisterUpgradeHandlers() f...
(HAVE_A_LOOK)
🔇 Additional comments (1)
CHANGELOG.md (1)
Line range hint 1-1000: Overall changelog structure and content looks good
The changelog follows good practices:
- Clear version sections with dates
- Detailed categorization of changes
- Links to relevant PRs and issues
- Breaking changes clearly marked
- Migration notes included
Description
Finally remove the
x/paramsmoduleAuthor 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
Improvements
Bug Fixes
API Breaking Changes
x/paramsmodule, transitioning to a new parameters system.