-
Notifications
You must be signed in to change notification settings - Fork 127
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
refactor: bump up irismod #2950
Conversation
WalkthroughThe changes involve significant updates to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (3)go.mod (3)Line range hint The migration from a monolithic
Let's verify the codebase adaptation: #!/bin/bash
# Check for any remaining old imports
echo "Checking for old irismod imports..."
rg -l "github.com/irisnet/irismod"
# Check for new module imports
echo "Checking usage of new modules..."
for module in $(rg -o 'mods\.irisnet\.org/modules/[a-z]+' go.mod | cut -d'/' -f4 | sort -u); do
echo "Usage of $module module:"
rg -l "mods.irisnet.org/modules/$module"
done
The indirect dependencies (api, simapp) use an older version (20240628054110) compared to the direct module dependencies (20240705013837). While this might be intentional, it's worth verifying that these versions are compatible. Let's check for any compatibility issues between these versions: #!/bin/bash
# Compare the changes between these versions to identify potential breaking changes
echo "Checking changes between versions..."
for repo in api simapp; do
echo "Changes in $repo between versions:"
gh api repos/mods.irisnet.org/$repo/compare/$(rg -o '20240628054110[^"]+' | head -1)...$(rg -o '20240705013837[^"]+' | head -1)
done
The changes introduce multiple dependencies using unreleased versions:
While this allows for faster iteration during development, it's important to ensure these versions are stable for production use. Let's verify if these are the latest versions and if there are any released versions available: Also applies to: 15-24 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
modules/mint/types/expected_keepers.go (1)
Line range hint
9-16
:
Ensure proper documentation for interfaces.The
AccountKeeper
andBankKeeper
interfaces should have detailed documentation for each method to ensure clarity and maintainability.type AccountKeeper interface { + // GetModuleAddress returns the address of a module account. GetModuleAddress(name string) sdk.AccAddress // TODO remove with genesis 2-phases refactor https://github.com/cosmos/cosmos-sdk/issues/2862 + // SetModuleAccount sets a module account in the store. SetModuleAccount(sdk.Context, types.ModuleAccountI) + // GetModuleAccount returns a module account by name. GetModuleAccount(ctx sdk.Context, moduleName string) types.ModuleAccountI } type BankKeeper interface { + // SendCoinsFromModuleToAccount sends coins from a module to an account. SendCoinsFromModuleToAccount( ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins, ) error + // SendCoinsFromModuleToModule sends coins from one module to another. SendCoinsFromModuleToModule( ctx sdk.Context, senderModule, recipientModule string, amt sdk.Coins, ) error + // MintCoins mints new coins. MintCoins(ctx sdk.Context, name string, amt sdk.Coins) error }Also applies to: 18-33
modules/internft/interface.go (1)
Line range hint
12-18
:
Ensure proper documentation for theAccountKeeper
interface.The
AccountKeeper
interface should have detailed documentation for each method to ensure clarity and maintainability.type AccountKeeper interface { + // NewAccountWithAddress creates a new account with the given address. NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI // Set an account in the store. SetAccount(sdk.Context, authtypes.AccountI) + // GetModuleAddress returns the address of a module account. GetModuleAddress(name string) sdk.AccAddress }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (20)
- app/ante/decorators.go (1 hunks)
- app/ante/handler_options.go (1 hunks)
- app/export.go (1 hunks)
- app/keepers/keepers.go (2 hunks)
- app/keepers/keys.go (1 hunks)
- app/modules.go (4 hunks)
- app/sim_test.go (1 hunks)
- cmd/iris/cmd/testnet.go (3 hunks)
- cmd/iris/cmd/util.go (1 hunks)
- go.mod (3 hunks)
- modules/internft/interface.go (1 hunks)
- modules/internft/keeper.go (1 hunks)
- modules/mint/keeper/migrations.go (1 hunks)
- modules/mint/migrations/v2/migrate.go (2 hunks)
- modules/mint/migrations/v2/migrate_test.go (1 hunks)
- modules/mint/module.go (2 hunks)
- modules/mint/types/expected_keepers.go (3 hunks)
- types/runtime.go (1 hunks)
- wrapper/farm.go (1 hunks)
- wrapper/token.go (3 hunks)
Files skipped from review due to trivial changes (7)
- app/ante/handler_options.go
- app/export.go
- app/sim_test.go
- cmd/iris/cmd/util.go
- modules/internft/keeper.go
- modules/mint/migrations/v2/migrate_test.go
- wrapper/farm.go
Additional comments not posted (29)
modules/mint/migrations/v2/migrate.go (1)
Line range hint
15-19
:
Ensure thetypes.Subspace
interface is correctly defined.Verify that the
types.Subspace
interface properly supports theGetParamSet
method.Verification successful
The
types.Subspace
interface correctly supports theGetParamSet
method.
- The
GetParamSet
method is defined within thetypes.Subspace
interface in themodules/mint/types/expected_keepers.go
file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `types.Subspace` interface supports the `GetParamSet` method. # Test: Search for the definition of `types.Subspace`. Expect: The interface should include `GetParamSet`. rg --type go --pattern 'type Subspace interface {'Length of output: 464
Script:
#!/bin/bash # Description: Verify the `types.Subspace` interface supports the `GetParamSet` method. # Test: Search for the definition of `types.Subspace`. Expect: The interface should include `GetParamSet`. rg 'type Subspace interface {'Length of output: 146
Script:
#!/bin/bash # Description: Verify the `types.Subspace` interface supports the `GetParamSet` method. # Test: Search for the definition of `types.Subspace`. Expect: The interface should include `GetParamSet`. rg 'type Subspace interface \{'Length of output: 98
Script:
#!/bin/bash # Description: Verify the `types.Subspace` interface supports the `GetParamSet` method. # Test: Search for the `GetParamSet` method within the `modules/mint/types/expected_keepers.go` file. rg 'GetParamSet' modules/mint/types/expected_keepers.goLength of output: 109
modules/mint/keeper/migrations.go (1)
7-7
: Ensure thetypes.Subspace
interface is correctly defined.Verify that the
types.Subspace
interface properly supports theGetParamSet
method.Also applies to: 13-13, 17-17
modules/mint/types/expected_keepers.go (1)
35-38
: Ensure theSubspace
interface is correctly defined.Verify that the
Subspace
interface properly supports theGetParamSet
method.modules/internft/interface.go (1)
9-9
: Ensure thenfttypes
module is correctly updated.Verify that the
nfttypes
module has been correctly updated tomods.irisnet.org/modules/nft/types
.Verification successful
The
nfttypes
module has been correctly updated.The import path
mods.irisnet.org/modules/nft/types
is correctly used in multiple files, includingmodules/internft/interface.go
.
modules/internft/keeper.go
modules/internft/interface.go
app/sim_test.go
app/modules.go
app/keepers/keepers.go
app/keepers/keys.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `nfttypes` module update. # Test: Search for the import path. Expect: The import path should be `mods.irisnet.org/modules/nft/types`. rg --type go --pattern 'mods.irisnet.org/modules/nft/types'Length of output: 471
Script:
#!/bin/bash # Description: Verify the `nfttypes` module update. # Test: Search for the import path. Expect: The import path should be `mods.irisnet.org/modules/nft/types`. rg 'mods.irisnet.org/modules/nft/types' --type goLength of output: 453
types/runtime.go (1)
17-18
: Imports updated correctly.The import paths for
tokentypes
andtokenv1
have been updated tomods.irisnet.org/modules/token/types
andmods.irisnet.org/modules/token/types/v1
, respectively. These changes are consistent with the rebranding effort mentioned in the PR objectives.wrapper/token.go (3)
21-21
: Imports updated correctly.The import path for
tokentypes
has been updated tomods.irisnet.org/modules/token/types
. This change is consistent with the rebranding effort mentioned in the PR objectives.
42-47
: Update inApplyMessage
method is correct.The
ApplyMessage
method now usestokentypes.Result
instead ofirismodtypes.Result
. This change is consistent with the rebranding effort and the new module path.
Line range hint
62-70
:
Update inEstimateGas
method is correct.The
EstimateGas
method now usestokentypes.EthCallRequest
instead ofirismodtypes.EthCallRequest
. This change is consistent with the rebranding effort and the new module path.app/ante/decorators.go (1)
11-15
: Imports updated correctly.The import paths for
coinswaptypes
,servicetypes
,tokenkeeper
,tokentypesv1
, andtokentypesv1beta1
have been updated tomods.irisnet.org/modules/...
. These changes are consistent with the rebranding effort mentioned in the PR objectives.app/keepers/keys.go (1)
28-37
: Imports updated correctly.The import paths for
coinswaptypes
,farmtypes
,htlctypes
,mttypes
,nfttypes
,oracletypes
,randomtypes
,recordtypes
,servicetypes
, andtokentypes
have been updated tomods.irisnet.org/modules/...
. These changes are consistent with the rebranding effort mentioned in the PR objectives.modules/mint/module.go (1)
94-94
: LGTM!The type change for
legacySubspace
fromexported.Subspace
totypes.Subspace
is appropriate and aligns with the module import path updates.Also applies to: 101-101
go.mod (2)
7-7
: LGTM!The version update for
github.com/bianjieai/tibc-go
tov0.5.1-0.20240703054905-6368161b801f
is appropriate.
15-24
: LGTM!The import path updates from
github.com/irisnet/irismod
tomods.irisnet.org
for multiple modules are consistent and align with the restructuring or rebranding effort.Also applies to: 102-103
cmd/iris/cmd/testnet.go (2)
15-15
: LGTM!The import path updates for
tokentypes
andservicetypes
tomods.irisnet.org
are consistent and align with the restructuring or rebranding effort.Also applies to: 44-45
383-383
: LGTM!The function call change from
randomtypes.GetSvcDefinition()
toservicetypes.GetRandomSvcDefinition()
is appropriate and aligns with the import path updates.app/modules.go (2)
51-70
: LGTM!The import path updates from
github.com/irisnet/irismod
tomods.irisnet.org
for multiple modules are consistent and align with the restructuring or rebranding effort.
140-140
: LGTM!The updates in the
appModules
andsimulationModules
functions to reflect the new import paths are consistent and align with the restructuring or rebranding effort.Also applies to: 285-285, 411-411
app/keepers/keepers.go (12)
85-86
: Import path updated tomods.irisnet.org
.The import path for
coinswapkeeper
andcoinswaptypes
has been updated correctly.
87-88
: Import path updated tomods.irisnet.org
.The import path for
farm
andfarmkeeper
has been updated correctly.
89-90
: Import path updated tomods.irisnet.org
.The import path for
farmtypes
andhtlckeeper
has been updated correctly.
91-92
: Import path updated tomods.irisnet.org
.The import path for
htlctypes
andmtkeeper
has been updated correctly.
93-94
: Import path updated tomods.irisnet.org
.The import path for
mttypes
andnftkeeper
has been updated correctly.
95-96
: Import path updated tomods.irisnet.org
.The import path for
nfttypes
andoraclekeeper
has been updated correctly.
97-98
: Import path updated tomods.irisnet.org
.The import path for
oracletypes
andrandomkeeper
has been updated correctly.
99-100
: Import path updated tomods.irisnet.org
.The import path for
randomtypes
andrecordkeeper
has been updated correctly.
101-102
: Import path updated tomods.irisnet.org
.The import path for
recordtypes
andservicekeeper
has been updated correctly.
103-104
: Import path updated tomods.irisnet.org
.The import path for
servicetypes
andtokenkeeper
has been updated correctly.
105-106
: Import path updated tomods.irisnet.org
.The import path for
tokentypes
andtokenv1
has been updated correctly.
518-518
: Handler registration updated.The proposal handler for
farmtypes
has been updated tofarm.NewProposalHandler
. Ensure this new handler is correctly implemented and tested.
Summary by CodeRabbit
New Features
Bug Fixes