Bump golang linter version #592
Conversation
|
deividaspetraitis
added
A:backport/v28.x
|
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-lint (1.62.2)level=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." WalkthroughThe pull request includes significant updates across multiple files, primarily focusing on upgrading dependencies from version 27 to version 28 of the Osmosis library. Key changes involve updating import paths, enhancing workflow configurations for linting and required labels, and adding new features to the changelog. The modifications maintain the existing functionality while ensuring compatibility with the latest library versions and improving error handling and logging mechanisms in various components. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
domain/mocks/msg_simulator_mock.go (1)
Based on the evidence gathered, I can now verify the interface implementation. The
PriceMsgsmethod in the mock has a signature mismatch with the interface:
- Interface (
domain/cosmos/tx/msg_simulator.go):PriceMsgs( ctx context.Context, encodingConfig cosmosclient.TxConfig, account *authtypes.BaseAccount, chainID string, msg ...sdk.Msg, ) domain.TxFeeInfo
- Mock implementation (
domain/mocks/msg_simulator_mock.go):PriceMsgs(ctx context.Context, encodingConfig client.TxConfig, account *authtypes.BaseAccount, chainID string, msg ...interface { ProtoMessage() Reset() String() string }) domain.TxFeeInfoThe mock's
PriceMsgsmethod accepts...interface{ ProtoMessage(); Reset(); String() string }instead of...sdk.Msg. This is a type mismatch that could lead to runtime errors.Fix the mock's PriceMsgs method signature to match the interface.
- Update the parameter type from
...interface{ ProtoMessage(); Reset(); String() string }to...sdk.Msgindomain/mocks/msg_simulator_mock.go🔗 Analysis chain
Line range hint
1-85: Verify interface implementation compatibility.Let's ensure the mock still correctly implements the
sqstx.MsgSimulatorinterface after the osmosis version bump.
Let me analyze the interface definition and implementation:
The interface
MsgSimulatorindomain/cosmos/tx/msg_simulator.gorequires:
BuildTx(ctx, keyring, encodingConfig, account, chainID, msg ...sdk.Msg)SimulateMsgs(encodingConfig, account, chainID, msgs []sdk.Msg)PriceMsgs(ctx, encodingConfig, account, chainID, msg ...sdk.Msg)The mock implementation has an issue in the
PriceMsgsmethod:
- Interface expects
msg ...sdk.Msg- Mock implements
msg ...interface{ ProtoMessage(); Reset(); String() string }Let me verify if
sdk.Msgis an interface with these methods:
Let me check the actual implementation of
sdk.Msginterface:
Let me try a different approach to find the sdk.Msg interface:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the interface implementation and usage # 1. Check the interface definition # 2. Look for any callers of this mock # Check the interface definition ast-grep --pattern 'type MsgSimulator interface { $$$ }' # Find usages of MsgSimulatorMock rg "MsgSimulatorMock" -A 5Length of output: 7916
Script:
#!/bin/bash # Check sdk.Msg interface definition ast-grep --pattern 'type Msg interface { $$$ }'Length of output: 51
Script:
#!/bin/bash # Let's try to find the sdk.Msg definition in vendor directory fd -t f "types.go" -x grep -l "type Msg interface" # Also check for any imports of sdk.Msg to understand its usage rg "sdk \"github.com/cosmos/cosmos-sdk/types\"" -A 3Length of output: 28951
Script:
#!/bin/bash # Check vendor directory for sdk.Msg interface fd -t f "types.go" -x cat {} | grep -A 10 "type Msg interface"Length of output: 64
Script:
#!/bin/bash # Look for sdk.Msg usage in the codebase to understand its interface rg "implements sdk\.Msg" -B 2 -A 2 # Also check for any direct references to the interface methods rg "ProtoMessage|Reset|String" --type go -C 2Length of output: 69500
ingest/usecase/ingest_usecase.go (1)
Line range hint
408-413: Refactor duplicated code for obtaining pool denominationsThe logic for retrieving pool denominations based on whether
CosmWasmPoolModelisnilis duplicated in bothfilterExactMatchSearchandfilterPartialMatchSearchfunctions. Consider abstracting this logic into a helper function to improve maintainability.Also applies to: 442-447
pools/usecase/pools_usecase.go (1)
408-413: Refactor duplicated logic for retrieving pool denominationsThe code for obtaining
denomsbased on theCosmWasmPoolModelcheck is duplicated in bothfilterExactMatchSearchandfilterPartialMatchSearch. Refactoring this into a shared helper function can enhance code readability and maintainability.Also applies to: 442-447
🛑 Comments failed to post (4)
.github/workflows/required-labels.yml (1)
7-7:
⚠️ Potential issueInconsistency between branch name and required label versions
The branch pattern has been updated to
"v27.x", but the required label is"A:backport/v28.x". This version mismatch may cause confusion or enforce incorrect label requirements. Please verify that the branch patterns and required labels are consistent with the intended versioning strategy.Also applies to: 29-29
domain/mocks/chain_pool_mock.go (1)
7-7: 💡 Codebase verification
Interface compatibility check failed after v28 update
The mock implementation is missing several required methods from the v28 PoolI interface:
GetPoolLiquidityCap()GetUnderlyingPool()GetSQSPoolModel()GetTickModel()GetLiquidityCap()GetLiquidityCapError()GetAPRData()GetFeesData()Incentive()SetTickModel()SetLiquidityCap()SetLiquidityCapError()SetAPRData()SetFeesData()Validate()The mock needs to be updated to implement all methods from the v28 interface to maintain compatibility.
🔗 Analysis chain
LGTM! Verify interface compatibility after version bump.
The import path update from v27 to v28 is correct. The mock implementation remains unchanged and maintains the expected behavior.
Let's verify that the interface implementation is still compatible with v28:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that ChainPoolMock implements all required methods from poolmanagertypes.PoolI # Search for the PoolI interface definition in v28 ast-grep --pattern 'type PoolI interface { $$$ }'Length of output: 2976
Script:
#!/bin/bash # Let's check the ChainPoolMock implementation to verify interface compatibility ast-grep --pattern 'type ChainPoolMock struct { $$$ } $$$' # Also check for the interface compliance check rg "var _ .* = &ChainPoolMock{}"Length of output: 234
Script:
#!/bin/bash # Let's check the ChainPoolMock implementation to verify interface compatibility ast-grep --pattern 'type ChainPoolMock struct' # Also check for the interface compliance check (fixing the regex) rg "var _ \w+\.?\w*\s*=\s*&ChainPoolMock{}" # Get the full mock implementation rg -A 2 "func \(c \*ChainPoolMock\)"Length of output: 2315
Script:
#!/bin/bash # Let's check the full content of the mock file to see all implementations and the interface check cat domain/mocks/chain_pool_mock.goLength of output: 1732
router/usecase/pools/routable_stableswap_pool.go (1)
13-16: 💡 Codebase verification
Pool type implementation mismatch detected
The stableswap pool implementation incorrectly returns
poolmanagertypes.Balancertype instead of the expected stableswap type. This appears to be a bug in the implementation as the pool type doesn't match the actual pool model being used.
router/usecase/pools/routable_stableswap_pool.go: TheGetType()method returnspoolmanagertypes.Balancerfor a stableswap pool implementation.🔗 Analysis chain
LGTM! Verify pool type constants compatibility
The import path updates from v27 to v28 are consistent across all Osmosis packages. Let's verify that the pool type constants haven't changed in v28 to ensure compatibility.
Let me try a different approach to verify the pool type constants.
Let me try one more search to find the pool type definitions and their usage.
Let me try one final search to locate the pool type definitions.
Based on the search results, I can see that pool types are defined in the interface and the changes are only import path updates. Let me verify the specific stableswap pool implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any changes in pool type constants between v27 and v28 # Expected: Pool type constants should remain the same echo "Checking pool type constants usage..." ast-grep --pattern 'poolmanagertypes.Balancer' -l goLength of output: 137
Script:
#!/bin/bash # Search for pool type constant definitions and usages echo "Searching for pool type constants in poolmanager types..." rg -A 3 "const \(" router/usecase/pools/ x/poolmanager/types/ echo -e "\nSearching for stableswap pool type usage..." rg "StableSwap" router/usecase/pools/routable_stableswap_pool.goLength of output: 1888
Script:
#!/bin/bash # Search for pool type definitions and their usage in the domain and types echo "Searching for pool type definitions..." rg -A 5 "type PoolType" router/domain/ echo -e "\nSearching for pool type usage in stableswap pool..." ast-grep --pattern 'type RoutableStableSwapPool struct' router/usecase/pools/routable_stableswap_pool.goLength of output: 408
Script:
#!/bin/bash # Search for pool type definitions and their usage echo "Searching for pool type definitions in the codebase..." rg -A 5 "type Pool" --type go echo -e "\nSearching for stableswap pool implementation..." rg -A 10 "RoutableStableSwapPool" --type goLength of output: 13126
Script:
#!/bin/bash # Check the stableswap pool implementation and its dependencies echo "Checking stableswap pool implementation..." rg -l "stableswap" --type go echo -e "\nChecking pool type usage in stableswap pool..." rg -A 5 "func.*GetType\(\)" router/usecase/pools/routable_stableswap_pool.goLength of output: 890
go.mod (1)
3-5:
⚠️ Potential issueVerify Go version and toolchain compatibility
The Go version (1.22.7) and toolchain (1.23.3) configuration appears unusual:
- The toolchain version (1.23.3) is higher than the Go version (1.22.7)
- Go 1.23 is not yet released (as of December 2024)
Consider using a matching toolchain version or remove the toolchain directive if not needed.
Summary
This updates the github action that runs golangci-lint and the linter to the last version