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

chore: SDK 0.47 related cleanups #2334

Merged
merged 2 commits into from
Nov 21, 2023
Merged

chore: SDK 0.47 related cleanups #2334

merged 2 commits into from
Nov 21, 2023

Conversation

robert-zaremba
Copy link
Member

@robert-zaremba robert-zaremba commented Nov 21, 2023

Description

Summary by CodeRabbit

  • Documentation

    • Updated release notes to include the new version "v6.2" for Cosmovisor and a notice for validators to upgrade to umee/v2.3.0 or newer.
  • Refactor

    • Simplified message types in the leverage module by removing redundant Route and Type methods and adding GetSignBytes methods.
    • Removed the ProposalContents function from the oracle module, indicating a change in governance proposal simulations.
  • Tests

    • Updated incentive and leverage test suites to reflect changes in message types and to use the new legacytx.LegacyMsg type.
    • Renamed test functions to TestLegacyMsg and adjusted assertions accordingly.

@robert-zaremba robert-zaremba requested review from a team as code owners November 21, 2023 14:29
Copy link
Contributor

coderabbitai bot commented Nov 21, 2023

Walkthrough

The updates reflect a significant shift in the codebase, primarily affecting message handling and testing within a Go project. The changes remove deprecated methods like Route and Type from message types, aligning with newer standards for message signing. Additionally, tests are updated to use legacy message types, indicating a transition to newer message formats. Validators must upgrade to umee/v2.3.0 to maintain compatibility with Cosmos SDK v0.47, and governance proposal simulation functions have been removed, suggesting a possible streamlining or refactoring of the governance process.

Changes

File Path Change Summary
RELEASE_NOTES.md Added "v6.2" upgrade title for Cosmovisor; noted requirement for validators to update to umee/v2.3.0.
x/leverage/types/tx.go Removed Route and Type methods; added GetSignBytes methods for several message types.
x/oracle/module.go Removed ProposalContents function from AppModule.
x/incentive/msgs_test.go
x/leverage/types/tx_test.go
Updated tests: renamed TestRoutes to TestLegacyMsg, changed message types to legacytx.LegacyMsg, and adjusted assertions.

Poem

In the code where bytes align,
A rabbit hops through the refactor vine.
With autumn's chill and leaves that twirl,
We celebrate the code's new world. 🍂🐰✨


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • 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/coderabbit-overrides.v2.json

Copy link
Contributor

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 6f4b1f1 and 49585b9.
Files selected for processing (3)
  • RELEASE_NOTES.md (1 hunks)
  • x/leverage/types/tx.go (22 hunks)
  • x/oracle/module.go (1 hunks)
Additional comments: 13
RELEASE_NOTES.md (1)
  • 26-32: The release notes correctly highlight the requirement for validators to update to umee/v2.3.0 or newer due to incompatibility with Cosmos SDK v0.47. This is a critical update that validators need to be aware of to ensure the continued smooth operation of the network. It is important to ensure that all validators are notified of this change and that they understand the necessity of the update to maintain compatibility with the new SDK version.
x/leverage/types/tx.go (12)
  • 28-30: The Route method has been modified to return an empty string, which may indicate a change in the message routing logic. Ensure that this change is intentional and that the new routing logic (if any) is correctly implemented elsewhere in the codebase. Additionally, the Type method now uses sdk.MsgTypeURL(&msg) which seems to be a new way of handling message types in Cosmos SDK v0.47. Verify that this change aligns with the new SDK requirements and that it does not break any existing functionality.

  • 51-53: Similar to the previous comment, the Route method for MsgWithdraw is returning an empty string, and the Type method is using sdk.MsgTypeURL(&msg). This pattern is consistent across all message types in the provided code. It is crucial to ensure that these changes are in line with the new Cosmos SDK version's expectations and that they do not introduce any regressions.

  • 74-76: For MsgMaxWithdraw, the Route method returns an empty string, and the Type method uses sdk.MsgTypeURL(&msg). This change is consistent with the other message types and should be verified for correctness according to the new SDK's message handling mechanisms.

  • 97-99: The Route and Type methods for MsgCollateralize have been updated similarly. It is important to confirm that the new message handling logic is correctly implemented and that these changes are compatible with the rest of the codebase.

  • 120-122: The Route and Type methods for MsgSupplyCollateral are also returning empty strings and using sdk.MsgTypeURL(&msg), respectively. As with the other message types, verify that these changes are correct and do not cause any issues with message processing.

  • 143-145: The Route and Type methods for MsgDecollateralize follow the same pattern of changes. Ensure that these changes are part of the intended refactoring and that they are consistent with the new SDK's requirements.

  • 165-167: For MsgBorrow, the Route method returns an empty string, and the Type method uses sdk.MsgTypeURL(&msg). This is part of the broader refactoring pattern observed in the codebase. Confirm that these changes are correct and that they do not negatively impact the application's functionality.

  • 187-189: The Route and Type methods for MsgMaxBorrow have been updated in the same manner as the other message types. Verify that these changes are in accordance with the new SDK's guidelines and that they do not introduce any unintended behavior.

  • 209-211: For MsgRepay, the Route method returns an empty string, and the Type method uses sdk.MsgTypeURL(&msg). This consistent change across all message types should be verified for correctness and compatibility with the new SDK version.

  • 240-242: The Route and Type methods for MsgLiquidate are also returning empty strings and using sdk.MsgTypeURL(&msg), respectively. As with the other message types, verify that these changes are correct and do not cause any issues with message processing.

  • 287-289: For MsgLeveragedLiquidate, the Route method returns an empty string, and the Type method uses sdk.MsgTypeURL(&msg). This is part of the broader refactoring pattern observed in the codebase. Confirm that these changes are correct and that they do not negatively impact the application's functionality.

  • 294-296: The helper method validateSenderAndAsset is used to validate the sender's address and the asset. It is important to ensure that this method is being used consistently throughout the codebase wherever such validation is required. This will help maintain consistency and reduce the likelihood of errors.

@robert-zaremba robert-zaremba added the skip-e2e-test Skip the e2e tests label Nov 21, 2023
@robert-zaremba robert-zaremba added this pull request to the merge queue Nov 21, 2023
Merged via the queue into main with commit aa82893 Nov 21, 2023
22 of 36 checks passed
@robert-zaremba robert-zaremba deleted the robert/047-cleanup branch November 21, 2023 17:03
Copy link
Contributor

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 49585b9 and 29daa38.
Files selected for processing (2)
  • x/incentive/msgs_test.go (3 hunks)
  • x/leverage/types/tx_test.go (3 hunks)
Additional comments: 7
x/incentive/msgs_test.go (3)
  • 6-11: The addition of the github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx import is necessary for the updated tests that now use legacytx.LegacyMsg instead of the previous sdk.Msg interface. This change aligns with the Cosmos SDK's migration patterns for handling legacy transactions.

  • 61-79: The test function TestRoutes has been renamed to TestLegacyMsg, and the type of msgs has been changed from []sdk.Msg to []legacytx.LegacyMsg. The assertions within the function have been updated accordingly. This reflects the changes in the Cosmos SDK and ensures that the tests are compatible with the new transaction processing system. However, the assertion on line 75 should also check for the correctness of the GetSignBytes output, not just that it's non-zero.

  • 83-86: The addV1ToType function has been updated to replace "*incentive" with "incentive.v1". This change is likely related to versioning updates in the message types and ensures that the tests are checking against the correct message type strings. It's important to verify that this versioning pattern is consistent across the entire codebase.

x/leverage/types/tx_test.go (4)
  • 9-13: The import of gotest.tools/v3/assert is added, which suggests that the test suite is using this package for assertions. It's important to ensure that this package is included in the project's dependencies and that it is the preferred assertion library for consistency across the test suite.

  • 51-55: The test function TestLegacyMsg has been introduced, replacing the previous TestRoutes function. This change reflects the update to use legacytx.LegacyMsg instead of sdk.Msg. It is important to ensure that all tests that previously relied on sdk.Msg are updated to use the new legacytx.LegacyMsg type for compatibility with the Cosmos SDK v0.47.

  • 66-72: The loop is checking that GetSignBytes() returns a non-zero length byte slice, which is a good practice to ensure that the messages are correctly implementing the method required for signing. However, the call to addV1ToType and the comparison with tx.Type() have been removed, which is likely due to the removal of the Type method from the message types. This change aligns with the Cosmos SDK's move away from using Type for message identification.

  • 75-77: The addV1ToType function has been removed, which is consistent with the removal of the Type method from the message types. This function was previously used to manipulate the type string for testing purposes. Its removal should be verified to ensure that it is not used elsewhere in the codebase and that the tests no longer rely on the Type method for message identification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-e2e-test Skip the e2e tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants