feat: export genesis in simapp v2 #21199
Conversation
WalkthroughWalkthroughThe recent updates to the Cosmos SDK significantly enhance module structure and functionality. Key improvements include the addition of new dependencies that strengthen server capabilities, alongside methods for retrieving the latest blockchain height and exporting genesis data. Existing methods have been refined for better error handling and state management, collectively advancing the server infrastructure toward a production-ready state, ensuring improved integration and stability across modules. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App[T]
participant DB as Database
participant Manager as ModuleManager
App->>DB: LoadLatestHeight()
DB-->>App: return latest height
App->>Manager: ExportGenesis(ctx, version)
Manager->>App: Retrieve genesis data
App-->>Manager: Exported data
Manager-->>App: return exported genesis data
sequenceDiagram
participant AppManager as AppManager[T]
participant DB as Database
AppManager->>DB: StateAt(version)
DB-->>AppManager: return current state
AppManager->>AppManager: RunWithCtx(callback)
AppManager->>AppManager: Check if exportGenesis is set
AppManager-->>AppManager: return error if not set
AppManager->>DB: Export genesis state
DB-->>AppManager: return exported state
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 Configuration File (
|
github-actions
bot
added
C:x/staking
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, codebase verification and nitpick comments (1)
runtime/v2/builder.go (1)
178-190: Ensure robust error handling and consider logging.The
ExportGenesismethod handles errors appropriately, but consider adding logging for better traceability.+ import "log" ... if err != nil { + log.Printf("Error exporting genesis: %v", err) return nil, fmt.Errorf("failed to export genesis: %w", err) } ... if err != nil { + log.Printf("Error marshaling genesis: %v", err) return nil, fmt.Errorf("failed to marshal genesis: %w", err) }
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumruntime/v2/go.sumis excluded by!**/*.sum
Files selected for processing (12)
- go.mod (1 hunks)
- runtime/v2/app.go (1 hunks)
- runtime/v2/builder.go (1 hunks)
- runtime/v2/go.mod (2 hunks)
- runtime/v2/manager.go (1 hunks)
- server/v2/appmanager/appmanager.go (1 hunks)
- server/v2/cometbft/go.mod (1 hunks)
- simapp/v2/app_di.go (2 hunks)
- simapp/v2/app_test.go (1 hunks)
- simapp/v2/export.go (1 hunks)
- simapp/v2/go.mod (4 hunks)
- x/staking/genesis.go (2 hunks)
Additional context used
Path-based instructions (8)
simapp/v2/export.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/app_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"runtime/v2/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/appmanager/appmanager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/app_di.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/builder.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/manager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (22)
simapp/v2/export.go (4)
10-15: Context creation and height retrieval logic.The context creation and height retrieval logic are appropriate. Ensure that
context.Background()is suitable for your use case and consider using a more specific context if needed.
16-23: Error handling for height retrieval.The error handling for height retrieval is correct.
24-27: Review the commented-out section for zero-height genesis preparation.The commented-out section for zero-height genesis preparation indicates potential future functionality. Ensure this is revisited and implemented if required.
34-39: Incomplete implementation for CometBFT consensus params.The TODO comment indicates that the CometBFT consensus params are not yet implemented. Ensure this is completed before the final release.
x/staking/genesis.go (2)
18-18: Context type change tocontext.Context.The change to use
context.Contextinstead ofsdk.Contextaligns with Go context package conventions and enhances interoperability.
18-18: Verify the internal logic ofWriteValidators.The internal logic of the function remains unchanged and appears correct.
simapp/v2/app_test.go (3)
24-68: Initialization logic forNewTestApp.The initialization logic for
NewTestAppappears correct and handles errors appropriately.
70-103: State advancement logic forMoveNextBlock.The state advancement logic for
MoveNextBlockappears correct and handles errors appropriately.
105-114: Test logic forTestSimAppExportAndBlockedAddrs.The test logic for
TestSimAppExportAndBlockedAddrsappears correct and handles errors appropriately.runtime/v2/app.go (1)
82-85: LGTM!The method
LoadLatestHeightis correctly implemented and adheres to best practices.server/v2/appmanager/appmanager.go (1)
94-111: LGTM!The updated
ExportGenesismethod includes improved error handling and control flow, making it more robust and reliable.simapp/v2/app_di.go (1)
94-94: Verify the reason for commenting out the line.The line that sets the configuration flag in the
viperinstance is commented out. Ensure this change is intentional and does not affect the initialization process.runtime/v2/go.mod (3)
29-29: Verify the necessity and compatibility of the updated dependency version.Ensure that the new version
v0.0.0-20240802110823-cffeedff643dofcosmossdk.io/server/v2/appmanageris compatible with the project and necessary for the new features.
32-32: Verify the necessity and compatibility of the updated dependency version.Ensure that the new version
v0.0.0-20240226161501-23359a0b6d91ofcosmossdk.io/x/bankis compatible with the project and necessary for the new features.
44-165: Verify the necessity and compatibility of the new indirect dependencies.Ensure that the newly added indirect dependencies are necessary for the project and do not introduce any conflicts.
server/v2/cometbft/go.mod (1)
29-29: Verify the necessity and compatibility of the updated dependency version.Ensure that the new version
v0.0.0-20240802110823-cffeedff643dofcosmossdk.io/server/v2/appmanageris compatible with the project and necessary for the new features.go.mod (1)
15-15: Dependency Addition Approved.The addition of the
cosmossdk.io/server/v2/appmanagerdependency with the specified version is appropriate.simapp/v2/go.mod (3)
15-15: Change from Indirect to Direct Dependency Approved.The change of
cosmossdk.io/store/v2from an indirect to a direct dependency is appropriate.
47-47: New Dependency Addition Approved.The addition of the
github.com/cometbft/cometbft/apidependency is appropriate and necessary for the project.
61-61: Dependency Version Update Approved.The version update of the
cosmossdk.io/server/v2/appmanagerdependency is appropriate and necessary.runtime/v2/manager.go (2)
224-225: Control Flow Improvement Approved.The addition of the
elseclause to handle cases where the module does not satisfy the conditions for eitherModuleIorappmodulev2.HasGenesisis appropriate and improves the control flow.
230-230: Logging Statement Addition Approved.The addition of the logging statement enhances the traceability of the export process.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
simapp/v2/export.go (1)
4-5: Add a blank line after import statements.To improve readability, add a blank line after the import statements.
import ( "context" cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" servertypes "github.com/cosmos/cosmos-sdk/server/types" )
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumruntime/v2/go.sumis excluded by!**/*.sum
Files selected for processing (12)
- go.mod (1 hunks)
- runtime/v2/app.go (1 hunks)
- runtime/v2/builder.go (1 hunks)
- runtime/v2/go.mod (2 hunks)
- runtime/v2/manager.go (1 hunks)
- server/v2/appmanager/appmanager.go (1 hunks)
- server/v2/cometbft/go.mod (1 hunks)
- simapp/v2/app_di.go (2 hunks)
- simapp/v2/app_test.go (1 hunks)
- simapp/v2/export.go (1 hunks)
- simapp/v2/go.mod (4 hunks)
- x/staking/genesis.go (2 hunks)
Additional context used
Path-based instructions (8)
simapp/v2/export.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/app_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"runtime/v2/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/appmanager/appmanager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/app_di.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/builder.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/manager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (24)
simapp/v2/export.go (3)
18-23: Proper error handling for loading the latest height.The error handling for loading the latest height is correctly implemented. Ensure that the error messages are logged for better debugging.
29-32: Proper error handling for exporting genesis.The error handling for exporting genesis is correctly implemented. Ensure that the error messages are logged for better debugging.
34-39: Handle consensus parameters TODO.The consensus parameters are currently set to an empty struct with a TODO comment. Ensure that the consensus parameters are correctly set before deploying to production.
x/staking/genesis.go (3)
4-4: Import context package for context management.The
contextpackage has been correctly imported to support the context management changes.
18-18: Update function signature for context management.The function
WriteValidatorshas been updated to usecontext.Contextinstead ofsdk.Context. This change aligns with Go's standard context management practices.
18-18: Ensure correct context usage in keeper methods.Ensure that all
keepermethods called withinWriteValidatorsare compatible withcontext.Contextand handle context appropriately.simapp/v2/app_test.go (4)
24-24: Initialize new test application.The function
NewTestAppinitializes a newSimAppinstance for testing. Ensure that all necessary components are correctly initialized.
70-70: Move to the next block in the test application.The function
MoveNextBlockadvances the blockchain state by one block for testing purposes. Ensure that all necessary state changes are correctly committed.
80-86: Remove TODO comment and handle context properly.The TODO comment indicates a hack to set the comet info in the context. Ensure that this is properly handled before deploying to production.
105-114: Test exporting app state and blocked addresses.The function
TestSimAppExportAndBlockedAddrstests the export of the app state and blocked addresses. Ensure that the test covers all necessary scenarios and edge cases.runtime/v2/app.go (1)
82-85: LGTM! But verify the method usage in the codebase.The code changes are approved.
However, ensure that all method calls to
LoadLatestHeightare correctly implemented.Verification successful
LGTM! The method
LoadLatestHeightis correctly implemented and its usage in the codebase is appropriate.
simapp/v2/export.go: The method is used with proper error handling.simapp/v2/app_test.go: The method is used with proper error handling in the test case.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `LoadLatestHeight` are correctly implemented. # Test: Search for the method usage. Expect: Correct implementation of the method calls. rg --type go -A 5 $'LoadLatestHeight'Length of output: 1038
Script:
#!/bin/bash # Extracting more context from simapp/v2/export.go around the usage of LoadLatestHeight rg --type go -A 10 -B 10 $'LoadLatestHeight' simapp/v2/export.go # Extracting more context from simapp/v2/app_test.go around the usage of LoadLatestHeight rg --type go -A 10 -B 10 $'LoadLatestHeight' simapp/v2/app_test.goLength of output: 1384
server/v2/appmanager/appmanager.go (1)
94-111: Enhanced error handling and state management.The changes improve the robustness of the
ExportGenesisfunction by adding better error handling and state management.However, ensure that all method calls to
ExportGenesisare correctly implemented and tested.Verification successful
Enhanced error handling and state management.
The changes improve the robustness of the
ExportGenesisfunction by adding better error handling and state management.The method calls to
ExportGenesisare correctly implemented and thoroughly tested across various modules and scenarios.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `ExportGenesis` are correctly implemented and tested. # Test: Search for the method usage. Expect: Correct implementation and testing of the method calls. rg --type go -A 5 $'ExportGenesis'Length of output: 76653
simapp/v2/app_di.go (1)
94-94: Verify the application's initialization process.Commenting out the line
viper.Set(serverv2.FlagHome, DefaultNodeHome)could affect the application's initialization, particularly regarding its home directory configuration. The TODO note suggests that this setting might need to be applied earlier.Ensure that the application's initialization process is not adversely affected by this change.
runtime/v2/builder.go (1)
178-190: LGTM! Ensure the method is used correctly in the codebase.The
ExportGenesismethod is well-implemented with proper error handling. The code changes are approved.However, ensure that the method is correctly integrated and used in the codebase.
runtime/v2/go.mod (2)
44-165: LGTM! Ensure the new dependencies are necessary.The addition of new indirect dependencies is approved. The changes expand the functionality of the module.
However, verify that the new dependencies are necessary for the project.
29-32: LGTM! Ensure the updated dependencies are necessary.The dependency updates to specific commit versions are approved. The changes enhance stability by replacing placeholders with defined versions.
However, verify that the updated dependencies are necessary for the project.
Verification successful
The updated dependencies are necessary and used throughout the codebase.
The dependencies
cosmossdk.io/server/v2/appmanager,cosmossdk.io/server/v2/stf,cosmossdk.io/store/v2, andcosmossdk.io/x/bankare all utilized in various files, confirming their necessity.
cosmossdk.io/server/v2/appmanageris used inserver/v2/types.go,server/v2/server_test.go,server/v2/cometbft/abci.go,runtime/v2/builder.go, andruntime/v2/app.go.cosmossdk.io/server/v2/stfis used inserver/v2/stf/stf.go,server/v2/stf/stf_test.go,server/v2/stf/core_branch_service_test.go,runtime/v2/builder.go,runtime/v2/manager.go,runtime/v2/app.go,runtime/v2/store.go, andruntime/v2/module.go.cosmossdk.io/store/v2is used in numerous files such asstore/v2/migration/manager.go,store/v2/store.go,store/v2/migration/manager_test.go,store/v2/database.go,store/v2/migration/stream.go,store/v2/storage/store.go,store/v2/snapshots/manager_test.go,store/v2/snapshots/chunk_test.go,store/v2/snapshots/store.go,store/v2/snapshots/helpers_test.go,store/v2/snapshots/store_test.go,store/v2/snapshots/snapshotter.go,store/v2/root/store_test.go,store/v2/pruning/manager_test.go,store/v2/storage/storage_test_suite.go,store/v2/root/store.go,store/v2/root/migrate_test.go,store/v2/snapshots/manager.go,store/v2/storage/sqlite/batch.go,store/v2/root/factory.go,store/v2/root/reader.go,store/v2/storage/sqlite/db_test.go,store/v2/storage/pebbledb/db.go,store/v2/storage/pebbledb/db_test.go,store/v2/storage/rocksdb/db_test.go,store/v2/storage/storage_bench_test.go,store/v2/snapshots/chunk.go,store/v2/storage/sqlite/db.go,store/v2/storage/rocksdb/batch.go,store/v2/proof/proof.go,store/v2/proof/commit_info.go,store/v2/storage/database.go,store/v2/db/goleveldb.go,store/v2/commitment/store.go,store/v2/storage/pebbledb/batch.go,store/v2/db/rocksdb_noflag.go,store/v2/storage/rocksdb/db.go,store/v2/db/pebbledb.go,store/v2/commitment/mem/tree.go,store/v2/commitment/tree.go,store/v2/db/db.go,store/v2/db/memdb.go,store/v2/commitment/metadata.go,store/v2/db/prefixdb.go,store/v2/db/rocksdb.go,store/v2/commitment/iavl/tree.go,store/v2/commitment/iavl/tree_test.go,store/v2/commitment/store_bench_test.go,store/v2/commitment/iavl/exporter.go,store/v2/commitment/iavl/importer.go,store/v2/commitment/store_test_suite.go,store/v2/pruning/manager.go,simapp/v2/app_test.go,server/v2/cometbft/server.go,server/v2/cometbft/snapshots.go,server/v2/cometbft/types/store.go,server/v2/cometbft/options.go,server/v2/store/commands.go,server/v2/store/config.go,server/v2/cometbft/abci.go,runtime/v2/builder.go,runtime/v2/store.go.cosmossdk.io/x/bankis used inx/group/simulation/genesis.go,x/group/simulation/genesis_test.go,x/group/testutil/expected_keepers_mocks.go,x/group/testutil/app_config.go,x/group/keeper/tally_test.go,x/group/keeper/msg_server_test.go,x/group/keeper/genesis_test.go,x/group/keeper/abci_test.go,x/group/genesis_test.go,x/group/testutil/expected_keepers.go,x/group/simulation/operations_test.go,x/group/keeper/keeper_test.go,x/group/client/cli/tx_test.go,x/gov/types/v1/msgs_test.go,x/gov/keeper/msg_server_test.go,x/gov/migrations/v5/store_test.go,x/gov/keeper/common_test.go,x/gov/client/cli/util_test.go,x/distribution/migrations/v4/migrate_funds_test.go,x/genutil/types/genesis_state_test.go,x/genutil/types/expected_keepers.go,x/genutil/testutil/expected_keepers_mocks.go,x/genutil/gentx_test.go,x/genutil/gentx.go,x/genutil/genaccounts.go,x/feegrant/filtered_fee_test.go,x/bank/simulation/operations.go,x/bank/types/send_authorization_test.go,x/bank/types/metadata_test.go,x/bank/types/restrictions_test.go,x/bank/types/balance_test.go,x/bank/types/balance.go,x/authz/generic_authorization_test.go,x/authz/testutil/bank_helpers.go,x/authz/simulation/operations.go,x/authz/simulation/genesis.go,x/authz/simulation/decoder_test.go,x/authz/msgs_test.go,x/authz/keeper/grpc_query_test.go,x/authz/migrations/v2/store_test.go,x/authz/simulation/genesis_test.go,x/authz/keeper/keys_test.go,x/authz/keeper/msg_server_test.go,x/authz/keeper/keeper_test.go,x/authz/module/abci_test.go,x/authz/module/abci_test.go,x/authz/module/autocli.go,x/accounts/defaults/multisig/utils_test.go,x/bank/simulation/genesis_test.go,x/accounts/defaults/lockup/utils_test.go,x/accounts/defaults/lockup/lockup.go,x/genutil/client/cli/gentx_test.go,x/accounts/cli/cli_test.go,x/authz/codec.go,x/genutil/client/cli/commands.go,x/authz/client/cli/tx_test.go,x/authz/client/cli/tx.go,x/bank/simulation/proposals.go,x/bank/depinject.go,x/bank/testutil/helpers.go,x/bank/client/cli/tx_test.go,x/bank/client/cli/tx.go,x/bank/module.go,x/bank/keeper/genesis.go,x/bank/keeper/msg_server.go,x/bank/keeper/keeper.go,x/bank/keeper/view.go,x/bank/keeper/keeper_test.go,x/bank/keeper/send.go,x/bank/keeper/invariants.go,x/bank/keeper/msg_server_test.go,x/bank/keeper/grpc_query.go,x/bank/keeper/genesis_test.go,x/bank/keeper/grpc_query_test.go,x/bank/keeper/collections_test.go,x/bank/keeper/export_test.go,testutil/network/network.go,testutil/sims/state_helpers.go,testutil/sims/app_helpers.go,tests/sims/slashing/app_config.go,tests/sims/slashing/operations_test.go,tests/sims/gov/operations_test.go,testutil/network/util.go,tests/sims/nft/operations_test.go,tests/sims/protocolpool/app_config.go,tests/sims/protocolpool/operations_test.go,tests/sims/distribution/app_config.go,tests/sims/distribution/operations_test.go,tests/sims/bank/operations_test.go,tests/sims/feegrant/operations_test.go,tests/sims/authz/operations_test.go,tests/e2e/bank/grpc.go,tests/e2e/tx/service_test.go,tests/sims/nft/app_config.go,tests/e2e/staking/suite.go,tests/integration/gov/genesis_test.go,tests/integration/bank/bench_test.go,tests/integration/bank/app_test.go,tests/integration/gov/keeper/keeper_test.go,tests/e2e/tx/benchmarks_test.go,tests/integration/gov/common_test.go,tests/integration/distribution/appconfig.go,tests/integration/distribution/keeper/msg_server_test.go,tests/e2e/bank/suite.go,tests/integration/bank/keeper/deterministic_test.go,tests/e2e/distribution/withdraw_all_suite.go,tests/integration/gov/abci_test.go,tests/integration/types/pagination_test.go,tests/integration/types/filtered_pagination_test.go,tests/e2e/baseapp/utils.go,tests/integration/staking/app_config.go,tests/e2e/authz/grpc.go,tests/integration/tx/decode_test.go,tests/integration/types/fuzz_test.go,tests/integration/staking/simulation/operations_test.go,tests/e2e/group/suite.go,tests/e2e/authz/tx.go,tests/integration/staking/keeper/delegation_test.go,tests/integration/staking/keeper/unbonding_test.go,tests/integration/staking/keeper/validator_bench_test.go,tests/integration/staking/keeper/slash_test.go,tests/integration/staking/keeper/msg_server_test.go,tests/integration/tx/aminojson/aminojson_test.go,tests/integration/staking/keeper/common_test.go,tests/integration/staking/keeper/genesis_test.go,tests/integration/staking/keeper/validator_test.go,tests/e2e/accounts/wiring_test.go,tests/e2e/auth/suite.go,tests/integration/slashing/slashing_test.go,tests/integration/server/grpc/out_of_gas_test.go,tests/integration/staking/keeper/deterministic_test.go,tests/integration/slashing/abci_test.go,tests/integration/slashing/keeper/slash_redelegation_test.go,tests/e2e/accounts/multisig/test_suite.go,tests/integration/server/grpc/server_test.go,tests/e2e/accounts/lockup/utils.go,tests/e2e/auth/keeper/app_config.go,tests/e2e/accounts/base_account_test.go,tests/integration/slashing/app_config.go,tests/integration/slashing/keeper/keeper_test.go,tests/integration/protocolpool/module_test.go,tests/integration/evidence/keeper/infraction_test.go,tests/integration/mint/app_config.go,tests/integration/rapidgen/rapidgen.go,tests/integration/auth/client/cli/suite_test.go,tests/integration/auth/keeper/msg_server_test.go,tests/integration/runtime/query_test.go,tests/integration/protocolpool/app_config.go,simapp/app_di.go,simapp/app_test.go,simapp/app.go,simapp/v2/app_di.go,simapp/test_helpers.go,simapp/v2/simdv2/cmd/testnet.go,simapp/v2/app_config.go,simapp/simd/cmd/testnet.go,simapp/simd/cmd/testnet_test.go,simapp/mint_fn.go,simapp/app_config.go,tests/integration/evidence/app_config.go,client/v2/autocli/common_test.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the necessity of the updated dependencies. # Test: Search for the usage of the updated dependencies. Expect: Necessary usage in the codebase. rg --type go -A 5 $'cosmossdk.io/server/v2/appmanager'Length of output: 1737
Script:
#!/bin/bash # Description: Verify the necessity of the updated dependencies. # Test: Search for the usage of the updated dependencies. Expect: Necessary usage in the codebase. rg --type go -A 5 $'cosmossdk.io/server/v2/stf' rg --type go -A 5 $'cosmossdk.io/store/v2' rg --type go -A 5 $'cosmossdk.io/x/bank'Length of output: 105565
server/v2/cometbft/go.mod (1)
29-29: LGTM! Ensure the updated dependency is necessary.The dependency update to a specific commit version is approved. The change enhances stability by replacing the placeholder with a defined version.
However, verify that the updated dependency is necessary for the project.
Verification successful
Dependency Update Verified
The update to the
cosmossdk.io/server/v2/appmanagerdependency is necessary and used across multiple files in the project:
runtime/v2/builder.goserver/v2/server_test.goserver/v2/types.goruntime/v2/app.goserver/v2/cometbft/abci.goThis confirms that the dependency is essential for the project's functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the necessity of the updated dependency. # Test: Search for the usage of the updated dependency. Expect: Necessary usage in the codebase. rg --type go -A 5 $'cosmossdk.io/server/v2/appmanager'Length of output: 1737
go.mod (1)
15-15: Dependency Addition ApprovedThe addition of the
cosmossdk.io/server/v2/appmanagerdependency looks good and is correctly formatted.simapp/v2/go.mod (4)
15-15: Indirect Comment Removal ApprovedThe removal of the indirect comment from
cosmossdk.io/store/v2is appropriate and looks good.
47-47: New Dependency Addition ApprovedThe addition of the
github.com/cometbft/cometbft/apidependency is correctly formatted and looks good.
61-61: Version Update ApprovedThe version update for
cosmossdk.io/server/v2/appmanageris appropriate and correctly formatted.
48-48: Indirect Comment Removal ApprovedThe removal of the indirect comment from
github.com/cometbft/cometbft/apiis appropriate and looks good.runtime/v2/manager.go (2)
224-225: Control Flow Improvement ApprovedThe addition of the
elseclause to handle cases where the module does not satisfy previous type assertions improves robustness.
230-230: Logging Statement ApprovedThe logging statement before the call to
moduleI.ExportGenesisenhances visibility into the function's execution flow.
| func (app *SimApp[T]) ExportAppStateAndValidators(forZeroHeight bool, jailAllowedAddrs, modulesToExport []string) (servertypes.ExportedApp, error) { | ||
| panic("not implemented") | ||
| // as if they could withdraw from the start of the next block | ||
| ctx := context.Background() |
There was a problem hiding this comment.
Use context with timeout or cancellation.
Using context.Background() does not allow for timeout or cancellation. Consider using context.WithTimeout or context.WithCancel to handle potential long-running operations.
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()|
|
||
| bz := sha256.Sum256([]byte{}) | ||
|
|
||
| ctx := context.Background() |
There was a problem hiding this comment.
Use context with timeout or cancellation.
Using context.Background() does not allow for timeout or cancellation. Consider using context.WithTimeout or context.WithCancel to handle potential long-running operations.
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
simapp/v2/app_di.go
Outdated
| @@ -92,7 +91,7 @@ func NewSimApp[T transaction.Tx]( | |||
| logger log.Logger, | |||
| viper *viper.Viper, | |||
| ) *SimApp[T] { | |||
| viper.Set(serverv2.FlagHome, DefaultNodeHome) // TODO possibly set earlier when viper is created | |||
| //viper.Set(serverv2.FlagHome, DefaultNodeHome) // TODO possibly set earlier when viper is created | |||
There was a problem hiding this comment.
this was the only way to enforce another folder than the home one. For the tests in order to be able to clear it in the end I want to use t.TempDir().
I even wonder if having the viper.set there if it is correct.
There was a problem hiding this comment.
It was for avoiding it doing it in newApp (in root.go or somewhere there). We should move it there then
|
@randygrok your pull request is missing a changelog! |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (3)
- server/v2/appmanager/appmanager.go (1 hunks)
- simapp/v2/app_test.go (1 hunks)
- simapp/v2/export.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- server/v2/appmanager/appmanager.go
- simapp/v2/app_test.go
Additional context used
Path-based instructions (1)
simapp/v2/export.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (3)
simapp/v2/export.go (3)
16-20: LGTM!The logic for loading the latest height and handling errors is correct.
23-26: LGTM!The logic for exporting the genesis state and handling errors is correct.
28-33: LGTM! But address the TODO item.The construction of the
ExportedAppobject is correct. Ensure to address the TODO item for the CometBFT consensus params.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- simapp/v2/app_test.go (1 hunks)
- simapp/v2/export.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- simapp/v2/app_test.go
Additional context used
Path-based instructions (1)
simapp/v2/export.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (5)
simapp/v2/export.go (5)
4-5: Imports look good.The added imports for
contextandstakingare necessary for the new functionality.
11-12: Comments are informative.The added comments effectively describe the purpose of the
ExportAppStateAndValidatorsfunction.
15-15: Use context with timeout or cancellation.Using
context.Background()does not allow for timeout or cancellation. Consider usingcontext.WithTimeoutorcontext.WithCancelto handle potential long-running operations.ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel()
17-21: Error handling is implemented correctly.The function correctly handles errors when loading the latest height.
24-35: Error handling is correct, but address the TODO.The function correctly handles errors when exporting genesis and validators. However, the consensus parameters are currently a placeholder (TODO).
Ensure that the consensus parameters are correctly implemented before finalizing the PR.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (3)
- server/v2/appmanager/appmanager.go (2 hunks)
- simapp/v2/app_test.go (1 hunks)
- simapp/v2/export.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- server/v2/appmanager/appmanager.go
- simapp/v2/app_test.go
Additional context used
Path-based instructions (1)
simapp/v2/export.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (3)
simapp/v2/export.go (3)
18-22: LGTM: Robust error handling for loading the latest height.The code correctly loads the latest height and handles errors, ensuring robustness in case of failures.
25-28: LGTM: Effective error handling for exporting genesis state.The code correctly exports the genesis state and handles errors, ensuring that any issues during export are managed.
30-40: LGTM: Context-aware validator handling with error management.The use of
RunWithCtxallows for context-aware operations, and error handling ensures any issues are addressed.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
simapp/app.go (1)
267-267: Inconsistent Refactoring DetectedThe refactoring from
consensusparamtypestoconsensustypesis incomplete. The following files still contain references toconsensusparamtypes:
testutil/integration/router.gotests/integration/evidence/keeper/infraction_test.goPlease ensure that all occurrences are updated to maintain consistency across the codebase.
Analysis chain
Confirm consistency of refactoring.
The store key reference has been updated from
consensusparamtypes.StoreKeytoconsensustypes.StoreKey. Ensure that all related references have been consistently updated throughout the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent renaming of `consensusparamtypes` to `consensustypes`. # Test: Search for any remaining instances of `consensusparamtypes` in the codebase. Expect: No occurrences. rg --type go 'consensusparamtypes'Length of output: 950
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (11)
- runtime/v2/app.go (1 hunks)
- runtime/v2/builder.go (2 hunks)
- runtime/v2/manager.go (2 hunks)
- server/v2/appmanager/appmanager.go (2 hunks)
- server/v2/cometbft/go.mod (1 hunks)
- simapp/app.go (4 hunks)
- simapp/v2/app_di.go (2 hunks)
- simapp/v2/app_test.go (1 hunks)
- simapp/v2/export.go (1 hunks)
- simapp/v2/go.mod (5 hunks)
- simapp/v2/simdv2/cmd/commands.go (5 hunks)
Files skipped from review due to trivial changes (1)
- server/v2/cometbft/go.mod
Files skipped from review as they are similar to previous changes (5)
- runtime/v2/app.go
- runtime/v2/builder.go
- runtime/v2/manager.go
- simapp/v2/app_test.go
- simapp/v2/go.mod
Additional context used
Path-based instructions (5)
simapp/v2/export.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/simdv2/cmd/commands.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/appmanager/appmanager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/app_di.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (13)
simapp/v2/export.go (4)
4-5: Use context with timeout or cancellation.Using
context.Background()does not allow for timeout or cancellation. Consider usingcontext.WithTimeoutorcontext.WithCancelto handle potential long-running operations.ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel()
15-18: LGTM: Correct error handling forLoadLatestHeight.The error handling for
LoadLatestHeightis correctly implemented by returning the error.
20-23: LGTM: Correct error handling forExportGenesis.The error handling for
ExportGenesisis correctly implemented by returning the error.
25-28: LGTM: Correct construction ofv2.ExportedApp.The return statement correctly constructs and returns the
v2.ExportedAppinstance.simapp/v2/simdv2/cmd/commands.go (3)
31-32: LGTM: Imports forgenutilv2andv2.The imports for
genutilv2andv2are correctly added and used in the file.
86-89: LGTM: UpdatedgenesisCommandto usegenutilv2.AppExporter.The
genesisCommandfunction correctly usesgenutilv2.AppExporterand aligns with the new module structure.
Line range hint
149-164:
LGTM: UpdatedappExportto returngenutilv2.ExportedApp.The
appExportfunction correctly returnsgenutilv2.ExportedAppand aligns with the new module structure.server/v2/appmanager/appmanager.go (4)
92-95: LGTM: Correct error handling forStateAt.The error handling for
StateAtis correctly implemented by returning a formatted error message.
97-101: LGTM: Check forexportGenesisfunction configuration.The logic correctly checks if
exportGenesisis set and returns an error if not, ensuring proper configuration.
103-106: LGTM: Correct error handling forexportGenesis.The error handling for
exportGenesisis correctly implemented by returning a formatted error message.
190-203: Note onUnsafeRunWithCtx: Intended for genesis support.The function is marked as unsafe and is intended for genesis support. Consider its removal as indicated by the TODO comment.
simapp/app.go (2)
521-521: Ensure module name consistency.The module name has been updated from
consensusparamtypes.ModuleNametoconsensustypes.ModuleName. Verify that this change is consistently applied across all module references.Verification successful
Module Name Consistency Verified
The module name change from
consensusparamtypes.ModuleNametoconsensustypes.ModuleNamehas been consistently applied across the codebase. Instances of the new module name are found in various files, confirming the update.
- Files with
consensustypes.ModuleName:
simapp/v2/app_config.gosimapp/app.gosimapp/app_config.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent application of the new module name `consensustypes.ModuleName`. # Test: Search for any remaining instances of `consensusparamtypes.ModuleName` in the codebase. Expect: No occurrences. rg --type go 'consensusparamtypes.ModuleName'Length of output: 45
Script:
#!/bin/bash # Description: Verify presence and usage of the new module name `consensustypes.ModuleName`. # Test: Search for occurrences of `consensustypes.ModuleName` in the codebase to ensure it is being used. rg --type go 'consensustypes.ModuleName'Length of output: 358
290-290: Verify the initialization ofConsensusParamsKeeper.The
ConsensusParamsKeeperis now initialized usingconsensustypes.StoreKey. Confirm that this change aligns with the updated package structure and does not affect the application's functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (4)
- runtime/v2/manager.go (2 hunks)
- server/v2/appmanager/appmanager.go (1 hunks)
- simapp/v2/go.mod (4 hunks)
- x/genutil/v2/cli/commands.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- runtime/v2/manager.go
- server/v2/appmanager/appmanager.go
- simapp/v2/go.mod
Additional context used
Path-based instructions (1)
x/genutil/v2/cli/commands.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (4)
x/genutil/v2/cli/commands.go (4)
3-15: Imports are well-organized.The import statements are correctly grouped with a blank line separating standard library imports from third-party and local imports, adhering to the Uber Go style guide.
17-20: InterfacegenesisMMis well-defined.The interface is concise and appropriately named, following Go's conventions for interface naming.
22-25: FunctionCommandsis well-implemented.The function effectively delegates to
CommandsWithCustomMigrationMap, promoting code reuse and simplicity.
27-46: FunctionCommandsWithCustomMigrationMapis well-structured.The function initializes a Cobra command with appropriate subcommands, ensuring modularity and clarity. The use of
client.ValidateCmdforRunEis a good practice for command validation.Ensure that the integration of this new command into the overall CLI structure is verified.
…-sdk into randy/export-genesis
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- simapp/v2/simdv2/cmd/commands.go (4 hunks)
Additional context used
Path-based instructions (1)
simapp/v2/simdv2/cmd/commands.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (5)
simapp/v2/simdv2/cmd/commands.go (5)
37-41: FunctionnewAppappears correct.The function initializes a new SimApp instance and sets a default node home using Viper. Ensure that the use of Viper aligns with best practices.
Line range hint
43-78: FunctioninitRootCmdis well-structured.The function correctly initializes the root command with various subcommands and handles logger creation. Verify that the logger setup is consistent with the rest of the application.
Line range hint
170-209: TypegenericTxDecodercorrectly implementstransaction.Codec.The implementation of transaction decoding appears correct. Ensure that error messages are informative and consistent with the rest of the application.
88-95: FunctiongenesisCommandcorrectly usesgenutilv2.AppExporter.The function builds a command for genesis-related operations using the updated
genutilv2.AppExportertype. Verify that this change aligns with the intended refactoring and does not introduce breaking changes.
150-168: FunctionappExportcorrectly transitions togenutilv2.ExportedApp.The function exports the application state and validators using the new
genutilv2.ExportedApptype. Ensure that this transition is seamless and consistent across the codebase.
Co-authored-by: marbar3778 <marbar3778@yahoo.com> (cherry picked from commit aeeaca6) # Conflicts: # runtime/v2/app.go # runtime/v2/builder.go # runtime/v2/manager.go # server/v2/appmanager/appmanager.go # server/v2/cometbft/go.mod # simapp/app.go # simapp/v2/go.mod
Co-authored-by: marbar3778 <marbar3778@yahoo.com>
* main: (76 commits) docs: more app v2 renaming (#21336) chore: update link in disclaimer (#21339) refactor(x/distribution): audit QA (#21316) docs: rename app v2 to app di when talking about runtime v0 (#21329) feat(schema): specify JSON mapping (#21243) fix(x/authz): bring back msg response in `DispatchActions` (#21044) chore: fix all lint issue since golangci-lint bump (#21326) refactor(x/mint): v0.52 audit x/mint (#21301) chore: fix spelling errors (#21327) feat: export genesis in simapp v2 (#21199) fix(baseapp)!: Halt at height now does not produce the halt height block (#21256) refactor(scripts): remove unused variable (#21320) chore(schema/testing): upgrade to go 1.23 iterators (#21282) chore: readmes + upgrading docs (#21271) feat(client): add auto cli for node service (#21074) ci: fix github workflow vulnerable to script injection (#21304) build(deps): Bump github.com/prometheus/client_golang from 1.19.1 to 1.20.0 (#21307) build(deps): use Go 1.23 instead of Go 1.22 (#21280) refactor(x/auth): audit x/auth changes (#21146) chore: remove todo: "abstract out staking message back to staking" (#21266) ...
Description
Closes: #20514
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
Bug Fixes
Chores