refactor: merge confirm handler

[zakir-code]

Summary by CodeRabbit

• New Features

  • Introduced new confirmation messages for cross-chain transactions, enhancing the Gravity module's functionality.
  • Added a new Confirm method for processing confirmation messages.

• Bug Fixes

  • Updated expected values in tests to reflect correct counts for assertions.

• Refactor

  • Consolidated keeper architecture to focus on cross-chain capabilities, removing direct dependencies on the Tron module.
  • Streamlined confirmation handling logic by centralizing it in a new ConfirmHandler.

• Documentation

  • Removed detailed descriptions for certain message types in the API documentation, simplifying the content.

[coderabbitai]

Walkthrough

The changes in this pull request involve significant modifications across various files to enhance the cross-chain functionality of the application. Key updates include adjustments to expected assertion values in tests, the introduction of new message types and handlers in the Gravity module, and the refactoring of keeper interfaces to unify cross-chain operations. Additionally, deprecated methods and types related to the Tron module have been removed, streamlining the codebase and focusing on the new cross-chain architecture.

Changes

Files	Change Summary
app/encoding_test.go	Updated expected assertion values in TestRegisterInterfaces for count1 (from 31 to 32) and count2 (from 305 to 307).
app/interface_registry.json	Added new entries for the Gravity module, including types and message paths related to confirmations.
app/keepers/keepers.go	Removed tronkeeper import; updated TronKeeper to use crosschainkeeper for a unified keeper interface. Adjusted route additions in crosschainRouter and govRouter.
docs/swagger-ui/swagger.yaml	Removed detailed descriptions for MsgConfirmBatch and MsgOracleSetConfirm, simplifying documentation.
proto/fx/gravity/crosschain/v1/tx.proto	Deprecated several RPC methods and added a new Confirm method with associated message types for handling confirmations.
x/crosschain/keeper/confirm.go	Introduced a new Keeper struct to handle confirmation messages, defining various handler methods for different message types.
x/crosschain/keeper/keeper_v1_test.go	Removed Tron-specific logic from KeeperTestSuite methods, simplifying the code.
x/crosschain/keeper/msg_server.go	Refactored confirmation methods to delegate logic to a new ConfirmHandler, simplifying the implementation.
x/crosschain/keeper/msg_server_router.go	Added a new Confirm method to handle confirmation messages, integrating with the message server.
x/crosschain/types/codec.go	Registered new Confirm interface and associated message types in codec registration functions.
x/crosschain/types/msgs.go	Introduced a new Confirm interface with method signatures for confirmation messages, implemented by existing message types.
x/tron/keeper/keeper.go	Deleted the keeper.go file, removing the Tron keeper's functionality and related methods.
x/tron/keeper/keeper_test.go	Updated KeeperTestSuite to remove Tron-specific imports and logic, consolidating keeper functionality.
x/tron/keeper/msg_server.go	Deleted the msg_server.go file, which handled Tron-specific message server implementations.
x/tron/module.go	Updated AppModule structure to use crosschainkeeper instead of keeper, modifying associated methods accordingly.

Possibly related PRs

• refactor: use ExternalAddrToStr #448: Refactors address handling relevant to cross-chain operations.
• feat(x/crosschain): unified external claim msg #675: Introduces a unified external claim message relevant to cross-chain operations.
• chore: mark deprecated SendToExternal logic #686: Marks deprecated logic related to sending assets to external addresses, relevant to cross-chain operations.
• refactor: use interface for precompiled contract calls to crosschain module #690: Refactors precompiled contract calls to the crosschain module, potentially relevant to the assertion changes in the main PR.

Suggested labels

automerge

Poem

🐇 In fields where cross-chains play,
New messages hop, bright as day.
With keepers united, the code's refined,
Old paths are cleared, new ones defined.
So let’s celebrate this joyful spree,
In the world of code, we’re wild and free! 🌼

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 57e9fca and e1844fd.

🔇 Files ignored due to path filters (1)

• x/crosschain/types/tx.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (16)

• app/encoding_test.go (1 hunks)
• app/interface_registry.json (3 hunks)
• app/keepers/keepers.go (5 hunks)
• docs/swagger-ui/swagger.yaml (0 hunks)
• proto/fx/gravity/crosschain/v1/tx.proto (4 hunks)
• x/crosschain/keeper/confirm.go (1 hunks)
• x/crosschain/keeper/keeper_v1_test.go (1 hunks)
• x/crosschain/keeper/msg_server.go (2 hunks)
• x/crosschain/keeper/msg_server_router.go (1 hunks)
• x/crosschain/precompile/contract_test.go (0 hunks)
• x/crosschain/types/codec.go (3 hunks)
• x/crosschain/types/msgs.go (1 hunks)
• x/tron/keeper/keeper.go (0 hunks)
• x/tron/keeper/keeper_test.go (1 hunks)
• x/tron/keeper/msg_server.go (0 hunks)
• x/tron/module.go (3 hunks)

💤 Files not reviewed due to no reviewable changes (4)

• docs/swagger-ui/swagger.yaml
• x/crosschain/precompile/contract_test.go
• x/tron/keeper/keeper.go
• x/tron/keeper/msg_server.go

🔇 Additional comments not posted (28)

app/encoding_test.go (1)

46-46: Verify the increase in impl interfaces.

The assertion for count2 has been updated from 305 to 307, indicating an increase of 2 in the number of impl interfaces. This change correlates with the previous increase in count1, suggesting a coordinated change in the system's interfaces.

Please confirm if this change is intentional and aligns with recent modifications in the codebase. To help verify this, you can run the following script:

#!/bin/bash
# Description: Check for recent changes in interface implementations and their usage

# Search for new interface implementations or changes in existing ones
rg --type go -e 'type.*struct' -e 'func \(.*\).*interface' --sort path

# Search for new uses of interfaces
rg --type go -e 'var.*interface' -e '.*:.*interface' --sort path

This will help identify any new or modified interface implementations and their usage that could explain the increase in both counts.

x/tron/module.go (3)

81-81: LGTM: Keeper type updated correctly

The change from keeper.Keeper to crosschainkeeper.Keeper is consistent with the refactoring objective. This update consolidates the keeper functionality under the crosschainkeeper interface, which should improve code organization and maintainability.

---

103-103: LGTM: Migrator creation updated correctly

The NewMigrator function call has been correctly updated to use am.keeper directly, which is consistent with the changes made to the AppModule struct and NewAppModule function. This change simplifies the code and maintains consistency with the new keeper type.

---

114-114: LGTM: Genesis methods updated correctly

The InitGenesis and ExportGenesis methods have been correctly updated to use am.keeper directly. These changes are consistent with the modifications made to the AppModule struct and other methods, maintaining the new keeper type usage throughout the module.

Also applies to: 120-120

x/crosschain/types/codec.go (3)

44-44: LGTM: New message type registered correctly.

The addition of &MsgConfirm{} to the list of message implementations is correct and follows the standard practice for registering new message types in Cosmos SDK.

---

Line range hint 1-115: Summary: Confirm interface and associated messages successfully integrated.

The changes in this file successfully introduce the new Confirm interface and its associated message types. These modifications enhance the module's functionality by providing a common interface for confirmation-related operations. The changes are consistent across both the new interface registry and the legacy Amino codec, ensuring compatibility with different versions of the Cosmos SDK.

Key points:

1. New MsgConfirm type added to the message implementations.
2. Confirm interface registered with appropriate message types.
3. Confirm interface added to the legacy Amino codec for backwards compatibility.

These changes align well with the PR objective of refactoring and merging the confirm handler. The code structure and registration patterns follow Cosmos SDK conventions, which should facilitate easy integration and future maintenance.

---

78-78: LGTM: Confirm interface registered for legacy Amino codec. Verify usage.

The addition of the Confirm interface registration for the legacy Amino codec is correct and necessary for maintaining backwards compatibility.

To ensure consistency, let's verify that all message types implementing the Confirm interface are also registered with the Amino codec:

This script will help ensure that all message types implementing the Confirm interface are properly registered with the Amino codec for backwards compatibility.

x/crosschain/keeper/keeper_v1_test.go (1)

82-82: LGTM: Simplified Tron keeper return statement

The change to directly return suite.App.TronKeeper aligns with the PR objective of refactoring and merging confirm handlers. This simplification maintains the existing functionality while reducing code complexity.

x/tron/keeper/keeper_test.go (3)

65-65: LGTM! Ensure adequate test coverage.

The change from tronkeeper.NewMsgServerImpl to keeper.NewMsgServerImpl is consistent with the previous change and aligns with the PR objective of merging the confirm handler.

To ensure the new implementation is properly tested, please run the following script to check the test coverage for the new keeper.NewMsgServerImpl:

#!/bin/bash
# Description: Check test coverage for keeper.NewMsgServerImpl

# Test: Search for test cases involving keeper.NewMsgServerImpl
rg --type go 'keeper\.NewMsgServerImpl.*testing'

# If no results are found, consider adding more test cases

---

62-65: Overall LGTM! Recommend comprehensive testing.

The changes in this file consistently replace tronkeeper with keeper, aligning with the PR objective of merging the confirm handler. This refactoring appears to be part of a larger effort to consolidate keeper functionality under a single interface.

To ensure these changes don't introduce any regressions, please run a comprehensive test suite:

---

62-62: LGTM! Verify impact on TronKeeper usage.

The change from tronkeeper.NewQueryServerImpl(suite.app.TronKeeper.Keeper) to keeper.NewQueryServerImpl(suite.app.TronKeeper) aligns with the PR objective of merging the confirm handler. This consolidation simplifies the keeper interface.

To ensure this change doesn't introduce any issues, please run the following script to check for any other occurrences of TronKeeper.Keeper that might need updating:

✅ Verification successful

Verification Successful: No additional instances of TronKeeper.Keeper found.

The change in x/tron/keeper/keeper_test.go is isolated and does not affect other parts of the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of TronKeeper.Keeper that might need updating

# Test: Search for TronKeeper.Keeper usage
rg --type go 'TronKeeper\.Keeper'

Length of output: 50

---

Script:

#!/bin/bash
# Description: Re-verify occurrences of TronKeeper and related patterns to ensure no instances are missed.

# Test: Search for various TronKeeper usage patterns
rg --type go 'TronKeeper\.Keeper'
rg --type go 'keeper\.NewMsgServerImpl'

Length of output: 3305

x/crosschain/keeper/msg_server_router.go (1)

185-191: Verify the existence of the Confirm method in the underlying interface.

The new Confirm method seems to be well-integrated with the existing code structure. However, it's crucial to ensure that the Confirm method exists in the underlying message server interface (types.MsgServer).

Please run the following script to verify the existence of the Confirm method in the types.MsgServer interface:

If the Confirm method is not found in the interface, it needs to be added to ensure consistency and prevent runtime errors.

✅ Verification successful

Verified that the Confirm method exists in the types.MsgServer interface. No issues found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of the Confirm method in the types.MsgServer interface

# Test: Search for the Confirm method in the MsgServer interface
ast-grep --lang go --pattern $'type MsgServer interface {
  $$$
  Confirm(context.Context, *MsgConfirm) (*MsgConfirmResponse, error)
  $$$
}'

Length of output: 3260

proto/fx/gravity/crosschain/v1/tx.proto (3)

38-39: Verify the deprecation of RequestBatch

The RequestBatch RPC method has been marked as deprecated. Please ensure that:

1. The functionality provided by this method is either no longer needed or has been replaced by the new Confirm method or another appropriate method.
2. All clients using this method are updated accordingly.
3. Documentation is updated to reflect this change.

To check for any remaining usages of this method, run:

#!/bin/bash
# Search for RequestBatch usage in the codebase
rg --type go "RequestBatch"

---

45-47: Verify the deprecation of BridgeCallConfirm

The BridgeCallConfirm RPC method has been marked as deprecated. Please ensure that:

1. The bridge call confirmation functionality is adequately replaced by the new Confirm method.
2. All clients using this method are updated to use the new confirmation process.
3. Documentation is updated to reflect this change in bridge call handling.
4. Any specific logic related to bridge call confirmations is properly migrated to the new method.

To check for any remaining usages of this method, run:

#!/bin/bash
# Search for BridgeCallConfirm usage in the codebase
rg --type go "BridgeCallConfirm"

---

62-62: Verify the implementation of the new Confirm method and MsgConfirm message

The addition of the Confirm RPC method and MsgConfirm message appears to be a good approach for generalizing the confirmation process. The use of google.protobuf.Any for the confirm field allows for flexibility in handling different types of confirmations. However, please ensure that:

1. The new Confirm method can handle all the confirmation scenarios previously covered by the deprecated methods (OracleSetConfirm, ConfirmBatch, BridgeCallConfirm).
2. Proper documentation is added to explain how to use the new Confirm method and MsgConfirm message, including examples of different confirmation types.
3. Client code is updated to use this new method correctly.
4. Adequate test coverage is implemented for the new method, covering all possible confirmation types.

To check the implementation of the new Confirm method, run:

#!/bin/bash
# Search for the implementation of the Confirm method
rg --type go "func \(.*\) Confirm\("

Also applies to: 387-397

app/interface_registry.json (3)

229-232: LGTM: New TypeURLMap entries for MsgConfirm added correctly.

The new entries for "/fx.gravity.crosschain.v1.MsgConfirm" and "/fx.gravity.crosschain.v1.MsgConfirmResponse" have been added correctly to the TypeURLMap. These additions are consistent with the AI-generated summary and follow the existing naming convention and structure.

---

437-437: LGTM: New Msgs entry for MsgConfirm added correctly.

The new entry "/fx.gravity.crosschain.v1.MsgConfirm" has been added correctly to the Msgs section. This addition is consistent with the changes made in the TypeURLMap section and follows the existing naming convention and structure.

---

Line range hint 20-532: Summary: MsgConfirm functionality added to interface registry

The changes in this file add support for the new MsgConfirm functionality in the Gravity module. The additions have been made consistently in both the TypeURLMap and Msgs sections, following the existing naming conventions and structure. These changes align with the PR objectives of merging the confirm handler.

Key points:

1. New TypeURLMap entries for MsgConfirm and MsgConfirmResponse added.
2. New Msgs entry for MsgConfirm added.

The changes look good and should enable the system to recognize and handle the new MsgConfirm messages correctly.

x/crosschain/types/msgs.go (2)

164-169: LGTM: Well-defined Confirm interface

The new Confirm interface is well-structured with clear and consistent method signatures. It covers essential information needed for confirmation operations across different chain types.

---

171-175: LGTM: Correct interface implementation declarations

The declarations for MsgBridgeCallConfirm, MsgConfirmBatch, and MsgOracleSetConfirm implementing the Confirm interface are correct and follow Go conventions.

To ensure the correctness of the implementations, please run the following command:

x/crosschain/keeper/confirm.go (1)

61-64: Verify external address formatting consistency.

Ensure that msg.ExternalAddress is correctly formatted and consistent with the expected external address format for the blockchain module (e.g., Tron or Ethereum). Incorrect formatting may lead to validation failures in ValidateConfirmSign.

Run the following script to check the format of ExternalAddress:

x/crosschain/keeper/msg_server.go (3)

468-469: Refactored ConfirmBatch to use ConfirmHandler

The ConfirmBatch method now delegates to the ConfirmHandler, promoting code reuse and simplifying the codebase.

---

474-475: Refactored OracleSetConfirm to use ConfirmHandler

The OracleSetConfirm method has been updated to call ConfirmHandler, reducing duplication and enhancing maintainability.

---

480-481: Refactored BridgeCallConfirm to use ConfirmHandler

The BridgeCallConfirm method now utilizes ConfirmHandler, streamlining the confirmation logic across different methods.

app/keepers/keepers.go (4)

94-94: Updated TronKeeper to use crosschainkeeper.Keeper

The TronKeeper in the CrossChainKeepers struct now uses crosschainkeeper.Keeper, aligning it with other cross-chain keepers and promoting consistency across the codebase.

---

Line range hint 468-481: Refactored TronKeeper initialization to use crosschainkeeper.NewKeeper

The initialization of appKeepers.TronKeeper now uses crosschainkeeper.NewKeeper, consistent with other cross-chain modules. This unifies the keeper implementations and simplifies the codebase.

---

493-493: Updated module handler for TronKeeper in the cross-chain router

The route addition for the Tron module now uses crosschainkeeper.NewModuleHandler(appKeepers.TronKeeper), ensuring it utilizes the unified cross-chain module handler.

---

506-506: Added TronKeeper to the precompile router

The TronKeeper is now included in the precompile router using AddRoute(trontypes.ModuleName, appKeepers.TronKeeper), aligning it with other cross-chain modules.

---

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.