Skip to content
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

bump up mods.irisnet.org/modules/oracle to cosmos-sdk v0.50.10 #456

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

wangjiulian
Copy link
Collaborator

@wangjiulian wangjiulian commented Nov 19, 2024

Closed: #443

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced message handling structure with updated method signatures in the oracle module.
    • Streamlined integer handling in simulation operations for better consistency.
  • Bug Fixes

    • Improved handling of ServiceFeeCap across various test cases for better validation.
  • Documentation

    • Updated context handling across multiple interfaces to align with standard Go practices.
  • Chores

    • Comprehensive updates to dependencies and Go version for improved compatibility and security.

Copy link

coderabbitai bot commented Nov 19, 2024

Walkthrough

The pull request introduces several changes primarily focused on updating package imports and modifying method signatures to align with the latest version of the Cosmos SDK. Key changes include the transition from store to storetypes, updates to the Go version and various dependencies in the go.mod file, and adjustments to method signatures for improved error handling and context management. The overall functionality remains intact, with no new features introduced or existing functionalities altered beyond these structural updates.

Changes

File Change Summary
modules/oracle/depinject.go Updated import from store to storetypes; changed Key field type in Inputs struct.
modules/oracle/go.mod Updated Go version from 1.19 to 1.21; upgraded several dependencies including cosmossdk.io/core and github.com/cosmos/cosmos-sdk.
modules/oracle/handler.go Removed NewHandler function.
modules/oracle/keeper/depinject_test.go Removed import for capabilitymodulev1; added imports for evidencetypes, feegrant, and upgradetypes; removed capabilitytypes.ModuleName from various arrays.
modules/oracle/keeper/feed.go Updated iterator methods to use storetypes instead of sdk; added import for storetypes.
modules/oracle/keeper/grpc_query.go Changed import path for prefix from github.com/cosmos/cosmos-sdk/store/prefix to cosmossdk.io/store/prefix.
modules/oracle/keeper/keeper.go Updated import for storetypes from github.com/cosmos/cosmos-sdk/store/types to cosmossdk.io/store/types.
modules/oracle/keeper/keeper_test.go Added import for math from cosmossdk.io; replaced sdk.NewInt(100) with math.NewInt(100).
modules/oracle/module.go Refactored BeginBlock and EndBlock methods to accept context.Context and return error. Updated RegisterStoreDecoder parameter type.
modules/oracle/simulation/operation.go Added import for math; replaced sdk.NewInt with math.NewInt; simplified NewOperationMsg calls.
modules/oracle/types/expected_keepers.go Updated context parameter type from sdk.Context to context.Context in several methods.
modules/oracle/types/msgs_test.go Replaced sdk.NewInt(100) with math.NewInt(100) in multiple test cases.

Assessment against linked issues

Objective Addressed Explanation
Bump up mods.irisnet.org/modules/oracle to cosmos-sdk v0.50.10 ( #443 )

Possibly related issues

Possibly related PRs

Suggested reviewers

  • mitch1024
  • dreamer-zq

🐇 In the code where I hop and play,
Imports change and grow each day.
From store to storetypes we leap,
Upgrading paths, our code runs deep.
With new versions, we refine,
In the world of Go, our code will shine! 🌟


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between bb78ac5 and 7943e19.

📒 Files selected for processing (1)
  • modules/oracle/handler.go (0 hunks)
💤 Files with no reviewable changes (1)
  • modules/oracle/handler.go

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (8)
modules/oracle/go.mod (2)

Line range hint 208-210: Address TODO: Remove deprecated jwt-go dependency

The github.com/dgrijalva/jwt-go package is deprecated and no longer receives security updates. It's been replaced with github.com/golang-jwt/jwt/v4 v4.4.2. A TODO is noted to remove it (cosmos/cosmos-sdk#13134).

Would you like assistance in identifying and refactoring code to eliminate the usage of dgrijalva/jwt-go? I can help generate a plan or code suggestions to expedite this process.


Line range hint 211-213: Review necessity of gin-gonic/gin dependency

The github.com/gin-gonic/gin package has been updated to v1.9.0 to fix the GHSA-h395-qcrw-5vmq vulnerability. A TODO is noted to remove it (cosmos/cosmos-sdk#10409). Consider evaluating whether this dependency is still required.

Would you like assistance in assessing the usage of gin-gonic/gin within the module and exploring alternatives or removal strategies?

modules/oracle/types/expected_keepers.go (1)

ServiceKeeper interface should be updated to use context.Context

The ServiceKeeper interface should be updated to use context.Context instead of sdk.Context to maintain consistency with other modules in the codebase. This is evidenced by:

  • Other keeper interfaces like NFT, Oracle and Token modules have already migrated to using context.Context
  • The AccountKeeper and BankKeeper interfaces in those modules use context.Context for their methods
  • This change aligns with the Cosmos SDK v0.50.x context standardization efforts

Key locations that need updates:

  • modules/oracle/types/expected_keepers.go: ServiceKeeper interface methods should use context.Context instead of sdk.Context
  • All method signatures need to be updated to use ctx context.Context instead of ctx sdk.Context
🔗 Analysis chain

Line range hint 13-89: Verify if ServiceKeeper interface needs context updates

The ServiceKeeper interface still uses sdk.Context while other interfaces have been updated to use context.Context. This inconsistency might need to be addressed.

Let's check if other modules in cosmos-sdk v0.50.10 have updated their service-related interfaces:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for service keeper interfaces in the codebase that might have been updated
rg -l "type ServiceKeeper interface" | xargs rg "ctx.*?Context"

# Check if there are any recent changes to similar interfaces
git log -p --grep="context" -- "**/keeper.go" "**/expected_keepers.go"

Length of output: 7625


Script:

#!/bin/bash
# Let's search for ServiceKeeper interface and its context usage
rg -l "type ServiceKeeper interface" | xargs cat

# Also check for any recent context-related changes in service module
git log -p --grep="context" -- "modules/service/**/*"

# Check for any migration guides or documentation about context changes
fd -g "MIGRATING*.md" -g "UPGRADE*.md" -g "CHANGELOG.md" | xargs rg -i "context"

Length of output: 12974


Script:

#!/bin/bash
# Let's check the service module's implementation to see if it's using context.Context
ast-grep --pattern 'func ($KEEPER *Keeper) $METHOD($CTX context.Context, $$$)'

# Also check for any recent PRs or issues discussing context migration
git log --oneline --grep="context\|sdk.Context" -n 10

# Check the cosmos-sdk v0.50.x upgrade guide
fd -g "UPGRADING*.md" | xargs rg "context.*?Context"

Length of output: 283258


Script:

#!/bin/bash
# Let's check the service module's keeper implementation to see if it's using context.Context
rg -g "keeper.go" "func.*?\(.*?context\.Context.*?\)" modules/service/

# Check if there are any recent PRs or commits related to context migration
git log --oneline -p --grep="context" -- "modules/service/**/*" | head -n 50

# Check for any migration guides in the service module
fd -g "MIGRATING*.md" -g "UPGRADE*.md" modules/service/ | xargs cat

Length of output: 2549


Script:

#!/bin/bash
# Let's check the AccountKeeper and BankKeeper interfaces in the service module to see if they've been updated to use context.Context
rg -g "expected_keepers.go" "type (Account|Bank)Keeper interface" -A 5 modules/service/

# Check for any context-related changes in the cosmos-sdk v0.50.x
git log --oneline -n 5 --grep="context" origin/release/v0.50.x

# Check if other modules have been updated to use context.Context
rg -g "expected_keepers.go" "ctx context.Context" modules/

Length of output: 1051

modules/oracle/module.go (1)

164-164: Consider implementing store decoder for simulation support

While the type change to simtypes.StoreDecoderRegistry is correct, the empty implementation might limit simulation capabilities. Consider implementing a proper store decoder to support debugging and testing during simulations.

Example implementation:

-func (am AppModule) RegisterStoreDecoder(sdr simtypes.StoreDecoderRegistry) {}
+func (am AppModule) RegisterStoreDecoder(sdr simtypes.StoreDecoderRegistry) {
+    sdr[types.StoreKey] = simulation.NewDecodeStore(am.cdc)
+}
modules/oracle/keeper/keeper.go (1)

Line range hint 291-297: Consider making timeout and error codes configurable

The ModuleServiceRequest function has some hardcoded values that could be made configurable:

  • 5-minute timeout for value expiration
  • HTTP-style error codes (400, 401, 402)

Consider:

  1. Making the timeout configurable through module parameters
  2. Defining error codes as constants with meaningful names
+const (
+    ErrCodeFeedNotFound = "400"
+    ErrCodeNoValue      = "401"
+    ErrCodeValuesExpired = "402"
+)
+
 func (k Keeper) ModuleServiceRequest(ctx sdk.Context, input string) (result, output string) {
     feedName := gjson.Get(input, serviceexported.PATH_BODY).Get("pair").String()
     if _, found := k.GetFeed(ctx, feedName); !found {
-        result = `{"code":"400","message":"feed not found"}`
+        result = fmt.Sprintf(`{"code":"%s","message":"feed not found"}`, ErrCodeFeedNotFound)
         return
     }
modules/oracle/keeper/depinject_test.go (2)

Line range hint 52-61: Remove outdated comment about capability module

The comment mentioning that "Capability module must occur first" is now outdated since the capability module has been removed from the initialization sequence.

- // 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.

Inconsistency in capability module handling across test files

The removal of the capability module in modules/oracle/keeper/depinject_test.go appears to be inconsistent with other module test files in the codebase. Other module test files (token, random, record, farm, htlc, coinswap) still include the capability module in their module ordering and configuration.

Key findings:

  • The capability module is still present in cosmos-sdk v0.50.10
  • Other test files in the codebase maintain capability module configuration
  • All other test files include the comment "capability module's beginblocker must come before any modules using capabilities"
🔗 Analysis chain

Line range hint 63-67: Verify removal of capability module

While the module ordering looks correct, the removal of the capability module is a significant change that warrants verification.

Let's verify if this change is consistent with cosmos-sdk v0.50.10:


Let me gather more information about the capability module usage in the codebase to understand if its removal is part of a broader migration pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if capability module is still used in cosmos-sdk v0.50.10
# and if other repos have made similar changes

# Check cosmos-sdk v0.50.10 for capability module usage
gh api repos/cosmos/cosmos-sdk/contents/x/capability?ref=v0.50.10 || echo "Capability module not found in v0.50.10"

# Search for similar migration patterns in other cosmos-sdk based projects
rg -l "capabilitytypes.ModuleName" --type go

Length of output: 610


Script:

#!/bin/bash
# Check the content of other depinject_test.go files to see how they handle capability module
rg -A 5 -B 5 "ModuleName.*capability" modules/*/keeper/depinject_test.go

# Check if there are any capability-related imports that might need cleanup
rg "capabilitytypes" --type go

# Check for any capability keeper or store key references
rg "capability" --type go | grep -i "keeper\|storekey"

Length of output: 8989

modules/oracle/types/msgs_test.go (1)

Based on the search results, I can generate the final response since we have comprehensive information about math-related operations in the codebase:

Migration to cosmossdk.io/math package is incomplete

Several modules still use the legacy sdk.Int, sdk.Dec, and related math operations from the cosmos-sdk types package. These need to be migrated to the new cosmossdk.io/math package:

  • modules/token/: Uses sdk.NewInt, sdk.Dec, sdk.NewDec extensively for token operations and calculations
  • modules/coinswap/: Uses sdk.Dec for fee calculations and price computations
  • modules/farm/: Uses sdk.Dec for reward calculations and tax rates
  • modules/htlc/: Uses sdk.NewInt for limits and fee calculations
🔗 Analysis chain

Line range hint 1-379: Verify complete migration of math operations

Let's ensure all math-related operations have been migrated to the new package across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining instances of sdk.NewInt that might need migration
rg "sdk\.NewInt\(" --type go

# Search for other potential math operations that might need migration
rg "sdk\.(Int|Dec|NewDec)" --type go

Length of output: 47173

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3452658 and bb78ac5.

⛔ Files ignored due to path filters (1)
  • modules/oracle/go.sum is excluded by !**/*.sum
📒 Files selected for processing (12)
  • modules/oracle/depinject.go (2 hunks)
  • modules/oracle/go.mod (2 hunks)
  • modules/oracle/handler.go (2 hunks)
  • modules/oracle/keeper/depinject_test.go (2 hunks)
  • modules/oracle/keeper/feed.go (6 hunks)
  • modules/oracle/keeper/grpc_query.go (1 hunks)
  • modules/oracle/keeper/keeper.go (1 hunks)
  • modules/oracle/keeper/keeper_test.go (4 hunks)
  • modules/oracle/module.go (2 hunks)
  • modules/oracle/simulation/operation.go (8 hunks)
  • modules/oracle/types/expected_keepers.go (2 hunks)
  • modules/oracle/types/msgs_test.go (17 hunks)
✅ Files skipped from review due to trivial changes (1)
  • modules/oracle/keeper/grpc_query.go
🔇 Additional comments (31)
modules/oracle/go.mod (6)

3-3: Go version updated to 1.21: Ensure environment compatibility

The Go version has been updated to 1.21. Please verify that all development, testing, and production environments are using Go 1.21 to prevent any compatibility issues. Update any CI/CD pipelines, Dockerfiles, or documentation accordingly.


6-24: Updated core dependencies: Review for compatibility and breaking changes

Several core dependencies have been updated to newer versions:

  • cosmossdk.io/core to v0.11.1
  • cosmossdk.io/depinject to v1.0.0
  • github.com/cometbft/cometbft to v0.38.12
  • github.com/cosmos/cosmos-sdk to v0.50.10
  • Other related dependencies.

Please review the release notes and changelogs for these dependencies to identify any breaking changes or required code modifications. Ensure that the module remains compatible and all tests pass with these updates.


19-21: Google dependencies updated: Confirm API changes

The following Google packages have been updated:

  • google.golang.org/genproto/googleapis/api to v0.0.0-20240318140521-94a12d6c2237
  • google.golang.org/grpc to v1.64.1
  • google.golang.org/protobuf to v1.34.2

Verify that these updates do not introduce any breaking changes in API usage within the module. Update any affected code accordingly.


22-24: Module dependencies updated: Ensure compatibility with internal modules

Updated internal module dependencies:

  • mods.irisnet.org/api to v0.0.0-20241118093307-345265846e1d
  • mods.irisnet.org/modules/service to v0.0.0-20241118093307-345265846e1d
  • mods.irisnet.org/simapp to v0.0.0-20241118093307-345265846e1d

Confirm that these internal modules are compatible with the updates and that any inter-module interactions function as expected.


27-34: New dependencies added: Validate necessity and integration

A new require block has been introduced with additional dependencies:

  • cosmossdk.io/api v0.7.5
  • cosmossdk.io/math v1.3.0
  • cosmossdk.io/store v1.1.1
  • cosmossdk.io/x/evidence v0.1.1
  • cosmossdk.io/x/feegrant v0.1.1
  • cosmossdk.io/x/upgrade v0.1.4

Ensure that these new dependencies are necessary for the module's functionality and have been properly integrated. Check for any conflicts or redundant packages.


Line range hint 220-220: Indirect dependencies updated: Monitor for side effects

Several indirect dependencies have been updated or added. While indirect, these can sometimes introduce unforeseen issues. Keep an eye on:

  • github.com/hashicorp/go-metrics v0.5.3
  • github.com/sagikazarmark/locafero v0.4.0
  • Other indirect dependencies listed.

Ensure that the module operates correctly with these updated indirect dependencies, and run comprehensive tests to detect any anomalies.

modules/oracle/handler.go (2)

5-5: LGTM: Required import for SDK v0.50.10 compatibility

The addition of the baseapp import is necessary for the new message handler pattern in Cosmos SDK v0.50.10.


Line range hint 14-38: LGTM: Correct implementation of the new message handler pattern

The changes properly implement the new message handler pattern from Cosmos SDK v0.50.10:

  • Return type changed to baseapp.MsgServiceHandler
  • Maintains existing message processing logic
  • Correctly uses sdk.WrapServiceResult for all message handlers
  • Proper error handling with errorsmod.Wrapf

Let's verify the consistency of message handling across the module:

✅ Verification successful

All message handlers consistently follow the new pattern

The verification confirms that all message handlers in the oracle module follow the same pattern:

  • Each handler properly wraps the SDK context using sdk.WrapSDKContext(ctx)
  • All handlers consistently use sdk.WrapServiceResult(ctx, res, err) for returning results
  • The pattern is uniform across CreateFeed, StartFeed, PauseFeed, and EditFeed handlers
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all message handlers follow the same pattern
# Expected: All handlers should use WrapServiceResult and WrapSDKContext

# Check for consistent usage of WrapServiceResult and WrapSDKContext
rg -A 1 "msgServer\." modules/oracle/

Length of output: 704

modules/oracle/depinject.go (2)

6-6: LGTM: Import path aligns with cosmos-sdk v0.50.10

The change from store to cosmossdk.io/store/types is correct and consistent with the cosmos-sdk v0.50.10 package structure.

Let's verify this is the correct import path in cosmos-sdk v0.50.10:

#!/bin/bash
# Description: Verify the store types import path in cosmos-sdk v0.50.10
# Expected: Should find references to this import path in go.mod

rg -l "cosmossdk.io/store/types" "go.*"

35-35: LGTM: Store key type updated correctly

The change from *store.KVStoreKey to *storetypes.KVStoreKey is the correct type migration for cosmos-sdk v0.50.10. This maintains compatibility with the keeper's store key requirements.

Let's verify this type is used consistently across the module:

✅ Verification successful

Store key type is consistently using storetypes.KVStoreKey across the codebase

The verification confirms that:

  • No instances of the old store.KVStoreKey type were found
  • The new storetypes.KVStoreKey type is consistently used across the codebase in:
    • Oracle module's dependency injection
    • MT module's dependency injection
    • SimApp's store key management
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of storetypes.KVStoreKey
# Expected: Should find only the new type being used

# Search for any remaining old store.KVStoreKey references
rg "store\.KVStoreKey" --type go

# Search for the new storetypes.KVStoreKey usage
rg "storetypes\.KVStoreKey" --type go

Length of output: 721

modules/oracle/types/expected_keepers.go (4)

4-5: LGTM: Required import for context.Context

The addition of the context import is appropriate for the interface changes.


92-92: LGTM: Updated context type in Authorized method

The change from sdk.Context to context.Context aligns with the cosmos-sdk v0.50.10 update.


98-98: LGTM: Updated context type in SpendableCoins method

The change from sdk.Context to context.Context aligns with the cosmos-sdk v0.50.10 update.


101-101: LGTM: Updated context type in GetAccount method

The change from sdk.Context to context.Context aligns with the cosmos-sdk v0.50.10 update.

Let's verify the return type sdk.AccountI is the correct type in cosmos-sdk v0.50.10:

#!/bin/bash
# Check the AccountKeeper interface definition in cosmos-sdk v0.50.10
rg "type AccountKeeper interface" -A 10
modules/oracle/keeper/feed.go (2)

6-12: LGTM: Import changes align with SDK upgrade

The addition of storetypes import is consistent with the migration to cosmos-sdk v0.50.10, where store types were moved to a dedicated package.


42-42: LGTM: Consistent iterator migration pattern

The changes correctly migrate all iterator usage from the SDK store package to the new storetypes package. The modifications maintain proper iterator handling with consistent patterns:

  • Proper use of defer iterator.Close()
  • Consistent prefix and reverse prefix iterator usage
  • Preserved existing functionality while adapting to the new SDK version

The changes affect the following methods:

  • IteratorFeeds
  • IteratorFeedsByState
  • GetFeedValues
  • getFeedValuesCnt
  • deleteOldestFeedValue

Also applies to: 58-58, 99-99, 138-138, 148-148

modules/oracle/module.go (2)

151-151: LGTM: BeginBlock signature updated correctly

The method signature has been properly updated to match Cosmos SDK v0.50.x module interface requirements.


154-154: LGTM: EndBlock signature updated correctly

The method signature has been properly updated to match Cosmos SDK v0.50.x module interface requirements. The removal of validator updates return is aligned with the new SDK version.

modules/oracle/keeper/keeper.go (3)

Line range hint 20-24: Implementation maintains compatibility with new storetypes

The Keeper struct and NewKeeper implementation correctly use the storetypes interfaces from the new import path. The implementation remains compatible with cosmos-sdk v0.50.10.

Also applies to: 27-42


Line range hint 1-300: Error handling implementation is robust

The file demonstrates consistent and thorough error handling:

  • Proper use of errorsmod.Wrapf for error context
  • Comprehensive error logging in handler functions
  • Clear error messages with relevant context

Line range hint 1-300: Successfully adapted to cosmos-sdk v0.50.10

The changes successfully update the module to be compatible with cosmos-sdk v0.50.10 while maintaining all existing functionality. The implementation:

  • Correctly uses the new import paths
  • Maintains compatibility with all interfaces
  • Preserves existing behavior
modules/oracle/keeper/depinject_test.go (1)

27-29: LGTM: Import changes align with cosmos-sdk v0.50.10

The new imports correctly use the cosmossdk.io/x/ paths, which is consistent with the module structure in cosmos-sdk v0.50.10.

modules/oracle/keeper/keeper_test.go (2)

8-8: LGTM: Correct import for cosmos-sdk v0.50.10

The addition of cosmossdk.io/math import aligns with the cosmos-sdk v0.50.10 upgrade, where math operations were moved to a dedicated package.


81-81: LGTM: Consistent usage of new math package

The changes correctly migrate from sdk.NewInt to math.NewInt for ServiceFeeCap values, maintaining consistency across all test cases while adapting to the new cosmos-sdk v0.50.10 API.

Also applies to: 149-149

modules/oracle/types/msgs_test.go (2)

4-4: LGTM: Import change aligns with cosmos-sdk v0.50.x

The addition of cosmossdk.io/math import is correct as math operations were moved to a dedicated package in cosmos-sdk v0.50.x.


35-35: LGTM: Consistent migration of NewInt calls to math package

The replacement of sdk.NewInt with math.NewInt across all test cases is correct and consistent with the cosmos-sdk v0.50.x upgrade. The test logic and values remain unchanged, maintaining the original test coverage.

Also applies to: 53-53, 71-71, 89-89, 107-107, 125-125, 143-143, 161-161, 179-179, 197-197, 215-215, 318-318, 330-330, 342-342, 354-354, 366-366

modules/oracle/simulation/operation.go (5)

7-7: LGTM: Import change aligns with cosmos-sdk v0.50.10

The addition of cosmossdk.io/math import is consistent with the module upgrade to cosmos-sdk v0.50.10, where the math package has been moved to this new location.


38-38: LGTM: Simplified GetOrGenerate calls

The removal of redundant parameters in GetOrGenerate calls maintains the same functionality while making the code more concise.

Also applies to: 44-44, 50-50, 56-56


123-123: LGTM: Updated to use cosmossdk.io/math package

The replacement of sdk.NewInt with math.NewInt is consistent with the upgrade to cosmos-sdk v0.50.10 and maintains the same functionality for creating coin amounts.

Also applies to: 389-389


185-185: LGTM: Simplified NewOperationMsg calls

The removal of the unused nil parameter in NewOperationMsg calls has been consistently applied across all simulation functions, making the code more concise while maintaining the same functionality.

Also applies to: 264-264, 344-344, 445-445


Line range hint 1-545: Verify simulation test coverage

While the changes look correct, please ensure that the simulation tests are run to verify that these modifications don't affect the simulation behavior.

✅ Verification successful

Let me try another approach to verify the simulation tests.


Let me check the module.go file to see how simulations are integrated.


Based on the findings, I can now provide a final response:

Simulation code is properly integrated and no issues found

The verification shows that:

  • The simulation operations are correctly integrated into the module's AppModuleSimulation implementation
  • The module properly implements the required simulation interfaces (verified by module.AppModuleSimulation = AppModule{})
  • The WeightedOperations are properly wired in the module.go file

The changes in the simulation code are safe as they maintain the expected integration patterns with the Cosmos SDK simulation framework.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for simulation test files and their coverage

# Look for simulation test files
echo "Checking for simulation test files..."
fd -e go -e _test.go simulation

# Check if there are any existing test coverage files
echo "Checking for test coverage files..."
fd -e out -e coverage

Length of output: 223


Script:

#!/bin/bash
# Search for test files that import the simulation package and check for test functions
echo "Searching for test files using the simulation package..."
rg -l "\"mods.irisnet.org/modules/oracle/simulation\"" --type go

echo -e "\nChecking for simulation test functions..."
ast-grep --pattern 'func Test$_($$$) {
  $$$
}' --lang go

Length of output: 371


Script:

#!/bin/bash
# Check module.go for simulation integration
echo "Checking module.go for simulation setup..."
rg -A 10 "simulation" modules/oracle/module.go

# Check for any simulation-related test files in the module directory
echo -e "\nChecking for simulation tests in module directory..."
fd -e go test modules/oracle

# Look for AppModuleSimulation implementation
echo -e "\nChecking for AppModuleSimulation implementation..."
ast-grep --pattern 'func (am AppModule) $_(ctx sdk.Context, $$$) {
  $$$
}' modules/oracle/module.go

Length of output: 1487

@@ -7,9 +7,9 @@ import (
"time"

errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Update all store/types imports across the codebase

The verification reveals that multiple modules are still using the old import path github.com/cosmos/cosmos-sdk/store/types. For consistency with cosmos-sdk v0.50.x, the following files need similar import updates:

  • modules/record/depinject.go
  • modules/record/keeper/keeper.go
  • modules/token/keeper/keeper.go
  • modules/token/depinject.go
  • modules/random/depinject.go
  • modules/random/keeper/keeper.go
  • modules/htlc/keeper/keeper.go
  • modules/htlc/depinject.go
  • modules/coinswap/keeper/keeper.go
  • modules/coinswap/depinject.go
  • modules/farm/depinject.go
  • modules/farm/keeper/keeper.go

The usage of storetypes interfaces (StoreKey, KVStore, Iterator) remains compatible across versions, but all imports should be updated to maintain consistency.

🔗 Analysis chain

Verify compatibility with cosmos-sdk v0.50.10

The import path change from github.com/cosmos/cosmos-sdk/store/types to cosmossdk.io/store/types is part of the SDK's v0.50.x breaking changes. Let's verify that all dependent modules are compatible with this version.

Consider adding a compatibility matrix in the module's documentation to clearly specify which cosmos-sdk versions are supported.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check go.mod for cosmos-sdk version and potential incompatibilities
echo "Checking cosmos-sdk version and dependencies..."
grep -r "cosmos-sdk" go.mod

# Check for any other modules still using the old import path
echo -e "\nChecking for modules still using old import path..."
rg "github.com/cosmos/cosmos-sdk/store/types"

# Verify no breaking changes in storetypes interfaces used by this module
echo -e "\nChecking storetypes usage..."
rg "storetypes\.(StoreKey|KVStore|Iterator)"

Length of output: 8604

@@ -54,7 +54,7 @@ func (suite *KeeperTestSuite) SetupTest() {
app := simapp.Setup(suite.T(), false, depInjectOptions)

suite.cdc = app.LegacyAmino()
suite.ctx = app.BaseApp.NewContext(false, tmproto.Header{})
suite.ctx = app.BaseApp.NewContext(false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the evidence gathered, I can now generate the final response:

Context creation change requires tmproto.Header parameter

The simplified context creation app.BaseApp.NewContext(false) appears to be incomplete. All other test files in the codebase consistently use app.BaseApp.NewContext(false, tmproto.Header{}) with the header parameter. This includes:

  • Other keeper tests like modules/record/keeper/keeper_test.go
  • Migration tests like modules/token/migrations/v2/migrate_test.go
  • HTLC tests with timestamp requirements: modules/htlc/keeper/params_test.go
🔗 Analysis chain

Verify context creation approach in cosmos-sdk v0.50.10

The simplified context creation looks correct, but let's verify this is the recommended approach in v0.50.10.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar context creation patterns in test files
rg -g '**/*_test.go' 'BaseApp.*NewContext.*false' --no-heading

# Search for any documentation or examples of context creation
rg -g '*.md' 'NewContext.*false'

Length of output: 1806


Script:

#!/bin/bash
# Let's check the imports in the test file to understand the context better
rg -g '**/oracle/keeper/keeper_test.go' '^import \([\s\S]*?\)' -A 20

# Let's also check for any recent changes or migrations related to context creation
rg -g '**/migrations/**/*.go' 'NewContext.*tmproto.Header'

# Search for BaseApp.NewContext method signature
ast-grep --pattern 'func (app $_) NewContext($$$) $_'

Length of output: 910


Script:

#!/bin/bash
# Let's check the go.mod file for cosmos-sdk version and any related changes
cat go.mod

# Let's also check the specific test file to see the full context
cat modules/oracle/keeper/keeper_test.go

# Check for any related test setup patterns in the same directory
rg -g '**/oracle/keeper/*_test.go' 'func.*Setup' -A 10

Length of output: 10927

@mitch1024 mitch1024 merged commit 2406be9 into main Nov 20, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bump up mods.irisnet.org/modules/oracle to cosmos-sdk v0.50.10
3 participants