-
Notifications
You must be signed in to change notification settings - Fork 4.1k
refactor!: remove global config from x/auth and client #19447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughWalkthroughThe overall change involves integrating 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: .coderabbit.yml
Files selected for processing (23)
- baseapp/abci_utils_test.go (2 hunks)
- baseapp/baseapp_test.go (2 hunks)
- baseapp/msg_service_router_test.go (1 hunks)
- client/context.go (2 hunks)
- client/keys/add.go (1 hunks)
- client/keys/parse.go (5 hunks)
- client/keys/parse_test.go (2 hunks)
- client/keys/show.go (1 hunks)
- client/tx/tx_test.go (1 hunks)
- crypto/ledger/ledger_test.go (1 hunks)
- simapp/app.go (2 hunks)
- simapp/simd/cmd/root.go (2 hunks)
- simapp/simd/cmd/root_v2.go (1 hunks)
- testutil/integration/router.go (1 hunks)
- testutil/sims/simulation_helpers.go (1 hunks)
- types/address.go (1 hunks)
- types/config.go (3 hunks)
- types/module/testutil/codec.go (2 hunks)
- x/auth/ante/feegrant_test.go (1 hunks)
- x/auth/ante/sigverify_test.go (1 hunks)
- x/auth/tx/config.go (4 hunks)
- x/auth/tx/config/depinject.go (3 hunks)
- x/auth/tx/config_test.go (1 hunks)
Additional comments: 34
client/keys/parse_test.go (1)
- 26-26: The change from using a
configobject to a hardcoded"cosmos"string simplifies the test setup in alignment with the PR's objectives. However, consider the implications of hardcoding values in tests, especially regarding flexibility and test coverage for different configurations.x/auth/tx/config_test.go (1)
- 26-26: The inclusion of explicit
"cosmos"and"cosmosvaloper"parameters in theNewTxConfigfunction call aligns with the PR's goal of enhancing modularity by removing global config dependencies. Ensure these hardcoded values are suitable for the intended test scenarios.types/module/testutil/codec.go (2)
- 33-33: The addition of explicit Bech32 prefix parameters in the
tx.NewTxConfigcall withinMakeTestEncodingConfigfunction is consistent with the PR's objectives. Ensure the hardcoded"cosmos"and"cosmosvaloper"values are appropriate for all use cases of this utility function.- 49-49: Similarly, the update in the
MakeTestTxConfigfunction to include explicit Bech32 prefix parameters aligns with the PR's direction. Review the hardcoded values to confirm their suitability across different testing scenarios.client/keys/parse.go (3)
- 20-27: The refactoring of
bech32Prefixesto use an explicitmainBech32Prefixparameter instead of relying on a global config is a positive change towards more explicit and modular code. Ensure comprehensive testing to validate the correct handling of Bech32 prefixes across different contexts.- 48-49: The
newBech32Outputfunction's adaptation to accept abech32Prefixdirectly enhances clarity and reduces dependency on global state. Verify that this change integrates well with all calling contexts and maintains the expected functionality.- 95-95: The update to
doParseKeyto directly use abech32Prefixstring parameter aligns with the PR's objectives. It's important to ensure that this change does not limit the function's utility or introduce potential issues with prefix handling.simapp/simd/cmd/root_v2.go (1)
- 119-121: Adding
WithAddressPrefixandWithValidatorPrefixmethods in theProvideClientContextfunction is a constructive step towards explicit configuration. It's crucial to thoroughly test the application to ensure these changes do not impact its initialization and runtime behavior adversely.types/config.go (1)
- 13-18: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The structural changes in
Config, including the removal offullFundraiserPath, align with the PR's objectives to simplify and make configuration handling more explicit. Ensure that the removal of this field and the deprecation ofSetFullFundraiserPathdo not eliminate essential functionality or flexibility. Guidance on alternative configuration methods should be clear for developers.simapp/simd/cmd/root.go (1)
- 52-54: Introducing
WithAddressPrefixandWithValidatorPrefixmethods in theNewRootCmdfunction to explicitly set address prefixes is in line with the PR's objectives. Thorough testing is recommended to ensure these changes do not negatively impact the application's initialization and runtime behavior.baseapp/msg_service_router_test.go (1)
- 141-141: The change to initialize
TxConfigwith"cosmos"and"cosmosvaloper"as parameters aligns with the PR's goal of enhancing modularity by explicitly passing configuration details. However, it would be beneficial to add a comment explaining why"cosmos"and"cosmosvaloper"are chosen as the default values for address and validator prefixes. This would improve code readability and maintainability.testutil/integration/router.go (1)
- 57-57: The addition of
"cosmos"and"cosmosvaloper"as parameters in thetxConfiginitialization is a positive step towards explicit configuration, aligning with the PR's objectives. Including a comment to explain the choice of these default values would enhance clarity and maintainability.client/keys/show.go (1)
- 178-178: Replacing
sdk.GetConfig().GetBech32AccountAddrPrefix()withclientCtx.AddressPrefixis a good practice in moving towards localized configuration handling. Please ensure thatclientCtx.AddressPrefixis correctly initialized and validated elsewhere in the code to maintain robustness and prevent potential issues.testutil/sims/simulation_helpers.go (1)
- 58-58: The update to include
"cosmos"and"cosmosvaloper"as parameters in theTxConfiginitialization within theSimulationOperationsfunction aligns with the PR's goal of explicit configuration. Adding a comment to explain the choice of these default values would be helpful for clarity and future reference.x/auth/tx/config/depinject.go (3)
- 13-15: The addition of imports for
authv1,bankv1beta1, andstakingv1modules aligns with the PR's objective to refactor the global config system. Ensure that these modules are used effectively within the file and that there are no unused imports.- 50-51: The introduction of
AuthConfigandStakingConfigfields in theModuleInputsstruct is a significant change. It's crucial to ensure that these fields are populated correctly whereverModuleInputsis instantiated. This change supports the move towards more localized configuration management.- 88-89: The initialization of
AddressPrefixandValidatorPrefixusingin.AuthConfig.Bech32Prefixandin.StakingConfig.Bech32PrefixValidatorrespectively in theProvideModulefunction is a key part of removing global config usage. This approach enhances modularity by allowing these prefixes to be passed explicitly. However, ensure thatAuthConfigandStakingConfigare guaranteed to be non-nil whereProvideModuleis called to avoid potential nil pointer dereferences.x/auth/tx/config.go (5)
- 60-63: Adding
AddressPrefixandValidatorPrefixfields to theConfigOptionsstruct aligns with the goal of removing global config usage and making configurations more explicit. This is a positive change for modularity and clarity.- 83-90: The modification of the
NewTxConfigfunction signature to includeaddressPrefixandvalidatorPrefixparameters is a necessary change to support the removal of global config. It's important to ensure that all calls to this function throughout the codebase have been updated to pass these new parameters.- 98-103: Renaming
NewDefaultSigningOptionstoNewSigningOptionsand updating its signature to includeaddressPrefixandvalidatorPrefixparameters is consistent with the refactor's objectives. However, ensure that the newauthcodec.NewBech32Codecusage correctly handles the prefixes for both addresses and validators.- 113-113: The use of
NewSigningOptionswithinNewSigningHandlerMapto dynamically create signing options based on the provided prefixes is a good approach. It ensures that the signing handlers are configured with the correct address and validator prefixes. Just make sure that error handling is consistent and that any potential issues with prefix configuration are caught.- 187-187: The decision to fallback to
NewSigningOptionsinNewTxConfigWithOptionswhenSigningOptionsis not provided directly is logical. It ensures that the transaction configuration is always initialized with valid address and validator prefixes. This is crucial for maintaining the integrity of transaction signing.client/keys/add.go (1)
- 272-272: Replacing
sdk.GetConfig().GetBech32AccountAddrPrefix()withctx.AddressPrefixwhen using ledger for key addition is a direct application of the refactor's goals. This change makes the configuration more explicit and reduces reliance on global state. Ensure thatctx.AddressPrefixis correctly initialized and passed through the context in all relevant scenarios.client/tx/tx_test.go (1)
- 32-32: The addition of explicit
"cosmos"and"cosmosvaloper"prefixes in thenewTestTxConfigfunction aligns with the PR's objectives to remove global config usage and enhance modularity. However, consider the implications of hardcoded prefixes on flexibility and configuration in different environments.client/context.go (2)
- 84-85: The addition of
AddressPrefixandValidatorPrefixfields to theContextstruct, along with corresponding methods for setting these prefixes, is a positive step towards explicit and localized configuration. Ensure that documentation and examples are updated to reflect these new configuration options.- 344-354: The implementation of
WithAddressPrefixandWithValidatorPrefixmethods follows the established pattern for context modification methods, enhancing consistency and maintainability. This approach aligns well with the PR's objectives.x/auth/ante/sigverify_test.go (1)
- 105-106: The addition of
AddressPrefixandValidatorPrefixto thetxConfigOptsstruct in theTestSigVerificationfunction aligns with the PR's objective to remove global config usage and enhance modularity. This change ensures that address and validator prefixes are explicitly passed and managed, improving clarity and reducing potential errors. It's a positive step towards making the Cosmos SDK's codebase more modular and maintainable.baseapp/abci_utils_test.go (2)
- 330-330: The addition of
"cosmos"and"cosmosvaloper"as parameters toNewTxConfigaligns with the PR's objective to remove global config usage by explicitly passing configuration details. This change ensures that the tests are adapted to the new approach, enhancing clarity and reducing potential errors by avoiding reliance on a shared global state.- 425-425: Similar to the previous comment, the inclusion of
"cosmos"and"cosmosvaloper"as parameters in theNewTxConfigfunction call within theTestDefaultProposalHandler_PriorityNonceMempoolTxSelectiontest function is consistent with the PR's goal. It demonstrates a clear move towards explicit configuration handling, which is expected to improve the modularity and maintainability of the codebase.types/address.go (1)
- 74-97: The newly added functions
GetBech32PrefixAccPub,GetBech32PrefixValAddr,GetBech32PrefixValPub,GetBech32PrefixConsAddr, andGetBech32PrefixConsPubare straightforward and follow a consistent pattern for constructing Bech32 prefixes based on a given main prefix. This approach aligns with the PR's objective to remove global config usage and enhance modularity by allowing more explicit and localized configuration.However, it's important to ensure that these functions are thoroughly tested, especially given their critical role in address and public key formatting across the SDK. Consider adding unit tests that cover various scenarios, including edge cases for the input
mainPrefix.Additionally, documentation for these functions could be enhanced to provide more context on their usage and the expected format of the
mainPrefixargument. This would improve maintainability and ease of understanding for developers who might work with this code in the future.Overall, the changes are aligned with the PR's objectives and represent a positive step towards a more modular and maintainable codebase. Just ensure thorough testing and consider enhancing the documentation for clarity.
baseapp/baseapp_test.go (2)
- 69-69: The modification to
NewTxConfigto include"cosmos"and"cosmosvaloper"as explicit parameters aligns with the PR's objective to remove global config usage. This change ensures that address and validator prefixes are explicitly provided, enhancing clarity and reducing potential errors. Ensure that all tests and utility functions that rely ontxConfigare updated to reflect this new approach.- 506-506: Similar to the previous comment, the addition of explicit
"cosmos"and"cosmosvaloper"parameters toNewTxConfighere is consistent with the PR's goal. It's crucial to verify that the tests using thistxConfiginstance are correctly adjusted to accommodate these changes, especially since this instance is used for encoding and decoding transactions in tests. This explicit configuration approach contributes to the SDK's modularity and maintainability.simapp/app.go (2)
- 210-210: The initialization of
txConfignow explicitly includes address prefixes (AddressPrefixandValidatorPrefix) obtained from the SDK configuration. This change aligns with the PR's objective to remove global config usage and enhance modularity by explicitly passing configuration details.- 318-319: The re-initialization of
txConfigwithAddressPrefixandValidatorPrefixusingauthtx.NewTxConfigWithOptionsfurther emphasizes the move towards explicit configuration. This approach not only aligns with the PR's objectives but also improves clarity and reduces potential errors by avoiding reliance on global state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yml
Files selected for processing (12)
- baseapp/abci_utils_test.go (2 hunks)
- baseapp/baseapp_test.go (3 hunks)
- baseapp/msg_service_router_test.go (1 hunks)
- client/context.go (2 hunks)
- client/tx/tx_test.go (1 hunks)
- simapp/app.go (2 hunks)
- simapp/simd/cmd/root.go (3 hunks)
- simapp/simd/cmd/root_v2.go (3 hunks)
- testutil/integration/router.go (1 hunks)
- testutil/sims/simulation_helpers.go (1 hunks)
- types/module/testutil/codec.go (2 hunks)
- x/auth/tx/config.go (4 hunks)
Files skipped from review as they are similar to previous changes (11)
- baseapp/abci_utils_test.go
- baseapp/baseapp_test.go
- baseapp/msg_service_router_test.go
- client/context.go
- client/tx/tx_test.go
- simapp/app.go
- simapp/simd/cmd/root.go
- simapp/simd/cmd/root_v2.go
- testutil/integration/router.go
- testutil/sims/simulation_helpers.go
- types/module/testutil/codec.go
Additional comments: 3
x/auth/tx/config.go (3)
- 80-88: The addition of
AddressCodecandValidatorAddressCodecfields to theSigningOptionsstruct in theNewTxConfigfunction is a positive change towards removing global config usage. This approach enhances modularity by allowing configurations to be passed explicitly. However, ensure that all existing calls toNewTxConfigare updated to include the new parameters, as this change could break compatibility with existing code that relies on the previous function signature.Verification successful
The verification process has revealed that the changes to the
NewTxConfigfunction, including the addition ofAddressCodecandValidatorAddressCodecfields, have been widely adopted across the codebase. This supports the initial review comment about the positive impact of these changes towards removing global config usage and enhancing modularity. It appears that all necessary updates to accommodate the new parameters have been made, ensuring compatibility with the updated function signature.* 96-101: Renaming `NewDefaultSigningOptions` to `NewSigningOptions` and updating its signature to include `addressPrefix` and `validatorPrefix` parameters is a clear and necessary change to support the new configuration approach. This change directly aligns with the objective of reducing global state dependencies by requiring explicit prefix parameters. It's crucial to verify that all usages of the old `NewDefaultSigningOptions` function are updated to reflect the new name and signature.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that import the tx package and use NewTxConfig. Expecting list of dependent files. rg --type go $'NewTxConfig\('Length of output: 2308
Verification successful
The verification process did not find any occurrences of
NewDefaultSigningOptions, suggesting that all instances have been successfully updated toNewSigningOptionswith the new signature across the codebase. This aligns with the expectations set in the review comment regarding the renaming and signature update to support the new configuration approach.* 182-182: The error message "signing options not provided" is clear and appropriately indicates a missing configuration. This is a good practice for error handling, as it provides specific feedback that can help developers quickly identify configuration issues. No changes are needed here, but it's important to ensure that all potential error paths in the configuration process are similarly well-handled.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that import the tx package and use NewDefaultSigningOptions. Expecting no results. rg --type go $'NewDefaultSigningOptions\('Length of output: 43
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (4)
- x/auth/ante/feegrant_test.go (1 hunks)
- x/auth/ante/sigverify_test.go (4 hunks)
- x/auth/tx/config.go (4 hunks)
- x/auth/tx/config_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- x/auth/ante/feegrant_test.go
- x/auth/ante/sigverify_test.go
- x/auth/tx/config.go
- x/auth/tx/config_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- x/auth/tx/config_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/auth/tx/config_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- x/auth/tx/config_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/auth/tx/config_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (7)
- baseapp/baseapp_test.go (2 hunks)
- client/tx/tx_test.go (1 hunks)
- simapp/app.go (2 hunks)
- types/module/testutil/codec.go (2 hunks)
- x/auth/ante/feegrant_test.go (1 hunks)
- x/auth/tx/config.go (4 hunks)
- x/auth/tx/config_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- baseapp/baseapp_test.go
- client/tx/tx_test.go
- simapp/app.go
- types/module/testutil/codec.go
- x/auth/ante/feegrant_test.go
- x/auth/tx/config.go
- x/auth/tx/config_test.go
julienrbrt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work.
The signing context of the interface registry will be empty most of the time here. Why not having NewTxConfig take two address codec?
I actually meant doing that only for the sim function.
x/auth/tx/config_test.go
Outdated
| protoCodec := codec.NewProtoCodec(interfaceRegistry) | ||
| configOptions := tx.ConfigOptions{} | ||
| configOptions := tx.ConfigOptions{SigningOptions: &signing.Options{ | ||
| AddressCodec: protoCodec.InterfaceRegistry().SigningContext().AddressCodec(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use testutil.CodecOptions{}.NewInterfaceRegistry() instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (11)
- baseapp/abci_utils_test.go (2 hunks)
- baseapp/baseapp_test.go (3 hunks)
- baseapp/msg_service_router_test.go (1 hunks)
- client/tx/tx_test.go (1 hunks)
- simapp/app.go (2 hunks)
- testutil/integration/router.go (1 hunks)
- testutil/sims/simulation_helpers.go (1 hunks)
- types/module/testutil/codec.go (2 hunks)
- x/auth/ante/feegrant_test.go (1 hunks)
- x/auth/tx/config.go (4 hunks)
- x/auth/tx/config_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (9)
- baseapp/abci_utils_test.go
- baseapp/baseapp_test.go
- baseapp/msg_service_router_test.go
- simapp/app.go
- testutil/integration/router.go
- testutil/sims/simulation_helpers.go
- types/module/testutil/codec.go
- x/auth/ante/feegrant_test.go
- x/auth/tx/config_test.go
Additional comments: 5
x/auth/tx/config.go (4)
- 79-87: The addition of
AddressCodecandValidatorAddressCodecto theSigningOptionsstruct and their usage inNewTxConfigfunction is a significant improvement towards removing global config dependencies. This change enhances modularity by allowing these codecs to be passed directly to functions that require them, rather than relying on a global state. This approach aligns well with best practices for software design, promoting better modularity and flexibility.- 95-100: The update to
NewSigningOptionsto includeaddressCodecandvalidatorAddressCodecas parameters is a clear and concise way to configure signing options. This change ensures that the necessary codecs are explicitly provided where needed, further supporting the goal of eliminating global config reliance. It's a straightforward and effective solution to the problem at hand.- 110-110: The error handling in
NewSigningHandlerMapfor whensigning options not providedis clear and necessary. However, it's important to ensure that all potential error paths are similarly well-handled throughout the codebase, especially in the context of this refactor. Proper error handling is crucial for maintainability and reliability.Verification successful
The script output confirms the presence of multiple error handling patterns (
if err != nil) inx/auth/tx/config.go, indicating that the file employs consistent error handling mechanisms. This supports the review comment's emphasis on the importance of proper error handling, especially in the context of a refactor. Therefore, the approach to error handling inNewSigningHandlerMapand throughout the file appears to be consistent and well-implemented.* 181-181: The error handling for `signing options not provided` in `NewTxConfigWithOptions` is consistent with the approach in `NewSigningHandlerMap`, which is good for maintaining a uniform error handling strategy across the codebase. This consistency is important for readability and maintainability.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling patterns in the modified files to ensure consistency. rg 'if err != nil {' x/auth/tx/config.goLength of output: 136
Script:
#!/bin/bash # Corrected search for error handling patterns in the modified files to ensure consistency. rg 'if err != nil' x/auth/tx/config.goLength of output: 184
client/tx/tx_test.go (1)
- 32-34: The modifications to
newTestTxConfigto include the creation ofcdcandsigningCtxvariables before theauthtx.NewTxConfigcall are well-aligned with the PR's goal of enhancing modularity and reducing global config usage. By explicitly creating and passing these variables, the test setup becomes more flexible and easier to understand. This change also ensures that tests are more aligned with the new approach to handling address prefixes, which is crucial for validating the refactor's effectiveness.
julienrbrt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly lgtm!
Can you add an api breaking change changelog about NewTxConfig in x/auth/changelog.md and NewDefaultSigningOptions -> NewSigningOptions
And update the UPGRADING.md about client.Context adding the address prefix in the diff (there is a section for that)
testutil/integration/router.go
Outdated
| interfaceRegistry := codectypes.NewInterfaceRegistry() | ||
| moduleManager := module.NewManagerFromMap(modules) | ||
| moduleManager.RegisterInterfaces(interfaceRegistry) | ||
| signingCtx := interfaceRegistry.SigningContext() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one will be nil, we should take the address codecs as arguments of NewIntegrationApp
| Codec: codec, | ||
| TxConfig: tx.NewTxConfig(codec, tx.DefaultSignModes), | ||
| Codec: cdc, | ||
| TxConfig: tx.NewTxConfig(cdc, signingCtx.AddressCodec(), signingCtx.ValidatorAddressCodec(), tx.DefaultSignModes), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a doubt about this, if another chain use this helper, the address codec will have the wrong bech32 prefix as we provide no way to overwrite it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can add an options struct to provide address codec and when empty we use those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a struct containing both prefixes will make it as CodecOptions prefixes can be overwritten before calling NewInterfaceRegistry()
cosmos-sdk/types/module/testutil/codec.go
Lines 25 to 27 in 7155a1c
| func MakeTestEncodingConfig(modules ...module.AppModule) TestEncodingConfig { | |
| aminoCodec := codec.NewLegacyAmino() | |
| interfaceRegistry := testutil.CodecOptions{}.NewInterfaceRegistry() |
Another option is to pass CodecOptions directly as function argument. Worst part of this is codec/testutil will bleed in 100+ places 😱

What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmh, that's a tricky one, breaking the api would be annoying indeed.., however we probably have to.
Given that CodecOptions accepts prefixes, I think we should instead use NewInterfaceRegistryWithOptions.
Maybe we should create a MaketestEncodingConfigWithOptions that has the two required address codecs as argument -- however ux wise that may not be best, idk?
I am actually for breaking the api, as otherwise MakeTestEncodingConfig will be useful for the sdk and the hub only 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opted to pass codecoptions as arguments. Zones can set it with its requieres prefixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 3
Configuration used: .coderabbit.yml
Files selected for processing (107)
- CHANGELOG.md (1 hunks)
- UPGRADING.md (1 hunks)
- client/context_test.go (2 hunks)
- client/grpc_query_test.go (2 hunks)
- client/keys/add_ledger_test.go (4 hunks)
- client/keys/add_test.go (5 hunks)
- client/keys/delete_test.go (2 hunks)
- client/keys/export_test.go (2 hunks)
- client/keys/import_test.go (3 hunks)
- client/keys/list_test.go (3 hunks)
- client/keys/migrate_test.go (2 hunks)
- client/keys/rename_test.go (2 hunks)
- client/keys/show_test.go (2 hunks)
- client/tx/aux_builder_test.go (2 hunks)
- client/tx/tx_test.go (3 hunks)
- client/v2/autocli/common_test.go (2 hunks)
- codec/any_test.go (3 hunks)
- codec/unknownproto/regression_test.go (1 hunks)
- server/util_test.go (2 hunks)
- simapp/app.go (5 hunks)
- simapp/simd/cmd/testnet_test.go (2 hunks)
- tests/integration/auth/client/cli/suite_test.go (2 hunks)
- tests/integration/bank/app_test.go (5 hunks)
- tests/integration/bank/bench_test.go (3 hunks)
- tests/integration/bank/keeper/deterministic_test.go (3 hunks)
- tests/integration/distribution/cli_tx_test.go (2 hunks)
- tests/integration/distribution/keeper/msg_server_test.go (3 hunks)
- tests/integration/evidence/keeper/infraction_test.go (3 hunks)
- tests/integration/example/example_test.go (5 hunks)
- tests/integration/gov/keeper/keeper_test.go (3 hunks)
- tests/integration/slashing/keeper/keeper_test.go (3 hunks)
- tests/integration/staking/keeper/common_test.go (3 hunks)
- tests/integration/staking/keeper/deterministic_test.go (3 hunks)
- tests/integration/tx/aminojson/aminojson_test.go (5 hunks)
- tests/integration/tx/decode_test.go (2 hunks)
- testutil/integration/router.go (3 hunks)
- testutil/key_test.go (4 hunks)
- types/mempool/mempool_test.go (2 hunks)
- types/module/testutil/codec.go (2 hunks)
- x/auth/CHANGELOG.md (1 hunks)
- x/auth/ante/testutil_test.go (2 hunks)
- x/auth/client/cli/encode_test.go (2 hunks)
- x/auth/client/tx_test.go (4 hunks)
- x/auth/keeper/deterministic_test.go (2 hunks)
- x/auth/keeper/keeper_test.go (2 hunks)
- x/auth/tx/aux_test.go (2 hunks)
- x/auth/types/genesis_test.go (2 hunks)
- x/auth/vesting/types/vesting_account_test.go (2 hunks)
- x/authz/client/cli/tx_test.go (2 hunks)
- x/authz/keeper/genesis_test.go (2 hunks)
- x/authz/keeper/keeper_test.go (2 hunks)
- x/authz/migrations/v2/store_test.go (2 hunks)
- x/authz/module/abci_test.go (2 hunks)
- x/authz/simulation/decoder_test.go (1 hunks)
- x/authz/simulation/genesis_test.go (1 hunks)
- x/bank/client/cli/tx_test.go (2 hunks)
- x/bank/keeper/collections_test.go (2 hunks)
- x/bank/keeper/keeper_test.go (3 hunks)
- x/circuit/ante/circuit_test.go (2 hunks)
- x/circuit/keeper/genesis_test.go (2 hunks)
- x/circuit/keeper/keeper_test.go (2 hunks)
- x/consensus/keeper/keeper_test.go (2 hunks)
- x/crisis/keeper/genesis_test.go (2 hunks)
- x/crisis/keeper/keeper_test.go (4 hunks)
- x/crisis/keeper/msg_server_test.go (3 hunks)
- x/distribution/keeper/allocation_test.go (4 hunks)
- x/distribution/keeper/delegation_test.go (10 hunks)
- x/distribution/keeper/keeper_test.go (2 hunks)
- x/distribution/migrations/v4/migrate_funds_test.go (2 hunks)
- x/distribution/migrations/v4/migrate_test.go (2 hunks)
- x/distribution/simulation/decoder_test.go (2 hunks)
- x/evidence/keeper/keeper_test.go (3 hunks)
- x/feegrant/client/cli/tx_test.go (2 hunks)
- x/feegrant/filtered_fee_test.go (2 hunks)
- x/feegrant/grant_test.go (2 hunks)
- x/feegrant/keeper/genesis_test.go (2 hunks)
- x/feegrant/keeper/keeper_test.go (2 hunks)
- x/feegrant/migrations/v2/store_test.go (2 hunks)
- x/feegrant/module/abci_test.go (2 hunks)
- x/feegrant/simulation/decoder_test.go (2 hunks)
- x/feegrant/simulation/genesis_test.go (1 hunks)
- x/genutil/client/cli/genaccount_test.go (2 hunks)
- x/genutil/client/cli/gentx_test.go (2 hunks)
- x/genutil/client/cli/migrate_test.go (2 hunks)
- x/genutil/gentx_test.go (2 hunks)
- x/genutil/types/genesis_state_test.go (3 hunks)
- x/gov/client/cli/tx_test.go (2 hunks)
- x/gov/client/utils/query_test.go (2 hunks)
- x/gov/keeper/common_test.go (2 hunks)
- x/gov/migrations/v5/store_test.go (1 hunks)
- x/gov/migrations/v6/store_test.go (1 hunks)
- x/group/client/cli/tx_test.go (2 hunks)
- x/group/keeper/genesis_test.go (2 hunks)
- x/group/keeper/grpc_query_test.go (2 hunks)
- x/group/keeper/keeper_test.go (2 hunks)
- x/group/migrations/v2/migrate_test.go (2 hunks)
- x/group/proposal_test.go (2 hunks)
- x/group/simulation/decoder_test.go (1 hunks)
- x/group/simulation/genesis_test.go (1 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/grpc_query_test.go (2 hunks)
- x/mint/keeper/keeper_test.go (2 hunks)
- x/mint/simulation/genesis_test.go (3 hunks)
- x/nft/keeper/keeper_test.go (2 hunks)
- x/nft/simulation/decoder_test.go (2 hunks)
- x/nft/simulation/genesis_test.go (1 hunks)
- x/params/keeper/common_test.go (1 hunks)
Files not processed due to max files limit (13)
- x/params/keeper/keeper_test.go
- x/params/types/subspace_test.go
- x/protocolpool/keeper/keeper_test.go
- x/slashing/keeper/keeper_test.go
- x/slashing/migrations/v4/migrate_test.go
- x/slashing/simulation/decoder_test.go
- x/staking/client/cli/tx_test.go
- x/staking/keeper/keeper_test.go
- x/staking/migrations/v5/migrations_test.go
- x/staking/simulation/decoder_test.go
- x/upgrade/keeper/abci_test.go
- x/upgrade/keeper/grpc_query_test.go
- x/upgrade/keeper/keeper_test.go
Files skipped from review as they are similar to previous changes (4)
- client/tx/tx_test.go
- simapp/app.go
- testutil/integration/router.go
- types/module/testutil/codec.go
Additional comments: 273
x/group/proposal_test.go (2)
- 11-11: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 20-20: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/feegrant/simulation/genesis_test.go (2)
- 15-15: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 22-22: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/params/keeper/common_test.go (2)
- 9-9: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 16-16: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.codec/unknownproto/regression_test.go (2)
- 10-10: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 18-18: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/authz/simulation/genesis_test.go (2)
- 16-16: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 23-23: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/nft/simulation/genesis_test.go (2)
- 16-16: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 23-23: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/authz/simulation/decoder_test.go (2)
- 16-16: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 23-23: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/distribution/migrations/v4/migrate_test.go (2)
- 14-14: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 23-23: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/group/simulation/genesis_test.go (2)
- 16-16: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 23-23: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/genutil/client/cli/migrate_test.go (2)
- 10-10: The addition of
codectestutilimport is appropriate for enabling codec options customization in tests.- 58-58: Using
codectestutil.CodecOptions{}inMakeTestEncodingConfigis a good practice for enhancing modularity in test configurations.x/feegrant/simulation/decoder_test.go (2)
- 15-15: The addition of the
codectestutilpackage import is aligned with the PR's objective to refactor codec usage and testing. This change enhances the test setup by providing more flexible codec options.- 29-29: The modification in the
TestDecodeStorefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a necessary adjustment following the import ofcodectestutil. This ensures that the test encoding configuration is properly initialized with the intended codec options, promoting consistency and flexibility in testing.x/gov/migrations/v5/store_test.go (2)
- 16-16: The import of
codectestutilis appropriate for the context of testing codec configurations. This aligns with the PR's goal of enhancing modularity and testing capabilities.- 23-23: Updating the
MakeTestEncodingConfigcall to includecodectestutil.CodecOptions{}is a crucial step in ensuring that the test setup is aligned with the new codec testing strategy. This change facilitates more accurate and flexible codec configurations for testing purposes.x/distribution/simulation/decoder_test.go (2)
- 14-14: The addition of the
codectestutilpackage import supports the PR's objective to improve codec testing by providing more flexible codec options. This is a positive change for enhancing test setups.- 28-28: Modifying the
TestDecodeDistributionStorefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall ensures that the test encoding configuration is properly initialized. This adjustment is necessary for maintaining consistency and flexibility in codec testing.x/gov/migrations/v6/store_test.go (2)
- 16-16: The import of
codectestutilis consistent with the PR's aim to enhance codec testing capabilities. This import is beneficial for setting up more flexible and accurate test configurations.- 23-23: The update to the
MakeTestEncodingConfigcall, includingcodectestutil.CodecOptions{}, is a necessary adjustment for the new codec testing strategy. This ensures that the test encoding configuration is properly initialized, promoting better testing practices.x/auth/client/cli/encode_test.go (3)
- 14-14: The addition of the
codectestutilpackage import aligns with the PR's goal to improve codec testing by providing more flexible codec options. This enhances the test setup and is a positive change.- 21-21: Updating the
TestGetCommandEncodefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall ensures that the test encoding configuration is properly initialized. This adjustment is crucial for maintaining consistency and flexibility in codec testing.- 51-51: Similarly, modifying the
TestGetCommandDecodefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is necessary for the new codec testing strategy. This ensures that the test encoding configuration is properly initialized, promoting better testing practices.x/nft/simulation/decoder_test.go (2)
- 14-14: The addition of the
codectestutilpackage import supports the PR's objective to enhance codec testing capabilities. This import is beneficial for setting up more flexible and accurate test configurations.- 27-27: Modifying the
TestDecodeStorefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall ensures that the test encoding configuration is properly initialized. This adjustment is crucial for maintaining consistency and flexibility in codec testing.testutil/key_test.go (4)
- 8-8: The import of
codectestutilaligns with the PR's aim to enhance codec testing capabilities. This import is beneficial for setting up more flexible and accurate test configurations.- 17-17: Updating the
TestGenerateCoinKeyfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall ensures that the test encoding configuration is properly initialized. This adjustment is crucial for maintaining consistency and flexibility in codec testing.- 32-32: Similarly, modifying the
TestGenerateSaveCoinKeyfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is necessary for the new codec testing strategy. This ensures that the test encoding configuration is properly initialized, promoting better testing practices.- 57-57: The update to the
TestGenerateSaveCoinKeyOverwriteFlagfunction, includingcodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall, is a necessary adjustment for the new codec testing strategy. This ensures that the test encoding configuration is properly initialized, promoting better testing practices.client/grpc_query_test.go (2)
- 16-16: The import of
testutilis consistent with the PR's aim to enhance codec testing capabilities. This import is beneficial for setting up more flexible and accurate test configurations.- 43-43: Updating the configuration creation in the
SetupSuitefunction by addingtestutil.CodecOptions{}tomoduletestutil.MakeTestEncodingConfig()is a crucial step in ensuring that the test setup is aligned with the new codec testing strategy. This change facilitates more accurate and flexible codec configurations for testing purposes.simapp/simd/cmd/testnet_test.go (2)
- 19-19: The import of
codectestutilaligns with the PR's goal to improve codec testing by providing more flexible codec options. This enhances the test setup and is a positive change.- 50-50: Updating the
Test_TestnetCmdfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall ensures that the test encoding configuration is properly initialized. This adjustment is crucial for maintaining consistency and flexibility in codec testing.x/crisis/keeper/genesis_test.go (2)
- 14-14: The addition of the
codectestutilpackage import supports the PR's objective to enhance codec testing capabilities. This import is beneficial for setting up more flexible and accurate test configurations.- 41-41: Modifying the
SetupTestfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall ensures that the test encoding configuration is properly initialized. This adjustment is crucial for maintaining consistency and flexibility in codec testing.x/circuit/ante/circuit_test.go (2)
- 15-15: The addition of
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"import is noted. This change is part of the broader effort to enhance testing by utilizing codec test utilities, which is a positive step towards improving code quality and maintainability.- 41-41: The modification in the
initFixturefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a crucial update. This change ensures that the encoding configuration used in tests is aligned with the new approach of handling codec options, which is essential for maintaining consistency and reliability in test setups.x/circuit/keeper/genesis_test.go (2)
- 18-18: The import of
codectestutilis correctly added to leverage codec test utilities for improved testing practices. This aligns with the goal of enhancing test reliability and code quality.- 41-41: Including
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall within theSetupTestfunction is a necessary update. It ensures that the test encoding configuration is consistent with the new approach to codec options, which is crucial for test accuracy and maintainability.x/group/simulation/decoder_test.go (2)
- 15-15: The addition of the
codectestutilimport is appropriate for utilizing codec test utilities, which supports the objective of improving test practices and code quality.- 22-22: Modifying the
TestDecodeStorefunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a critical update. It ensures the test encoding configuration adheres to the updated approach for handling codec options, which is vital for the consistency and reliability of tests.client/keys/delete_test.go (2)
- 12-12: The import of
codectestutilfor codec test utilities is a positive step towards enhancing test practices and improving code quality.- 38-38: Including
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall within theTest_runDeleteCmdfunction is an essential update. It ensures the test encoding configuration is in line with the updated approach to codec options, crucial for maintaining test accuracy and reliability.x/genutil/client/cli/genaccount_test.go (2)
- 15-15: The addition of the
codectestutilimport for codec test utilities is a commendable step towards improving test practices and enhancing code quality.- 73-73: Modifying the
TestAddGenesisAccountCmdfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a necessary update. It ensures the test encoding configuration aligns with the new approach to codec options, which is crucial for the consistency and reliability of tests.x/feegrant/grant_test.go (2)
- 15-15: The import of
codectestutilfor codec test utilities is a positive step towards enhancing test practices and improving code quality.- 25-25: Including
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall within theTestGrantfunction is an essential update. It ensures the test encoding configuration is consistent with the updated approach to codec options, which is vital for maintaining test accuracy and reliability.x/crisis/keeper/keeper_test.go (4)
- 12-12: The addition of the
codectestutilimport for codec test utilities is a commendable step towards improving test practices and enhancing code quality.- 30-30: Including
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall within theTestLoggerfunction is a necessary update. It ensures the test encoding configuration aligns with the new approach to codec options, which is crucial for the consistency and reliability of tests.- 44-44: The modification in the
TestInvariantsfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a critical update. It ensures that the test encoding configuration used is consistent with the new approach to codec options, which is essential for maintaining test accuracy and reliability.- 61-61: Including
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall within theTestAssertInvariantsfunction is an essential update. It ensures the test encoding configuration is in line with the updated approach to codec options, which is crucial for the consistency and reliability of tests.x/mint/keeper/grpc_query_test.go (2)
- 19-19: The import of
codectestutilfor codec test utilities is a positive step towards enhancing test practices and improving code quality.- 35-35: Including
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall within theSetupTestfunction is a necessary update. It ensures the test encoding configuration is consistent with the new approach to codec options, which is vital for maintaining test accuracy and reliability.x/bank/keeper/collections_test.go (2)
- 21-21: The addition of
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"is a good practice for modularizing test utilities. This change aligns with the overall goal of enhancing modularity in the Cosmos SDK.- 32-32: The modification to
encCfginitialization by passingcodectestutil.CodecOptions{}toMakeTestEncodingConfigis a positive change. It ensures that the encoding configuration for tests is more flexible and can be easily adjusted to accommodate different testing scenarios. This change supports the broader refactor goals by making the test setup more modular and adaptable.x/feegrant/migrations/v2/store_test.go (2)
- 18-18: The addition of
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"in thestore_test.gofile is consistent with the approach taken in thecollections_test.gofile. It enhances the modularity of test utilities, which is beneficial for maintaining a clean and adaptable testing environment.- 27-27: Modifying the
encodingConfigdeclaration by passingcodectestutil.CodecOptions{}as an argument toMakeTestEncodingConfigis a good practice. It ensures that the encoding configuration for tests is flexible and can be easily adjusted, which is particularly important for migration tests that may require specific configurations. This change supports the refactor's goal of enhancing modularity and adaptability in the Cosmos SDK.x/mint/keeper/genesis_test.go (1)
- 46-46: The addition of
codectestutil.CodecOptions{}as a parameter tomoduletestutil.MakeTestEncodingConfigis a good practice for allowing flexible codec configurations in tests. This change enhances modularity and aligns with the PR's objectives.x/mint/simulation/genesis_test.go (1)
- 25-25: The use of
codectestutil.CodecOptions{}inmoduletestutil.MakeTestEncodingConfigfor simulation tests is a positive change, enhancing modularity and flexibility in test configurations. This aligns well with the PR's objectives.client/keys/list_test.go (1)
- 43-43: Incorporating
codectestutil.CodecOptions{}into the codec configuration for key listing tests is a beneficial change, improving the flexibility of the test setup. This aligns with the PR's objectives of enhancing modularity.x/authz/keeper/genesis_test.go (1)
- 53-53: Adding
codectestutil.CodecOptions{}tomoduletestutil.MakeTestEncodingConfigin theauthzmodule's genesis tests is a positive step towards more flexible and precise testing configurations. This aligns with the PR's objectives of enhancing modularity.x/mint/keeper/keeper_test.go (1)
- 42-42: Utilizing
codectestutil.CodecOptions{}in themintkeeper tests for customizing codec options is a commendable change, enhancing the flexibility of the test setup. This aligns with the PR's objectives of enhancing modularity.client/keys/rename_test.go (1)
- 37-37: Incorporating
codectestutil.CodecOptions{}into the codec configuration for key renaming tests is a beneficial change, improving the flexibility of the test setup. This aligns with the PR's objectives of enhancing modularity.x/feegrant/module/abci_test.go (1)
- 32-32: The use of
codectestutil.CodecOptions{}in thefeegrantmodule's ABCI tests for customizing codec options is a positive change, enhancing the flexibility of the test setup. This aligns with the PR's objectives of enhancing modularity.x/auth/types/genesis_test.go (2)
- 13-13: The addition of
codectestutilimport is appropriate for providing codec testing utilities, enhancing the test setup's flexibility and capability.- 58-58: The modification to initialize
encodingConfigusingmoduletestutil.MakeTestEncodingConfigwith specific codec options aligns with the PR's objectives to enhance modularity and reduce global config dependencies. This change improves the clarity and reliability of the test setup.x/genutil/types/genesis_state_test.go (3)
- 16-16: The addition of
codectestutilimport is appropriate for enhancing the test setup with codec testing utilities, supporting the modifications made in the test functions.- 52-52: The modification to initialize
encodingConfigwith specific app modules inTestValidateGenesisMultipleMessagesenhances the test setup's modularity and reliability, aligning with the PR's objectives to improve configuration management.- 68-68: The modification in
TestValidateGenesisBadMessageto explicitly define the test encoding configuration with specific app modules contributes to the clarity and reliability of the test setup, aligning with the PR's objectives.x/authz/migrations/v2/store_test.go (2)
- 19-19: The addition of
codectestutilimport is appropriate for providing codec testing utilities, supporting the modifications made in theTestMigrationfunction and enhancing the test setup's capability.- 29-29: The modification to initialize
encodingConfigwith specific app modules inTestMigrationenhances the test setup's modularity and reliability, aligning with the PR's objectives to improve configuration management in migration tests.x/auth/CHANGELOG.md (1)
- 46-46: The changelog entry for PR #19447 accurately reflects the significant changes made, including the introduction of
AddressPrefixandValidatorPrefixas arguments inNewTxConfigand the replacement ofNewDefaultSigningOptionswithNewSigningOptions. This documentation is crucial for users to understand the changes and their impact.x/authz/module/abci_test.go (1)
- 34-34: The update to the encoding configuration in
TestExpiredGrantsQueueto includecodectestutil.CodecOptions{}aligns with the PR's objectives of refactoring Bech32 address prefix handling. This change ensures that the test setup reflects the new approach, enhancing the test's relevance and accuracy.x/distribution/migrations/v4/migrate_funds_test.go (1)
- 38-38: The modification to include
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall for the test setup is a positive change. It aligns with the PR's objectives of enhancing modularity and ensuring that tests accurately reflect the new approach to handling encoding configurations.tests/integration/distribution/cli_tx_test.go (1)
- 42-42: The update to the encoding configuration in the integration tests by including
codectestutil.CodecOptions{}aligns with the PR's objectives of enhancing modularity and refactoring encoding configurations. This ensures that the tests accurately reflect the new approach, maintaining the relevance and accuracy of the tests.x/group/migrations/v2/migrate_test.go (1)
- 38-38: The inclusion of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall for theTestMigratefunction is a positive change. It aligns with the PR's objectives of enhancing modularity and ensuring that tests accurately reflect the new approach to handling encoding configurations.x/genutil/client/cli/gentx_test.go (2)
- 21-21: The addition of
codectestutilimport is aligned with the PR's objective to refactor the global config usage. This change facilitates testing by providing codec options that are more modular and customizable.- 46-46: The modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}is a necessary change to ensure that the encoding configuration used in tests is consistent with the new approach of handling address prefixes without relying on a global config. This enhances the modularity and testability of the code.codec/any_test.go (3)
- 9-9: The import of
codectestutilis correctly added to support the new encoding configuration approach in the test functions. This change is consistent with the PR's goal of enhancing modularity.- 75-75: The modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}in theTestMarshalProtoPubKeyfunction is appropriate. It ensures that the test uses an encoding configuration that aligns with the new approach of explicit address prefix handling.- 115-115: Similarly, the modification in the
TestMarshalProtoInterfacePubKeyfunction to includecodectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is correct. It supports the PR's objective by using a more modular encoding configuration in tests.x/bank/client/cli/tx_test.go (2)
- 19-19: The addition of the
codectestutilimport is appropriate for the file's context, supporting the PR's goal of removing global config usage and enhancing modularity in tests.- 41-41: The update to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}is a necessary adjustment to align the test's encoding configuration with the new approach of explicit address prefix handling. This change supports the PR's objectives well.client/keys/export_test.go (2)
- 13-13: The import of
codectestutilin this file is consistent with the PR's objective to refactor the global config usage and enhance modularity in tests. This change is appropriate.- 23-23: The modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}in theTest_runExportCmdfunction is correctly implemented. It ensures that the test uses an encoding configuration that aligns with the new approach of explicit address prefix handling.x/feegrant/keeper/genesis_test.go (2)
- 20-20: The addition of the
codectestutilimport in this file supports the PR's goal of removing global config usage and enhancing modularity in tests. This change is well-aligned with the objectives.- 47-47: The update to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}is a necessary adjustment to align the test's encoding configuration with the new approach of explicit address prefix handling. This change supports the PR's objectives effectively.x/gov/client/utils/query_test.go (2)
- 18-18: The addition of
codectestutilimport is aligned with the PR's objective to refactor the global config usage and enhance modularity. This change supports the updated test configurations.- 58-58: The modification to include
codectestutil.CodecOptions{}as an argument in theMakeTestEncodingConfigfunction call is a necessary change following the removal of the global config. This ensures that the tests are aligned with the new configuration approach.tests/integration/bank/bench_test.go (3)
- 18-18: The addition of
codectestutilimport is consistent with the PR's goal to refactor the global config system. This import is necessary for the updated benchmark tests to function correctly with the new configuration approach.- 86-86: The modification to include
codectestutil.CodecOptions{}as an argument in theMakeTestTxConfigfunction call is appropriate and aligns with the PR's objectives. This ensures that the benchmark tests are compatible with the new configuration system.- 144-144: The repetition of the modification to include
codectestutil.CodecOptions{}in another instance of theMakeTestTxConfigfunction call further ensures consistency across the test suite. This change is necessary for maintaining alignment with the new configuration approach.client/keys/migrate_test.go (2)
- 16-16: The addition of
codectestutilimport supports the PR's objective to refactor the global config usage. This change is crucial for the updated test configurations in the key migration tests.- 45-45: Including
codectestutil.CodecOptions{}as an argument in theMakeTestEncodingConfigfunction call is a necessary adjustment following the removal of the global config. This ensures that the key migration tests are aligned with the new configuration approach.x/circuit/keeper/keeper_test.go (2)
- 19-19: The addition of
codectestutilimport aligns with the PR's goal to enhance modularity by removing the global config. This import is necessary for the updated keeper tests to function correctly with the new configuration approach.- 44-44: The modification to include
codectestutil.CodecOptions{}as a parameter in theMakeTestEncodingConfigfunction call is appropriate and necessary for the keeper tests. This ensures that the tests are compatible with the new configuration system.x/crisis/keeper/msg_server_test.go (3)
- 13-13: The addition of
codectestutilimport is consistent with the PR's objectives to refactor the global config system. This import is necessary for the updated msg server tests to function correctly with the new configuration approach.- 41-41: Including
codectestutil.CodecOptions{}as an argument in theMakeTestEncodingConfigfunction call is a necessary adjustment following the removal of the global config. This ensures that the msg server tests are aligned with the new configuration approach.- 55-55: The repetition of the modification to include
codectestutil.CodecOptions{}in another instance of theMakeTestEncodingConfigfunction call further ensures consistency across the test suite. This change is necessary for maintaining alignment with the new configuration approach.tests/integration/tx/decode_test.go (1)
- 47-47: The addition of
codectestutil.CodecOptions{}toMakeTestEncodingConfigis a good practice for explicitly configuring codec options in tests. Ensure that this change does not inadvertently affect the expected outcomes of the tests.x/feegrant/filtered_fee_test.go (1)
- 26-26: The addition of
codectestutil.CodecOptions{}toMakeTestEncodingConfigis a good practice for explicitly configuring codec options in tests. Ensure that this change does not inadvertently affect the expected outcomes of the tests.x/distribution/keeper/keeper_test.go (1)
- 42-42: The addition of
codectestutil.CodecOptions{}toMakeTestEncodingConfigis a good practice for explicitly configuring codec options in tests. Ensure that this change does not inadvertently affect the expected outcomes of the tests.tests/integration/gov/keeper/keeper_test.go (1)
- 56-56: The addition of
codectestutil.CodecOptions{}toMakeTestEncodingConfigis a good practice for explicitly configuring codec options in tests. Ensure that this change does not inadvertently affect the expected outcomes of the tests.x/auth/client/tx_test.go (4)
- 15-15: The addition of
codectestutilimport is appropriate and aligns with the PR's objective to enhance modularity in testing configurations. This change facilitates the direct specification of codec options in tests, which is crucial for the new approach to handling address prefixes.- 42-42: The modifications to
MakeTestEncodingConfigcalls, including the addition ofcodectestutil.CodecOptions{}as an argument, are appropriate and necessary for the new testing setup. This change allows for more flexible and explicit configuration of codec options in tests, aligning with the PR's objectives.- 78-78: The modifications to
MakeTestEncodingConfigcalls, including the addition ofcodectestutil.CodecOptions{}as an argument, are consistent and necessary across different test functions. This approach enhances the modularity and flexibility of test configurations.- 142-142: The adjustments in the
TestBatchScanner_Scanfunction, including the passing ofcodectestutil.CodecOptions{}andauth.AppModule{}toMakeTestEncodingConfig, are appropriate and align with the PR's objectives. These changes enhance the modularity and flexibility of the test setup, which is crucial for the new approach to handling address prefixes.client/keys/import_test.go (3)
- 22-22: The update to pass
codectestutil.CodecOptions{}as an argument toMakeTestEncodingConfigis a good practice for ensuring that codec configurations are explicit and customizable for testing purposes. This change enhances the modularity and flexibility of test setups.- 126-126: Similar to the previous comment, the consistent application of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall across different test functions maintains uniformity and ensures that codec configurations can be easily adjusted if needed in the future.- 188-188: Again, the use of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall for codec initialization is consistent with the changes made in other test functions within this file. This consistency is crucial for maintainability and readability of the test code.client/context_test.go (2)
- 16-16: The addition of the
codectestutilimport is necessary for utilizingCodecOptions{}in the test setup. This change supports the goal of making codec configurations explicit and customizable, which is beneficial for testing different scenarios.- 102-102: Modifying the
cfginitialization to includecodectestutil.CodecOptions{}aligns with the approach taken in other test files to enhance the flexibility and modularity of codec configurations. This is a positive change for maintaining consistent testing practices across the codebase.client/keys/add_ledger_test.go (4)
- 18-18: The introduction of
codectestutilfor codec testing utilities is consistent with the changes made in other test files. This import is essential for usingCodecOptions{}in the test setup, promoting explicit and customizable codec configurations.- 43-43: Updating the
MakeTestEncodingConfigfunction call withCodecOptions{}for codec initialization in this test function is in line with the approach taken across the codebase. This consistency is key for maintainable and flexible test configurations.- 101-101: Similar to previous comments, the consistent application of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall across different test functions within this file ensures uniformity and flexibility in codec configurations for testing.- 148-148: Again, the use of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall for codec initialization is consistent with the changes made in other test functions within this file and other files. This consistency is crucial for maintainability and readability of the test code.client/v2/autocli/common_test.go (2)
- 25-25: The addition of the
testutilimport is necessary for utilizingCodecOptions{}in the test setup. This change supports the goal of making codec configurations explicit and customizable, which is beneficial for testing different scenarios.- 55-55: Modifying the
initFixturefunction to includetestutil.CodecOptions{}alongsidebank.AppModule{}in theMakeTestEncodingConfigcall aligns with the approach taken in other test files to enhance the flexibility and modularity of codec configurations. This is a positive change for maintaining consistent testing practices across the codebase.client/tx/aux_builder_test.go (2)
- 10-10: The addition of the
testutilimport is appropriate for the changes made in the test setup. This aligns with the PR's objective to refactor the global config usage.- 37-37: Modification to
MakeTestEncodingConfigto includetestutil.CodecOptions{}as an argument is correct and necessary for the updated codec configuration. This change supports the PR's goal of enhancing modularity.tests/integration/staking/keeper/common_test.go (3)
- 29-29: The addition of the
codectestutilimport is appropriate for the changes made in the test setup. This aligns with the PR's objective to refactor the global config usage.- 107-107: Modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}as an argument is correct and necessary for the updated codec configuration. This change supports the PR's goal of enhancing modularity.- 152-159: The modifications to the
integrationAppinitialization, including additional parameters related to encoding configuration, are correct and necessary for the updated codec configuration. This ensures that theintegrationAppis correctly initialized with the new settings, supporting the PR's goal of enhancing modularity.x/evidence/keeper/keeper_test.go (2)
- 24-24: The addition of the
codectestutilimport is appropriate for the changes made in the test setup. This aligns with the PR's objective to refactor the global config usage.- 88-88: Modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}as an argument is correct and necessary for the updated codec configuration. This change supports the PR's goal of enhancing modularity.types/mempool/mempool_test.go (2)
- 17-17: The addition of
codectestutilimport is appropriate for the context of testing, ensuring that codec utilities are available for encoding and decoding operations within tests. This aligns with best practices for comprehensive testing.- 240-240: The modification in the
unmarshalTxfunction to includecodectestutil.CodecOptions{}as an argument toMakeTestEncodingConfigis a necessary change following the refactor to remove global config usage. This ensures that the test encoding configuration is explicitly defined, enhancing modularity and clarity in test setups.x/auth/tx/aux_test.go (2)
- 12-12: The addition of
codectestutilimport is consistent with the need for codec utilities in testing scenarios, especially when dealing with encoding and decoding within the Cosmos SDK. This import is crucial for maintaining the integrity and reliability of tests.- 43-43: The modification in the
TestBuilderWithAuxfunction to passcodectestutil.CodecOptions{}toMakeTestEncodingConfigis a direct consequence of the refactor aiming to eliminate global config usage. This change ensures that tests are aligned with the new approach of handling configurations, promoting better practices in test design.tests/integration/example/example_test.go (6)
- 21-21: The import of
codectestutilis essential for the integration tests, providing codec utilities that are necessary for encoding and decoding operations. This import supports the overall goal of enhancing modularity and reducing reliance on global configurations.- 33-33: The modification to include
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is aligned with the refactor's objectives. This ensures that the encoding configuration used in integration tests is explicitly defined, enhancing clarity and modularity.- 34-34: The initialization of
signingCtxusingencodingCfg.InterfaceRegistry.SigningContext()is a good practice, ensuring that the signing context is properly configured for the tests. This addition is crucial for tests that involve signing and address handling, aligning with the refactor's goals to improve modularity.- 69-70: The usage of
signingCtx.AddressCodec()andsigningCtx.ValidatorAddressCodec()to initialize the integration app is a significant improvement. It ensures that the address and validator address codecs are explicitly set, promoting better practices in handling addresses and enhancing the clarity of the test setup.- 126-126: The inclusion of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall for theExample_oneModulefunction is consistent with the refactor's objectives. This ensures that the encoding configuration is explicitly defined, enhancing the clarity and modularity of the test setup.- 155-156: The usage of
encodingCfg.InterfaceRegistry.SigningContext().AddressCodec()and.ValidatorAddressCodec()in theExample_oneModulefunction is a good practice. It ensures that the address and validator address codecs are explicitly set for the integration app, aligning with the refactor's goals to improve modularity and clarity in test setups.x/gov/keeper/common_test.go (2)
- 25-25: The addition of
codectestutilimport is appropriate for enhancing testing capabilities related to codec functionalities.- 102-102: The modification in
encCfginitialization to includecodectestutil.CodecOptions{}is necessary for configuring the encoding with specific options for testing. This change enhances the flexibility and accuracy of codec-related tests.x/auth/keeper/keeper_test.go (2)
- 18-18: The addition of
codectestutilimport is appropriate for enhancing testing capabilities related to codec functionalities.- 50-50: The modification in
encCfginitialization to includecodectestutil.CodecOptions{}is necessary for configuring the encoding with specific options for testing. This change enhances the flexibility and accuracy of codec-related tests.x/group/keeper/genesis_test.go (2)
- 24-24: The addition of
codectestutilimport is appropriate for enhancing testing capabilities related to codec functionalities.- 56-56: The modification in
encCfginitialization to includecodectestutil.CodecOptions{}is necessary for configuring the encoding with specific options for testing. This change enhances the flexibility and accuracy of codec-related tests.x/auth/ante/testutil_test.go (2)
- 28-28: The addition of
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"import is noted. Ensure that this new import is utilized effectively within the file, particularly in the context of the changes made toMakeTestEncodingConfigcall.- 71-71: The modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}as an argument is a significant change. This adjustment aligns with the PR's objective to enhance modularity by explicitly specifying codec options rather than relying on a global configuration. It's crucial to verify that all instances whereMakeTestEncodingConfigis called have been updated to include the new argument format to maintain consistency across the codebase.Verification successful
The verification process has revealed that the majority of calls to
MakeTestEncodingConfigacross the codebase have been updated to include the new argument format (codectestutil.CodecOptions{}along withAppModuleinstances), aligning with the PR's objective to enhance modularity. This indicates a significant effort to maintain consistency across the codebase with the updated method signature. However, there are instances whereMakeTestEncodingConfigis called with onlycodectestutil.CodecOptions{}and noAppModuleinstances, which might be intentional based on the specific testing context. Without further context, it's challenging to definitively conclude whether these instances represent an oversight or are designed for generic testing purposes.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of `MakeTestEncodingConfig` to ensure they've been updated. rg "MakeTestEncodingConfig" --type goLength of output: 19985
x/genutil/gentx_test.go (2)
- 22-22: The addition of
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"import is consistent with the changes made in the other file reviewed. This import is crucial for the new approach to specifying codec options explicitly in test setups. Ensure that this import is utilized wherever necessary within the file to support the changes made.- 56-56: The modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}as an argument is observed here as well. This change is part of the broader effort to remove global config usage and enhance modularity within the Cosmos SDK. It's important to ensure that this new approach to configuring encoding options is applied consistently across all relevant test files to maintain uniformity and avoid potential issues arising from inconsistent configurations.Verification successful
The modification to
MakeTestEncodingConfigto includecodectestutil.CodecOptions{}as an argument is consistently applied across all relevant test files, aligning with the effort to enhance modularity and reduce reliance on global configurations within the Cosmos SDK. This consistency supports the importance of uniformity in the new configuration approach as mentioned in the original review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of `MakeTestEncodingConfig` in test files to ensure consistency. rg "MakeTestEncodingConfig" --type go --glob "*_test.go"Length of output: 19756
client/keys/show_test.go (2)
- 15-15: The addition of
codectestutilimport is appropriate for enhancing test setups with specific codec options.- 60-60: Modification to the
MakeTestEncodingConfigfunction call, includingcodectestutil.CodecOptions{}as an argument, aligns with the PR's objectives to enhance modularity and configurability.x/consensus/keeper/keeper_test.go (2)
- 15-15: The addition of
codectestutilimport is appropriate for enhancing test setups with specific codec options.- 36-36: Modification to the
MakeTestEncodingConfigfunction call, includingcodectestutil.CodecOptions{}as an argument, aligns with the PR's objectives to enhance modularity and configurability.x/nft/keeper/keeper_test.go (2)
- 20-20: The addition of
codectestutilimport is correctly implemented and necessary for the changes in the test setup.- 56-56: The modification to include
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall enhances test configuration flexibility and is well-aligned with the PR's objectives.x/group/keeper/grpc_query_test.go (2)
- 21-21: The addition of
codectestutilimport is correctly implemented and necessary for the changes in the test setup.- 49-49: The modification to include
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall enhances test configuration flexibility and is well-aligned with the PR's objectives.x/gov/client/cli/tx_test.go (1)
- 47-47: The update to
s.encCfg = testutilmod.MakeTestEncodingConfig(codectestutil.CodecOptions{}, gov.AppModule{})in theSetupSuitemethod is a significant change towards removing the global config usage. This approach enhances modularity by allowing tests to specify codec options directly, aligning with the PR's objectives to improve the Cosmos SDK's configuration flexibility. However, it's crucial to ensure that all tests within this suite still function as expected after this change, especially since the global config system's removal might affect various parts of the SDK differently.x/auth/keeper/deterministic_test.go (2)
- 22-22: The addition of
codectestutilimport is appropriate for the context of testing. It's used to provide codec utilities that are essential for setting up the encoding configuration in tests, ensuring that the tests have the necessary infrastructure to serialize and deserialize data structures as expected.- 56-56: The modification to the
MakeTestEncodingConfigcall to includecodectestutil.CodecOptions{}as an argument is a necessary change following the introduction of thecodectestutilimport. This ensures that the test encoding configuration is correctly initialized with any codec options required for the tests, aligning with the PR's objective to refactor global config usage and enhance modularity.client/keys/add_test.go (2)
- 16-16: The addition of
codectestutilimport is appropriate for the context of testing. It's used to provide codec utilities that are essential for setting up the encoding configuration in tests, ensuring that the tests have the necessary infrastructure to serialize and deserialize data structures as expected. This aligns with the PR's objective to refactor global config usage and enhance modularity.- 32-32: The modifications to the
MakeTestEncodingConfigcall across various test functions to includecodectestutil.CodecOptions{}as an argument are necessary changes following the introduction of thecodectestutilimport. This ensures that the test encoding configuration is correctly initialized with any codec options required for the tests. These changes are consistent and correctly applied across the file, enhancing the modularity and flexibility of the test setup.Also applies to: 153-153, 221-221, 353-353
x/feegrant/keeper/keeper_test.go (1)
- 48-48: The modification to include
codectestutil.CodecOptions{}as an argument toMakeTestEncodingConfigis a direct adaptation to the changes in the encoding configuration setup. This change ensures that the test environment aligns with the new approach of handling encoding configurations without relying on a global state. It's crucial to verify that this change does not introduce any unintended side effects in the test setups or the behavior of the tests themselves.tests/integration/bank/app_test.go (5)
- 29-29: The addition of
cdctestutil "github.com/cosmos/cosmos-sdk/codec/testutil"is aligned with the PR's objective to refactor the handling of Bech32 address prefixes by removing global config usage. This import is necessary for utilizingCodecOptionsin test configurations.- 179-179: The introduction of
cdctestutil.CodecOptions{}inmoduletestutil.MakeTestTxConfig(cdctestutil.CodecOptions{})represents a significant shift towards a more modular and explicit configuration approach in tests. This change is crucial for ensuring that tests are aligned with the new method of handling address prefixes without relying on a global config.- 259-259: Similar to the previous comment, the use of
cdctestutil.CodecOptions{}here ensures that the transaction configuration in tests is consistent with the new approach of explicitly passing address prefixes. This change is essential for maintaining the integrity and relevance of the tests in light of the refactor.- 312-312: Again, the application of
cdctestutil.CodecOptions{}in this context underscores the PR's commitment to enhancing modularity and reducing reliance on global state. By adapting the test configurations to this new paradigm, the tests remain robust and reflective of the SDK's evolving architecture.- 367-367: The consistent use of
cdctestutil.CodecOptions{}across different test cases demonstrates a thorough and systematic approach to aligning the tests with the SDK's architectural changes. This consistency is key to ensuring that all tests accurately simulate the new configuration mechanisms introduced by the PR.x/distribution/keeper/allocation_test.go (2)
- 23-23: The import of
codectestutilis correctly added to facilitate the new approach of handling encoding configurations without relying on global state. This change aligns with the PR's objective to enhance modularity.- 35-35: The modification to
encCfginitialization by includingcodectestutil.CodecOptions{}withinmoduletestutil.MakeTestEncodingConfig()is a crucial part of the refactor. This change ensures that encoding configurations are explicitly defined, promoting better modularity and reducing reliance on global state. It's important to verify that all instances whereencCfgis used have been updated to reflect this new initialization pattern.Verification successful
The verification process has successfully identified numerous instances across the codebase where
encCfginitialization has been updated to includecodectestutil.CodecOptions{}withinmoduletestutil.MakeTestEncodingConfig(). This confirms that the modification mentioned in the review comment has been broadly applied, ensuring that encoding configurations are explicitly defined, which promotes better modularity and reduces reliance on global state.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all instances of `encCfg` initialization in tests have been updated. rg --type go 'MakeTestEncodingConfig\(codectestutil.CodecOptions{},'Length of output: 257
Script:
#!/bin/bash # Correcting the regex pattern to properly escape special characters and verify that all instances of `encCfg` initialization in tests have been updated. rg --type go 'MakeTestEncodingConfig\(codectestutil.CodecOptions\{\},'Length of output: 14797
tests/integration/bank/keeper/deterministic_test.go (3)
- 26-26: The addition of
codectestutilfor codec testing is a positive change, enhancing the robustness of codec-related tests. This aligns well with the PR's objective of improving modularity and reducing reliance on global state.- 67-68: The update to the encoding configuration using
codectestutil.CodecOptionsis a crucial change. It ensures that the tests are aligned with the new approach of handling Bech32 address prefixes directly. This modification is essential for maintaining consistency in encoding across the SDK, especially after the removal of the global config.- 106-112: The adjustments in the initialization of
integrationAppwith additional parameters related to encoding configuration are noteworthy. By explicitly passingAddressCodecandValidatorAddressCodecobtained fromencodingCfg.InterfaceRegistry.SigningContext(), the tests are made more flexible and modular. This change directly supports the PR's goal of enhancing modularity and reducing reliance on global state.x/authz/keeper/keeper_test.go (2)
- 21-21: The addition of
codectestutilimport is appropriate for the changes made in theMakeTestEncodingConfigcall. Ensure that this import is used elsewhere in the file if needed to avoid unused imports.- 55-55: The modification to include
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a positive change, enhancing the configurability of encoding in tests. Ensure that the provided codec options align with the testing requirements and do not inadvertently alter test behaviors.server/util_test.go (2)
- 21-21: The addition of
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"import is noted. This import is presumably used for the modifications in theTestEmptyMinGasPricesfunction to utilize the new codec options. Ensure that this new dependency is properly managed and does not introduce unnecessary overhead or complexity.- 454-454: The modification to initialize
encCfgwithtestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{})in theTestEmptyMinGasPricesfunction is a direct reflection of the PR's objective to enhance modularity and reduce reliance on global state. This change allows for more explicit control over codec options during testing, which is a positive step towards achieving better test isolation and configurability.x/authz/client/cli/tx_test.go (1)
- 55-55: The addition of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a positive change, enhancing the flexibility and customization of test setups. Ensure that this change integrates well with the rest of the test suite and does not adversely affect test execution.x/group/keeper/keeper_test.go (2)
- 27-27: The addition of
codectestutilimport is appropriate for supporting codec testing utilities, aligning with the PR's objectives to refactor encoding configurations.- 59-59: The modification to include
codectestutil.CodecOptionsin theencCfgdeclaration aligns with the PR's objectives to enhance modularity in encoding configurations. This change is a positive step towards more explicit and flexible configurations in tests.tests/integration/evidence/keeper/infraction_test.go (3)
- 44-44: The addition of
codectestutilfrommygithub.libinneed.workers.dev/cosmos/cosmos-sdk/codec/testutilis a positive change, as it aligns with the PR's objective to enhance modularity by using codec testing utilities. This should facilitate more isolated and reliable codec tests.- 91-91: The update to the
encodingCfgin theinitFixturefunction to includecodectestutil.CodecOptions{}is a crucial change. It ensures that the encoding configuration used throughout the tests is consistent with the new approach of handling address prefixes directly. This change is necessary for the tests to reflect the modifications made in the main codebase and to ensure that the tests remain valid and effective after the refactor.- 146-155: The adjustments made to the initialization of
integrationAppwithin theinitFixturefunction are significant. By utilizing the new methods from the encoding configuration, specificallyencodingCfg.InterfaceRegistry.SigningContext().AddressCodec()andencodingCfg.InterfaceRegistry.SigningContext().ValidatorAddressCodec(), the test setup aligns with the changes made in the main codebase. This ensures that the tests accurately reflect the new way address prefixes are managed, contributing to the overall goal of enhancing modularity and reducing reliance on global state.tests/integration/slashing/keeper/keeper_test.go (3)
- 33-33: The addition of
codectestutilfor codec testing utilities is a good practice for enhancing test robustness and readability. It's important to ensure that these utilities are used effectively throughout the test suite to maintain consistency.- 60-60: The introduction of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigfunction call is a significant change. It aligns with the PR's objective of enhancing modularity by allowing for more flexible codec configuration in tests. Ensure that this change is reflected in all relevant test setups to maintain consistency.- 105-112: Updating the
integrationAppinitialization to include additional parameters related to encoding configuration is a crucial change. This ensures that the application's integration tests are aligned with the new approach to handling address prefixes and other configurations without relying on a global state. It's important to verify that these changes do not introduce any unintended side effects in the integration tests, especially regarding address encoding and validation.x/feegrant/client/cli/tx_test.go (2)
- 28-28: The addition of
codectestutilimport aligns with the changes made to theMakeTestEncodingConfigcall, ensuring that the necessary utilities for codec testing are available. This is a good practice for maintaining the modularity and readability of test code.- 65-65: The modification to the
MakeTestEncodingConfigcall to includecodectestutil.CodecOptions{}as an argument is a crucial change. This adjustment ensures that the encoding configuration used in tests is flexible and can be customized with codec options. This change is particularly important for tests that may need to simulate different encoding behaviors or configurations, enhancing the robustness and coverage of the test suite.However, it's essential to ensure that this change does not inadvertently affect other tests that rely on the default encoding configuration. Given the scope of the PR, this change seems to be in line with the objective of enhancing modularity and reducing reliance on global state by explicitly specifying codec options where needed.
x/group/client/cli/tx_test.go (1)
- 53-53: The addition of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall during theSetupSuitemethod is a notable change. This modification aligns with the PR's objective to enhance modularity by allowing for more flexible encoding configurations. It's important to ensure that this change does not inadvertently affect the encoding or decoding processes in tests, especially since the PR involves significant refactoring related to configuration management.tests/integration/tx/aminojson/aminojson_test.go (5)
- 64-64: The introduction of
codectestutilaligns with the PR's objectives to refactor tests for a more modular approach in handlingCodecOptions. This change is necessary and correctly implemented.- 96-97: The use of
codectestutil.CodecOptions{}inMakeTestEncodingConfigis correctly implemented, ensuring that tests are aligned with the new approach of handling codec options without relying on global config. This change is crucial for enhancing modularity and flexibility in the Cosmos SDK.- 212-212: Similar to previous test functions, the use of
codectestutil.CodecOptions{}inMakeTestEncodingConfigforTestAminoJSON_LegacyParityis correctly applied. This consistency across test functions is important for maintaining a unified approach to handling codec options in the absence of global config.- 516-516: The application of
codectestutil.CodecOptions{}inMakeTestEncodingConfigforTestSendAuthorizationtest function is consistent with the PR's objectives. This change ensures that the test configuration is modular and does not rely on a global state, which is a significant improvement in terms of test design and maintainability.- 566-566: The use of
codectestutil.CodecOptions{}inMakeTestEncodingConfigforTestDecimalMutationis correctly implemented. This change is part of the broader effort to refactor tests to be more modular and independent of global config, which is commendable for improving the test infrastructure's flexibility and maintainability.tests/integration/staking/keeper/deterministic_test.go (3)
- 30-30: The introduction of
codectestutilfor codec testing is a positive change, enhancing the robustness of codec-related tests by providing more comprehensive testing utilities.- 72-72: The update to the encoding configuration to include
codectestutil.CodecOptionsis crucial for ensuring that the tests accurately reflect the encoding setup used throughout the application, especially with the new address codec configurations.- 116-123: Modifying the initialization of
integrationAppto utilize new encoding configuration methods for address codecs is a significant improvement. It ensures that the test environment closely mirrors the actual application setup, which is essential for the reliability of integration tests.tests/integration/distribution/keeper/msg_server_test.go (3)
- 36-36: The addition of
codectestutilfor codec testing utilities is a positive change, enhancing the testing framework's capabilities and allowing for more comprehensive codec-related tests.- 70-71: The update to the encoding configuration to include
codectestutil.CodecOptionsis a necessary change following the introduction ofcodectestutil. This ensures that the test encoding configuration is aligned with the new testing utilities.- 143-152: Adjusting the initialization of
integrationAppto incorporate new codec-related functionalities is a crucial update. It ensures that the application is correctly set up with the necessary codec options for testing. This change is directly related to the previous updates and is essential for maintaining the integrity and functionality of the integration tests.x/auth/vesting/types/vesting_account_test.go (2)
- 19-19: Adding
codectestutilas a new import is appropriate for the changes described, ensuring that the necessary utilities for codec testing are available. This aligns with the objective of refactoring the global config usage and enhancing test modularity.- 42-42: The modification in the
SetupTestmethod to passcodectestutil.CodecOptions{}as an argument toMakeTestEncodingConfigis a significant improvement. It directly supports the PR's objective of removing global config usage by allowing more flexible and explicit configuration for tests. This change enhances the modularity and maintainability of the test setup, making it easier to adapt to different testing scenarios without relying on a shared global state.x/distribution/keeper/delegation_test.go (2)
- 21-21: The addition of
codectestutilimport is consistent with the changes described in the PR summary, which involve refactoring how address prefixes are managed. This import is necessary for the updatedMakeTestEncodingConfigfunction calls that now require codec options to be explicitly provided.- 33-33: The modification of
MakeTestEncodingConfigfunction calls across multiple test functions to includecodectestutil.CodecOptions{}as an argument is a direct consequence of the PR's objective to remove global config usage. By passing codec options explicitly, the tests are aligned with the new approach of managing address prefixes and other configurations without relying on a global state. This change enhances the modularity and configurability of the SDK, which is beneficial for blockchain applications requiring customization. The consistent application of this change across the file demonstrates attention to detail and thoroughness in implementing the refactor.Also applies to: 135-135, 240-240, 366-366, 465-465, 542-542, 660-660, 799-799, 1000-1000
tests/integration/auth/client/cli/suite_test.go (1)
- 60-60: The addition of
codectestutil.CodecOptions{}in theMakeTestEncodingConfigcall is a significant change. This modification aligns with the PR's objective to enhance modularity by allowing more flexible configuration of encoding options. It's crucial to ensure that this change does not inadvertently affect the encoding behavior expected in other parts of the test suite or the application.UPGRADING.md (59)
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [4-4]
The term "SimApp" is correctly used in the context of Cosmos SDK and does not require correction.
- 19-19: The diff block is correctly formatted for showing the changes in code. The mention of "clientCtx" is appropriate in the context of demonstrating how to apply the new configurations.
- 30-30: The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [41-41]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [45-45]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [55-55]
The merging of
GasConsumptionDecoratorandIncreaseSequenceDecoratorwithSigVerificationDecoratoris a significant change that affects the AnteHandler setup. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [63-63]
The term "nonces" is correctly used in the context of blockchain transactions and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [68-68]
The code snippet provided for setting up unordered transactions is correctly formatted and provides valuable guidance for enabling this feature.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [99-99]
The term "SnapshotManager" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [100-100]
The term "manager" is correctly used in the context of managing snapshots within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [111-111]
The term "tx" is a common abbreviation for "transaction" in blockchain contexts and is correctly used here.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [135-135]
The term "Protobuf" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [137-137]
The term "CometBFT" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [148-148]
The term "appmodule" is correctly used in the context of Cosmos SDK modules and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [165-165]
The term "Params" is correctly used in the context of Cosmos SDK modules and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [171-171]
The change from
sdk.Contexttocontext.Contextin genesis interfaces is a significant update that aligns with Go's standard context package, enhancing the modularity and interoperability of the Cosmos SDK.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [187-187]
The migration to collections is a notable improvement that simplifies the API and enhances the efficiency of Cosmos SDK modules. This change is correctly highlighted and is crucial for developers to understand.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [192-192]
The term "Auth" is correctly used in the context of the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [195-195]
The term "Authz" is correctly used in the context of the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [206-206]
The term "protocolpool" is correctly used in the context of the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [226-226]
The term "parameters" is more appropriate than "params" in formal documentation. However, in the context of module names within the Cosmos SDK, "params" is the correct term.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [230-230]
The term "protocolpool" is correctly used in the context of the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [263-263]
The term "CometBFT" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [280-280]
The term "CometBFT" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [303-303]
The term "BaseApp" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [317-317]
The term "DeliverTx" is correctly used in the context of Cosmos SDK transactions and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [325-325]
The term "SetPreBlocker" is correctly used in the context of Cosmos SDK's BaseApp configuration and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [328-328]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [342-342]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [364-364]
The term "abci.TxResult" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [367-367]
The term "BeginBlock" and "EndBlock" are correctly used in the context of Cosmos SDK block execution and do not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [369-369]
The term "FinalizeBlock" is correctly used in the context of Cosmos SDK block execution and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [398-398]
The term "Protobuf" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [400-400]
The migration from
gogo/protobuftocosmos/gogoprotois a significant change that affects how protobuf files are generated and used within the Cosmos SDK. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [408-408]
The removal of the
gogoproto.goproto_stringer = falseannotation and the implications for the generatedString()methods are important for developers to understand, especially regarding their usage in state-machine code.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [411-411]
The term "SimApp" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [418-418]
The relaxation of module assertions in the Cosmos SDK enhances flexibility in application wiring. This change is correctly highlighted and is crucial for developers to understand.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [422-422]
The update to modules'
NewKeeperfunctions to take aKVStoreServiceinstead of aStoreKeyis a significant change that affects how modules interact with the store. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [438-438]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [440-440]
The guidance for manually wiring chains to use the
runtime.NewKVStoreServicemethod is valuable for developers who are not using dependency injection. This information is crucial for ensuring correct module configuration.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [453-453]
The term "CometBFT" is correctly used in the context of Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [455-455]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [476-476]
The term "ModuleBasics" is correctly used in the context of Cosmos SDK modules and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [495-495]
The guidance for manually wiring chains to use the
module.NewBasicManagerFromManagerfunction is valuable for developers who are not using dependency injection. This information is crucial for ensuring correct module configuration.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [544-544]
The term "store" is correctly used in the context of Cosmos SDK modules and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [551-551]
The guidance for continuing to use state streaming by replacing
streaming.LoadStreamingServicesis valuable for developers upgrading their applications. This information is crucial for ensuring correct streaming service configuration.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [561-561]
The change in the return type of
TxConfig.SignModeHandler()fromx/auth/signing.SignModeHandlertox/tx/signing.HandlerMapis a significant update that affects how transactions are signed. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [565-565]
The introduction of textual sign mode in the SDK is a notable feature that enhances the readability of signed transactions. This information is crucial for developers to understand and utilize in their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [590-590]
The term "depinject" is correctly used in the context of dependency injection within the Cosmos SDK and does not require correction.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [609-609]
The guidance for using
depinject/app v2with the textual sign mode is valuable for developers upgrading their applications. This information is crucial for ensuring correct transaction configuration.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [617-617]
The simplification of the
sdk.Msginterface by removing the requirement for theValidateBasicmethod is a significant change that affects how messages are validated. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [683-683]
The update to
authz.NewMsgGrantandauthz.NewGrantto handle expiration and block time is a significant change that affects how grants are created and validated. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1091-1091]
The keyring refactor in v0.46, including changes to interfaces and serialization, is a significant update that affects how keys are managed and used within the Cosmos SDK. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1105-1105]
The introduction of a
postHandleras a counterpart to theanteHandleris a significant change that affects how transactions are processed. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1109-1109]
The introduction of the "fast" index in IAVL v0.19.0 is a significant improvement that enhances query and iteration performance. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1130-1130]
The deprecation of the
x/paramsmodule in favor of module-specific parameter handling is a significant change that affects how parameters are managed and used within the Cosmos SDK. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1136-1136]
The update to the
govmodule, including the introduction ofv1and changes to proposal submission, is a significant change that affects governance functionality. This information is crucial for developers upgrading their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1143-1143]
The addition of a message type to cancel unbonding delegations in the
stakingmodule is a notable feature that enhances flexibility for users. This information is crucial for developers to understand and utilize in their applications.
- 12-30: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1145-1145]
The migration from
gogo/protobuftocosmos/gogoprotoand the use ofbuffor managing protobuf dependencies are significant changes that affect how protobuf files are generated and used within the Cosmos SDK. This information is crucial for developers upgrading their applications.x/bank/keeper/keeper_test.go (3)
- 30-30: The import of
codectestutilis added to facilitate codec testing. Ensure that this package provides the necessary utilities for encoding and decoding, and that it's being used appropriately in the tests.- 133-133: The usage of
codectestutil.CodecOptions{}inMakeTestEncodingConfigcall is correct and aligns with the goal of configuring encoding options for testing purposes. This change ensures that tests can be configured with specific codec options, enhancing test flexibility and accuracy.- 307-307: The modification to use
codectestutil.CodecOptions{}inMakeTestEncodingConfigwithin theNewKeeperWithAuthorityfunction is appropriate. It demonstrates a consistent approach to configuring encoding options across different test setups, which is crucial for maintaining test reliability and ease of understanding.CHANGELOG.md (2)
- 97-97: The documentation of the change to
module.testutil.MakeTestEncodingConfigis clear and accurately reflects the modifications made in the codebase. It's good practice to link the PR for easy reference.- 97-97: The documentation of the removal of
module.BasicManagerand related functions is concise and informative. It clearly explains the rationale behind the change and how to adapt to it. For clarity, it might be helpful to include an example or a link to documentation on how to inject the module manager using the app config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (13)
- CHANGELOG.md (1 hunks)
- baseapp/abci_utils_test.go (2 hunks)
- client/grpc_query_test.go (2 hunks)
- simapp/app.go (5 hunks)
- tests/integration/distribution/keeper/msg_server_test.go (3 hunks)
- tests/integration/gov/keeper/keeper_test.go (3 hunks)
- testutil/integration/router.go (3 hunks)
- x/distribution/keeper/allocation_test.go (4 hunks)
- x/distribution/keeper/delegation_test.go (10 hunks)
- x/distribution/keeper/keeper_test.go (2 hunks)
- x/distribution/migrations/v4/migrate_funds_test.go (2 hunks)
- x/distribution/migrations/v4/migrate_test.go (2 hunks)
- x/gov/keeper/common_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (13)
- CHANGELOG.md
- baseapp/abci_utils_test.go
- client/grpc_query_test.go
- simapp/app.go
- tests/integration/distribution/keeper/msg_server_test.go
- tests/integration/gov/keeper/keeper_test.go
- testutil/integration/router.go
- x/distribution/keeper/allocation_test.go
- x/distribution/keeper/delegation_test.go
- x/distribution/keeper/keeper_test.go
- x/distribution/migrations/v4/migrate_funds_test.go
- x/distribution/migrations/v4/migrate_test.go
- x/gov/keeper/common_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- tests/integration/evidence/keeper/infraction_test.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/integration/evidence/keeper/infraction_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- client/v2/autocli/common_test.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- client/v2/autocli/common_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (4)
- simapp/app.go (5 hunks)
- x/authz/keeper/genesis_test.go (2 hunks)
- x/authz/keeper/keeper_test.go (2 hunks)
- x/authz/module/abci_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- simapp/app.go
- x/authz/keeper/genesis_test.go
- x/authz/keeper/keeper_test.go
- x/authz/module/abci_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (4)
- simapp/app.go (5 hunks)
- x/group/keeper/genesis_test.go (2 hunks)
- x/group/keeper/grpc_query_test.go (2 hunks)
- x/group/keeper/keeper_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- simapp/app.go
- x/group/keeper/genesis_test.go
- x/group/keeper/grpc_query_test.go
- x/group/keeper/keeper_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 25
Configuration used: .coderabbit.yml
Files selected for processing (4)
- UPGRADING.md (1 hunks)
- simapp/app.go (5 hunks)
- tests/integration/gov/keeper/keeper_test.go (3 hunks)
- x/gov/keeper/common_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- simapp/app.go
- tests/integration/gov/keeper/keeper_test.go
- x/gov/keeper/common_test.go
julienrbrt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! nice work!
Can add in codetestutil.CodecOptions address codecs as well?
And make prefixes and codec mutually exclusive there, as not everyone may want to use bech32 addresses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- codec/testutil/codec.go (1 hunks)
Additional comments: 6
codec/testutil/codec.go (6)
- 14-21: The
CodecOptionsstruct introduces a way to specify address and validator codecs or prefixes for creating codecs. This approach enhances flexibility in codec configuration, allowing for either direct codec specification or prefix-based codec creation. This change supports the PR's goal of reducing global state dependencies by enabling more explicit codec management.- 24-30: The
NewCodecOptionsWithPrefixesfunction provides a straightforward way to createCodecOptionsusing address and validator prefixes. This method simplifies the process of configuring codecs based on prefixes, aligning with the PR's objectives of enhancing modularity and configurability.- 32-38: The
NewCodecOptionsWithCodecsfunction allows for the creation ofCodecOptionswith pre-defined address and validator codecs. This method further supports the PR's aim of reducing global configuration by enabling explicit codec management, offering an alternative to prefix-based codec creation.- 40-52: The
NewInterfaceRegistrymethod withinCodecOptionsutilizes the provided address and validator codecs if available, otherwise defaults to creating new ones based on prefixes. This implementation ensures that codec configuration is flexible and adaptable, supporting the PR's goal of enhancing modularity. However, it's crucial to ensure that the panic on error is acceptable in this context, as it might halt execution unexpectedly in test environments.Consider verifying the appropriateness of using
panicfor error handling in this context. In production code, it might be preferable to return an error and handle it gracefully.
- 61-74: The
GetAddressCodecmethod provides a mechanism to retrieve or create an address codec based on theAccAddressPrefix. This method ensures that a codec is always available, either from the providedAddressCodecor by creating a new one using the specified prefix. This approach aligns with the PR's objectives by enabling more explicit and configurable codec management.- 76-89: The
GetValidatorCodecmethod functions similarly toGetAddressCodec, providing a way to retrieve or create a validator codec based on theValAddressPrefix. This method supports the PR's goals by ensuring that codec management is explicit and configurable, allowing for the use of provided codecs or the creation of new ones based on prefixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- x/auth/signing/adapter_test.go (1 hunks)
Additional comments: 2
x/auth/signing/adapter_test.go (2)
- 11-11: The addition of
codectestutilimport aligns with the PR's objectives to enhance codec testing utilities and reduce global config dependencies. This change is appropriate and improves modularity.- 18-18: Modifying the
MakeTestEncodingConfigfunction call to acceptcodectestutil.CodecOptions{}as an argument is a key change that aligns with the PR's objectives. This enhances the flexibility and modularity of configuration management within the Cosmos SDK.
Description
ref:
#7448
This PR removes the use of the global config in x/auth and client package.
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.
I have...
Summary by CodeRabbit