Bump IAVL to v0.20.1 (from v0.19.6) (and the sdk to v0.46.13-pio-4 from v0.46.13-pio-3). #1874
Conversation
…to v6.2.0-pio-2 (from v6.2.0-pio-1). Bump wasmd to v0.30.0-pio-7 (from v0.30.0-pio-6).
WalkthroughThe update focuses on transitioning database management from Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sum
Files selected for processing (21)
- CHANGELOG.md (2 hunks)
- app/app.go (1 hunks)
- app/app_test.go (1 hunks)
- app/sim_test.go (1 hunks)
- app/store_loader.go (3 hunks)
- app/store_loader_test.go (11 hunks)
- app/test_helpers.go (1 hunks)
- cmd/dbmigrate/utils/badgerdb.go (1 hunks)
- cmd/dbmigrate/utils/boltdb.go (1 hunks)
- cmd/dbmigrate/utils/cleveldb.go (1 hunks)
- cmd/dbmigrate/utils/migrator.go (1 hunks)
- cmd/dbmigrate/utils/migrator_test.go (1 hunks)
- cmd/dbmigrate/utils/rocksdb.go (1 hunks)
- cmd/provenanced/cmd/root.go (1 hunks)
- cmd/provenanced/config/manager.go (1 hunks)
- internal/handlers/msg_service_router_test.go (1 hunks)
- testutil/network.go (1 hunks)
- x/exchange/keeper/orders.go (1 hunks)
- x/hold/keeper/grpc_query.go (1 hunks)
- x/ibchooks/ibc_middleware_test.go (1 hunks)
- x/ibcratelimit/module/ibc_middleware_test.go (1 hunks)
Additional comments: 42
cmd/dbmigrate/utils/boltdb.go (1)
- 7-7: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives.cmd/dbmigrate/utils/rocksdb.go (1)
- 7-7: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives.cmd/dbmigrate/utils/badgerdb.go (1)
- 7-7: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives.cmd/dbmigrate/utils/cleveldb.go (1)
- 7-7: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives.testutil/network.go (1)
- 8-8: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives. The static analysis hint flagging this import as not allowed seems to be a false positive in this context.app/store_loader.go (2)
- 12-12: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives.- 12-13: The removal of logic related to the
disable-iavl-fastnodeconfig value aligns with the PR's objectives to simplify configuration and potentially improve performance.x/hold/keeper/grpc_query.go (1)
- 10-10: The import path update from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis correctly implemented and aligns with the PR objectives.app/store_loader_test.go (10)
- 55-57: The adjustments to the
MockAppOptionsstruct and related test logic, including the removal of thefastNodefield, align with the PR's objectives to simplify configuration.- 84-86: The use of
MockAppOptionsin test functions correctly reflects the updated configuration options without thefastNodefield.- 129-129: The adjustments in test cases to reflect the removal of the
fastNodefield and focus on the updated configuration options are correctly implemented.- 138-138: The test case adjustments, including the removal of the
fastNodefield fromMockAppOptions, correctly align with the PR's objectives.- 200-202: The test configurations correctly reflect the updated database management strategy without the
fastNodefield.- 209-211: The test configurations, including the updated database management strategy, are correctly implemented in the test cases.
- 223-225: The test case adjustments to reflect the updated configuration options, including the database management strategy, are correctly implemented.
- 237-239: The test configurations, including the updated database management strategy without the
fastNodefield, are correctly reflected in the test cases.- 251-253: The comprehensive test case adjustments, including the updated database management strategy, are correctly implemented.
- 267-269: The test case adjustments, including the updated database management strategy without the
fastNodefield, are correctly implemented.cmd/provenanced/cmd/root.go (1)
- 17-18: The change from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis noted. Ensure thatcometbft-dbprovides all necessary functionalities and is compatible with the existing codebase. It's also important to verify that this change does not introduce any regressions or issues in database interactions.app/app_test.go (1)
- 20-21: The addition of
github.com/cometbft/cometbft-dband removal oftm-dbin the test file align with the project's database management package transition. Ensure that the new database package is fully compatible with the existing tests and does not affect the integrity of the test outcomes.app/sim_test.go (1)
- 18-21: The changes to the imports, including the addition of
cometbft-dband the reordering oficqtypes, align with the PR objectives and do not introduce any issues.app/test_helpers.go (1)
- 18-19: The change in the import of the
dbmpackage fromtm-dbtocometbft-dbaligns with the PR objectives to transition the database management package. This is a necessary update and appears to be correctly implemented.x/ibchooks/ibc_middleware_test.go (1)
- 23-24: The switch from
tm-dbtocometbft-dbis noted. Ensure that all instances and references to the database within this test suite and related components are updated to usecometbft-dbcorrectly. This change should also be reflected in any documentation or comments that mention the database dependency.cmd/provenanced/config/manager.go (1)
- 93-93: Changing the
IAVLDisableFastNodesetting fromfalsetotruealigns with the PR's objective to enhance performance by disabling the IAVL fast node feature. Ensure that this change is well-documented in the project's configuration documentation, highlighting the reasons and potential impacts of this setting on the application's behavior.x/ibcratelimit/module/ibc_middleware_test.go (2)
- 12-27: The import statements have been reordered, and new imports have been added while unused ones have been removed. This aligns with the PR's objective of updating dependencies and transitioning to a new database management package (
github.com/cometbft/cometbft-db). Ensure that all newly added imports are used within the test suite and that the removal of any imports does not affect the functionality of the tests.- 9-35: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-200]
Ensure that the test suite adequately covers the functionality and any new features introduced by the updated dependencies (
github.com/cosmos/cosmos-sdk,github.com/cosmos/ibc-go/v6,github.com/CosmWasm/wasmd) and the new database management package (github.com/cometbft/cometbft-db). Given the PR's focus on updating dependencies and transitioning to a new database package, it's crucial to verify that the tests reflect these changes accurately and that any new functionality is properly tested.x/exchange/keeper/orders.go (1)
- 8-8: The import
github.com/cometbft/cometbft-dbis introduced to replacegithub.com/tendermint/tm-db. Ensure that all instances wheretm-dbwas used are correctly updated to usecometbft-db. Additionally, verify thatcometbft-dbis compatible with the existing codebase and meets all required functionalities thattm-dbprovided.cmd/dbmigrate/utils/migrator_test.go (1)
- 10-10: Switching the import from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbaligns with the PR's objective of transitioning to a new database management package. Ensure that all references and usages of the database functionalities are compatible withcometbft-db.cmd/dbmigrate/utils/migrator.go (9)
- 16-16: The import of
github.com/cometbft/cometbft-dbastmdbreplaces the previousgithub.com/tendermint/tm-db. This change aligns with the PR's objective to transition to a new database management package. Ensure that all references and usages of the database functionalities are compatible with thecometbft-dbpackage's API.- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [23-26]
Constants
BytesPerMBandunknownDBBackendare correctly defined and serve their intended purposes within the migration logic.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [30-32]
The global variable
PossibleDBTypesis correctly initialized and used to maintain a list of possible database backend types. This approach facilitates easy addition of new types and checking for supported types.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [58-156]
The type definitions for
MigratorandmigrationManagerare well-designed, encapsulating all necessary fields and logic for database migration. The struct fields are appropriately named and typed, and comments provide clear explanations of their purposes.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [158-161]
The
Initializefunction correctly prepares theMigratorby applying defaults, validating the configuration, and reading the source data directory. This structured approach ensures that the migrator is ready for operation before starting the migration process.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [163-214]
The
ApplyDefaultsfunction intelligently fills in default values for various fields of theMigratorstruct based on the providedHomePath,StagingDir, and other fields. This function enhances usability by reducing the amount of configuration required from the user.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [216-250]
The
ValidateBasicfunction thoroughly checks theMigratorconfiguration for completeness and correctness. This validation step is crucial for preventing runtime errors due to misconfiguration. The detailed error messages will aid users in correcting their configurations.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [252-273]
The
ReadSourceDataDirfunction correctly identifies database directories and non-database entries within the source data directory. This functionality is essential for determining which directories need migration and which ones should simply be copied.
- 13-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [275-1019]
The
Migratefunction, along with its helper functions and methods onmigrationManager, implements the core logic of the database migration process. The function is well-structured, with clear separation of concerns and detailed logging. However, it's important to ensure that the migration logic is thoroughly tested, especially given the complexity and the potential impact of errors on data integrity.app/app.go (1)
- 21-21: The change in the import statement from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis noted. Ensure that the newcometbft-dbpackage is fully compatible with the existing database management code and that all necessary updates have been made throughout the application to accommodate this change.CHANGELOG.md (3)
- 49-50: The removal of the startup warning when
disable-iavl-fastnodeis set totrueis clearly documented. This change aligns with the PR objectives to enhance performance and operational efficiency.- 50-50: Switching to
github.com/cometbft/cometbft-dbfromgithub.com/tendermint/tm-dbis a significant change reflecting a strategic decision in database management technology. This change is well-documented and aligns with the PR's objectives to leverage improvements offered bycometbft-db.- 65-68: The updates to dependencies are clearly documented, indicating a comprehensive effort to stay aligned with the latest developments in the ecosystem. This includes updates to
github.com/cosmos/cosmos-sdk,github.com/cosmos/ibc-go/v6,github.com/CosmWasm/wasmd, andgithub.com/cosmos/iavl, which are crucial for maintaining compatibility and taking advantage of new features and optimizations.internal/handlers/msg_service_router_test.go (1)
- 15-16: The import change from
github.com/tendermint/tm-dbtogithub.com/cometbft/cometbft-dbis noted. Ensure that all references and usages of the database functionality in the tests are compatible with the newcometbft-dbpackage.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- .golangci.yml (1 hunks)
- app/app.go (2 hunks)
- app/store_loader.go (3 hunks)
- app/test_helpers.go (2 hunks)
- cmd/dbmigrate/utils/migrator.go (1 hunks)
- cmd/provenanced/cmd/root.go (2 hunks)
- x/hold/keeper/grpc_query.go (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- app/app.go
- app/store_loader.go
- app/test_helpers.go
- cmd/dbmigrate/utils/migrator.go
- cmd/provenanced/cmd/root.go
- x/hold/keeper/grpc_query.go
Additional comments: 1
.golangci.yml (1)
- 123-124: The addition of
github.com/cometbft/cometbft-dbto the allowed imports list underdepguardsettings aligns with the PR's objective of transitioning tocometbft-dbfor database management. This is a positive change that supports the architectural shift in the project.However, it's important to verify that this transition does not inadvertently allow unwanted imports elsewhere in the project.
Verification successful
The search results confirm that the transition to
cometbft-dbfor database management is comprehensive, affecting various parts of the project including tests, command-line utilities, and application logic. The addition ofgithub.com/cometbft/cometbft-dbto the allowed imports list in.golangci.ymlis appropriate and aligns with the project's objectives. There's no indication that these imports are unexpected or unwanted, suggesting that the transition has been implemented as intended.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any unexpected imports of `cometbft-db` outside of the intended contexts. rg --type go 'github.com/cometbft/cometbft-db'Length of output: 1257
…om v0.46.13-pio-3). (#1874) * Switch to github.com/cometbft/cometbft-db (from github.com/tendermint/tm-db). * Remove disable-iavl-fastnode warning. * Bump cosmos-sdk to v0.46.13-pio-4 (from v0.46.13-pio-3). Bump ibc-go to v6.2.0-pio-2 (from v6.2.0-pio-1). Bump wasmd to v0.30.0-pio-7 (from v0.30.0-pio-6). * Add changelog entries. * Add a couple more changelog entries. * Add cometbft-db to the list of allowed imports. * Lint fix: Fix some import orderings. # Conflicts: # CHANGELOG.md
…om v0.46.13-pio-3). (#1874) (#1878) * Switch to github.com/cometbft/cometbft-db (from github.com/tendermint/tm-db). * Remove disable-iavl-fastnode warning. * Bump cosmos-sdk to v0.46.13-pio-4 (from v0.46.13-pio-3). Bump ibc-go to v6.2.0-pio-2 (from v6.2.0-pio-1). Bump wasmd to v0.30.0-pio-7 (from v0.30.0-pio-6). * Add changelog entries. * Add a couple more changelog entries. * Add cometbft-db to the list of allowed imports. * Lint fix: Fix some import orderings. # Conflicts: # CHANGELOG.md
Description
This PR bumps IAVL to
v0.20.1(fromv0.19.6). In order to do that, several other libraries needed to be bumped too:github.com/cosmos/cosmos-sdktov0.46.13-pio-4(fromv0.46.13-pio-3)github.com/cosmos/ibc-go/v6tov6.2.0-pio-2(fromv6.2.0-pio-1)github.com/CosmWasm/wasmdtov0.30.0-pio-7(fromv0.30.0-pio-6)It also required switching to
github.com/cometbft/cometbft-db(fromgithub.com/tendermint/tm-db).This PR also removes the startup warning when the disable-iavl-fastnode config value is
true, and changes the default value back totrue. We recommend that folks continue to usetruefor that setting.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passesSummary by CodeRabbit