8000 chore(erc20): keep MsgConvertCoin interface registered by GAtom22 · Pull Request #2892 · evmos/evmos · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore(erc20): keep MsgConvertCoin interface registered #2892

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

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

GAtom22
Copy link
Contributor
@GAtom22 GAtom22 commented Sep 27, 2024

Description

Closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the correct branch (see PR Targeting)

Reviewers Checklist

All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.

I have...

  • added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • confirmed all author checklist items have been addressed
  • confirmed that this PR does not change production code
  • reviewed content
  • tested instructions (if applicable)
  • confirmed all CI checks have passed

Summary by CodeRabbit

  • Bug Fixes
    • Updated the changelog to include a new section for bug fixes.
    • Ensured the MsgConvertCoin interface remains registered for backwards compatibility, preventing potential issues.
  • New Features
    • Introduced a new constant, convertCoinName, to maintain backward compatibility when querying transactions.

@GAtom22 GAtom22 < 8000 a target="_blank" class="Link--secondary" rel="noopener noreferrer" href="https://github.com/evmos/evmos/actions/runs/11070868226/job/30761439171">temporarily deployed to release September 27, 2024 12:44 — with GitHub Actions Inactive
Copy link
Contributor
coderabbitai bot commented Sep 27, 2024

Walkthrough

The pull request introduces updates to the changelog and the x/erc20/types/codec.go file to ensure the MsgConvertCoin interface remains registered for backward compatibility. A new constant, convertCoinName, is added to facilitate this registration. The changes aim to maintain existing functionality and prevent issues arising from deregistration of the interface.

Changes

File Change Summary
CHANGELOG.md Updated to include a section for bug fixes, specifically noting the registration of MsgConvertCoin.
x/erc20/types/codec.go Added constant convertCoinName, updated RegisterInterfaces and RegisterLegacyAminoCodec to include MsgConvertCoin.

Possibly related PRs

Suggested labels

tests, precompile

🐇 In the code where changes bloom,
The MsgConvertCoin finds more room.
With constants bright and interfaces clear,
Backward paths are safe, have no fear!
A hop, a skip, through code we race,
Compatibility holds its rightful place! 🌼


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 27c9d9f and 96f54b9.

📒 Files selected for processing (1)
  • x/erc20/types/codec.go (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • x/erc20/types/codec.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.

@GAtom22 GAtom22 marked this pull request as ready for review September 27, 2024 12:46
@GAtom22 GAtom22 requested a review from a team as a code owner September 27, 2024 12:46
@GAtom22 GAtom22 requested review from Vvaradinov and 0xstepit and removed request for a team September 27, 2024 12:46
@GAtom22
Copy link
Contributor Author
GAtom22 commented Sep 27, 2024

https://github.com/Mergifyio backport release/v19.x.x

@GAtom22
Copy link
Contributor Author
GAtom22 commented Sep 27, 2024

https://github.com/Mergifyio backport rama/release-v20

Copy link
Contributor
mergify bot commented Sep 27, 2024

backport release/v19.x.x

✅ Backports have been created

Copy link
Contributor
mergify bot commented Sep 27, 2024

backport rama/release-v20

✅ Backports have been created

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.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
x/erc20/types/codec.go (1)

47-47: LGTM: MsgConvertCoin registered for backwards compatibility

The addition of &MsgConvertCoin{} to the RegisterInterfaces function is correct and maintains consistency with the module's architecture. This change ensures proper registration of the message type, supporting backwards compatibility for transaction queries.

Consider updating the comment to match the exact wording used for the constant:

-		&MsgConvertCoin{}, // keep it for backwards compatiblity when querying txs
+		&MsgConvertCoin{}, // keep it for backwards compatibility when querying txs

This minor change corrects the spelling of "compatibility" and ensures consistency with the comment on the constant.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 816aca9 and 27c9d9f.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • x/erc20/types/codec.go (3 hunks)
🔇 Additional comments (33)
x/erc20/types/codec.go (3)

31-31: LGTM: New constant added for backwards compatibility

The addition of the convertCoinName constant is well-documented and follows the existing naming conventions. This change supports backwards compatibility for transaction queries, which is a good practice for maintaining API stability.


69-69: LGTM: MsgConvertCoin registered with LegacyAmino codec

The addition of &MsgConvertCoin{} to the RegisterLegacyAminoCodec function is correct and consistent with the registration of other message types. This change ensures that the MsgConvertCoin type is properly serialized for Amino JSON and EIP-712 compatibility.


31-31: Summary: MsgConvertCoin interface successfully maintained

The changes in this file effectively achieve the PR's objective of keeping the MsgConvertCoin interface registered. The modifications include:

  1. Adding a new constant convertCoinName for backwards compatibility.
  2. Registering MsgConvertCoin 8000 in the RegisterInterfaces function.
  3. Registering MsgConvertCoin with the LegacyAmino codec.

These changes ensure that the MsgConvertCoin type is properly integrated into the existing codec system while maintaining backwards compatibility for transaction queries. The implementation is consistent with the module's architecture and coding conventions.

Also applies to: 47-47, 69-69

CHANGELOG.md (30)

68-71: New bug fix section added

A new "Bug Fixes" section has been added to the changelog for v16.0.0. This is a good practice as it clearly separates bug fixes from other types of changes.


70-70: Keep MsgConvertCoin interface registered for backwards compatibility

This bug fix ensures backwards compatibility by keeping the MsgConvertCoin interface registered. It's important to maintain compatibility with existing systems that may rely on this interface.


Line range hint 9-10: New secp256r1 curve precompile added

The addition of the secp256r1 curve precompile (RIP-7212) is a significant change that enhances the cryptographic capabilities of the system. This change is state machine breaking and will require careful testing and coordination during the upgrade process.


Line range hint 11-11: New ClaimRewards custom transaction added to distribution precompile

Adding a ClaimRewards custom transaction to the distribution precompile is a useful feature that allows for more efficient reward claiming. This change is state machine breaking and will require updates to any systems interacting with reward claiming.


Line range hint 12-13: New bech32 conversion precompile added

The addition of a bech32 conversion precompile is a helpful feature for address format conversions. This change is state machine breaking and will require careful integration and testing.


Line range hint 15-15: New CreateValidator function added to staking precompile

Adding the CreateValidator function to the staking precompile expands the functionality available through precompiles. This is a state machine breaking change that will require updates to any systems interacting with validator creation.


Line range hint 16-16: Transaction fee tokens restricted

Restricting transaction fee tokens is a significant change that may affect how users interact with the system. This change should be clearly communicated to all stakeholders, including users and developers.


Line range hint 20-20: x/recovery module removed

The removal of the x/recovery module is a major change that may affect existing functionality. Ensure that any dependencies on this module are addressed and that users are informed about the removal of any related features.


Line range hint 21-21: bank precompile added

The addition of a bank precompile is a useful feature that allows for easier interaction with bank module functionality. This change is state machine breaking and will require careful testing and integration.


Line range hint 22-22: x/incentives module removed and incentives pool balance burned

The removal of the x/incentives module and the burning of the incentives pool balance is a significant change. This may affect existing incentive structures and should be clearly communicated to all stakeholders.


Line range hint 23-24: x/claims params removed and EVMChannels param migrated

The removal of x/claims params and the migration of the EVMChannels param to the x/evm module params is a notable change. Ensure that any systems relying on the old parameter structure are updated accordingly.


Line range hint 25-25: BurnDecorator added to PostHandler

Adding a BurnDecorator to the PostHandler to burn cosmos transaction fees is an important change that affects the economics of the system. This should be clearly explained in the documentation and communicated to users.


Line range hint 26-26: Channel selector added based on chain ID

The addition of a channel selector based on chain ID is a useful feature for multi-chain environments. Ensure that the selection logic is well-documented and tested.


Line range hint 27-27: Inflation reduced by 2/3

A significant reduction in inflation by 2/3 is a major economic change. This should be thoroughly explained in the documentation, including the rationale behind the change and its expected impacts.


Line range hint 28-29: Stride and Osmosis outposts refactored

The refactoring of Stride and Osmosis outposts to hardcode the WEVMOS address is a notable change. Ensure that this doesn't introduce any security vulnerabilities and that the hardcoded address is correctly set.


Line range hint 30-30: feecollector Burner role added in upgrade handler

Adding the feecollector Burner role in the upgrade handler is an important change for fee management. Ensure that the implications of this change are well-documented and that it doesn't introduce any unexpected behavior.


Line range hint 31-31: Usage incentives pool balance burned during v16 upgrade

Burning the usage incentives pool balance during the v16 upgrade is a significant economic change. This should be clearly communicated to all stakeholders, and the implications of this action should be thoroughly explained.


Line range hint 35-35: inflation module renamed to inflation/v1

Renaming the inflation module to inflation/v1 is an API breaking change that may require updates to client applications. Ensure that this change is clearly documented and that migration guides are provided for users of the inflation module.


Line range hint 36-36: Legacy EIP-712 ante handler deprecated

The deprecation of the legacy EIP-712 ante handler is an important change that may affect existing transactions. Provide clear documentation on the new recommended approach and any necessary migration steps.


Line range hint 40-41: Lint tests added for consistency

Adding lint tests to ensure consistency between test code and non-test code is a good practice. This will help maintain code quality and readability across the project.


Line range hint 42-42: Golang dependency vulnerability checker added to CI

Adding a Golang dependency vulnerability checker to the CI process is an excellent security improvement. This will help identify and address potential vulnerabilities in dependencies more quickly.


Line range hint 43-44: Default File store listener added for application

Adding a default File store listener based on ADR38 is a good improvement for state management and observability. Ensure that the implementation is well-documented and that any performance implications are considered.


Line range hint 45-45: Audits page added to documentation

Adding an audits page to the documentation is a great improvement for transparency and security. This will help users and developers understand the security measures in place.


Line range hint 46-47: New MsgUpdateVestingFunder message added

The addition of a new MsgUpdateVestingFunder message to update the Funder field of a clawback vesting account is a useful feature. Ensure that proper access controls are in place for this operation.


Line range hint 62-62: Fix for TraceTx KVGasConfig setup

Fixing the TraceTx KVGasConfig setup is important for accurate gas estimation and tracing. Ensure that this fix is thoroughly tested with various transaction types.


Line range hint 63-63: Fix for inflation info returned by epoch mint provision getter

Correcting the inflation info returned by the epoch mint provision getter is crucial for accurate economic calculations. Verify that this fix doesn't have any unintended consequences on inflation-related functionality.


Line range hint 64-64: Add burned cosmos transactions fees metric

Adding a metric for burned cosmos transaction fees is a good improvement for monitoring and analytics. Ensure that this metric is properly documented and integrated with existing monitoring systems.


65-65: Fix store uploader for x/recovery module

Fixing the store uploader for the x/recovery module is important for data integrity during upgrades. Thoroughly test this fix with various upgrade scenarios to ensure it works as expected.


66-66: Fix unbound metrics and remove labels that keep increasing

Addressing unbound metrics and removing continuously increasing labels is crucial for the stability and performance of the metrics system. Verify that this fix doesn't break any existing monitoring or alerting rules.


Line range hint 1-71: Comprehensive update with significant changes

The v16.0.0 release introduces numerous substantial changes, including state machine breaking modifications, API changes, new features, and important bug fixes. Key points to note:

  1. Several modules have been removed or significantly altered (e.g., recovery, incentives).
  2. New precompiles and functions have been added, enhancing functionality.
  3. Economic parameters have been adjusted, including inflation reduction.
  4. Various improvements have been made to testing, documentation, and development processes.
  5. Several critical bug fixes have been implemented.

Given the scope of these changes, it is crucial to:

  • Thoroughly test all modifications, especially state machine breaking changes.
  • Update all relevant documentation and provide clear upgrade guides.
  • Communicate these changes clearly to all stakeholders, including users, developers, and node operators.
  • Consider the potential impact on existing integrations and provide necessary migration paths.

Overall, this release represents a significant evolution of the system and should be approached with careful planning and testing.

Copy link
Contributor
@MalteHerrmann MalteHerrmann left a comment

Choose a reason for hiding this comment

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

Thanks @GAtom22 🙌

@GAtom22 GAtom22 enabled auto-merge (squash) September 27, 2024 13:07
@GAtom22 GAtom22 merged commit 9c452de into main Sep 27, 2024
50 of 52 checks passed
@GAtom22 GAtom22 deleted the GAtom22/legacy-msg branch September 27, 2024 13:32
mergify bot pushed a commit that referenced this pull request Sep 27, 2024
* chore(erc20): keep MsgConvertCoin interface registered

* add changelog entry

* fix lint warnings

(cherry picked from commit 9c452de)

# Conflicts:
#	CHANGELOG.md
#	x/erc20/types/codec.go
mergify bot pushed a commit that referenced this pull request Sep 27, 2024
* chore(erc20): keep MsgConvertCoin interface registered

* add changelog entry

* fix lint warnings

(cherry picked from commit 9c452de)
GAtom22 added a commit that referenced this pull request Sep 27, 2024
… (#2894)

* chore(erc20): keep MsgConvertCoin interface registered (#2892)

* chore(erc20): keep MsgConvertCoin interface registered

* add changelog entry

* fix lint warnings

(cherry picked from commit 9c452de)

* update chlog

---------

Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: tom <tomasguerraalda@hotmail.com>
GAtom22 added a commit that referenced this pull request Sep 27, 2024
… (#2893)

* chore(erc20): keep MsgConvertCoin interface registered (#2892)

* chore(erc20): keep MsgConvertCoin interface registered

* add changelog entry

* fix lint warnings

(cherry picked from commit 9c452de)

# Conflicts:
#	CHANGELOG.md
#	x/erc20/types/codec.go

* fix conflicts

* add methods to comply with Msg interface

---------

Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: tom <tomasguerraalda@hotmail.com>
GAtom22 added a commit that referenced this pull request Sep 27, 2024
… (#2893)

* chore(erc20): keep MsgConvertCoin interface registered (#2892)

* chore(erc20): keep MsgConvertCoin interface registered

* add changelog entry

* fix lint warnings

(cherry picked from commit 9c452de)

* fix conflicts

* add methods to comply with Msg interface

---------

Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: tom <tomasguerraalda@hotmail.com>
@coderabbitai coderabbitai bot mentioned this pull request Sep 27, 2024
9 tasks
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.

4 participants
0