-
Notifications
You must be signed in to change notification settings - Fork 29
fix: break link-coin command #1063
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 2 comments
sui/README.md
Outdated
```bash | ||
ts-node sui/its link-coin <symbol> <name> <decimals> <destinationChain> <destinationAddress> | ||
|
||
node sui/its.js link-coin-with-channel <symbol> <destinationChain> <destinationAddress> --channel <channelId> --tokenManagerMode <lock_unlock|mint_burn> --gasValue <amount_in_SUI> --saltAddress <addr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Inconsistent command invocation - uses 'node' instead of 'ts-node' like other commands in the documentation
node sui/its.js link-coin-with-channel <symbol> <destinationChain> <destinationAddress> --channel <channelId> --tokenManagerMode <lock_unlock|mint_burn> --gasValue <amount_in_SUI> --saltAddress <addr> | |
ts-node sui/its.js link-coin-with-channel <symbol> <destinationChain> <destinationAddress> --channel <channelId> --tokenManagerMode <lock_unlock|mint_burn> --gasValue <amount_in_SUI> --saltAddress <addr> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: sui/README.md
Line: 522:522
Comment:
style: Inconsistent command invocation - uses 'node' instead of 'ts-node' like other commands in the documentation
```suggestion
ts-node sui/its.js link-coin-with-channel <symbol> <destinationChain> <destinationAddress> --channel <channelId> --tokenManagerMode <lock_unlock|mint_burn> --gasValue <amount_in_SUI> --saltAddress <addr>
```
How can I resolve this? If you propose a fix, please make it concise.
new Option('--tokenManagerMode <mode>', 'Token Manager Mode').choices(['lock_unlock', 'mint_burn']).makeOptionMandatory(true), | ||
) | ||
.addOption(new Option('--destinationOperator <address>', 'Operator that can control flow limits on the destination chain')) | ||
.requiredOption('--gasValue <amount>', 'Amount to pay gas (SUI full units)', parseSuiUnitAmount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Missing import for parseSuiUnitAmount
function used in gasValue validation
Prompt To Fix With AI
This is a comment left during a code review.
Path: sui/its.js
Line: 1069:1069
Comment:
syntax: Missing import for `parseSuiUnitAmount` function used in gasValue validation
How can I resolve this? If you propose a fix, please make it concise.
coin.objects.Metadata, | ||
[linkedToken], | ||
saltAddress, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Token Linking Overwrites Data, Gas Wasted
The linkCoinWithChannel
function overwrites existing linked token data when saving, causing previously linked chains to be lost if a token is linked multiple times. Separately, the token existence check occurs after the transaction is broadcast, potentially wasting gas if the token isn't in the config.
This work was done in #1059 |
why?
how?
testing
Note
Splits linking flow by adding
link-coin-with-channel
(uses existing channel, pays gas, sends link message) and trimminglink-coin
to deploy/register only; updates docs and CLI.link-coin-with-channel <symbol> <destinationChain> <destinationAddress> --channel <channelId> --tokenManagerMode <lock_unlock|mint_burn> --gasValue <amount_in_SUI> [--saltAddress <addr>]
.GasService::pay_gas
, sends link message viaAxelarGateway::gateway::send_message
, and saves[linkedToken]
to config.link-coin
: Remove in-command linking and message send; now only deploys/registers coin and saves deployment with emptylinkedTokens
.channelId
.link-coin-with-channel
alongsidelink-coin
.Written by Cursor Bugbot for commit 40c2870. This will update automatically on new commits. Configure here.
Greptile Overview
Updated On: 2025-10-01 13:16:51 UTC
Summary
This PR refactors the `link-coin` command in the Sui ITS (Interchain Token Service) implementation by breaking it into two separate, more focused commands. The original `link-coin` command was performing both token deployment/registration AND cross-chain linking in a single operation, which has been split for better separation of concerns.The refactored
link-coin
command now only handles:The new
link-coin-with-channel
command handles the actual cross-chain linking functionality:link-coin
operation)This change aligns with the principle of single responsibility, allowing developers to deploy tokens first and then link them to destination chains in separate steps. The documentation in
sui/README.md
has been updated to reflect both command options, providing users with examples of the simplerlink-coin
command and the more advancedlink-coin-with-channel
command with its additional parameters.Important Files Changed
Changed Files
Confidence score: 4/5
Sequence Diagram