bump up mods.irisnet.org/modules/htlc to cosmos-sdk v0.50.10 #458
Conversation
WalkthroughThe pull request introduces significant updates to the HTLC module, primarily focused on enhancing the structures and methods related to hashed time-locked contracts. Key modifications include the addition of new fields to various structures, updates to method signatures for better context management, and a transition to a new math library for integer handling. The changes also involve upgrading dependencies, particularly the Cosmos SDK, to version 0.50.10, ensuring compatibility across multiple modules. Overall, these updates aim to improve functionality and maintainability within the HTLC module. Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lintlevel=warning msg="The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar." 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)modules/htlc/module.go (3)
The EndBlock method correctly implements a no-op end blocker with proper error handling, aligning with Cosmos SDK v0.50.10 standards.
The RegisterStoreDecoder method signature has been properly updated to use simtypes.StoreDecoderRegistry, aligning with Cosmos SDK v0.50.10 standards.
Previous error propagation comment is still valid The BeginBlock method should propagate any errors from BeginBlocker instead of discarding them. This is critical for proper error handling during block processing. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (14)
modules/htlc/simulation/genesis.go (1)
68-78: Consider reducing code duplication in AssetSupply initializationThe migration to
math.ZeroInt()is correct, but there's repetitive code in the AssetSupply initialization.Consider creating a helper function to reduce duplication:
+func newZeroAssetSupply(denom string) types.AssetSupply { + zeroCoin := sdk.NewCoin(denom, math.ZeroInt()) + return types.NewAssetSupply( + zeroCoin, zeroCoin, zeroCoin, zeroCoin, + time.Duration(0), + ) +} Supplies: []types.AssetSupply{ - types.NewAssetSupply( - sdk.NewCoin("htltbnb", math.ZeroInt()), - sdk.NewCoin("htltbnb", math.ZeroInt()), - sdk.NewCoin("htltbnb", math.ZeroInt()), - sdk.NewCoin("htltbnb", math.ZeroInt()), - time.Duration(0), - ), - types.NewAssetSupply( - sdk.NewCoin("htltinc", math.ZeroInt()), - sdk.NewCoin("htltinc", math.ZeroInt()), - sdk.NewCoin("htltinc", math.ZeroInt()), - sdk.NewCoin("htltinc", math.ZeroInt()), - time.Duration(0), - ), + newZeroAssetSupply("htltbnb"), + newZeroAssetSupply("htltinc"), },modules/htlc/types/genesis_test.go (1)
68-68: LGTM: Correct migration to math package for test casesThe changes to use
math.ZeroInt()andmath.NewInt(-1)are correct and maintain the original test semantics.Consider adding test cases for additional edge cases with the new math package, such as:
}, { + "max int overflow", + args{ + swaps: []types.HTLC{{Amount: sdk.Coins{sdk.Coin{Denom: "test", Amount: math.MaxInt}}}}, + previousBlockTime: types.DefaultPreviousBlockTime, + }, + false, +}, {Also applies to: 82-82
modules/htlc/integration_test.go (2)
64-67: LGTM: Asset supplies correctly initialized with potential for refactoring.The changes correctly migrate to
math.ZeroInt()while maintaining proper zero initialization of all supply fields.Consider reducing code duplication by extracting a helper function:
+func newZeroAssetSupply(denom string) types.AssetSupply { + return types.NewAssetSupply( + sdk.NewCoin(denom, math.ZeroInt()), + sdk.NewCoin(denom, math.ZeroInt()), + sdk.NewCoin(denom, math.ZeroInt()), + sdk.NewCoin(denom, math.ZeroInt()), + time.Duration(0), + ) +} func NewHTLTGenesis(deputyAddress sdk.AccAddress) *types.GenesisState { // ... params initialization ... Supplies: []types.AssetSupply{ - types.NewAssetSupply( - sdk.NewCoin("htltbnb", math.ZeroInt()), - sdk.NewCoin("htltbnb", math.ZeroInt()), - sdk.NewCoin("htltbnb", math.ZeroInt()), - sdk.NewCoin("htltbnb", math.ZeroInt()), - time.Duration(0), - ), - types.NewAssetSupply( - sdk.NewCoin("htltinc", math.ZeroInt()), - sdk.NewCoin("htltinc", math.ZeroInt()), - sdk.NewCoin("htltinc", math.ZeroInt()), - sdk.NewCoin("htltinc", math.ZeroInt()), - time.Duration(0), - ), + newZeroAssetSupply("htltbnb"), + newZeroAssetSupply("htltinc"), },Also applies to: 71-74
Migration from sdk to math package incomplete
Several files still use the deprecated
sdk.NewInt/sdk.ZeroIntfunctions that need to be migrated to the math package:
modules/htlc/migrations/v2/migrate_test.go: Multiple instances ofsdk.NewInt,sdk.ZeroInt, andsdk.OneIntmodules/htlc/genesis_test.go: Usessdk.ZeroInt()modules/htlc/types/conmmon_test.goandmodules/htlc/keeper/integration_test.go: Usesdk.NewInt64Coinwhich is fine as it's a Cosmos SDK coin functionThe migration needs to be completed by:
- Replacing
sdk.NewIntwithmath.NewInt- Replacing
sdk.ZeroIntwithmath.ZeroInt- Replacing
sdk.OneIntwithmath.OneInt🔗 Analysis chain
Line range hint
1-99: Verify complete migration from sdk to math package.Let's ensure no instances of
sdk.NewIntorsdk.ZeroIntwere missed in the migration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of sdk.NewInt or sdk.ZeroInt echo "Checking for remaining sdk.NewInt/ZeroInt instances..." rg -l 'sdk\.(New|Zero)Int' modules/htlc/ # Search for mixed usage of sdk and math packages for integer operations echo "Checking for mixed integer package usage..." rg -l 'math\.(New|Zero)Int.*sdk\.(New|Zero)Int|sdk\.(New|Zero)Int.*math\.(New|Zero)Int' modules/htlc/Length of output: 509
Script:
#!/bin/bash # Let's examine the actual content of files that still use sdk.NewInt/ZeroInt echo "=== Checking modules/htlc/migrations/v2/migrate_test.go ===" rg 'sdk\.(New|Zero)Int' modules/htlc/migrations/v2/migrate_test.go -B 2 -A 2 echo -e "\n=== Checking modules/htlc/types/conmmon_test.go ===" rg 'sdk\.(New|Zero)Int' modules/htlc/types/conmmon_test.go -B 2 -A 2 echo -e "\n=== Checking modules/htlc/genesis_test.go ===" rg 'sdk\.(New|Zero)Int' modules/htlc/genesis_test.go -B 2 -A 2 echo -e "\n=== Checking modules/htlc/keeper/integration_test.go ===" rg 'sdk\.(New|Zero)Int' modules/htlc/keeper/integration_test.go -B 2 -A 2Length of output: 1653
modules/htlc/keeper/integration_test.go (1)
85-95: LGTM: Zero initialization with math packageThe migration to
math.ZeroInt()for initializing asset supplies is correct. Consider adding test cases to verify:
- Supply initialization
- Supply limit enforcement
- Time-based limit functionality (especially for htltinc)
// Example test case structure func TestAssetSupplyLimits(t *testing.T) { // Test initial supply is zero // Test supply cannot exceed Limit // Test time-based limit for htltinc }proto/irismod/htlc/htlc.proto (1)
Line range hint
112-123: Ensure consistent type updates across related fields.While the
limitfield has been correctly updated to usecosmossdk.io/math.Int, thetime_based_limitfield at line 121 should also be updated for consistency, as both fields represent similar numeric values.Apply this update to maintain consistency:
string time_based_limit = 4 [ - (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int", + (gogoproto.customtype) = "cosmossdk.io/math.Int", (gogoproto.nullable) = false ];modules/htlc/module.go (1)
Line range hint
1-191: Ensure consistent adoption of cosmos-sdk v0.50.10 patternsThe module is being updated to follow cosmos-sdk v0.50.10 patterns, which include:
- Using
context.Contextinstead ofsdk.Context- Adding error returns for better error handling
- Moving to dedicated type packages (e.g.,
simtypes)Please ensure these patterns are consistently applied across the entire module, including:
- All ABCI method signatures
- Error handling in all methods
- Type imports from their new packages
modules/htlc/simulation/operation.go (2)
144-144: LGTM with suggestion: Consider enhancing error contextWhile the return statement simplification is good, consider enhancing error context in the simulation by including relevant details (e.g., amount, addresses) in error messages for better debugging.
Example enhancement:
- return simtypes.NewOperationMsg(msg, true, ""), nil, nil + return simtypes.NewOperationMsg(msg, true, fmt.Sprintf( + "created HTLC from %s to %s with amount %s", + msg.Sender, + msg.To, + msg.Amount, + )), nil, nil
229-229: LGTM with suggestion: Align error context with create operationThe return statement simplification is good. Consider applying the same error context enhancement suggested for the create operation.
Example enhancement:
- return simtypes.NewOperationMsg(msg, true, ""), nil, nil + return simtypes.NewOperationMsg(msg, true, fmt.Sprintf( + "claimed HTLC %s by %s", + msg.Id, + msg.Sender, + )), nil, nilmodules/htlc/types/params_test.go (1)
Migration to math package is incomplete in test files
The verification reveals several files still using the deprecated
sdk.Intoperations that need to be migrated tomathpackage:
modules/htlc/keeper/integration_test.go: Usingsdk.NewInt64Coinmodules/htlc/migrations/v2/migrate_test.go: Usingsdk.NewInt,sdk.ZeroInt,sdk.OneIntmodules/htlc/genesis_test.go: Usingsdk.ZeroIntmodules/htlc/types/conmmon_test.go: Usingsdk.NewInt64CoinThese test files need to be updated to use the corresponding
mathpackage operations for consistency with the main code migration to cosmos-sdk v0.50.10.🔗 Analysis chain
Line range hint
1-371: Verify complete migration of integer operations in HTLC moduleLet's ensure all integer operations in the HTLC module have been migrated from sdk to math package.
Let me check the specific usage context in the test files that still use sdk.Int to determine if they need migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining sdk.Int usage in HTLC module echo "Checking for remaining sdk.Int usage..." rg -l "sdk\.(NewInt|Int|ZeroInt)" modules/htlc/ echo "Checking for new math.Int usage..." rg -l "math\.(NewInt|Int|ZeroInt)" modules/htlc/Length of output: 807
Script:
#!/bin/bash # Check the actual usage of sdk.Int in test files echo "Checking sdk.Int usage in test files:" rg -A 2 "sdk\.(NewInt|Int|ZeroInt)" modules/htlc/types/conmmon_test.go modules/htlc/genesis_test.go modules/htlc/migrations/v2/migrate_test.go modules/htlc/keeper/integration_test.go echo -e "\nChecking imports in these files:" rg "^import.*sdk" modules/htlc/types/conmmon_test.go modules/htlc/genesis_test.go modules/htlc/migrations/v2/migrate_test.go modules/htlc/keeper/integration_test.goLength of output: 1784
modules/htlc/keeper/depinject_test.go (1)
Line range hint
60-64: Remove outdated capability module commentThe comment above
genesisModuleOrderstill references the capability module's initialization requirements, but the module has been removed as part of the SDK upgrade.Apply this diff to remove the outdated comment:
// NOTE: The genutils module must occur after staking so that pools are // properly initialized with tokens from genesis accounts. // NOTE: The genutils module must also occur after auth so that it can access the params from auth. - // NOTE: Capability module must occur first so that it can initialize any capabilities - // so that other modules that want to create or claim capabilities afterwards in InitChain - // can do so safely. genesisModuleOrder = []string{modules/htlc/keeper/asset_test.go (1)
646-685: LGTM! Consistent migration of integer types with a minor formatting suggestionThe changes correctly migrate all integer fields to use
math.NewInt. However, the indentation in the test case structure could be improved for better readability.Consider applying consistent indentation to improve readability:
SupplyLimit: types.SupplyLimit{ - Limit: math.NewInt(350000000000000), - TimeLimited: false, - TimeBasedLimit: math.ZeroInt(), + Limit: math.NewInt(350000000000000), + TimeLimited: false, + TimeBasedLimit: math.ZeroInt(),modules/htlc/keeper/htlc_test.go (1)
Line range hint
4-4: Consider adding test coverage for concurrent operationsWhile the test suite is comprehensive for individual operations, it might be valuable to add test cases for concurrent HTLC operations to ensure thread safety.
Consider adding test cases that simulate:
- Multiple simultaneous claims
- Concurrent creation and expiration
- Race conditions between claim and refund operations
api/irismod/htlc/htlc.pulsar.go (1)
Line range hint
1-1: Avoid committing generated code to the repositoryThe file
htlc.pulsar.gois auto-generated byprotoc-gen-go-pulsaras indicated by the header comment// Code generated by protoc-gen-go-pulsar. DO NOT EDIT.It's generally recommended to exclude generated files from version control to prevent merge conflicts and reduce repository size. Instead, consider adding this file to.gitignoreand updating the build process to generate it when needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
modules/htlc/go.sumis excluded by!**/*.summodules/htlc/types/htlc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (22)
api/irismod/htlc/htlc.pulsar.go(2 hunks)modules/htlc/abci.go(2 hunks)modules/htlc/depinject.go(2 hunks)modules/htlc/go.mod(1 hunks)modules/htlc/integration_test.go(2 hunks)modules/htlc/keeper/asset.go(4 hunks)modules/htlc/keeper/asset_test.go(4 hunks)modules/htlc/keeper/depinject_test.go(2 hunks)modules/htlc/keeper/grpc_query_test.go(1 hunks)modules/htlc/keeper/htlc.go(3 hunks)modules/htlc/keeper/htlc_test.go(1 hunks)modules/htlc/keeper/integration_test.go(3 hunks)modules/htlc/keeper/keeper.go(1 hunks)modules/htlc/keeper/params_test.go(1 hunks)modules/htlc/module.go(2 hunks)modules/htlc/simulation/genesis.go(3 hunks)modules/htlc/simulation/operation.go(3 hunks)modules/htlc/types/expected_keepers.go(1 hunks)modules/htlc/types/genesis_test.go(4 hunks)modules/htlc/types/msgs_test.go(2 hunks)modules/htlc/types/params_test.go(18 hunks)proto/irismod/htlc/htlc.proto(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- modules/htlc/keeper/keeper.go
🔇 Additional comments (44)
modules/htlc/abci.go (3)
4-4: LGTM: Import changes align with Cosmos SDK v0.50.x practices
The addition of the "context" import is appropriate for the transition to using standard Go contexts.
15-16: LGTM: Context handling follows Cosmos SDK v0.50.x patterns
The transition to using context.Context and the context unwrapping logic is implemented correctly, following the standard patterns for Cosmos SDK v0.50.x modules.
Let's verify that similar context handling patterns are used consistently across other BeginBlocker functions in the codebase:
✅ Verification successful
Based on the results, I can now generate the final response since I have sufficient information to verify the context handling patterns.
LGTM: Context handling follows Cosmos SDK v0.50.x patterns consistently
The transition to using context.Context and the context unwrapping logic is implemented correctly in the HTLC module's BeginBlocker, following the standard patterns for Cosmos SDK v0.50.x modules. The same pattern is consistently used across other modules in the codebase, as evidenced by:
- All modules (service, random, htlc, etc.) use
sdk.UnwrapSDKContextto handle context conversion - The unwrapped context is used immediately after conversion for module-specific operations
- Logger configuration is preserved with the new context (as seen in htlc/abci.go)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent context handling patterns in BeginBlocker functions
# Expected: All BeginBlocker functions should use context.Context and UnwrapSDKContext
# Search for BeginBlocker function declarations
ast-grep --pattern 'func BeginBlocker($ctx context.Context, $$$) {
$$$
}'
# Search for context unwrapping patterns
rg 'UnwrapSDKContext' -A 1
Length of output: 17080
Line range hint 19-42: Verify the UpdateTimeBasedSupplyLimits integration
While the core HTLC expiration and refund logic remains correctly preserved, let's verify the integration of UpdateTimeBasedSupplyLimits to ensure it's properly implemented in the keeper.
✅ Verification successful
Let me gather more information about the UpdateTimeBasedSupplyLimits implementation since the first search didn't show the actual implementation.
Based on the extensive search results, I can now generate the final response since I have enough information about the UpdateTimeBasedSupplyLimits implementation and its integration:
UpdateTimeBasedSupplyLimits is properly integrated and maintains asset supply limits
The implementation in modules/htlc/keeper/asset.go correctly:
- Updates time-based supply limits for each asset by tracking elapsed time between blocks
- Resets time-limited supplies when the time period has elapsed
- Maintains proper state through the keeper's store
The function is well-tested in modules/htlc/keeper/asset_test.go with comprehensive test cases covering:
- Time period resets
- Supply limit enforcement
- Asset supply tracking
- Various time durations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify UpdateTimeBasedSupplyLimits implementation
# Expected: Find the implementation and any related tests
# Search for the implementation
ast-grep --pattern 'func ($k Keeper) UpdateTimeBasedSupplyLimits($ctx $$$) {
$$$
}'
# Search for related tests
rg 'TestUpdateTimeBasedSupplyLimits' -A 5
Length of output: 511
Script:
#!/bin/bash
# Search for UpdateTimeBasedSupplyLimits implementation in keeper files
rg -t go "UpdateTimeBasedSupplyLimits" modules/htlc/keeper/ -B 2 -A 10
# Search for asset supply related code to understand the context
ast-grep --pattern 'type AssetSupply struct {
$$$
}'
# Look for any asset supply limit related code
rg "AssetSupply" -t go modules/htlc/ -B 2 -A 5
Length of output: 94259
modules/htlc/types/expected_keepers.go (2)
4-7: LGTM: Import changes align with Cosmos SDK v0.50.10
The addition of the context import and retention of the sdk import properly support the interface updates.
11-34: LGTM: BankKeeper interface updates are consistent
The migration from sdk.Context to context.Context across all methods is correct and aligns with Cosmos SDK v0.50.10 standards.
Let's verify that all implementations of this interface have been updated:
modules/htlc/simulation/genesis.go (3)
34-43: LGTM! Integer type migration is complete and consistent
All integer fields in the first AssetParam have been correctly migrated to use math.NewInt and math.ZeroInt. The values remain appropriate for their respective purposes.
50-59: LGTM! Integer type migration is complete and consistent
All integer fields in the second AssetParam have been correctly migrated to use math.NewInt and math.ZeroInt. The values are appropriate and properly differentiated from the first AssetParam.
8-8: LGTM! Verify complete migration to math package
The addition of the math package import is correct and aligns with the Cosmos SDK's migration from sdk.Int to math.Int.
✅ Verification successful
Let me gather more specific information about the HTLC module to verify the migration status.
Let me check the specific file in question to verify if the migration is complete in the simulation package.
Migration to math package is complete in this file
The verification shows that modules/htlc/simulation/genesis.go has been fully migrated to use math.Int instead of sdk.Int. All integer operations in this file are using the new math package correctly, including:
- SupplyLimit fields using
math.NewIntandmath.ZeroInt - FixedFee, MinSwapAmount, and MaxSwapAmount using
math.NewInt - Asset supply amounts using
math.ZeroInt
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that all Int operations in the htlc module use the new math package
rg -l 'sdk\.NewInt|sdk\.ZeroInt' --type go
Length of output: 1655
Script:
#!/bin/bash
# Check for any remaining sdk.Int usages specifically in the HTLC module
rg 'sdk\.(Int|NewInt|ZeroInt)' --type go modules/htlc/
Length of output: 649
Script:
#!/bin/bash
# Check the content of the genesis.go file in the HTLC simulation package
cat modules/htlc/simulation/genesis.go
Length of output: 2474
modules/htlc/types/genesis_test.go (1)
7-7: LGTM: Correct import for the new math package
The addition of cosmossdk.io/math import is appropriate for the migration from sdk.Int to the new math package, aligning with cosmos-sdk v0.50.x standards.
modules/htlc/integration_test.go (3)
7-7: LGTM: Import addition aligns with cosmos-sdk upgrade.
The addition of cosmossdk.io/math import is consistent with the migration to cosmos-sdk v0.50.10.
31-33: LGTM: Consistent migration to cosmossdk.io/math package.
The changes correctly migrate from sdk.NewInt/sdk.ZeroInt to math.NewInt/math.ZeroInt while preserving the original values.
Also applies to: 38-40
47-49: LGTM: Consistent parameter updates for htltinc asset.
The changes maintain consistency with the htltbnb asset while using appropriate values for htltinc.
Also applies to: 54-56
modules/htlc/keeper/integration_test.go (3)
7-7: LGTM: Import change aligns with cosmos-sdk v0.50.10 upgrade
The addition of cosmossdk.io/math import is consistent with the migration from sdk.Int to math.Int types.
67-76: LGTM: Consistent math package migration for htltinc asset
The migration from sdk.Int to math.Int for "htltinc" asset parameters is correct. The values are appropriate for a time-limited asset:
- Limit: 100000000000000
- TimeBasedLimit: 50000000000
- FixedFee: 1000
- MinSwapAmount: 1
- MaxSwapAmount: 100000000000
51-60: LGTM: Verify numerical values after math package migration
The migration from sdk.Int to math.Int for "htltbnb" asset parameters is correct. The numerical values are preserved:
- Limit: 350000000000000
- FixedFee: 1000
- MinSwapAmount: 1
- MaxSwapAmount: 1000000000000
✅ Verification successful
Migration from sdk.Int to math.Int is consistent across test files
The verification shows that the numerical values for the htltbnb asset parameters are consistently used across test files and during the migration:
- The Limit value of 350000000000000 is consistently used in test files
- The FixedFee value of 1000 is consistently maintained across all test configurations
- The migration from
sdk.NewInttomath.NewIntis properly implemented while preserving the values
The changes in the integration test align with the values used in other test files and the migration path.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that these values are consistent with production configuration
rg -A 1 'Limit:.*350000000000000' --type go
rg -A 1 'FixedFee:.*1000.*htltbnb' --type go
Length of output: 997
Script:
#!/bin/bash
# Let's search for FixedFee with a broader pattern to understand the values used across the codebase
rg -B 5 -A 1 'FixedFee:.*1000' --type go
# Also search for htltbnb asset configurations
rg -B 10 -A 5 'htltbnb.*Asset' --type go
Length of output: 5277
modules/htlc/keeper/grpc_query_test.go (2)
Line range hint 1-150: Test coverage looks comprehensive
The test suite maintains good coverage of HTLC functionality including:
- Asset supply queries (single and multiple)
- HTLC queries
- Parameter queries
- Comprehensive test setup with multiple HTLC scenarios
The test logic remains robust despite the cosmos-sdk upgrade.
51-51: Verify the context creation approach for cosmos-sdk v0.50.10
The change from NewContext to NewContextLegacy appears to be related to the cosmos-sdk v0.50.10 upgrade. However, we should verify if this is the recommended approach and whether there's a non-legacy alternative we should consider.
Consider adding a comment explaining why NewContextLegacy is used to help future maintainers understand the context of this change:
+ // Using NewContextLegacy for compatibility with cosmos-sdk v0.50.10
suite.ctx = app.BaseApp.NewContextLegacy(false, tmproto.Header{Height: 1, Time: time.Now()})✅ Verification successful
The previous results show that within the HTLC module, all test files consistently use NewContextLegacy. However, we need to:
- Verify the specific context around these changes in HTLC module tests
- Confirm the cosmos-sdk version being used to understand if this is the recommended approach
Based on the search results, I can now provide a final response as we have sufficient context about the usage pattern in the HTLC module:
Usage of NewContextLegacy is consistent across HTLC module tests
The change from NewContext to NewContextLegacy is consistent with all other test files in the HTLC module. All test files in the module (grpc_query_test.go, asset_test.go, params_test.go, and htlc_test.go) use the same NewContextLegacy approach, showing this is a deliberate pattern across the module's test suite.
modules/htlc/keeper/grpc_query_test.gomodules/htlc/keeper/asset_test.gomodules/htlc/keeper/params_test.gomodules/htlc/keeper/htlc_test.go
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for context creation patterns in the codebase
echo "Searching for context creation patterns..."
rg "NewContext|NewContextLegacy" --type go
# Check cosmos-sdk documentation or examples
echo "Checking go.mod for cosmos-sdk version..."
rg "github.com/cosmos/cosmos-sdk" go.mod
Length of output: 8560
Script:
#!/bin/bash
# Let's check the HTLC module test files more specifically
echo "Checking HTLC module test files for context creation patterns..."
rg "NewContext|NewContextLegacy" modules/htlc/ --type go -B 2 -A 2
# Let's also check the go.mod file in the repository root
echo "Checking cosmos-sdk version in go.mod..."
cat go.mod | grep -A 5 "require"
Length of output: 4257
modules/htlc/keeper/params_test.go (1)
Line range hint 1-156: Test coverage looks comprehensive!
The test suite thoroughly covers all parameter-related functionality including asset management, deputy addresses, fees, swap amounts, and block locks. Both positive and negative test cases are well-handled.
modules/htlc/module.go (2)
166-167: LGTM!
The EndBlock method has been correctly updated to match the new cosmos-sdk v0.50.10 signature, and its no-op implementation is appropriate.
180-180: Verify other simulation type imports
The RegisterStoreDecoder signature has been correctly updated to use simtypes.StoreDecoderRegistry. Let's verify that all other simulation-related types are consistently using the new simtypes package.
✅ Verification successful
All simulation types have been properly migrated to simtypes package
Based on the search results, all simulation-related types in the HTLC module are correctly using the simtypes package from cosmos-sdk. This includes:
StoreDecoderRegistryin module.goWeightedOperation,Operation,OperationMsg, and other simulation types in operation.go- All simulation helper functions using
simtypesfor random number generation and account handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining sdk simulation type usages that should be migrated to simtypes
# Search for simulation-related types from sdk package
rg --type go 'sdk\.(StoreDecoderRegistry|SimulationState|WeightedOperation)' ./modules/htlc
Length of output: 93
Script:
#!/bin/bash
# Let's check for simulation-related code in the htlc module more broadly
# to verify the types being used
# Search for files containing simulation-related code
rg --type go -l "simulation" ./modules/htlc
# Then let's examine the content of these files to check the imports and type usage
rg --type go -B3 -A3 "simulation|simtypes" ./modules/htlc
Length of output: 13729
modules/htlc/simulation/operation.go (2)
34-37: LGTM: Parameter generation simplified
The removal of the codec parameter from GetOrGenerate calls simplifies the API while maintaining the appropriate weight distribution between create and claim operations.
Also applies to: 40-43
Line range hint 1-279: Verify simulation test coverage
Since these simulations are crucial for testing the HTLC module's behavior, let's verify that they are properly integrated into the test suite.
✅ Verification successful
Simulations are properly integrated and tested
The verification shows that the HTLC module's simulation operations are well integrated into the application's test suite:
-
The e2e simulation tests in
e2e/sim_test.goinclude comprehensive simulation testing:TestFullAppSimulation: Tests full application simulationTestAppImportExport: Verifies state consistency through export/importTestAppSimulationAfterImport: Tests simulation after state importTestAppStateDeterminism: Ensures deterministic simulation results
-
The HTLC module's store is explicitly included in the simulation store comparisons, ensuring its state is properly validated during import/export tests:
{app.GetKey(htlctypes.StoreKey), newApp.GetKey(htlctypes.StoreKey), [][]byte{}}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for simulation test coverage and integration
# Look for simulation test files
echo "Checking for simulation test files..."
fd -e go -e yaml simulation test
# Search for simulation test execution in go files
echo "Checking for simulation test execution..."
rg -l "simulation.*Test" -g "*.go"
# Check if these simulations are integrated into any test suites
echo "Checking for test suite integration..."
rg -l "WeightedOperations.*simulation" -g "*.go"
Length of output: 479
Script:
#!/bin/bash
# Let's try a different approach to find simulation tests
# Look for simulation test files in the entire repo
echo "Checking for simulation test files..."
fd simulation -e go
# Search for test files that import the simulation package
echo "Checking for test files importing simulation..."
rg -l "\"mods.irisnet.org/modules/htlc/simulation\"" -g "*_test.go"
# Look for any test files in the htlc module
echo "Checking for test files in htlc module..."
fd -e go . modules/htlc | grep "_test.go"
# Check for simulation test setup patterns
echo "Checking for simulation setup patterns..."
rg -l "func.*TestAll.*Simulation|func.*TestApp.*Simulation" -g "*.go"
Length of output: 1087
Script:
#!/bin/bash
# Let's check the e2e simulation test and integration test contents
# Check the e2e simulation test
echo "Checking e2e simulation test content..."
cat e2e/sim_test.go
# Check integration test for simulation usage
echo "Checking integration test for simulation usage..."
cat modules/htlc/integration_test.go
# Look for any simulation-related code in test files
echo "Checking for simulation usage in test files..."
rg -A 5 "simulation" modules/htlc/*_test.go modules/htlc/**/*_test.go
Length of output: 19227
modules/htlc/types/params_test.go (3)
7-7: LGTM: Appropriate migration to cosmossdk.io/math package
The addition of the math package import aligns with the cosmos-sdk v0.50.x architecture where the math package is separated from the core sdk package.
24-26: LGTM: Consistent migration of integer operations
The integer operations have been correctly migrated from sdk to math package while maintaining the same test setup values and behavior.
Also applies to: 30-32
63-65: LGTM: Comprehensive migration of test cases to math package
All test cases have been systematically updated to use the math package for integer operations while preserving:
- The original test coverage
- The validation scenarios
- The test values and conditions
The migration is thorough and consistent across all test cases, including:
- Valid parameter combinations
- Invalid parameter validations
- Edge cases and boundary testing
Also applies to: 82-84, 101-103, 113-115, 131-133, 150-152, 169-171, 188-190, 207-209, 226-228, 245-247, 261-264, 267-269, 284-286, 290-292, 307-310, 314-316, 333-335, 345-347
modules/htlc/keeper/depinject_test.go (2)
27-29: LGTM: Import paths correctly updated for cosmos-sdk v0.50.10
The migration from github.com/cosmos/cosmos-sdk/x/* to cosmossdk.io/x/* package paths aligns with the SDK's new package organization.
Line range hint 1-256: Verify HTLC module's capability dependencies
The removal of the capability module as part of the SDK v0.50.10 upgrade requires verification that the HTLC module doesn't have any remaining capability dependencies.
modules/htlc/keeper/asset.go (2)
7-8: LGTM: Import changes align with cosmos-sdk v0.50.10
The addition of cosmossdk.io/math and cosmossdk.io/store/types imports is consistent with the migration to cosmos-sdk v0.50.10, which moves these packages out of the main SDK package.
219-219: LGTM: Consistent use of math.ZeroInt() in supply limit reset
The change maintains functionality while aligning with the new math package usage.
modules/htlc/types/msgs_test.go (2)
8-8: LGTM: Import change aligns with Cosmos SDK v0.50.x guidelines
The addition of cosmossdk.io/math import follows the SDK's architectural change of moving math operations to a dedicated package.
25-25: LGTM: Correct migration of integer initialization
The change from sdk.NewInt(10) to math.NewInt(10) properly implements the new math package usage while maintaining the same functionality.
modules/htlc/keeper/htlc.go (4)
10-10: LGTM: Import change aligns with Cosmos SDK v0.50.10 upgrade
The addition of storetypes import from the new path cosmossdk.io/store/types is consistent with the modular architecture in Cosmos SDK v0.50.10.
407-407: LGTM: Iterator update maintains existing functionality
The change from sdk.KVStorePrefixIterator to storetypes.KVStorePrefixIterator is a clean upgrade that preserves the existing iteration behavior while aligning with the new SDK version.
429-429: LGTM: Consistent iterator update for expired queue
The iterator change in IterateHTLCExpiredQueueByHeight follows the same pattern as IterateHTLCs, maintaining consistency across the codebase while upgrading to the new SDK version.
407-407: Verify HTLC cleanup process with new iterators
While the iterator changes look correct, let's verify that the HTLC cleanup process still works as expected, particularly for expired HTLCs.
Also applies to: 429-429
✅ Verification successful
Let me gather more information about the iterator implementation and cleanup process.
Let me gather more specific information about the HTLC cleanup implementation.
Based on the gathered information, I can now provide the final response:
Iterator usage in HTLC module is properly implemented and safe
The code correctly implements iterator cleanup with defer iterator.Close() statements immediately after iterator creation. The HTLC cleanup process uses standard store prefix iteration patterns with proper resource management:
- Proper cleanup with
defer iterator.Close()at line 408 - Consistent usage of
KVStorePrefixIteratorwithHTLCKeyprefix - Safe iteration pattern with standard validity checks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the HTLC cleanup process is properly implemented with the new iterators
# Check for any TODO/FIXME comments related to iterators or cleanup
rg -i "todo|fixme" -g "modules/htlc/**/*.go"
# Look for any test cases covering the iterator functionality
rg -l "IterateHTLCs|IterateHTLCExpiredQueueByHeight" -g "modules/htlc/**/*_test.go"
# Check for any related issues or PRs mentioning iterator-related bugs
gh issue list --search "htlc iterator"
gh pr list --search "htlc iterator"
Length of output: 331
Script:
#!/bin/bash
# Check the implementation of iterator cleanup and deferred Close() calls
ast-grep --pattern 'defer $_.Close()'
# Look for the actual implementation of cleanup logic
rg -A 5 "CleanupHTLC|ExpiredHTLC" modules/htlc/
# Check test coverage for expired HTLCs
rg -A 10 "TestCleanupExpiredHTLCs|TestHandleHTLCExpired" modules/htlc/
# Check if there are any existing iterator patterns in the codebase
ast-grep --pattern 'KVStorePrefixIterator($_, $_)'
Length of output: 3163
Script:
#!/bin/bash
# Check the implementation around the iterator usage in HTLC module
rg -A 10 "HTLCKey|ExpiredHTLCKey" modules/htlc/
# Look for the cleanup and expiry handling logic
rg -A 15 "func.*CleanupHTLCs|func.*HandleExpiredHTLC" modules/htlc/keeper/
# Check for any error handling around iterator usage
rg -B 2 -A 5 "iterator.*Close\(\)" modules/htlc/keeper/
Length of output: 5893
modules/htlc/keeper/asset_test.go (3)
8-8: LGTM! Import change aligns with cosmos-sdk v0.50.10
The migration from sdk.Int to cosmossdk.io/math is part of the SDK's modularization effort.
52-54: LGTM! Consistent usage of math.NewInt
The changes correctly migrate from sdk.NewInt to math.NewInt while maintaining the same test values.
42-42: Verify the usage of NewContextLegacy
While this works, we should verify if this is the recommended approach in cosmos-sdk v0.50.10 or if we should be using a different context creation method.
modules/htlc/keeper/htlc_test.go (2)
Line range hint 1-600: Test coverage looks comprehensive
The test suite thoroughly covers the HTLC functionality with various test cases:
- Creation with different parameters and edge cases
- Claiming with valid/invalid secrets
- Refunding with proper/improper timing
- Asset supply tracking
- Error cases and validation
60-60: Consider the implications of using NewContextLegacy
The change from NewContext to NewContextLegacy suggests a transition in the Cosmos SDK. While this change maintains compatibility, it's important to note that using legacy methods might be deprecated in future SDK versions.
Consider creating a tracking issue to migrate away from legacy context creation methods in the future when the new context API stabilizes.
✅ Verification successful
The previous search revealed consistent usage of NewContextLegacy across the HTLC module tests, but we need to verify if this is part of a planned migration and check the SDK version being used to provide more accurate guidance.
Based on the search results, I can now provide a conclusive response:
Using NewContextLegacy is consistent with the codebase pattern
The usage of NewContextLegacy in the HTLC test file is consistent with its usage across other test files in the codebase, particularly within the HTLC module and other modules (oracle, farm, service, etc.). This appears to be a deliberate pattern rather than a deprecated approach.
- All HTLC module tests use
NewContextLegacyconsistently - Other modules like oracle, farm, and service follow the same pattern in their test files
- No deprecation notices or migration warnings were found related to context creation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other occurrences of NewContextLegacy to ensure consistent usage
rg "NewContextLegacy" -A 2 -B 2
# Check for any deprecation notices or migration guides in SDK
rg -g "*.md" "NewContextLegacy|NewContext.*deprecated"
Length of output: 1884
Script:
#!/bin/bash
# Let's check for NewContext usage to understand the transition pattern
rg "NewContext[^L]" -A 2 -B 2
# Let's also check for any SDK upgrade or migration related comments
rg -g "*.go" -B 3 "TODO|FIXME|@deprecated.*Context"
# Check go.mod for cosmos-sdk version
cat go.mod | grep cosmos-sdk
Length of output: 34657
modules/htlc/go.mod (3)
3-5: LGTM: Go version and toolchain update
The update to go 1.21 and addition of toolchain go1.23.1 are appropriate and keep the module up-to-date with the latest Go features.
8-19: LGTM: Dependency updates align with PR objectives
The dependencies have been updated to align with the upgrade to cosmos-sdk v0.50.10, as per the PR objectives. These updates include cosmossdk.io/api, cosmossdk.io/core, cosmossdk.io/math, github.com/cometbft/cometbft, and github.com/cosmos/cosmos-sdk. The updates appear consistent and should enhance compatibility and performance.
Line range hint 199-201: Review the replace directive for gin-gonic/gin
The replace directive for github.com/gin-gonic/gin addresses the GHSA-h395-qcrw-5vmq vulnerability. The TODO comment references issue #10409 for its removal. Please verify if the latest version of gin-gonic/gin has resolved this vulnerability so that the replace directive can be updated or removed.
I can help check if the latest version of gin-gonic/gin addresses the vulnerability. Would you like me to verify this and propose necessary updates?
api/irismod/htlc/htlc.pulsar.go (1)
Line range hint 4622-4691: Verify that the generated code matches the updated proto definitions
After upgrading to cosmos-sdk v0.50.10, ensure that the htlc.pulsar.go file is regenerated to reflect any changes in the .proto files, preventing inconsistencies between the code and the data structures.
Run the following script to check if the generated Go files are up-to-date with their corresponding proto files:
This script uses fd to locate .proto files and checks if the corresponding .pb.go files are older, indicating they need to be regenerated.
✅ Verification successful
Let me run another script to check the specific file mentioned in the review comment.
Let me check the content of both the proto file and generated pulsar file to verify if they match.
Let me try one more verification to check the file modification times using a different command.
Based on the verification results, I can now provide a final response:
The generated pulsar code is up-to-date with proto definitions
The file timestamps show that both htlc.proto and htlc.pulsar.go were last modified at the same time (Nov 20 07:20), indicating the pulsar file was regenerated along with any proto changes. The binary content in the pulsar file matches the expected protobuf encoding patterns, including the correct message structure for HTLC, HTLCState, and SwapDirection enums.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that generated Go files are up-to-date with the proto files.
# Find all .proto files and compare their timestamps with the generated .go files.
for proto_file in $(fd -e proto); do
go_file="$(dirname "$proto_file")/$(basename "${proto_file%.*}.pb.go")"
if [ -f "$go_file" ]; then
if [[ "$proto_file" -nt "$go_file" ]]; then
echo "Warning: $go_file is older than $proto_file. Regeneration needed."
fi
else
echo "Generated file $go_file does not exist. Generation needed."
fi
done
Length of output: 21936
Script:
#!/bin/bash
# Check if the htlc.pulsar.go file exists and compare with proto file
if [ -f "api/irismod/htlc/htlc.pulsar.go" ]; then
echo "htlc.pulsar.go exists"
# Get file info
ls -l api/irismod/htlc/htlc.pulsar.go
# Check corresponding proto file
if [ -f "proto/irismod/htlc/htlc.proto" ]; then
echo -e "\nCorresponding proto file exists:"
ls -l proto/irismod/htlc/htlc.proto
fi
else
echo "htlc.pulsar.go does not exist at the specified path"
fi
# Check for any other pulsar generated files
echo -e "\nSearching for other pulsar generated files:"
fd -e pulsar.go
Length of output: 2604
Script:
#!/bin/bash
# Compare the proto definitions with generated code
echo "=== Proto file content ==="
head -n 50 proto/irismod/htlc/htlc.proto
echo -e "\n=== Generated pulsar file relevant section ==="
# Extract the relevant section around line 4630
awk 'NR>=4620 && NR<=4700' api/irismod/htlc/htlc.pulsar.go
# Get file timestamps to check if pulsar file is newer
echo -e "\n=== File timestamps ==="
stat -c "Proto file: %y" proto/irismod/htlc/htlc.proto
stat -c "Pulsar file: %y" api/irismod/htlc/htlc.pulsar.go
Length of output: 9314
Script:
#!/bin/bash
# Check file modification times using ls
echo "=== File modification times ==="
ls -l proto/irismod/htlc/htlc.proto api/irismod/htlc/htlc.pulsar.go
# Check if the pulsar file contains the correct package path after cosmos-sdk upgrade
echo -e "\n=== Package path in pulsar file ==="
grep -A 5 "go_package" api/irismod/htlc/htlc.pulsar.go
# Check cosmos-sdk version in go.mod
echo -e "\n=== Cosmos-SDK version ==="
grep "cosmos-sdk" go.mod
Length of output: 582
| GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI | ||
| SetAccount(ctx context.Context, acc sdk.AccountI) | ||
| NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI | ||
| GetModuleAddress(name string) sdk.AccAddress | ||
| GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI | ||
| SetModuleAccount(ctx sdk.Context, macc authtypes.ModuleAccountI) | ||
| GetModuleAccount(ctx context.Context, name string) sdk.ModuleAccountI | ||
| SetModuleAccount(ctx context.Context, macc sdk.ModuleAccountI) | ||
| GetModuleAddressAndPermissions(moduleName string) (sdk.AccAddress, []string) | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistency found in AccountKeeper interface implementations
The AccountKeeper interface in different modules shows inconsistencies with the updated interface in htlc:
- Some modules still use
sdk.Contextandauthtypes.AccountI(coinswap, farm, mt, oracle) - Others have been updated to
context.Contextandsdk.AccountI(service, record, random, nft) - The htlc module has the most comprehensive interface with additional methods
This needs to be harmonized across all modules for consistency with Cosmos SDK v0.50.10.
Affected files:
modules/coinswap/types/expected_keepers.gomodules/farm/types/expected_keeper.gomodules/mt/types/expected_keepers.gomodules/oracle/types/expected_keepers.gomodules/token/types/expected_keepers.go
🔗 Analysis chain
LGTM: AccountKeeper interface updates are well-structured
The changes properly update both context types and return types to use SDK interfaces, improving modularity and maintaining consistency with Cosmos SDK v0.50.10.
Let's verify the AccountKeeper implementations:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for AccountKeeper implementations to ensure they're updated
ast-grep --pattern 'type $_ struct {
$$$
}
func ($_ $_) GetAccount(ctx sdk.Context, $$$) $$$'
# Check for any remaining authtypes.AccountI usage
rg "authtypes.AccountI" modules/htlc/
Length of output: 136
Script:
#!/bin/bash
# Let's try a broader search for AccountKeeper implementations
rg -A 5 "type.*AccountKeeper.*struct" .
# Search for GetAccount implementations with context.Context
ast-grep --pattern 'func ($_ $_) GetAccount(ctx context.Context, $$$) sdk.AccountI'
# Search for any GetAccount implementations with sdk.Context
ast-grep --pattern 'func ($_ $_) GetAccount(ctx sdk.Context, $$$) $$$'
# Check keeper implementations
rg -l "AccountKeeper.*interface" .
Length of output: 656
Script:
#!/bin/bash
# Let's check the AccountKeeper interface definition in other modules
rg -A 10 "type AccountKeeper interface" ./modules/*/types/expected_keeper*.go
# Check if there are any implementations in keeper directories
rg -A 5 "func \(.*\) GetAccount\(" ./modules/*/keeper/
Length of output: 8424
| @@ -3,8 +3,8 @@ package htlc | |||
| import ( | |||
| "cosmossdk.io/core/appmodule" | |||
| "cosmossdk.io/depinject" | |||
| storetypes "cosmossdk.io/store/types" | |||
There was a problem hiding this comment.
💡 Codebase verification
The import path cosmossdk.io/store/types needs to be updated consistently across the codebase
Several files are still using the old import path github.com/cosmos/cosmos-sdk/store/types which needs to be updated to cosmossdk.io/store/types for consistency with Cosmos SDK v0.50.10:
modules/token/depinject.goe2e/sim_test.gomodules/token/keeper/keeper.gomodules/oracle/depinject.gomodules/oracle/keeper/keeper.gomodules/coinswap/depinject.gomodules/farm/depinject.gomodules/farm/keeper/keeper.gomodules/coinswap/keeper/keeper.go
🔗 Analysis chain
LGTM: Import path update aligns with Cosmos SDK v0.50.10
The change from github.com/cosmos/cosmos-sdk/store/types to cosmossdk.io/store/types is correct and consistent with the Cosmos SDK v0.50.10 module structure.
Let's verify this change is consistent across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining old store types imports that need updating
# Expected: No results from old import path
rg "github.com/cosmos/cosmos-sdk/store/types" --type go
Length of output: 882
| @@ -42,7 +42,7 @@ | |||
|
|
|||
| Config *modulev1.Module | |||
| Cdc codec.Codec | |||
| Key *store.KVStoreKey | |||
| Key *storetypes.KVStoreKey | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent KVStoreKey type usage across modules
The search results show that while modules/htlc and modules/mt use *storetypes.KVStoreKey, other modules like token, random, service, record, oracle, coinswap, and farm are still using *store.KVStoreKey. This indicates an inconsistency in the type usage across the codebase.
modules/htlc/depinject.go: Uses*storetypes.KVStoreKeymodules/mt/depinject.go: Uses*storetypes.KVStoreKeymodules/token/depinject.go: Uses*store.KVStoreKeymodules/random/depinject.go: Uses*store.KVStoreKeymodules/service/depinject.go: Uses*store.KVStoreKeymodules/record/depinject.go: Uses*store.KVStoreKeymodules/oracle/depinject.go: Uses*store.KVStoreKeymodules/coinswap/depinject.go: Uses*store.KVStoreKeymodules/farm/depinject.go: Uses*store.KVStoreKey
🔗 Analysis chain
LGTM: KVStoreKey type update matches the new import
The type change from *store.KVStoreKey to *storetypes.KVStoreKey correctly reflects the updated import path and maintains the same functionality.
Let's verify the keeper initialization still works with this type:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check keeper initialization patterns to ensure consistent type usage
# Expected: All keeper initializations should use storetypes.KVStoreKey
ast-grep --pattern 'NewKeeper($$$, $key, $$$)'
Length of output: 48
Script:
#!/bin/bash
# Let's try a different approach to find keeper initialization patterns
# First, let's find the NewKeeper function definition
ast-grep --pattern 'func NewKeeper($$$) $$ {'
# Also search for any direct usage of KVStoreKey type
rg "KVStoreKey" -A 2 -B 2
Length of output: 3152
| @@ -17,7 +18,7 @@ type GenesisTestSuite struct { | |||
| } | |||
|
|
|||
| func (suite *GenesisTestSuite) SetupTest() { | |||
| coin := sdk.NewCoin("htltbnb", sdk.OneInt()) | |||
| coin := sdk.NewCoin("htltbnb", math.OneInt()) | |||
There was a problem hiding this comment.
💡 Codebase verification
Migration to math.OneInt() is incomplete across the codebase
Several files still use sdk.OneInt() and need to be migrated to math.OneInt():
modules/token/keeper/fees.goe2e/htlc/tx.gomodules/coinswap/keeper/swap.gomodules/htlc/migrations/v2/migrate_test.go
🔗 Analysis chain
LGTM: Correct migration to math.OneInt()
The change from sdk.OneInt() to math.OneInt() is correct and consistent with the cosmos-sdk v0.50.x upgrade.
Let's verify consistent usage across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining instances of sdk.OneInt() that should be migrated
rg "sdk\.OneInt\(\)" --type go
# Check for proper usage of math.OneInt()
rg "math\.OneInt\(\)" --type go
Length of output: 1100
| @@ -39,7 +39,7 @@ func (suite *ParamsTestSuite) SetupTest() { | |||
| return state | |||
| }, | |||
| ) | |||
| suite.ctx = app.BaseApp.NewContext(false, tmproto.Header{Height: 1, Time: time.Now()}) | |||
| suite.ctx = app.BaseApp.NewContextLegacy(false, tmproto.Header{Height: 1, Time: time.Now()}) | |||
There was a problem hiding this comment.
💡 Codebase verification
Similar context creation updates needed in multiple test files
Several test files are still using the old NewContext method and need to be updated to NewContextLegacy for SDK v0.50.10 compatibility:
modules/token/keeper/keeper_test.gomodules/record/keeper/keeper_test.gomodules/service/keeper/keeper_test.gomodules/nft/keeper/keeper_test.gomodules/oracle/keeper/keeper_test.gomodules/random/keeper/keeper_test.gomodules/farm/keeper/keeper_test.gomodules/coinswap/keeper/keeper_test.goe2e/sim_test.go
🔗 Analysis chain
LGTM! Context creation updated for SDK v0.50.10 compatibility.
The change from NewContext to NewContextLegacy aligns with the PR objective of upgrading to Cosmos SDK v0.50.10.
Let's verify if similar context creation updates are needed in other test files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other test files that might need similar context creation updates
# Look for test files using the old NewContext method that might need updating
# Search for test files using NewContext
rg -t go "NewContext\(" --glob "**/*_test.go"
# Search for test files already using NewContextLegacy for comparison
rg -t go "NewContextLegacy\(" --glob "**/*_test.go"
Length of output: 2826
Closed: #439
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Refactor
Tests