feat(store): migrate db backends to store/db and remove the cosmos-db deps completely
#21765
Conversation
cool-develope
requested review from
a team,
kocubinski,
aaronc,
testinginprod,
lucaslopezf,
julienrbrt and
JulianToledano
as code owners
github-actions
bot
added
C:x/staking
C:CLI
C:x/gov
C:x/distribution
|
@cool-develope your pull request is missing a changelog! |
WalkthroughWalkthroughThe changes encompass a transition from the Changes
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/consensus
C:collections
C:x/circuit
C:server/v2
![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 and nitpick comments (3)
runtime/v2/go.mod (1)
Line range hint
1-89: Consider running a dependency check and updating outdated dependencies.While the go.mod file uses specific versions or commit hashes for dependencies, which is good for reproducibility, it's important to regularly update dependencies to include security patches and bug fixes. Some dependencies might be outdated or have known vulnerabilities.
Consider running a dependency check tool like
go list -u -m allor using a service like Dependabot to identify and update outdated dependencies. Pay special attention to dependencies with known security vulnerabilities.You can use the following command to check for outdated dependencies:
#!/bin/bash # Description: Check for outdated dependencies and potential vulnerabilities # Test: List outdated dependencies go list -u -m all | grep '\[' # Test: Check for known vulnerabilities (requires govulncheck tool) which govulncheck && govulncheck ./... || echo "govulncheck not found. Install with: go install golang.org/x/vuln/cmd/govulncheck@latest"Review the output and update dependencies as necessary, ensuring compatibility with your project.
x/accounts/defaults/lockup/go.mod (1)
Line range hint
1-165: Consider dependency management and module couplingWhile reviewing the entire
go.modfile, I noticed:
- This module has a large number of dependencies, which might indicate a complex module with many integrations.
- There's extensive use of replace directives, suggesting tight coupling with other parts of the Cosmos SDK.
Consider the following recommendations:
- Evaluate if all these dependencies are necessary. Could some be consolidated or removed to simplify the module?
- Assess the need for so many replace directives. While they're sometimes necessary for development, they can make versioning and releases more challenging.
- Consider creating a separate document or comments explaining the rationale behind each replace directive to aid future maintenance.
These suggestions aim to improve maintainability and reduce potential version conflicts in the long term.
x/epochs/go.mod (1)
Line range hint
1-224: Consider future cleanup of replace directives and TODO commentsWhile not directly related to the current changes, I noticed that the
go.modfile contains multiplereplacedirectives and a TODO comment. These are often used during development but can lead to dependency management issues if not properly maintained.Consider the following suggestions for future improvements:
- Review and potentially remove unnecessary
replacedirectives, especially those pointing to local paths.- Address the TODO comment: "TODO remove post spinning out all modules". This suggests that some cleanup or refactoring might be planned. It would be beneficial to create a tracking issue for this task if one doesn't already exist.
- Regularly review and update dependencies to their latest stable versions to ensure security and performance improvements.
These suggestions aim to improve the long-term maintainability and clarity of the module's dependency management.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (33)
client/v2/go.sumis excluded by!**/*.sumcollections/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumorm/go.sumis excluded by!**/*.sumruntime/v2/go.sumis excluded by!**/*.sumserver/v2/cometbft/go.sumis excluded by!**/*.sumserver/v2/go.sumis excluded by!**/*.sumsimapp/go.sumis excluded by!**/*.sumsimapp/v2/go.sumis excluded by!**/*.sumstore/go.sumis excluded by!**/*.sumstore/v2/go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sumx/accounts/defaults/base/go.sumis excluded by!**/*.sumx/accounts/defaults/lockup/go.sumis excluded by!**/*.sumx/accounts/defaults/multisig/go.sumis excluded by!**/*.sumx/accounts/go.sumis excluded by!**/*.sumx/authz/go.sumis excluded by!**/*.sumx/bank/go.sumis excluded by!**/*.sumx/circuit/go.sumis excluded by!**/*.sumx/consensus/go.sumis excluded by!**/*.sumx/distribution/go.sumis excluded by!**/*.sumx/epochs/go.sumis excluded by!**/*.sumx/evidence/go.sumis excluded by!**/*.sumx/feegrant/go.sumis excluded by!**/*.sumx/gov/go.sumis excluded by!**/*.sumx/group/go.sumis excluded by!**/*.sumx/mint/go.sumis excluded by!**/*.sumx/nft/go.sumis excluded by!**/*.sumx/params/go.sumis excluded by!**/*.sumx/protocolpool/go.sumis excluded by!**/*.sumx/slashing/go.sumis excluded by!**/*.sumx/staking/go.sumis excluded by!**/*.sumx/upgrade/go.sumis excluded by!**/*.sum
Files selected for processing (33)
- client/v2/go.mod (1 hunks)
- collections/go.mod (1 hunks)
- go.mod (1 hunks)
- orm/go.mod (4 hunks)
- runtime/v2/go.mod (1 hunks)
- server/v2/cometbft/go.mod (1 hunks)
- server/v2/go.mod (1 hunks)
- simapp/go.mod (2 hunks)
- simapp/v2/go.mod (2 hunks)
- store/go.mod (2 hunks)
- store/v2/go.mod (1 hunks)
- tests/go.mod (2 hunks)
- x/accounts/defaults/base/go.mod (1 hunks)
- x/accounts/defaults/lockup/go.mod (1 hunks)
- x/accounts/defaults/multisig/go.mod (1 hunks)
- x/accounts/go.mod (2 hunks)
- x/authz/go.mod (1 hunks)
- x/bank/go.mod (1 hunks)
- x/circuit/go.mod (1 hunks)
- x/consensus/go.mod (1 hunks)
- x/distribution/go.mod (1 hunks)
- x/epochs/go.mod (1 hunks)
- x/evidence/go.mod (1 hunks)
- x/feegrant/go.mod (1 hunks)
- x/gov/go.mod (2 hunks)
- x/group/go.mod (1 hunks)
- x/mint/go.mod (2 hunks)
- x/nft/go.mod (1 hunks)
- x/params/go.mod (1 hunks)
- x/protocolpool/go.mod (1 hunks)
- x/slashing/go.mod (1 hunks)
- x/staking/go.mod (2 hunks)
- x/upgrade/go.mod (1 hunks)
Files skipped from review as they are similar to previous changes (20)
- go.mod
- orm/go.mod
- simapp/go.mod
- simapp/v2/go.mod
- store/go.mod
- tests/go.mod
- x/accounts/defaults/multisig/go.mod
- x/accounts/go.mod
- x/authz/go.mod
- x/bank/go.mod
- x/circuit/go.mod
- x/distribution/go.mod
- x/evidence/go.mod
- x/feegrant/go.mod
- x/gov/go.mod
- x/group/go.mod
- x/mint/go.mod
- x/protocolpool/go.mod
- x/staking/go.mod
- x/upgrade/go.mod
Additional comments not posted (21)
collections/go.mod (1)
7-7: Approve version update and verify compatibilityThe update of
cosmossdk.io/core/testingto a specific version (v0.0.0-20240923163230-04da382a9f29) is a good practice. It ensures reproducibility and consistency across different environments.Please verify that this new version is compatible with the current codebase and doesn't introduce any breaking changes. Run the test suite to ensure everything still works as expected.
Additionally, the AI summary mentions the removal of a
replacedirective for this module, but this change is not visible in the provided code. Could you clarify if this directive was indeed removed? If so, ensure that this doesn't negatively impact local development workflows.To verify the module's dependencies and any
replacedirectives, you can run:store/v2/go.mod (1)
7-7: LGTM! Verify removal ofreplacedirective.The update of
cosmossdk.io/core/testingto a specific commit version (v0.0.0-20240923163230-04da382a9f29) is a positive change, moving from a placeholder version to a concrete one. This aligns with the PR objective of migrating and updating dependencies.Could you please confirm if the
replacedirective forcosmossdk.io/core/testinghas been removed as mentioned in the AI summary? If so, this change is not visible in the provided code snippet. You can use the following command to check:If the
replacedirective has indeed been removed, this further supports the migration away from local development dependencies, which is in line with the PR objectives.Verification successful
Verified removal of
replacedirective.The
replace cosmossdk.io/core/testingdirective has been successfully removed instore/v2/go.mod, aligning with the migration and dependency update objectives.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for the presence of a replace directive for cosmossdk.io/core/testing grep -n "replace cosmossdk.io/core/testing" store/v2/go.modLength of output: 61
runtime/v2/go.mod (2)
Line range hint
3-11: Verify the impact of removing the replace directive forcosmossdk.io/core/testing.The removal of the replace directive for
cosmossdk.io/core/testing(as mentioned in the AI summary) means that the module will now use the version specified in the require block instead of a local path. This change could affect the development workflow, especially if developers were relying on local changes in thecore/testingmodule.Please ensure that:
- The specified version in the require block contains all necessary changes that were previously accessed through the local path.
- All developers are aware of this change and update their workflows accordingly.
- Any CI/CD pipelines that might have depended on the local path are updated.
You can run the following command to check for any remaining usage of the local path:
#!/bin/bash # Description: Check for any remaining usage of the local path for cosmossdk.io/core/testing # Test: Search for any remaining references to the local path rg --type go "cosmossdk.io/core/testing" | grep -v "v0.0.0-20240923163230-04da382a9f29"If this command returns any results, those files might need to be updated to work with the new setup.
32-32: LGTM! Verify the compatibility of the new version.The update of
cosmossdk.io/core/testingto a specific pseudo-versionv0.0.0-20240923163230-04da382a9f29is a good practice. However, please ensure that this version is compatible with the rest of the project and that it's the intended version for your current development stage.To verify the compatibility and intention of this version update, you can run:
This script will help you confirm if the version is up-to-date and review the changes introduced in this version.
server/v2/go.mod (1)
17-17: LGTM! Version update and removal of local replace directive.The update to
cosmossdk.io/core/testingand the removal of its local replace directive are good changes. This suggests moving from a local development version to an official release, which is generally a positive step.To ensure compatibility, please run the following verification script:
This script will verify that the new version can be downloaded, the module can be built, and any tests using the
core/testingpackage still pass.x/accounts/defaults/lockup/go.mod (1)
20-20: Dependency update and replace directive removalThe update to
cosmossdk.io/core/testingand the removal of its replace directive suggest a shift towards using the official version instead of a local path. This change may have the following implications:
- It could potentially affect the testing utilities used in this module.
- It might be part of a larger effort to standardize dependency management across the Cosmos SDK.
To ensure this change doesn't introduce any breaking changes or inconsistencies, please run the following script:
If any issues are found, please address them before merging this change.
x/params/go.mod (3)
Line range hint
1-1: Summary of changes in x/params/go.modThe modifications in this file represent a shift in dependency management:
- The
cosmossdk.io/core/testingmodule has been updated to a specific timestamped version.- The local replace directive for this module has been removed.
These changes indicate a move towards using published versions of dependencies, which is generally a good practice for improving maintainability and reproducibility of the project. However, it's crucial to ensure that these changes don't introduce any compatibility issues or unintended side effects.
To ensure the changes don't negatively impact the project, please:
- Run all tests and CI checks.
- Verify that the build process completes successfully.
- Check for any warnings or errors related to module resolution.
If any issues arise, consider temporarily reverting to the local replace directive until the published version is fully vetted.
Line range hint
1-1: Verify compatibility after removing local replace directiveThe removal of the replace directive for
cosmossdk.io/core/testingindicates a shift from using a local development version to a published version. This change can improve reproducibility and version management. However, it's crucial to ensure that the published version (v0.0.0-20240923163230-04da382a9f29) contains all necessary changes and is fully compatible with the current codebase.Please run the following commands to verify that the change doesn't introduce any issues:
8-8: Approved: Updatedcosmossdk.io/core/testingto a specific versionThe change from a placeholder version to a specific timestamped version (
v0.0.0-20240923163230-04da382a9f29) indicates that the module is now referencing a concrete, externally hosted version rather than a local development version. This change is consistent with the removal of the corresponding replace directive.To ensure this change doesn't introduce any compatibility issues, please run the following command and verify that all tests pass:
x/consensus/go.mod (2)
9-9: Removal of replace directive forcosmossdk.io/core/testingThe replace directive for
cosmossdk.io/core/testinghas been removed. This indicates that the module is no longer using a local version of the package and is now relying on the version specified in therequiresection.To ensure this change doesn't introduce any issues, please run the following verification script:
#!/bin/bash # Description: Verify that removing the replace directive doesn't cause any issues. # Test: Check if there are any unresolved dependencies go mod verify # Test: Ensure that the module can be downloaded and used without the replace directive go mod download cosmossdk.io/core/[email protected]
9-9: Update to a specific version ofcosmossdk.io/core/testingThe dependency
cosmossdk.io/core/testinghas been updated from a placeholder version to a specific timestamped version. This change suggests a move from a local or development version to a more stable, potentially released version.To ensure this change doesn't introduce any breaking changes or unexpected behavior, please run the following verification script:
x/epochs/go.mod (1)
9-9: Update tocosmossdk.io/core/testingdependency and removal of local replacementThe dependency version for
cosmossdk.io/core/testinghas been updated from a placeholder version to a specific commit hash (v0.0.0-20240923163230-04da382a9f29). Additionally, the local path replacement for this dependency has been removed.This change suggests a shift from using a local development version to a more stable, versioned dependency. It's a positive step towards better dependency management and reproducibility.
To ensure this change doesn't introduce any breaking changes or compatibility issues, please run the following verification script:
This script will verify that the module still builds and passes tests with the updated dependency.
x/slashing/go.mod (1)
9-9: Update tocosmossdk.io/core/testingmodule versionThe version of
cosmossdk.io/core/testinghas been updated from a pseudo-version to a specific timestamped version (v0.0.0-20240923163230-04da382a9f29). This change, along with the removal of the replace directive, indicates a shift from using a local development version to a more stable or recent version of the testing module.This update aligns with the PR objectives of migrating database backends and removing dependencies, as it suggests a move towards using standardized versions across the project.
To ensure this change doesn't introduce any compatibility issues, please run the following command to check for any breaking changes or deprecations in the new version:
This script uses the GitHub CLI to fetch recent commit messages for the
core/testingdirectory and searches for keywords related to breaking changes or deprecations.x/nft/go.mod (3)
28-28: Summary of changes inx/nft/go.modThe changes in this file contribute to the PR objectives of migrating database backends and removing cosmos-db dependencies. The update to a specific version of
cosmossdk.io/core/testingand the removal of the replace directive indicate a move towards more standardized dependency management. These changes should improve maintainability and align with the overall goals of the pull request.
28-28: Removal of replace directive forcosmossdk.io/core/testingThe replace directive for
cosmossdk.io/core/testinghas been removed. This change indicates a shift from using a local development path to a published version of the package.To ensure that the package is correctly resolved without the replace directive, please run:
#!/bin/bash # Verify package resolution go list -m cosmossdk.io/core/testing
28-28: Version update forcosmossdk.io/core/testingThe dependency
cosmossdk.io/core/testinghas been updated from a placeholder version to a specific timestamped version. This change suggests a move towards more precise dependency management.To ensure compatibility with other dependencies, please run the following command:
Verification successful
Dependency Update Verified
The
cosmossdk.io/core/testingdependency has been successfully updated tov0.0.0-20240923163230-04da382a9f29, and the replace directive has been removed. No issues found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify compatibility of the updated dependency go mod tidy go mod verifyLength of output: 190
Script:
#!/bin/bash # Check the version of cosmossdk.io/core/testing in go.mod rg 'cosmossdk\.io/core/testing\s+v0\.0\.0-20240923163230-04da382a9f29' x/nft/go.mod # Ensure there are no replace directives for cosmossdk.io/core/testing in go.mod rg '^replace\s+cosmossdk\.io/core/testing' x/nft/go.modLength of output: 217
client/v2/go.mod (3)
Line range hint
1-189: Summary of changes in go.modThe changes in this file are minimal and focused on dependency management:
- A new indirect dependency on
cosmossdk.io/core/testinghas been added.- There's a potential removal of a local replacement for
cosmossdk.io/core/testing(needs clarification).These changes seem to align with the PR objectives of migrating database backends and removing
cosmos-dbdependencies, although the direct impact on these objectives is not immediately clear from this file alone. The addition of the new testing dependency might be related to updates in the testing infrastructure to support the migration process.To ensure these changes don't introduce any unintended consequences, please run:
#!/bin/bash # Verify that cosmos-db is not being used anymore rg --type go "github.com/cosmos/cosmos-db" . # Check for any new usages of the core/testing package rg --type go "cosmossdk.io/core/testing" .This will help verify that
cosmos-dbis indeed being removed and that the newcore/testingdependency is being used appropriately.
Line range hint
180-189: Clarification needed oncosmossdk.io/core/testingreplacementThe AI summary mentioned the removal of a local path replacement for
cosmossdk.io/core/testing, but this change is not visible in the current diff. Could you please clarify if this removal was done in a previous commit or if it's still pending?To check the current status of replacements, please run:
#!/bin/bash # Check for any remaining replacements related to core/testing grep -n "replace.*cosmossdk.io/core/testing" client/v2/go.modThis will help verify if there are any remaining replacements for
cosmossdk.io/core/testingin thego.modfile.
26-26: New indirect dependency addedA new indirect dependency on
cosmossdk.io/core/testinghas been added with a pre-release version. This might indicate ongoing development or testing.To ensure this change is intentional and consistent with the PR objectives, please run the following command:
This will help verify if the new dependency is actually being used in the codebase.
Verification successful
Dependency Usage Verified
The new indirect dependency on
cosmossdk.io/core/testingis actively used in multiple test files across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the new dependency is used in the codebase rg --type go "cosmossdk.io/core/testing" .Length of output: 8463
server/v2/cometbft/go.mod (1)
44-44: Dependency update looks good, verify compatibilityThe update of
cosmossdk.io/core/testingto a specific timestamped version (v0.0.0-20240923163230-04da382a9f29) is appropriate. This change aligns with the PR objectives and potentially introduces new features or bug fixes.To ensure compatibility, please run the following commands:
x/accounts/defaults/base/go.mod (1)
21-21: LGTM: Version update and removal of replace directiveThe update to
cosmossdk.io/core/testingand the removal of itsreplacedirective are positive changes. This suggests a move towards using an official release rather than a local development version, which aligns with the PR objectives.To ensure this change doesn't introduce any breaking changes, please run the following command:
This will help identify any potential areas of the codebase that might be affected by the version update.
Description
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
Release Notes
New Features
Improvements
Dependency Management