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

feat(plugin-sui): add support for swapping and transferring by domain name, and converting domain name to address #2140

Closed
wants to merge 16 commits into from

Conversation

Vakurin
Copy link

@Vakurin Vakurin commented Jan 11, 2025

Relates to

N/A - New feature addition

Risks

Low - new features, but there may be instances where the model could trigger incorrect actions

Background

What does this PR do?

  • Integrates swap functionality using Aftermath, one of the top DEXs on the Sui
  • Updates the transfer function to support both SuiNS domains and wallet addresses
  • Implements SuiNS domain-to-address conversion for seamless user experience
  • Fixes wallet balance display to show the correct dollar equivalent

What kind of change is this?

Bug Fixes: Resolves issues with wallet balance calculations.
Feature Addition: Adds swap functionality using Aftermath and SuiNS domain-to-address conversion.
Improvements: Enhances the transfer function to support both SuiNS domains and wallet addresses.

Documentation changes needed?

Testing

Where should a reviewer start?

  1. Install the new dependencies required for domain and swap functionalities.
  2. Review all files in the packages/plugin-sui/src/actions directory to verify updates to the Sui plugin actions.

Detailed Testing Steps

  1. Test Domain Conversion

    • Ask the bot to convert a domain name to an address. Example:
      "Convert maxim.sui to address"
  2. Test New Transfer Functionality

    • Verify transfers by using both SuiNS domain and wallet address. Examples:
      • "Send 0.1 SUI to maxim.sui"
      • "Send 0.1 SUI to {sui_address}"
  3. Test Swap Functionality

    • Perform a swap by providing the coin type of two tokens, the amount, and the recipient. Example:
      • "Swap 0.1 SUI (0x0000000000000000000000000000000000000000000000000000000000000002::sui::SUI) to AAA (0xd976fda9a9786cda1a36dee360013d775a5e5f206f8e20f84fad3385e99eeb2d::aaa::AAA) my wallet: 0xe6e494d014eb41edacae84bfc5893ab5616be246064d52b452ba88828a548b8b"

Discord username

max_holasui.app

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi @Vakurin! Welcome to the ai16z community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now a ai16z contributor!

@Vakurin Vakurin changed the base branch from main to develop January 11, 2025 01:21
@wtfsayo
Copy link
Member

wtfsayo commented Jan 11, 2025

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Jan 11, 2025

📝 Walkthrough

Walkthrough

The pull request introduces updates to the Sui blockchain plugin, including new actions for converting SUI domain names to addresses and swapping tokens. It also enhances the transfer functionality to support Sui Name Service (SuiNS) names. Additionally, several new dependencies are added, utility functions are introduced for address resolution and coin type validation, and the price calculation method is refined for improved precision.

Changes

File Change Summary
packages/plugin-sui/README.md Updated SDK reference from @mysten/sui.js to @mysten/sui
packages/plugin-sui/package.json Added dependencies: @mysten/sui.js, @mysten/suins, aftermath-ts-sdk, and bignumber
packages/plugin-sui/src/actions/convertNameToAddress.ts New action to convert SUI domain names to addresses
packages/plugin-sui/src/actions/swap.ts New action for token swapping on Sui blockchain
packages/plugin-sui/src/actions/transfer.ts Enhanced transfer functionality with SuiNS name support
packages/plugin-sui/src/index.ts Added new actions to plugin configuration
packages/plugin-sui/src/providers/wallet.ts Modified price calculation method using BigNumber
packages/plugin-sui/src/utils.ts Added utility functions for coin type validation and address resolution

Finishing Touches

  • 📝 Generate Docstrings (Beta)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • 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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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.

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: 6

🔭 Outside diff range comments (1)
packages/plugin-sui/src/actions/transfer.ts (1)

Line range hint 164-171: Prevent potential numeric overflow

Using Number type for amount calculation could lead to precision loss or overflow with large values.

-            const adjustedAmount = BigInt(
-                Number(transferContent.amount) * Math.pow(10, SUI_DECIMALS)
-            );
+            const amount = new BigNumber(transferContent.amount);
+            const adjustedAmount = BigInt(
+                amount.times(new BigNumber(10).pow(SUI_DECIMALS)).toString()
+            );
🧹 Nitpick comments (13)
packages/plugin-sui/src/actions/convertNameToAddress.ts (5)

23-25: Consider extending from a more specific Content interface

The NameToAddressContent interface extends Content, but it might be more appropriate to extend from a more specific interface if available. This ensures better type safety and clarity.


27-32: Remove unnecessary console.log statements

The console.log at line 30 is for debugging purposes and should be removed in production code to keep the console output clean.

Apply this diff to remove the console.log:

 function isNameToAddressContent(
     content: Content
 ): content is NameToAddressContent {
-    console.log("Content for show address", content);
     return typeof content.recipientName === "string";
 }

58-61: Avoid using console.log for validation messages

Using console.log for validation messages can clutter the console. Consider using a logging library or remove the statement if it's not essential.

Apply this diff to remove the console.log:

 validate: async (runtime: IAgentRuntime, message: Memory) => {
-    console.log(
-        "Validating sui name to address from user:",
-        message.userId
-    );
     // Add custom validate logic here

93-98: Redundant state initialization check

The state check at line 94 is unnecessary because the state parameter is already provided. Since the handler expects state to be defined, this block can be removed.

Apply this diff to remove redundant code:

 // Initialize or update state
-if (!state) {
-    state = (await runtime.composeState(message)) as State;
-} else {
     state = await runtime.updateRecentMessageState(state);
-}

146-152: Validate the recipient name before processing

Before attempting to retrieve the address, validate that recipientName is a properly formatted SUI domain name to prevent unnecessary API calls and potential errors.

packages/plugin-sui/src/actions/swap.ts (4)

32-41: Remove console.log statements from production code

Logging internal content details might expose sensitive information. Remove or replace console.log statements with appropriate logging mechanisms.

Apply this diff:

 function isSwapContent(content: Content): content is SwapContent {
-    console.log("Content for swap", content);
     return (

68-88: Clean up commented-out code in validation function

The commented-out code within the validate function can be removed to improve readability.

Apply this diff to remove commented code:

 validate: async (runtime: IAgentRuntime, message: Memory) => {
     console.log("Validating sui swap from user:", message.userId);
-    //add custom validate logic here
-    /*
-        // Validation logic here
-    */
     return true;
 },

103-108: Remove redundant state initialization

The state initialization check is redundant since the handler expects state to be defined. This block can be simplified.

Apply this diff:

 // Initialize or update state
-if (!state) {
-    state = (await runtime.composeState(message)) as State;
-} else {
     state = await runtime.updateRecentMessageState(state);
-}

145-204: Handle potential errors with detailed logging

While the try-catch block exists, consider adding more specific error handling for known failure points, such as network issues, transaction failures, or invalid input data.

packages/plugin-sui/src/index.ts (2)

3-4: Align import statements with file extensions

The import statements for convertNameToAddress and swapToken include .ts extensions, whereas the first import does not. For consistency, consider removing the .ts extensions.

Apply this diff:

-import convertNameToAddress from "./actions/convertNameToAddress.ts";
-import swapToken from "./actions/swap.ts";
+import convertNameToAddress from "./actions/convertNameToAddress";
+import swapToken from "./actions/swap";

12-12: Maintain consistency in action order

If transferToken is the primary action, consider ordering the actions array accordingly, unless there is a specific reason to change the order.

packages/plugin-sui/src/utils.ts (1)

18-28: Enhance coin type validation

Consider adding more robust validation:

  • Check for valid characters (alphanumeric and dots only)
  • Validate format against Sui's coin type pattern
 const isCoinType = (coinType: string): boolean => {
+    const VALID_COIN_TYPE_PATTERN = /^[a-zA-Z0-9_.]+$/;
     if (
         coinType.includes("-") ||
         coinType === "" ||
         coinType === null ||
         coinType === undefined ||
+        !VALID_COIN_TYPE_PATTERN.test(coinType)
     ) {
         return false;
     }
     return true;
 };
packages/plugin-sui/src/actions/transfer.ts (1)

183-187: Enhance transaction feedback

Consider adding more details in the success message, such as network and gas fees.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0bdafda and 78441bd.

📒 Files selected for processing (8)
  • packages/plugin-sui/README.md (1 hunks)
  • packages/plugin-sui/package.json (1 hunks)
  • packages/plugin-sui/src/actions/convertNameToAddress.ts (1 hunks)
  • packages/plugin-sui/src/actions/swap.ts (1 hunks)
  • packages/plugin-sui/src/actions/transfer.ts (5 hunks)
  • packages/plugin-sui/src/index.ts (1 hunks)
  • packages/plugin-sui/src/providers/wallet.ts (1 hunks)
  • packages/plugin-sui/src/utils.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/plugin-sui/README.md
🔇 Additional comments (3)
packages/plugin-sui/src/actions/convertNameToAddress.ts (1)

90-91: Ensure walletInfo is correctly initialized

Assigning walletInfo to the state without checking if it's null might cause issues if the wallet information isn't available.

Confirm that walletProvider.get always returns valid walletInfo or handle the case when it's null.

packages/plugin-sui/src/actions/swap.ts (2)

99-101: Ensure walletInfo is correctly initialized

Assigning walletInfo to the state without checking if it's null might cause issues if the wallet information isn't available.

Confirm that walletProvider.get always returns valid walletInfo or handle the case when it's null.


181-190: Ensure sensitive transaction details are securely handled

Displaying transaction hashes and recipient addresses may have privacy implications. Ensure that sharing such details complies with privacy policies and user expectations.

@Vakurin
Copy link
Author

Vakurin commented Jan 12, 2025

@coderabbitai review

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: 3

🧹 Nitpick comments (2)
packages/plugin-sui/src/actions/convertNameToAddress.ts (2)

30-30: Improve logging clarity

The log message "Content for show address" doesn't match the function's purpose.

-    console.log("Content for show address", content);
+    console.log("Validating name to address content:", content);

46-47: Add domain suffix validation

The instruction to validate .sui domains isn't enforced in the code. Consider adding validation.

+const isSuiDomain = (name: string): boolean => {
+    return name.toLowerCase().endsWith('.sui');
+};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78441bd and 0cc7e2e.

📒 Files selected for processing (3)
  • packages/plugin-sui/package.json (1 hunks)
  • packages/plugin-sui/src/actions/convertNameToAddress.ts (1 hunks)
  • packages/plugin-sui/src/providers/wallet.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/plugin-sui/src/providers/wallet.ts
  • packages/plugin-sui/package.json
🔇 Additional comments (1)
packages/plugin-sui/src/actions/convertNameToAddress.ts (1)

57-80: Implement proper validation

The validate function bypasses all security checks by returning true. The commented-out admin validation suggests this was intentional but needs review.

Please confirm if this is intended for production or if the admin validation should be implemented.

@shakkernerd
Copy link
Member

Hello,

We are changing our plugin development strategy to be more scalable. We have moved the plugins out into their own repos and we're looking for people to either maintain those or own them on their own Github.

If you'd like to be a maintainer, file an issue in the plugin repo and join our Discord https://discord.gg/elizaos to coordinate.

If you'd like to control the plugin on your own Github, please add an issue to the plugin repo pointing to your repo, and add a modification to the registry. Submit a PR to edit the registry here: https://github.com/elizaos-plugins/registry

Closing this PR for now. Let us know if you have any questions.

@shakkernerd shakkernerd closed this Feb 7, 2025
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.

3 participants