feat(plugin-sui): add support for swapping and transferring by domain name, and converting domain name to address #2140
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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!
|
@coderabbitai review |
📝 WalkthroughWalkthroughThe 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
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 overflowUsing 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 interfaceThe
NameToAddressContentinterface extendsContent, 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 statementsThe
console.logat 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 messagesUsing
console.logfor 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 checkThe state check at line 94 is unnecessary because the
stateparameter is already provided. Since the handler expectsstateto 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 processingBefore attempting to retrieve the address, validate that
recipientNameis 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 codeLogging internal content details might expose sensitive information. Remove or replace
console.logstatements 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 functionThe commented-out code within the
validatefunction 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 initializationThe state initialization check is redundant since the handler expects
stateto 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 loggingWhile 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 extensionsThe import statements for
convertNameToAddressandswapTokeninclude.tsextensions, whereas the first import does not. For consistency, consider removing the.tsextensions.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 orderIf
transferTokenis 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 validationConsider 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 feedbackConsider adding more details in the success message, such as network and gas fees.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 initializedAssigning
walletInfoto the state without checking if it'snullmight cause issues if the wallet information isn't available.Confirm that
walletProvider.getalways returns validwalletInfoor handle the case when it'snull.packages/plugin-sui/src/actions/swap.ts (2)
99-101: Ensure walletInfo is correctly initializedAssigning
walletInfoto the state without checking if it'snullmight cause issues if the wallet information isn't available.Confirm that
walletProvider.getalways returns validwalletInfoor handle the case when it'snull.
181-190: Ensure sensitive transaction details are securely handledDisplaying transaction hashes and recipient addresses may have privacy implications. Ensure that sharing such details complies with privacy policies and user expectations.
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/plugin-sui/src/actions/convertNameToAddress.ts (2)
30-30: Improve logging clarityThe 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 validationThe 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
📒 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 validationThe 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.
|
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. |
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?
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?
Detailed Testing Steps
Test Domain Conversion
"Convert maxim.sui to address"Test New Transfer Functionality
"Send 0.1 SUI to maxim.sui""Send 0.1 SUI to {sui_address}"Test Swap Functionality
"Swap 0.1 SUI (0x0000000000000000000000000000000000000000000000000000000000000002::sui::SUI) to AAA (0xd976fda9a9786cda1a36dee360013d775a5e5f206f8e20f84fad3385e99eeb2d::aaa::AAA) my wallet: 0xe6e494d014eb41edacae84bfc5893ab5616be246064d52b452ba88828a548b8b"Discord username
max_holasui.app