-
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
feat: Updated cosmos-sdk to v0.50 #58
Conversation
## Walkthrough
The changes reflect a significant update to a Go project, primarily involving the removal of transaction tip handling functionality and associated tests. Additionally, there's an update to the network configuration for the Rosetta service, and the project's changelog indicates an upgrade to the cosmos-sdk version and the migration of the Rosetta implementation to a standalone repository.
## Changes
| File Path | Change Summary |
|-----------|----------------|
| `converter_test.go` | Commented out test functions related to Rosetta operations and signers. |
| `utils.go` | Added "authsigning" import, removed transaction tip parsing code, and disabled linting checks for specific blocks. |
| `.github/workflows/rosetta-cli-test.yaml` | Changed Rosetta service port from 8081 to 8080. |
| `CHANGELOG.md` | Documented cosmos-sdk version upgrade, removal of tip handling, and Rosetta migration to standalone repository. |
## Poem
> As autumn leaves fall, we prune the code tree, 🍂
> Tips are gone, ports shift, in the Rosetta sea. 🌊
> A rabbit hops along, through the GitHub mist,
> Celebrating changes, with a twist of the wrist. 🐰✨ TipsChat with CodeRabbit Bot (
|
This comment has been minimized.
This comment has been minimized.
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (2)
- converter_test.go (2 hunks)
- utils.go (3 hunks)
Additional comments: 2
utils.go (2)
- 3-9: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [3-17]
The import of the "authsigning" package at line 6 indicates new functionality related to authentication signing is being used in this file. Ensure that the new functionality is properly integrated and tested, and that any previous functionality it replaces is fully deprecated and removed if no longer needed.
- 63-74: The code for parsing transaction tips has been commented out. If this functionality is no longer needed due to changes in the
cosmos-sdk
v0.50, ensure that any related code and tests are also updated or removed to reflect this change. If this functionality has been moved to a different part of the codebase, verify that it has been correctly implemented there and that any references to the old implementation have been updated.
Signed-off-by: bizk <[email protected]>
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- tests/rosetta-cli/rosetta-cli.json
Files selected for processing (2)
- converter_test.go (2 hunks)
- utils.go (2 hunks)
Additional comments: 2
utils.go (2)
6-18: The import of the
authsigning
package from the Cosmos SDK has been added, which is likely necessary for the new functionality introduced in the updated code. However, it's important to ensure that the new import does not introduce any conflicts with existing imports and that it is used within the file. Unused imports should be removed to keep the code clean and maintainable.63-80: The function
parseTxTip
has been modified to comment out the logic that parses the tip amount and tipper from the transaction. This could be due to a change in how tips are handled in the updated version of the Cosmos SDK. It's important to ensure that this change is intentional and that any functionality depending on the parsing of tips is also updated accordingly. Additionally, thenolint
directives should be revisited once the final implementation is complete to ensure that they are still necessary and that the code adheres to linting standards.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/rosetta-cli-test.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/rosetta-cli-test.yaml
s.Require().Equal(len(ops), len(sdkTx.GetMsgs())*len(signerAddrs), "operation number mismatch") | ||
|
||
s.Require().Equal(len(signers), len(signerAddrs), "signers number mismatch") | ||
// s.Require().Equal(len(ops), len(sdkTx.GetMsgs())*len(signerAddrs), "operation number mismatch") |
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.
Ditto
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- tests/rosetta-cli/rosetta-cli.json
Files selected for processing (1)
- .github/workflows/rosetta-cli-test.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/rosetta-cli-test.yaml
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.
Can we get a chanegelog?
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.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- utils.go (4 hunks)
Additional comments: 3
utils.go (3)
6-18:
Verify that the newly added import "github.com/cosmos/cosmos-sdk/x/auth/signing" is being used in the file and there are no unused imports.60-61:
Ensure that the conversion ofsignerData.AccountNumber
to a string is intentional and compatible with the rest of the system that consumes this data.100-105: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [100-116]
Confirm that the removal of the
Tip
field from thetxv1beta1.AuthInfo
struct aligns with the changes in the cosmos-sdk version 0.50 and that no other parts of the codebase rely on this field.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- tests/rosetta-cli/rosetta-cli.json
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments: 3
CHANGELOG.md (3)
41-41:
The changelog entry is clear and follows the established format.45-45:
The changelog entry is clear and follows the established format.49-49:
The changelog entry is clear and follows the established format.
Summary by CodeRabbit
Refactor
Chores
Documentation
Bug Fixes