-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore: update to 0.52.0-rc.1 #180
Conversation
WalkthroughThis pull request introduces significant updates to the Rosetta project, focusing on dependency upgrades, Go version updates, and improvements to address handling and transaction processing. The changes primarily involve updating the Cosmos SDK dependencies, modifying import paths, enhancing configuration options with a Bech32 prefix flag, and updating the Go version across multiple workflow files and the project's dependencies. Changes
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
utils.go (1)
126-126
: Fix typo in TODO comment.The TODO comment contains a typo: "timeouthecigh". Did you mean "timeout height"?
-// todo: timeouthecigh +// TODO: timeout height
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (13)
.github/workflows/build.yml
(1 hunks).github/workflows/lint.yml
(1 hunks).github/workflows/rosetta-cli-test.yaml
(1 hunks).github/workflows/test.yml
(1 hunks)CHANGELOG.md
(1 hunks)client_online.go
(5 hunks)codec.go
(1 hunks)config.go
(5 hunks)converter.go
(3 hunks)converter_test.go
(4 hunks)go.mod
(1 hunks)plugins/cosmos-hub/main.go
(1 hunks)utils.go
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- codec.go
🔇 Additional comments (17)
plugins/cosmos-hub/main.go (1)
4-4
: LGTM! Verify import path consistency.The update to use
cosmossdk.io/x/bank/types
aligns with the new module path convention in cosmos-sdk 0.52.0-rc.1.Let's verify the consistency of this import path change across the codebase:
✅ Verification successful
Import path migration verified successfully
The new import path
cosmossdk.io/x/bank/types
is used consistently across all relevant files in the codebase, with no remaining instances of the old import path.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining old import paths # Expect: No occurrences of the old import path rg "github.com/cosmos/cosmos-sdk/x/bank/types"Length of output: 46
Script:
#!/bin/bash # Search for new import path and its usage rg -A 3 "cosmossdk.io/x/bank/types" # Also check for any potential variations that might indicate inconsistency rg -i "bank.*types"Length of output: 1809
utils.go (1)
131-131
: Verify the impact of switching from timeout height to timeout timestamp.The change from using
GetTimeoutHeight()
toGetTimeoutTimeStamp()
represents a significant modification in how transaction timeouts are handled. Please ensure this change is intentional and aligns with the cosmos-sdk 0.52.0-rc.1 requirements.Let's check the usage of timeout handling across the codebase:
✅ Verification successful
Timeout mechanism change is properly implemented
The switch from height-based to timestamp-based timeouts is complete and consistent across the codebase. The implementation aligns with cosmos-sdk v0.52.0-rc.1 requirements, with no remaining references to height-based timeouts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining timeout height usage and verify timestamp usage # Expect: Consistent usage of timestamp-based timeouts # Check for any remaining timeout height references rg "GetTimeoutHeight|TimeoutHeight" # Check timestamp timeout usage rg "GetTimeoutTimeStamp|TimeoutTimestamp"Length of output: 165
Script:
#!/bin/bash # Check for cosmos-sdk related timeout imports and configurations rg -i "cosmos.*timeout" -A 2 # Check go.mod for cosmos-sdk version cat go.mod | grep cosmos-sdkLength of output: 174
config.go (2)
69-69
: LGTM! Configuration changes for Bech32 prefix support.The addition of Bech32 prefix configuration is well-implemented with:
- New flag with appropriate default value
- Corresponding struct field
- Proper flag parsing
Also applies to: 106-107
240-243
: Verify error handling for the new Bech32 prefix flag.The error handling for the new flag follows the established pattern in the codebase.
Let's verify the validation of the Bech32 prefix:
✅ Verification successful
Error handling for Bech32 prefix is properly implemented
The implementation follows a robust pattern where:
- Basic flag parsing errors are properly wrapped with appropriate context
- Bech32 format validation occurs during usage via the SDK's address package
- Invalid prefixes will fail fast when creating address codecs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the Bech32 prefix validation is implemented # Expect: Validation logic for the prefix format ast-grep --pattern 'func $_(bech32Prefix string) error { $$$ }'Length of output: 67
Script:
#!/bin/bash # Find FlagBech32Prefix definition and Bech32-related validation echo "=== FlagBech32Prefix definition ===" rg "FlagBech32Prefix\s*=" -A 2 echo -e "\n=== Bech32 validation functions ===" rg -l "func.*[Bb]ech32" echo -e "\n=== Bech32 prefix usage patterns ===" rg "[Bb]ech32.*[Pp]refix" -A 2Length of output: 2067
Script:
#!/bin/bash # Find validate method and NewCachedBech32Codec implementation echo "=== Config validate method ===" ast-grep --pattern 'func (c *Config) validate() error { $$$ }' echo -e "\n=== NewCachedBech32Codec implementation ===" rg "func NewCachedBech32Codec" -A 5Length of output: 2262
converter_test.go (2)
12-12
: LGTM! Consistent updates for SDK changes.The changes correctly implement:
- Updated import path for bank types
- New txConfig instantiation with Bech32 codecs
Also applies to: 46-46
293-302
: LGTM! Updated event creation pattern.The balance operation events now use
sdk.NewEvent
with proper attributes for spender and receiver.Let's verify the consistency of the new event pattern:
✅ Verification successful
Event creation pattern is consistently implemented
The new event pattern using
sdk.NewEvent
with proper attributes is consistently used throughout the codebase, with no instances of old patterns found. The changes are properly contained within the test file, while other files only reference the event types in their supported operations list.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of the new event pattern # Expect: No usage of old event creation pattern # Check for new event pattern usage rg "sdk.NewEvent.*EventTypeCoin(Spent|Received)" # Check for potential old patterns rg "CoinSpentEvent|CoinReceivedEvent"Length of output: 90
Script:
#!/bin/bash # Try different search patterns for bank events and attributes fd -e go -x rg -l "bank\.EventType" {} rg "AttributeKeySpender|AttributeKeyReceiver" rg "bank\." -A 2 -B 2 --type goLength of output: 4892
client_online.go (4)
21-21
: Import path updated for bank package.The import path has been updated from the SDK to the new cosmossdk.io path, aligning with the cosmos-sdk v0.52.0-rc.1 update.
65-72
: LGTM: Proper initialization of address codecs with error handling.The code correctly initializes two cached Bech32 codecs with proper error handling:
- Account address codec with the base prefix
- Validator address codec with the validator prefix
74-74
: LGTM: Updated transaction configuration with new codecs.The transaction configuration now correctly uses both account and validator address codecs.
500-500
: Verify the field name change in transaction results.The code now uses
TxResults
instead ofTxsResults
. Let's verify this change is consistent with the new SDK version.Also applies to: 515-515
converter.go (2)
20-20
: Import path updated for bank types.The import path has been updated to use the new cosmossdk.io path, consistent with other files.
164-164
: API update: GetMsgV1Signers → GetMsgSigners.The code has been updated to use
GetMsgSigners
instead ofGetMsgV1Signers
, reflecting API changes in the new SDK version.Also applies to: 244-244
✅ Verification successful
API change verified: GetMsgV1Signers → GetMsgSigners is correct
The change is consistent with the SDK upgrade path and properly implemented across the codebase. This is confirmed by the changelog entry in PR #93 and the current SDK version (v0.52.0-rc.1) being used.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the method name change in the SDK # Test: Search for GetMsgSigners usage in the SDK rg "GetMsgSigners"Length of output: 135
Script:
#!/bin/bash # Search for any remaining instances of the old API name rg "GetMsgV1Signers" # Check go.mod for SDK version changes cat go.modLength of output: 8580
.github/workflows/rosetta-cli-test.yaml (1)
21-21
: Go version updated to 1.23.4.The Go version has been updated to align with the project's requirements. However, let's verify if this version is compatible with the new SDK version.
✅ Verification successful
Go version 1.23.4 is correctly aligned ✓
The Go version in the workflow matches the version specified in go.mod, and the use of the caret (^) notation allows for compatible updates while maintaining stability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Go version compatibility # Test: Search for Go version requirements in go.mod rg "go \d+\.\d+(\.\d+)?" go.modLength of output: 43
.github/workflows/lint.yml (1)
19-19
: Go version updated to 1.23.4.The Go version update in the lint workflow is consistent with other workflow files, maintaining version consistency across the CI pipeline.
.github/workflows/build.yml (1)
22-22
: Verify impact of removing ARM architecture supportThe build matrix no longer includes the "arm" architecture. This change could impact users running on 32-bit ARM systems.
Could you confirm if this removal is intentional and aligned with cosmos-sdk v0.52.0-rc.1's supported architectures?
CHANGELOG.md (1)
41-42
: LGTM! Well-documented changesThe changelog entries properly document both the cosmos-sdk version update and the addition of the bech32 prefix flag.
go.mod (1)
171-173
: Verify cosmos-sdk replacement versionThe replace directive ensures we use cosmos-sdk v0.52.0-rc.1. This matches the PR objective, but being a release candidate, we should verify its stability.
✅ Verification successful
RC version is appropriate and ready for integration
The v0.52.0-rc.1 is officially encouraged for integration by the Cosmos SDK team. This RC introduces a new modular architecture and has stable APIs. While still a release candidate, no critical issues have been reported, and proper migration documentation is available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any known issues with this RC version # Expected: Should show recent issues or discussions gh api graphql -f query=' { repository(owner: "cosmos", name: "cosmos-sdk") { issues(last: 5, states: OPEN, labels: ["v0.52.0-rc.1"]) { nodes { title url createdAt } } } }'Length of output: 270
Script:
#!/bin/bash # Check recent releases and discussions gh api graphql -f query=' { repository(owner: "cosmos", name: "cosmos-sdk") { releases(first: 5, orderBy: {field: CREATED_AT, direction: DESC}) { nodes { tagName isPrerelease createdAt } } discussions(first: 5, orderBy: {field: CREATED_AT, direction: DESC}) { nodes { title createdAt category { name } } } } }'Length of output: 1481
Script:
#!/bin/bash # Get release notes for v0.52.0-rc.1 gh api repos/cosmos/cosmos-sdk/releases/tags/v0.52.0-rc.1 | jq -r '.body'Length of output: 1029
@@ -21,7 +21,7 @@ jobs: | |||
- uses: actions/checkout@v4 | |||
- uses: actions/setup-go@v5 | |||
with: | |||
go-version: "1.22" | |||
go-version: "1.23.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Go version 1.23.4 appears to be incorrect
The specified Go version 1.23.4
seems to be a typo as:
- Go follows semantic versioning with major.minor format (e.g., 1.21)
- There is no Go 1.23.x series
- Latest stable Go version is 1.21.x (as of January 2025)
Please update to a valid Go version that aligns with the cosmos-sdk module requirements.
🔗 Analysis chain
Verify Go version compatibility with cosmos-sdk v0.52.0-rc.1
The Go version has been updated to 1.23.4. While this keeps the project up-to-date with the latest Go release, we should verify compatibility with cosmos-sdk v0.52.0-rc.1.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check cosmos-sdk's required Go version
# Expected: Should show Go version requirements in go.mod
rg -p "^go \d+\.\d+(\.\d+)?" $(fd -t f "go.mod$" --exec echo {//} | grep -i "cosmos-sdk")
Length of output: 111
Script:
#!/bin/bash
# Check go.mod for cosmos-sdk dependency and its version
cat go.mod | grep -A 5 "require"
# Check cosmos-sdk v0.52.0-rc.1 go.mod
gh api repos/cosmos/cosmos-sdk/contents/go.mod?ref=v0.52.0-rc.1 | jq -r '.content' | base64 -d | grep "^go "
Length of output: 786
cosmossdk.io/api v0.8.0-rc.3 | ||
cosmossdk.io/log v1.5.0 | ||
cosmossdk.io/math v1.4.0 | ||
cosmossdk.io/x/tx v0.13.7 | ||
cosmossdk.io/x/bank v0.0.0-20241218110910-47409028a73d | ||
cosmossdk.io/x/tx v1.0.0-alpha.3 | ||
github.com/coinbase/rosetta-sdk-go/types v1.0.0 | ||
github.com/cometbft/cometbft v0.38.12 | ||
github.com/cosmos/cosmos-sdk v0.50.11 | ||
github.com/cometbft/cometbft v1.0.0 | ||
github.com/cosmos/cosmos-sdk v0.52.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Update dependencies to available stable versions
The following dependencies have stable versions available:
cosmossdk.io/api
: Update fromv0.8.0-rc.3
tov0.8.0
cosmossdk.io/x/tx
: Update fromv1.0.0-alpha.3
tov1.0.0
🔗 Analysis chain
Verify compatibility of pre-release dependencies
Several dependencies are using pre-release versions:
cosmossdk.io/api v0.8.0-rc.3
cosmossdk.io/x/tx v1.0.0-alpha.3
Please ensure these pre-release versions are stable enough for production use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if these versions are the latest available
# Expected: Should show if newer versions exist
echo "Checking latest versions:"
for pkg in "cosmossdk.io/api" "cosmossdk.io/x/tx"; do
curl -s "https://proxy.golang.org/$pkg/@v/list" | sort -V | tail -n 5
done
Length of output: 439
This Pr updates cosmos-sdk to v0.52.0-rc.1
Summary by CodeRabbit
Release Notes
Improvements
Technical Updates
Build & Infrastructure