revert: recover transaction msg proto definition

[zakir-code]

Summary by CodeRabbit

• New Features

  • Introduced new message definitions for blockchain interactions, including token transfers and staking operations.
  • Added messages for Ethereum interactions, such as MsgSendToEth and MsgValsetConfirm.
  • New MsgTransfer message facilitates fungible token transfers between IBC-enabled chains.

• Bug Fixes

  • Updated import paths for legacy packages to improve organization and clarity.

• Chores

  • Modified cleanup behavior in the script for better resource management.

• Refactor

  • Reorganized legacy functionality into a centralized types package for improved structure.

• Removed

  • Deprecated governance message type MsgUpdateParams has been removed from the codebase.

[coderabbitai]

Walkthrough

The changes in this pull request involve updates to various files related to the restructuring of legacy message types and their import paths. Key modifications include the transition from the govlegacy package to the legacy package in the app/app_test.go file, adjustments to the go_package options in multiple proto files, and the introduction of new message definitions in several new proto files. Additionally, a cleanup script has been modified to alter its exit behavior, and certain legacy message definitions have been removed.

Changes

File Path	Change Summary
app/app_test.go	Updated import from govlegacy to legacy and changed message type mapping.
proto/ethermint/evm/v1/evm_proposal_testnet.proto	Changed go_package from github.com/functionx/fx-core/x/evm/legacy to github.com/functionx/fx-core/types/legacy.
proto/fx/gov/v1/gov_legacy.proto	Changed go_package from github.com/functionx/fx-core/x/gov/legacy to github.com/functionx/fx-core/types/legacy.
proto/fx/gravity/v1/gravity_legacy.proto	Introduced multiple new message definitions related to Ethereum interactions and token transfers.
proto/fx/ibc/applications/transfer/v1/ibc_legacy.proto	Added MsgTransfer for fungible token transfers between IBC-enabled chains.
proto/fx/staking/v1/staking_legacy.proto	Added MsgGrantPrivilege and MsgEditConsensusPubKey for validator management.
scripts/protoc-swagger-gen.sh	Modified cleanup logic to retain the ./build directory upon exit.
x/evm/types/codec.go	Updated import path for legacy from x/evm/legacy to types/legacy.
x/gov/legacy/legacy_msg.go	Removed file containing MsgUpdateParams message type and its associated methods.
x/gov/types/codec.go	Updated import path for legacy from x/gov/legacy to types/legacy.

Possibly related PRs

• chore: mark deprecated SendToExternal logic #686: The main PR modifies the mapping of deprecated message types, specifically changing from govlegacy to legacy, which aligns with the deprecation of messages in the retrieved PR that also deals with legacy functionality in the context of governance and cross-chain operations.

Suggested labels

automerge

Poem

In the code where bunnies hop,
Changes made, we can't stop!
Legacy paths now take a turn,
New messages for us to learn.
With every tweak, our project grows,
Hopping forward, as everyone knows! 🐇✨

---

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e3017dd and 4558222.

Files ignored due to path filters (5)

• types/legacy/evm_proposal_testnet.pb.go is excluded by !**/*.pb.go
• types/legacy/gov_legacy.pb.go is excluded by !**/*.pb.go
• types/legacy/gravity_legacy.pb.go is excluded by !**/*.pb.go
• types/legacy/ibc_legacy.pb.go is excluded by !**/*.pb.go
• types/legacy/staking_legacy.pb.go is excluded by !**/*.pb.go

Files selected for processing (10)

• app/app_test.go (2 hunks)
• proto/ethermint/evm/v1/evm_proposal_testnet.proto (1 hunks)
• proto/fx/gov/v1/gov_legacy.proto (1 hunks)
• proto/fx/gravity/v1/gravity_legacy.proto (1 hunks)
• proto/fx/ibc/applications/transfer/v1/ibc_legacy.proto (1 hunks)
• proto/fx/staking/v1/staking_legacy.proto (1 hunks)
• scripts/protoc-swagger-gen.sh (1 hunks)
• x/evm/types/codec.go (1 hunks)
• x/gov/legacy/legacy_msg.go (0 hunks)
• x/gov/types/codec.go (1 hunks)

Files not reviewed due to no reviewable changes (1)

• x/gov/legacy/legacy_msg.go

Additional context used

buf

proto/fx/gravity/v1/gravity_legacy.proto

4-4: import "cosmos/base/v1beta1/coin.proto": file does not exist

(COMPILE)

proto/fx/ibc/applications/transfer/v1/ibc_legacy.proto

4-4: import "cosmos/base/v1beta1/coin.proto": file does not exist

(COMPILE)

proto/fx/staking/v1/staking_legacy.proto

4-4: import "cosmos_proto/cosmos.proto": file does not exist

(COMPILE)

Additional comments not posted (17)

proto/fx/staking/v1/staking_legacy.proto (3)

18-23: MsgEditConsensusPubKey structure is well-defined.

The MsgEditConsensusPubKey message is correctly structured for editing a validator's consensus public key. The use of google.protobuf.Any with the cosmos_proto.accepts_interface option for the pubkey field is consistent with the previous message and follows Cosmos SDK conventions.

---

1-23: Overall, the staking_legacy.proto file is well-structured and follows Protobuf and Cosmos SDK conventions.

This new file successfully introduces two message types (MsgGrantPrivilege and MsgEditConsensusPubKey) for staking-related operations. The use of google.protobuf.Any with Cosmos SDK protobuf options enhances compatibility with public key interfaces, which is crucial for validator management.

The file structure, syntax, and message definitions are clean and follow best practices. There are only minor suggestions for improvement, such as comment alignment.

Remember to verify the cosmos_proto import as mentioned in the first comment, but otherwise, this file is ready for integration into the project.

Tools

buf

4-4: import "cosmos_proto/cosmos.proto": file does not exist

(COMPILE)

---

1-7: File structure looks good, but verify the cosmos_proto import.

The overall structure of the Protobuf file is correct, with proper syntax, package declaration, and imports. However, there's a static analysis hint about a missing file:

This might be a false positive, as cosmos_proto is a common dependency in Cosmos SDK projects.

Please run the following script to check if the cosmos_proto directory exists in the project:

If the cosmos_proto directory or cosmos.proto file is not found, you may need to add the cosmos-proto dependency to your project.

Verification successful

cosmos_proto import is correctly set up.

The cosmos_proto directory and cosmos.proto file exist in the project, and the cosmos-proto dependency is specified in go.mod. Therefore, the static analysis hint about a missing file is a false positive.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for the existence of cosmos_proto directory and cosmos.proto file

# Test 1: Check if cosmos_proto directory exists
if fd -t d "cosmos_proto"; then
    echo "cosmos_proto directory found"
else
    echo "cosmos_proto directory not found"
fi

# Test 2: Check if cosmos.proto file exists
if fd -t f "cosmos.proto"; then
    echo "cosmos.proto file found"
else
    echo "cosmos.proto file not found"
fi

# Test 3: Check go.mod for cosmos-proto dependency
if [ -f "go.mod" ]; then
    if grep -q "cosmos-proto" go.mod; then
        echo "cosmos-proto dependency found in go.mod"
    else
        echo "cosmos-proto dependency not found in go.mod"
    fi
else
    echo "go.mod file not found"
fi

Length of output: 308

Tools

buf

4-4: import "cosmos_proto/cosmos.proto": file does not exist

(COMPILE)

app/app_test.go (2)

21-21: LGTM. Verify impact on deprecated message handling.

The map entry has been updated to use the new legacy package, which is consistent with the import statement change. This modification aligns with the PR objective of recovering the transaction message proto definition.

To ensure that this change doesn't introduce any unintended side effects in the handling of deprecated messages, run the following script:

#!/bin/bash
# Description: Verify the usage of MsgUpdateParams in the codebase.

# Test 1: Search for other occurrences of MsgUpdateParams. 
echo "Searching for other occurrences of MsgUpdateParams:"
rg --type go 'MsgUpdateParams'

# Test 2: Search for any remaining references to govlegacy.MsgUpdateParams
echo "Searching for any remaining references to govlegacy.MsgUpdateParams:"
rg --type go 'govlegacy\.MsgUpdateParams'

Please review the results to ensure that all relevant occurrences have been updated and that there are no remaining references to the old govlegacy.MsgUpdateParams.

---

10-10: LGTM. Verify consistency across the codebase.

The import statement has been updated from "govlegacy" to "legacy", which aligns with the PR objective of recovering the transaction message proto definition. This change appears to be part of a package reorganization.

To ensure consistency across the codebase, run the following script:

x/gov/types/codec.go (1)

9-9: LGTM! Verify the new package structure.

The import path change from github.com/functionx/fx-core/v8/x/gov/legacy to github.com/functionx/fx-core/v8/types/legacy looks good. This change suggests a refactoring of the project structure, moving the legacy package to a more centralized location.

To ensure the refactoring was done correctly, please run the following script to verify the new package structure:

This script will help confirm that the package restructuring was done consistently across the project.

x/evm/types/codec.go (1)

11-11: LGTM! Verify impact on the codebase.

The import path change from github.com/functionx/fx-core/v8/x/evm/legacy to github.com/functionx/fx-core/v8/types/legacy aligns with the PR objective of reverting and recovering a transaction message proto definition. This change moves the legacy package to a more general types directory, which could improve code organization.

To ensure this change doesn't introduce any issues, please run the following script to check for any remaining occurrences of the old import path:

If the script returns any occurrences of the old import path, please update them accordingly.

Verification successful

Import Path Change Verified

The import path has been successfully updated to github.com/functionx/fx-core/v8/types/legacy across the codebase. No instances of the old import path remain, ensuring consistency and improved code organization.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining occurrences of the old import path

# Test: Search for the old import path. Expect: No results.
rg --type go "github.com/functionx/fx-core/v8/x/evm/legacy"

# Test: Verify that the new import path is used consistently. Expect: Consistent usage across the codebase.
rg --type go "github.com/functionx/fx-core/v8/types/legacy"

Length of output: 321

scripts/protoc-swagger-gen.sh (2)

Line range hint 1-38: Summary of changes and their implications

The main change in this script is the modification of the cleanup process in the trap command. The ./build directory is no longer being removed upon script exit, which aligns with the PR objective to recover transaction message proto definitions.

While the rest of the script remains unchanged and should function as before, it's crucial to ensure that this modification doesn't lead to unintended consequences such as:

1. Excessive disk usage due to accumulation of files in ./build over time.
2. Potential conflicts or unexpected behavior in subsequent runs due to preserved files.

The change appears to be intentional and aligned with the PR objectives. However, please consider the following recommendations:

1. Document this change in the script comments or in a README file.
2. Implement a strategy to manage the growth of the ./build directory over time.
3. Consider adding logging to track what's being preserved in ./build for easier debugging and maintenance.

---

6-6: Clarify the intention behind preserving the ./build directory

The trap command has been modified to no longer remove the ./build directory upon script exit. While this change aligns with the PR title suggesting a recovery of transaction message proto definitions, it's important to consider the following:

1. Preserving the ./build directory could lead to an accumulation of files over multiple script runs.
2. Old build files might potentially interfere with future runs if not properly managed.

To understand the impact, let's check the contents and size of the ./build directory:

Could you please clarify:

1. Is preserving the ./build directory intentional?
2. Are there specific files in ./build that need to be retained between runs?
3. Is there a cleanup mechanism in place to prevent excessive accumulation of files in ./build?

Depending on the answers, we might want to consider:

• Implementing a selective cleanup that preserves only necessary files in ./build.
• Adding a separate cleanup script for ./build that can be run periodically.
• Documenting the new behavior and any manual cleanup steps required.

proto/fx/ibc/applications/transfer/v1/ibc_legacy.proto (2)

1-2: LGTM: File structure and package declaration are appropriate.

The proto3 syntax is used, and the package name fx.ibc.applications.transfer.v1 aligns well with the file path. The Go package is correctly set to "github.com/functionx/fx-core/types/legacy".

Also applies to: 9-9

---

4-7: Verify the existence of imported proto file.

The static analysis tool reported that "cosmos/base/v1beta1/coin.proto" doesn't exist. Please ensure that this file is present in your project or in the correct import path.

Run the following script to check for the existence of the imported proto files:

Tools

buf

4-4: import "cosmos/base/v1beta1/coin.proto": file does not exist

(COMPILE)

proto/fx/gov/v1/gov_legacy.proto (1)

10-10: LGTM! Verify affected import statements.

The change to the go_package option aligns with the PR objective of recovering the transaction message proto definition. Moving the generated Go code to github.com/functionx/fx-core/types/legacy centralizes legacy types, which is a good architectural decision.

To ensure a smooth transition, please verify that all import statements referencing the old package path have been updated accordingly. Run the following script to check for any remaining references to the old path:

If the script returns any results, those files will need to be updated to use the new package path.

Verification successful

All import statements updated to the new package path.

The change to the go_package option has been successfully verified. No remaining references to the old package path were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for references to the old package path in Go files

# Search for import statements with the old path
echo "Checking for old import statements:"
rg --type go 'github\.com/functionx/fx-core/x/gov/legacy'

# Search for string literals containing the old path (e.g., in tests or config)
echo "Checking for string literals with the old path:"
rg --type go '"github\.com/functionx/fx-core/x/gov/legacy"'

Length of output: 307

---

Script:

#!/bin/bash
# Description: Re-verify references to the old package path in Go files with enhanced error handling

# Set the old package path
OLD_PACKAGE='github.com/functionx/fx-core/x/gov/legacy'

# Search for import statements with the old path
echo "Checking for old import statements in Go files:"
rg --type go "import\s+\"$OLD_PACKAGE\"" || echo "No import statements found with the old package path."

# Search for string literals containing the old path (e.g., in tests or config)
echo "Checking for string literals with the old package path in Go files:"
rg --type go "\"$OLD_PACKAGE\"" || echo "No string literals found with the old package path."

Length of output: 666

proto/ethermint/evm/v1/evm_proposal_testnet.proto (3)

Line range hint 1-89: File structure and syntax are consistent with protobuf conventions.

The overall structure of the file follows standard protobuf syntax and conventions. Package declarations, imports, and message definitions are all properly formatted.

While the content of the messages is deprecated, the structure and syntax of the file itself are correct and consistent.

Tools

buf

4-4: import "gogoproto/gogo.proto": file does not exist

(COMPILE)

---

Line range hint 8-89: Consider removing or archiving deprecated message definitions.

All message definitions in this file are marked as deprecated with the comment "// Deprecated: Do not use." This includes InitEvmParamsProposal, EVMParams, EVMChainConfig, and FeemarketParams.

Keeping deprecated code in active files, especially in a file specifically for testnet proposals, can lead to confusion and potential misuse. Consider the following actions:

1. If these messages are no longer used anywhere in the codebase, remove them entirely.
2. If they are still referenced but shouldn't be used in new code, move them to a separate "legacy" or "deprecated" file to clearly separate them from active code.
3. If they must be kept for some reason, add more detailed documentation explaining why they are still present and when they will be removed.

Please clarify the purpose of keeping these deprecated messages in a testnet-specific file. If they are truly not to be used, it would be best to remove them to prevent any accidental usage.

To help determine if these deprecated messages are still in use, run the following script:

#!/bin/bash
# Description: Check for usage of deprecated message types

echo "Checking for usage of deprecated message types:"
for type in InitEvmParamsProposal EVMParams EVMChainConfig FeemarketParams; do
  echo "Searching for $type:"
  rg --type go "$type"
  echo
done

This will help identify if these deprecated types are still referenced in the codebase, which can inform the decision on how to handle them.

Tools

buf

4-4: import "gogoproto/gogo.proto": file does not exist

(COMPILE)

---

6-6: Approve the go_package option change with caution.

The change from "github.com/functionx/fx-core/x/evm/legacy" to "github.com/functionx/fx-core/types/legacy" is likely part of a larger refactoring effort to centralize legacy types. This change is acceptable, but it's important to be aware of its implications.

Please ensure that:

1. All import statements referencing the old package path have been updated throughout the codebase.
2. Any build scripts or configuration files that might reference the old path have been updated.
3. The new types/legacy package exists and is ready to receive the generated code.

Run the following script to verify the usage of the old and new package paths:

This will help identify any places where updates might be needed or confirm that the change has been consistently applied.

Verification successful

go_package option change is verified successfully.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for occurrences of the old and new package paths

echo "Checking for old package path usage:"
rg --type go "github.com/functionx/fx-core/x/evm/legacy"

echo "Checking for new package path usage:"
rg --type go "github.com/functionx/fx-core/types/legacy"

echo "Checking for any remaining references to 'evm/legacy':"
rg --type go "evm/legacy"

Length of output: 13003

---

Script:

#!/bin/bash
# Description: Check for occurrences of the old and new package paths

echo "Checking for old package path usage:"
rg --type go "github.com/functionx/fx-core/x/evm/legacy"

echo "Checking for new package path usage:"
rg --type go "github.com/functionx/fx-core/types/legacy"

echo "Checking for any remaining references to 'evm/legacy':"
rg --type go "evm/legacy"

Length of output: 430

proto/fx/gravity/v1/gravity_legacy.proto (2)

9-110: Questioning the addition of deprecated messages

The entire file consists of messages marked as deprecated. Introducing new code that is already deprecated may lead to confusion and increased maintenance overhead.

Please confirm if adding these deprecated messages is necessary. If they are needed for backward compatibility, consider adding comments to explain their purpose.

---

54-57: ⚠️ Potential issue

Field type mismatch for amount in MsgDepositClaim

The field amount is declared as a string but uses a custom type cosmossdk.io/math.Int.

Apply this diff to correct the field type:

- string amount = 4 [
+ cosmossdk.io.math.Int amount = 4 [
    (gogoproto.customtype) = "cosmossdk.io/math.Int",
    (gogoproto.nullable) = false
];

Likely invalid or redundant comment.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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.