refactor: decouple from sdk/codec in amino codec usage #20369
Conversation
WalkthroughWalkthroughThe changes involve updating import paths and parameter types across various files in the repository to align with the new Changes
Possibly related issues
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 (
|
github-actions
bot
added
C:x/staking
C:x/gov
C:x/slashing
C:x/distribution
x/slashing/keeper/keeper.go
Outdated
There was a problem hiding this comment.
*codec.LegacyAmino can be removed from the constructor here as it's unused, wdyt?
kocubinski
force-pushed
the
kocu/rf-codec-usage
branch
from
c9df1e4
to
76546e1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (57)
- codec/amino.go (3 hunks)
- codec/amino_codec_test.go (1 hunks)
- codec/depinject.go (1 hunks)
- codec/legacy/amino_msg.go (1 hunks)
- core/legacy/amino.go (1 hunks)
- crypto/codec/amino.go (2 hunks)
- crypto/keyring/codec.go (1 hunks)
- crypto/keys/multisig/codec.go (1 hunks)
- crypto/ledger/amino.go (1 hunks)
- runtime/app.go (2 hunks)
- runtime/module.go (6 hunks)
- simapp/app_di.go (3 hunks)
- simapp/simd/cmd/root_di.go (3 hunks)
- std/codec.go (2 hunks)
- tests/sims/authz/operations_test.go (2 hunks)
- tests/sims/feegrant/operations_test.go (2 hunks)
- tests/sims/slashing/operations_test.go (2 hunks)
- testutil/network/network.go (3 hunks)
- types/codec.go (2 hunks)
- types/module/core_module.go (2 hunks)
- types/module/module.go (3 hunks)
- x/auth/ante/testutil_test.go (1 hunks)
- x/auth/migrations/legacytx/codec.go (1 hunks)
- x/auth/module.go (3 hunks)
- x/auth/types/codec.go (1 hunks)
- x/auth/vesting/module.go (2 hunks)
- x/auth/vesting/types/codec.go (1 hunks)
- x/authz/codec.go (1 hunks)
- x/authz/module/module.go (3 hunks)
- x/bank/module.go (2 hunks)
- x/bank/types/codec.go (1 hunks)
- x/consensus/module.go (2 hunks)
- x/consensus/types/codec.go (2 hunks)
- x/crisis/module.go (2 hunks)
- x/crisis/types/codec.go (1 hunks)
- x/distribution/module.go (2 hunks)
- x/distribution/types/codec.go (1 hunks)
- x/epochs/module.go (2 hunks)
- x/evidence/module.go (2 hunks)
- x/evidence/types/codec.go (1 hunks)
- x/feegrant/codec.go (1 hunks)
- x/feegrant/module/module.go (2 hunks)
- x/gov/module.go (2 hunks)
- x/gov/types/v1/codec.go (1 hunks)
- x/gov/types/v1beta1/codec.go (1 hunks)
- x/group/codec.go (1 hunks)
- x/group/module/module.go (2 hunks)
- x/mint/module.go (2 hunks)
- x/mint/types/codec.go (1 hunks)
- x/params/keeper/common_test.go (1 hunks)
- x/params/module.go (2 hunks)
- x/params/types/proposal/codec.go (1 hunks)
- x/protocolpool/module.go (2 hunks)
- x/slashing/depinject.go (2 hunks)
- x/slashing/keeper/keeper.go (1 hunks)
- x/slashing/module.go (2 hunks)
- x/slashing/types/codec.go (1 hunks)
Files not processed due to max files limit (4)
- x/staking/module.go
- x/staking/types/codec.go
- x/upgrade/module.go
- x/upgrade/types/codec.go
Files not reviewed due to errors (1)
- testutil/network/network.go (no review received)
Files skipped from review due to trivial changes (7)
- crypto/keyring/codec.go
- tests/sims/authz/operations_test.go
- tests/sims/feegrant/operations_test.go
- tests/sims/slashing/operations_test.go
- types/module/core_module.go
- x/auth/ante/testutil_test.go
- x/slashing/depinject.go
Additional Context Used
Path-based Instructions (50)
x/auth/migrations/legacytx/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.crypto/ledger/amino.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.core/legacy/amino.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/params/types/proposal/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.codec/legacy/amino_msg.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.std/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/consensus/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/slashing/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/crisis/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.crypto/keys/multisig/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/evidence/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.crypto/codec/amino.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/params/keeper/common_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/bank/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/types/v1beta1/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.codec/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/auth/vesting/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/feegrant/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/types/v1/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/authz/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/auth/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/auth/vesting/types/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/consensus/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/group/codec.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/params/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.codec/amino_codec_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/protocolpool/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/epochs/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/simd/cmd/root_di.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/feegrant/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/crisis/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.codec/amino.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/evidence/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/slashing/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/authz/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/group/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/auth/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/slashing/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/app_di.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.testutil/network/network.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.types/module/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (55)
x/auth/migrations/legacytx/codec.go (1)
7-8: LGTM! The function correctly uses the newlegacy.Aminointerface for codec registration.core/legacy/amino.go (1)
3-14: TheAminointerface andInterfaceOptionsstruct are well-defined and align with the PR's objectives.x/params/types/proposal/codec.go (1)
10-11: LGTM! The function correctly uses the newlegacy.Aminointerface for codec registration.codec/legacy/amino_msg.go (1)
14-18: LGTM! The function correctly uses the newlegacy.Aminointerface and includes a necessary check for message name length.std/codec.go (1)
14-14: LGTM! The function correctly uses the newlegacy.Aminointerface for codec registration.x/consensus/types/codec.go (1)
23-23: LGTM! The function correctly uses the newlegacy.Aminointerface for codec registration.x/mint/types/codec.go (2)
13-14: The migration tocorelegacy.AminoinRegisterLegacyAminoCodecaligns well with the PR's objectives. Good use of the new interface.
4-4: Ensure that all imported packages are used within the file to avoid unnecessary dependencies.Verification successful
The import
corelegacy "cosmossdk.io/core/legacy"is indeed used in the filex/mint/types/codec.go, specifically in the functionRegisterLegacyAminoCodec. This confirms that the import is necessary and not an unused dependency.Usage of
corelegacyimport confirmed inx/mint/types/codec.go:
- The alias
corelegacyis used in the declaration of the functionRegisterLegacyAminoCodec.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for unused imports in x/mint/types/codec.go # Test: List unused imports. Expect: No output if all imports are used. ast-grep --lang go --pattern $'import $$$ { $_ }' x/mint/types/codec.goLength of output: 417
Script:
#!/bin/bash # Description: Check for usage of the import 'corelegacy' in x/mint/types/codec.go # Test: Search for the alias 'corelegacy'. Expect: Some output if the import is used. rg 'corelegacy' x/mint/types/codec.goLength of output: 130
x/slashing/types/codec.go (2)
13-14: The updates toRegisterLegacyAminoCodecusingcorelegacy.Aminoare correctly implemented. This is consistent with the PR's goal of reducing dependencies onsdk/codec.
4-4: Check for any unused imports to maintain clean and efficient code.x/crisis/types/codec.go (2)
14-14: The refactoring ofRegisterLegacyAminoCodecto usecorelegacy.Aminoand the removal of thenilargument are well-executed, enhancing simplicity and clarity.
4-4: Verify that all imports are necessary and used within the file to avoid bloating the codebase.Verification successful
The verification process has confirmed that the import
corelegacy "cosmossdk.io/core/legacy"in the filex/crisis/types/codec.gois indeed used within the file. Specifically, it is utilized in the functionRegisterLegacyAminoCodec, which takes a parameter of typecorelegacy.Amino. This indicates that the import is necessary and not contributing to code bloat.
- The import is used in the function definition of
RegisterLegacyAminoCodecon line 6.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for unused imports in x/crisis/types/codec.go # Test: List unused imports. Expect: No output if all imports are used. ast-grep --lang go --pattern $'import $$$ { $_ }' x/crisis/types/codec.goLength of output: 419
Script:
#!/bin/bash # Correcting the pattern to check for the specific import and its usage in the file # First, confirm the import statement exists ast-grep --lang go --pattern $'import $$$ { "cosmossdk.io/core/legacy" as corelegacy }' x/crisis/types/codec.go # Then, check if 'corelegacy' is used anywhere in the file rg 'corelegacy' x/crisis/types/codec.goLength of output: 723
x/evidence/types/codec.go (2)
15-18: The updates toRegisterLegacyAminoCodecare correctly implemented, usingcorelegacy.Aminofor registering evidence types and messages. This aligns with the PR's objectives of decoupling fromsdk/codec.
4-4: Ensure that all imports are utilized to maintain a clean codebase.crypto/codec/amino.go (2)
16-33: The refactoring ofRegisterCryptoto uselegacy.Aminoand the removal of thenilargument in registration calls are well-executed. This enhances the clarity and simplicity of the code.
6-7: Verify that all imports are necessary and used within the file to maintain a clean and efficient codebase.Verification successful
The verification process has confirmed that all the imports listed in
crypto/codec/amino.goare indeed utilized within the file. Each import is referenced in the functionRegisterCrypto, where various cryptographic types and keys are registered. This ensures that the imports are necessary for the functionality provided by this file.
- GitHub.com/cometbft/cometbft/crypto/sr25519 is used for registering
sr25519.PubKeyandsr25519.PrivKey.- Cosmossdk.io/core/legacy is used as
legacy.Aminoin the function signature.- GitHub.com/cosmos/cosmos-sdk/crypto/keys/ed25519 is used for registering
ed25519.PubKeyanded25519.PrivKey.- GitHub.com/cosmos/cosmos-sdk/crypto/keys/multisig (aliased as
kmultisig) is used for registeringkmultisig.LegacyAminoPubKey.- GitHub.com/cosmos/cosmos-sdk/crypto/keys/secp256k1 is used for registering
secp256k1.PubKeyandsecp256k1.PrivKey.- GitHub.com/cosmos/cosmos-sdk/crypto/types (aliased as
cryptotypes) is used for registering interfaces and concrete types related to public and private keys.Given this information, the imports are verified to be necessary and used appropriately within the file.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for unused imports in crypto/codec/amino.go # Test: List unused imports. Expect: No output if all imports are used. ast-grep --lang go --pattern $'import $$$ { $_ }' crypto/codec/amino.goLength of output: 417
Script:
#!/bin/bash # Description: Check for unused imports in crypto/codec/amino.go using a corrected pattern # Test: List unused imports. Expect: No output if all imports are used. ast-grep --lang go --pattern $'import $$$' crypto/codec/amino.goLength of output: 622
Script:
#!/bin/bash # Description: Verify the usage of each import in crypto/codec/amino.go # Check for references to each import namespace or specific functions/classes grep -E "sr25519|legacy|ed25519|multisig|secp256k1|cryptotypes" crypto/codec/amino.goLength of output: 1015
x/bank/types/codec.go (2)
14-21: The updates toRegisterLegacyAminoCodecusingcorelegacy.Aminofor the bank module are correctly implemented. This aligns with the PR's objectives of reducing dependencies onsdk/codec.
4-4: Ensure that all imports are utilized to maintain a clean codebase.Verification successful
The verification process has confirmed that the import
corelegacy "cosmossdk.io/core/legacy"is indeed utilized within the filex/bank/types/codec.go. Thergcommand output indicates that there is at least one usage of thecorelegacynamespace in the file. This confirms that the import is not redundant and is necessary for the functionality of the code in this file.Conclusion:
- The import
corelegacy "cosmossdk.io/core/legacy"is used in the filex/bank/types/codec.go.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for unused imports in x/bank/types/codec.go # Test: List unused imports. Expect: No output if all imports are used. ast-grep --lang go --pattern $'import $$$ { $_ }' x/bank/types/codec.goLength of output: 417
Script:
#!/bin/bash # Description: Verify if the import 'corelegacy "cosmossdk.io/core/legacy"' is used in x/bank/types/codec.go # Step 1: Find the import statement for 'corelegacy' echo "Checking for 'corelegacy' import:" ast-grep --lang go --pattern $'import $$$ { "cosmossdk.io/core/legacy" as corelegacy }' x/bank/types/codec.go # Step 2: Check if 'corelegacy' is used anywhere in the file echo "Checking for usage of 'corelegacy':" rg --count 'corelegacy\.' x/bank/types/codec.goLength of output: 810
x/gov/types/v1beta1/codec.go (1)
14-20: Updated function signature to usecorelegacy.Aminoand correctly registers governance-related types with the codec.codec/depinject.go (1)
13-43: Introduced utility functions for creating and managing codecs and interface registries, enhancing modularity and maintainability.x/auth/vesting/module.go (1)
41-42: Updated method to use the newlegacy.Aminointerface, correctly delegating type registration to thetypespackage.x/distribution/types/codec.go (1)
15-22: Updated function signature to usecorelegacy.Aminoand correctly registers distribution-related types with the codec.x/feegrant/codec.go (1)
14-21: Updated function signature to usecorelegacy.Aminoand correctly registers fee grant-related types with the codec.x/gov/types/v1/codec.go (1)
14-14: Updated function signature to usecorelegacy.Aminoand correctly registers governance-related types with the codec.x/authz/codec.go (1)
16-22: Updated function signature to usecorelegacy.Aminoand correctly registers authz-related types with the codec.x/auth/types/codec.go (1)
16-23: Updated function signature to usecorelegacy.Aminoand correctly registers auth-related types with the codec.x/auth/vesting/types/codec.go (1)
17-23: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.x/consensus/module.go (1)
53-53: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.x/group/codec.go (1)
15-18: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.x/params/module.go (1)
55-55: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.codec/amino_codec_test.go (1)
20-21: Changes in test setup align with the main codebase modifications. Ensure comprehensive coverage of new interface methods if applicable.Verification successful
The provided output from the shell scripts has given us a comprehensive view of the test files related to the Amino codec, including specific tests for various functionalities and error handling scenarios. The tests cover a wide range of cases, ensuring that the Amino codec's behavior is thoroughly verified under different conditions.
From the detailed test cases provided in the output, it is evident that there is comprehensive test coverage for the Amino codec, including its new interface methods. The tests check for correct behavior, error handling, and equivalence between different encoding methods, which aligns with best practices in software testing.
Given this information, it can be concluded that the initial concern about ensuring comprehensive test coverage for new interface methods has been addressed adequately.
Final Response:
Comprehensive test coverage for the Amino codec's new interface methods is confirmed. The tests include various scenarios ensuring robust validation of functionality and error handling.
- Files with relevant tests:
codec/amino_codec_test.gocodec/legacy/amino_msg_test.gofuzz/tests/tendermint_amino_decodetime_test.gotests/integration/tx/aminojson/aminojson_test.gox/tx/signing/aminojson/aminojson_test.goScripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify comprehensive test coverage for new interface methods. # Test: Search for test cases covering new methods. Expect: At least one test case per method. rg --type go 'Test.*legacy.Amino'Length of output: 35
Script:
#!/bin/bash # Description: Broaden the search to find any test files related to the Amino codec. # Search for filenames that might indicate they are test files for the Amino codec. fd --type f 'amino.*test.*\.go$' # Additionally, list the contents of files that likely contain tests for the Amino codec to manually inspect for coverage of new methods. fd --type f 'amino.*test.*\.go$' --exec cat {}Length of output: 30167
x/protocolpool/module.go (1)
64-64: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.x/epochs/module.go (1)
59-59: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.simapp/simd/cmd/root_di.go (1)
Line range hint
104-121: Changes align well with the PR's objectives of interface abstraction and simplification of method signatures.x/feegrant/module/module.go (1)
70-70: Change to uselegacy.Aminointerface aligns with decoupling efforts.codec/amino.go (4)
28-29: Implementation oflegacy.Aminointerface is correctly done.
36-38: Simplification ofRegisterEvidencesby removingnilargument is a good improvement.
182-190: Refactoring ofRegisterInterfaceto uselegacy.InterfaceOptionsis consistent with the new interface design.
193-194: Removal ofnilargument inRegisterConcretesimplifies the method signature.x/evidence/module.go (1)
69-69: Change to uselegacy.Aminointerface aligns with decoupling efforts.Verification successful
The investigation into the usage of
corelegacy.Aminoacross the codebase reveals that it is an alias forcosmossdk.io/core/legacyand is used consistently in various modules. This usage appears to be a deliberate design choice, likely intended to handle specific legacy operations within the Cosmos SDK. There is no indication that this usage is incorrect or outdated. Therefore, the initial concern about the need to update all occurrences tolegacy.Aminois not applicable ascorelegacy.Aminoserves a distinct purpose.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `RegisterLegacyAminoCodec` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'RegisterLegacyAminoCodec'Length of output: 11946
Script:
#!/bin/bash # Description: Check the definition or relationship of `corelegacy.Amino` to understand if it's part of the new interface or an old one that needs updating. # Search for the definition or relevant comments about `corelegacy.Amino`. rg --type go "corelegacy.Amino"Length of output: 1281
Script:
#!/bin/bash # Description: Search for the definition or documentation of `corelegacy.Amino` to clarify its relationship with `legacy.Amino`. # Search for the definition or documentation of `corelegacy.Amino`. rg --type go -A 5 "type Amino" | rg "corelegacy"Length of output: 48
Script:
#!/bin/bash # Description: Broaden the search to find any references or documentation related to `corelegacy` to understand its context and usage in the codebase. # Search for any references or documentation related to `corelegacy`. rg --type go "corelegacy"Length of output: 2285
x/slashing/keeper/keeper.go (1)
23-24: LegacyAmino is deprecated but retained for backward compatibility.x/authz/module/module.go (1)
97-97: Change to uselegacy.Aminointerface aligns with decoupling efforts.Verification successful
The verification confirms that the function
RegisterLegacyAminoCodecconsistently uses thelegacy.Aminointerface across various modules in the codebase. This aligns with the intended refactoring efforts to standardize the use of this interface.
- x/upgrade/module.go
- x/staking/module.go
- x/slashing/module.go
- x/protocolpool/module.go
- x/params/module.go
- x/group/module/module.go
- x/mint/module.go
- x/gov/module.go
- x/feegrant/module/module.go
- x/evidence/module.go
- x/distribution/module.go
- x/crisis/module.go
- x/consensus/module.go
- x/authz/module/module.go
- x/auth/module.go
- x/auth/vesting/module.go
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `RegisterLegacyAminoCodec` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'RegisterLegacyAminoCodec'Length of output: 11946
x/group/module/module.go (1)
92-92: Change to uselegacy.Aminointerface aligns with decoupling efforts.Verification successful
The verification process confirms that the function
RegisterLegacyAminoCodecis consistently used with thelegacy.Aminointerface across the codebase. This aligns with the intended changes and confirms the correct implementation of the new function signature.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `RegisterLegacyAminoCodec` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'RegisterLegacyAminoCodec'Length of output: 11946
x/auth/module.go (1)
72-72: Change to uselegacy.Aminointerface aligns with decoupling efforts.Verification successful
All instances of
RegisterLegacyAminoCodecacross the codebase correctly use the newlegacy.Aminointerface as the parameter. This confirms that the function signature change has been consistently applied.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `RegisterLegacyAminoCodec` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go $'RegisterLegacyAminoCodec'Length of output: 11946
x/mint/module.go (1)
81-81: Change from*codec.LegacyAminotolegacy.Aminois correctly implemented.x/bank/module.go (1)
67-67: Change from*codec.LegacyAminotolegacy.Aminois correctly implemented.x/slashing/module.go (1)
85-85: Change from*codec.LegacyAminotolegacy.Aminois correctly implemented.x/distribution/module.go (1)
79-79: Change from*codec.LegacyAminotolegacy.Aminois correctly implemented.x/gov/module.go (1)
83-83: Change from*codec.LegacyAminotolegacy.Aminofor bothv1andv1beta1namespaces is correctly implemented.runtime/app.go (1)
50-50: Introduction oflegacy.Aminoin theAppstruct is correctly implemented and aligns with the PR's objectives.runtime/module.go (1)
166-166: Introduction oflegacy.Aminoin the dependency injection system is correctly implemented and aligns with the PR's objectives.simapp/app_di.go (2)
15-15: Import ofcosmossdk.io/core/legacyaligns with the PR's objective to decouple fromsdk/codec.
71-71: Change oflegacyAminotype from*codec.LegacyAminotolegacy.Aminois consistent with the broader refactoring to use the new interface, enhancing modularity.types/module/module.go (2)
81-81: Change to uselegacy.AminoinHasAminoCodecinterface aligns with decoupling efforts and is correctly implemented.
302-302: Update ofRegisterLegacyAminoCodecmethod to acceptlegacy.Aminois correctly implemented and necessary for interface compatibility.
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (45)
UPGRADING.md (45)
Line range hint
10-10: Add a missing comma for clarity.- In this section we describe the changes made in Cosmos SDK' SimApp. + In this section, we describe the changes made in Cosmos SDK's SimApp.
Line range hint
150-150: Add a missing comma for clarity.- The `cosmossdk.io/api/tendermint` package has been removed as CometBFT now publishes its protos to... + The `cosmossdk.io/api/tendermint` package has been removed, as CometBFT now publishes its protos to...
Line range hint
153-153: Revise the sentence for better readability.- If you were depending on `cosmossdk.io/api/tendermint`, please use the buf generated go SDK instead, or ask CometBFT host the generated proto v2 code. + If you were relying on `cosmossdk.io/api/tendermint`, please use the buf generated go SDK instead, or request CometBFT to host the generated proto v2 code.
Line range hint
163-163: Revise the sentence to use the infinitive form of the verb.- we strongly recommend to use the `cosmossdk.io/core/appmodule` interfaces for the modules. + we strongly recommend using the `cosmossdk.io/core/appmodule` interfaces for the modules.
Line range hint
207-207: Add a missing comma for clarity.- Previously `cosmossdk.io/core` held functions `Invoke`, `Provide` and `Register` were moved to `cosmossdk.io/depinject/appconfig`. + Previously, `cosmossdk.io/core` held functions `Invoke`, `Provide`, and `Register` were moved to `cosmossdk.io/depinject/appconfig`.
Line range hint
213-213: Consider rephrasing for clarity.- It is required to migrate to v0.50 prior to upgrading to v0.51 for not missing any module migrations. + It is required to migrate to v0.50 before upgrading to v0.51 to ensure no module migrations are missed.
Line range hint
234-234: Correct the determiner to match the plural noun.- Many functions have been removed due to this changes as the API can be smaller thanks to collections. + Many functions have been removed due to these changes as the API can be smaller thanks to collections.
Line range hint
246-246: Add a missing article for grammatical correctness.- Bank was spun out into its own `go.mod`. + Bank was spun out into its own `go.mod`.
Line range hint
282-282: Add a missing comma for clarity.- A standalone Go module was created and it is accessible at "cosmossdk.io/x/params". + A standalone Go module was created, and it is accessible at "cosmossdk.io/x/params".
Line range hint
317-317: Add a missing comma for clarity.- The Cosmos SDK has migrated in its previous versions, to CometBFT. + The Cosmos SDK has migrated in its previous versions to CometBFT.
Line range hint
350-350: Add a missing comma for clarity.- For existing chains this can be done in two ways: + For existing chains, this can be done in two ways:
Line range hint
352-352: Add a missing comma after the introductory phrase.- During an upgrade the value is set in an upgrade handler. + During an upgrade, the value is set in an upgrade handler.
Line range hint
359-359: Add a missing comma for clarity.- where a catastrophic failure has occurred and the application should halt. + where a catastrophic failure has occurred, and the application should halt.
Line range hint
361-361: Correct the typographical error.- will gracefully by handled by `BaseApp` on behalf of the application. + will gracefully be handled by `BaseApp` on behalf of the application.
Line range hint
366-366: Consider a shorter alternative to avoid wordiness.- `FinalizeBlock` is public and should be used in order to test and run operations. + `FinalizeBlock` is public and should be used to test and run operations.
Line range hint
379-379: Revise the sentence for better readability.- This is essential for BaseApp to run `PreBlock` which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics. + This is essential for BaseApp to run `PreBlock`, which operates before the begin blocker of other modules, allowing modifications to consensus parameters that are visible to subsequent state machine logics.
Line range hint
417-417: Add a missing comma after the introductory phrase.- Instead a new attribute is added to all messages indicating the `msg_index` which identifies which events and attributes relate the same transaction. + Instead, a new attribute is added to all messages indicating the `msg_index`, which identifies which events and attributes relate to the same transaction.
Line range hint
418-418: Add a missing preposition for clarity.- identifies which events and attributes relate the same transaction. + identifies which events and attributes relate to the same transaction.
Line range hint
444-444: Correct the verb form for consistency.- Use `confix` to clean-up your `app.toml`. + Use `confix` to clean up your `app.toml`.
Line range hint
448-448: Correct the article for grammatical accuracy.- To migrate from a unsupported database to a supported database please use a database migration tool. + To migrate from an unsupported database to a supported database, please use a database migration tool.
Line range hint
448-448: Add a missing comma for clarity.- To migrate from an unsupported database to a supported database please use a database migration tool. + To migrate from an unsupported database to a supported database, please use a database migration tool.
Line range hint
465-465: Add a missing comma for clarity.- In this section we describe the changes made in Cosmos SDK' SimApp. + In this section, we describe the changes made in Cosmos SDK's SimApp.
Line range hint
465-465: Correct the possessive form.- In this section, we describe the changes made in Cosmos SDK' SimApp. + In this section, we describe the changes made in Cosmos SDK's SimApp.
Line range hint
529-529: Add a missing comma for clarity.- The global variable has been removed and the basic module manager can be now created from the module manager. + The global variable has been removed, and the basic module manager can now be created from the module manager.
Line range hint
565-565: Add a missing comma after the introductory phrase.- Additionally `AutoCLI` automatically adds the custom modules commands to the root command for all modules implementing the [`appmodule.AppModule`](https://pkg.go.dev/cosmossdk.io/core/appmodule#AppModule) interface. + Additionally, `AutoCLI` automatically adds the custom modules commands to the root command for all modules implementing the [`appmodule.AppModule`](https://pkg.go.dev/cosmossdk.io/core/appmodule#AppModule) interface.
Line range hint
596-596: Add a missing preposition for grammatical accuracy.- a separate go.mod file which allows it be a standalone module. + a separate go.mod file which allows it to be a standalone module.
Line range hint
601-601: Add a missing bracket for syntactical correctness.- [ADR-38](https://docs.cosmos.network/main/architecture/adr-038-state-listening) has been implemented in the SDK. + [ADR-38](https://docs.cosmos.network/main/architecture/adr-038-state-listening) has been implemented in the SDK.
Line range hint
617-617: Correct the hyphenation for consistency.- more human readable output, currently only available on Ledger devices but soon to be implemented in other UIs. + more human-readable output, currently only available on Ledger devices but soon to be implemented in other UIs.
Line range hint
661-661: Correct the article for grammatical accuracy.- When using `depinject` / `app v2`, the a tx config should be recreated from the `txConfigOpts` to use `NewGRPCCoinMetadataQueryFn` instead of depending on the bank keeper (that is used in the server). + When using `depinject` / `app v2`, the tx config should be recreated from the `txConfigOpts` to use `NewGRPCCoinMetadataQueryFn` instead of depending on the bank keeper (that is used in the server).
Line range hint
669-669: Add a missing bracket for syntactical correctness.- [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules. + [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules.
Line range hint
677-677: Correct the determiner to match the singular noun.- Any module that has an interfaces for them (like "expected keepers") will need to update and re-generate mocks if needed: + Any module that has an interface for them (like "expected keepers") will need to update and re-generate mocks if needed:
Line range hint
701-701: Add a missing comma for clarity.- In case a module requires to return `abci.ValidatorUpdate` from `EndBlock`, it can use the `HasABCIEndBlock` interface instead. + In case a module requires to return `abci.ValidatorUpdate` from `EndBlock`, it can use the `HasABCIEndBlock` interface instead.
Line range hint
730-730: Add a missing comma for clarity.- To find out more please read the [signer field](https://github.com/cosmos/cosmos-sdk/blob/7352d0bce8e72121e824297df453eb1059c28da8/docs/docs/build/building-modules/02-messages-and-queries.md#L40) documentation. + To find out more, please read the [signer field](https://github.com/cosmos/cosmos-sdk/blob/7352d0bce8e72121e824297df453eb1059c28da8/docs/docs/build/building-modules/02-messages-and-queries.md#L40) documentation.
Line range hint
743-743: Correct the article for grammatical accuracy.- The Cosmos SDK has migrated from a CometBFT genesis to a application managed genesis file. + The Cosmos SDK has migrated from a CometBFT genesis to an application managed genesis file.
Line range hint
763-763: Correct the article for grammatical accuracy.- An expedited proposal must have an higher voting threshold than a classic proposal, that threshold is defined with the `ExpeditedThreshold` parameter. + An expedited proposal must have a higher voting threshold than a classic proposal, that threshold is defined with the `ExpeditedThreshold` parameter.
Line range hint
767-767: Add a missing possessive apostrophe for clarity.- The deposits burn rate will be determined by a new parameter called `ProposalCancelRatio` parameter. + The deposits' burn rate will be determined by a new parameter called `ProposalCancelRatio` parameter.
Line range hint
780-780: Add a missing comma for clarity.- The `x/evidence` module is extracted to have a separate go.mod file which allows it be a standalone module. + The `x/evidence` module is extracted to have a separate go.mod file, which allows it to be a standalone module.
Line range hint
780-780: Add a missing preposition for grammatical accuracy.- The `x/evidence` module is extracted to have a separate go.mod file which allows it be a standalone module. + The `x/evidence` module is extracted to have a separate go.mod file which allows it to be a standalone module.
Line range hint
787-787: Add a missing comma for clarity.- The `x/feegrant` module is extracted to have a separate go.mod file which allows it to be a standalone module. + The `x/feegrant` module is extracted to have a separate go.mod file, which allows it to be a standalone module.
Line range hint
801-801: Add a missing comma for clarity.- The `x/upgrade` module is extracted to have a separate go.mod file which allows it to be a standalone module. + The `x/upgrade` module is extracted to have a separate go.mod file, which allows it to be a standalone module.
Line range hint
808-808: Correct the possessive form.- Rosetta has moved to it's own [repo](https://github.com/cosmos/rosetta) and not imported by the Cosmos SDK SimApp by default. + Rosetta has moved to its own [repo](https://github.com/cosmos/rosetta) and not imported by the Cosmos SDK SimApp by default.
Line range hint
809-809: Correct the preposition for grammatical accuracy.- Any user who is interested on using the tool can connect it standalone to any node without the need to add it as part of the node binary. + Any user who is interested in using the tool can connect it standalone to any node without the need to add it as part of the node binary.
Line range hint
810-810: Correct the compound word for consistency.- The rosetta tool also allows multi chain connections. + The rosetta tool also allows multichain connections.
Line range hint
832-832: Consider a shorter alternative to avoid wordiness.- Remove `RandomizedParams` from `AppModuleSimulation` interface. Previously, it used to generate random parameter changes during simulations, however, it does so through ParamChangeProposal which is now legacy. + Remove `RandomizedParams` from `AppModuleSimulation` interface. Previously, it generated random parameter changes during simulations through ParamChangeProposal, which is now legacy.
Line range hint
834-834: Correct the plural form for consistency.- `AppModuleSimulation` now defines a `AppModule.ProposalMsgs` method in addition to `AppModule.ProposalContents`. That method defines the messages that can be used to submit a proposal and that should be tested in simulation. + `AppModuleSimulation` now defines an `AppModule.ProposalMsgs` method in addition to `AppModule.ProposalContents`. That method defines the messages that can be used to submit a proposal and that should be tested in simulation.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- UPGRADING.md (1 hunks)
Additional Context Used
LanguageTool (74)
UPGRADING.md (74)
Near line 10: It appears that a comma is missing.
Context: .... ## [Unreleased] ### SimApp In this section we describe the changes made in Cosmos ...
Near line 10: Unpaired symbol: ‘'’ seems to be missing
Context: ... describe the changes made in Cosmos SDK' SimApp. **These changes are directly ap...
Near line 150: Possible missing comma found.
Context: ...sdk.io/api/tendermint` package has been removed as CometBFT now publishes its protos to...
Near line 153: The verb ‘depend’ can be stative. If ‘depending’ describes a state, change the sentence structure and use the base form of the verb.
Context: ...ild/docs/bsr/generated-sdks/go). If you were depending oncosmossdk.io/api/tendermint, pleas...
Near line 163: The verb ‘recommend’ is used with the gerund form.
Context: ...precation ofsdk.Context, we strongly recommend to use thecosmossdk.io/core/appmoduleinter...
Near line 207: Possible missing comma found.
Context: ...error) ``` ##### Dependency Injection Previouslycosmossdk.io/coreheld functions `Inv...
Near line 213: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ved. It is required to migrate to v0.50 prior to upgrading to v0.51 for not missing any ...
Near line 234: The singular determiner ‘this’ may not agree with the plural noun ‘changes’. Did you mean “these”?
Context: ...Many functions have been removed due to this changes as the API can be smaller thank...
Near line 234: Possible missing comma found.
Context: ...functions have been removed due to this changes as the API can be smaller thanks to col...
Near line 246: Possible missing article found.
Context: ...cosmossdk.io/x/authz####x/bankBank was spun out into its owngo.mod. To ...
Near line 282: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ams` A standalone Go module was created and it is accessible at "cosmossdk.io/x/par...
Near line 317: Possible missing comma found.
Context: ...o CometBFT (Part 2) The Cosmos SDK has migrated in its previous versions, to CometBFT. ...
Near line 350: It appears that a comma is missing.
Context: ...genesis.json` file. For existing chains this can be done in two ways: * During an u...
Near line 352: Consider adding a comma after this introductory phrase.
Context: ...s can be done in two ways: * During an upgrade the value is set in an upgrade handler....
Near line 359: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...here a catastrophic failure has occurred and the application should halt. However, t...
Near line 361: Did you maybe mean “buy” or “be”?
Context: ... that returns an error, will gracefully by handled byBaseAppon behalf of the a...
Near line 366: Consider a shorter alternative to avoid wordiness.
Context: ...lizeBlock` is public and should be used in order to test and run operations. This means tha...
Near line 379: Did you mean “modifying”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...begin blocker other modules, and allows to modify consensus parameters, and the changes a...
Near line 417: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...he case of successful msg(s) execution. Instead a new attribute is added to all message...
Near line 418: Possible missing preposition found.
Context: ...fies which events and attributes relate the same transaction.BeginBlock& `EndB...
Near line 444: The word “clean-up” is a noun. The verb is spelled with a white space.
Context: ...s well as its settings. Useconfixto clean-up yourapp.toml. A nginx (or alike) rev...
Near line 448: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... not supported anymore. To migrate from a unsupported database to a supported dat...
Near line 448: Consider adding a comma here.
Context: ...pported database to a supported database please use a database migration tool. ### Pro...
Near line 465: It appears that a comma is missing.
Context: ...tate-machine code. ### SimApp In this section we describe the changes made in Cosmos ...
Near line 465: Unpaired symbol: ‘'’ seems to be missing
Context: ... describe the changes made in Cosmos SDK' SimApp. **These changes are directly ap...
Near line 529: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...on. The global variable has been removed and the basic module manager can be now cre...
Near line 565: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... not to be included in the binary. ::: AdditionallyAutoCLIautomatically adds the custom...
Near line 596: The preposition ‘to’ may be missing (allow someone to do something).
Context: ... a separate go.mod file which allows it be a standalone module. All the store impo...
Near line 601: Unpaired symbol: ‘[’ seems to be missing
Context: ...cross the SDK. ##### Streaming [ADR-38](https://docs.cosmos.network/main/archit...
Near line 617: This word is normally spelled with a hyphen.
Context: ...available in the SDK that produces more human readable output, currently only available on Led...
Near line 661: Two determiners in a row. Choose either “the” or “a”.
Context: ...``` When usingdepinject/ `app v2`, the a tx config should be recreated from the ...
Near line 669: Unpaired symbol: ‘[’ seems to be missing
Context: ... ### Modules ####**all*** [RFC 001](https://docs.cosmos.network/main/rfc/rf...
Near line 677: It looks like ‘interfaces’ doesn’t match ‘an’. Did you mean “an interface” or just “interfaces”?
Context: ...d ofsdk.Context. Any module that has an interfaces for them (like "expected keepers") will...
Near line 701: Possible missing comma found.
Context: ...EndBlock(context.Context) error ``` In case a module requires to return `abci.Valid...
Near line 730: Consider adding a comma here.
Context: ...ustom signer function. To find out more please read the [signer field](https://github....
Near line 743: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...has migrated from a CometBFT genesis to a application managed genesis file. The g...
Near line 763: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ameter. An expedited proposal must have an higher voting threshold than a classic ...
Near line 767: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...nt toProposalCancelDestaddress. The deposits burn rate will be determined by a new p...
Near line 780: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it be a standalone module....
Near line 780: The preposition ‘to’ may be missing (allow someone to do something).
Context: ... a separate go.mod file which allows it be a standalone module. All the evidence i...
Near line 787: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it to be a standalone modu...
Near line 801: Possible missing comma found.
Context: ... is extracted to have a separate go.mod file which allows it to be a standalone modu...
Near line 808: Did you mean the possessive pronoun “its”?
Context: ...ing #### Rosetta Rosetta has moved to it's own [repo](https://github.com/cosmos/ro...
Near line 809: After the verb ‘interested’ the preposition “in” is used.
Context: ... by default. Any user who is interested on using the tool can connect it standalon...
Near line 810: This word is normally spelled as one.
Context: ...de binary. The rosetta tool also allows multi chain connections. ## [v0.47.x](https://gith...
Near line 832: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...dom parameter changes during simulations, however, it does so through ParamChangeProposal ...
Near line 834: The noun should probably be in the singular form.
Context: ...teParamsgovernance proposals for each modules,AppModuleSimulationnow defines aA...
Near line 849: Consider using “formerly” to strengthen your wording.
Context: ... modules no longer support the REST API previously known as the LCD, and the `sdk.Msg#Rout...
Near line 866: Consider a shorter alternative to avoid wordiness.
Context: ...dd the following lines to yourapp.goin order to provide newer gRPC services: ```go aut...
Near line 883: Possible missing comma found.
Context: ...riod). These were unnecessary given as arguments as they were already present in theAp...
Near line 887: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...)was deprecated and has been removed. Instead you can use theTestEncodingConfig` fr...
Near line 897: The modal verb ‘must’ requires the verb’s base form.
Context: ... Replaces TheGoLevelDBversion must pinned to `v1.0.1-0.20210819022825-2ae1ddf74ef...
Near line 908: The word ‘replace’ is a verb. Did you mean the noun “replacement”?
Context: ...goproto. This allows you to remove the replace directivereplace github.com/gogo/prot...
Near line 916: You have used ‘I’ with an apostrophe, but either the verb is missing or you have not used the corresponding abbreviated form. Consider using one of the suggestions or removing the apostrophe.
Context: ...ly the rootproto/folder, set by theprotoc -Iflag). For example, assuming you put a...
Near line 926: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ace` annotations. We require them to be fully-scoped names. They will soon be used by code g...
Near line 944: The word “also” tends to be overused. Consider using a formal alternative to strengthen your wording.
Context: ...os.feegrant.v1beta1.FeeAllowanceI" ``` Please also check that in your own app's proto file...
Near line 945: Try using a more formal synonym for ‘check’.
Context: ...v1beta1.FeeAllowanceI" ``` Please also check that in your own app's proto files that...
Near line 945: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...otations. If so, then replace them with fully-qualified names, even though those names don't ac...
Near line 985: In American English, “take a look” is more commonly used.
Context: ...e modules keepers in previous versions. Have a look atsimapp.RegisterUpgradeHandlers()f...
Near line 1003: Did you mean “At a Time”, “At the Time”, or “At times”?
Context: ...x/gov##### Minimum Proposal Deposit At Time of Submission Thegovmodule has bee...
Near line 1010: Did you mean “at a time”, “at the time”, or “at times”?
Context: ...o utilize the minimum proposal deposits at time of submission, the migration logic need...
Near line 1042: A comma is probably missing here.
Context: ...ng Tendermint consensus parameters. For migration it is required to call a specific migra...
Near line 1069: Consider a shorter alternative to avoid wordiness.
Context: ...should still be imported in your app.go in order to handle this migration. Because the `x/...
Near line 1131: Consider a shorter alternative to avoid wordiness.
Context: ...ally, new packages have been introduced in order to further split the codebase. Aliases are...
Near line 1182: Possible missing article found.
Context: ...ur of each module housing and providing way to modify their parameters. Each module...
Near line 1183: It appears that a comma is missing.
Context: ...aintained until April 18, 2023. At this point the module will reach end of life and b...
Near line 1188: Consider a shorter alternative to avoid wordiness.
Context: ...the new implementation is calledv1. In order to submit a proposal with `submit-proposal...
Near line 1195: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ...g delegations. Users that have unbonded by accident or wish to cancel a undelegation can no...
Near line 1195: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... unbonded by accident or wish to cancel a undelegation can now specify the amount...
Near line 1199: The auxiliary verb ‘do’ requires the base form of the verb.
Context: ...v0.45.3/third_party/proto) now does not contains directly the [proto files](https://gith...
Near line 1201: Consider a shorter alternative to avoid wordiness.
Context: ...build/cosmos/cosmos-sdk` as dependency, in order to avoid having to copy paste these files....
Near line 1201: Did you mean “copy and paste”?
Context: ...dependency, in order to avoid having to copy paste these files. The protos can as well be...
Near line 1219: Possible missing comma found.
Context: ..."]; } ``` When clients interact with a node they are required to set a codec in in ...
Near line 1219: Possible typo: you repeated a word
Context: ...a node they are required to set a codec in in the grpc.Dial. More information can be ...
Path-based Instructions (2)
UPGRADING.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (4)
CHANGELOG.md (4)
121-121: Change correctly documented and linked.
121-121: Change correctly documented and linked.
121-121: Change correctly documented and linked.
121-121: Change correctly documented and linked.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- codec/depinject.go (1 hunks)
- runtime/module.go (6 hunks)
Files skipped from review as they are similar to previous changes (2)
- codec/depinject.go
- runtime/module.go
Description
Proposal to remove the hard coupling on sdk/codec in runtime, runtime/v2 (#20320), and modules.
core/legacy.AminoHasAminoCodecnow accepts that interface instead of the type incodecHasAminoCodec.RegisterAminoCodecaccordinglyProvideInterfaceRegistryto codec. This will allow runtime/v2 to avoid a direct dependency on sdk/codec.The new interface that modules will call through to register amino types looks like:
This is breaking on module implementations, they must adjust their
HasAminoCodecimplementation to accept the interface instead of type.Also note that the sig of
RegisterConretedropped an unused parameter which was always passed in asnil. It is an empty struct ingo-amino: https://github.com/tendermint/go-amino/blob/8e779b71f40d175cd1302d3cd41a75b005225a7a/codec.go#L103-L104Author 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
New Features
Refactor
Bug Fixes