[wallet] Add merge/split coins command to the wallet #657

patrickkuo · 2022-03-04T15:11:46Z

added merge-coin and split-coin commends to the wallet
added doc for merge and split coins
changed Client api's merge and split coin function to receive ObjectID instead of ObjectRef
Implemented display for Merge and Split coin response

patrickkuo · 2022-03-04T19:12:18Z

@Clay-Mysten can you please help review the docs?

Clay-Mysten · 2022-03-04T22:46:08Z

sui/src/wallet_commands.rs

+
+    /// Split a coin object into multiple coins.
+    SplitCoin {
+        /// Coin to Split, in 20 bytes Hex string


Optionally, lowercase hex throughout:
https://en.wikipedia.org/wiki/Hexadecimal

Totally not a big deal though.

Clay-Mysten · 2022-03-04T22:51:20Z

doc/src/wallet.md

 Sui wallet.  The config file contains information of the accounts and
-the Sui network. Sui wallet uses the network information to communicate
+the Sui network. The keystore file contains all the public private key pair of the created accounts.


public-private key pairs

lxfind

Nice! Like the Displays

Clay-Mysten · 2022-03-04T22:52:25Z

doc/src/wallet.md

-The `accounts` variable contains all of the account's address and key pairs.
-This will be used by the wallet to sign transactions. The `authorities`
-variable contains Sui network authorities' name, host and port information.
+The `accounts` variable contains account's address the wallet manages.


contains the

Clay-Mysten · 2022-03-04T22:53:47Z

doc/src/wallet.md

 and object data belonging to the account.

 #### Key management
-As you might have noticed, the key pairs are stored as plain text in `wallet.conf`, 
-which is not secure and shouldn't be used in a production environment. We have plans to 
+The key pairs are stored in `wallet.key`, however, it is not secure 


. However, this is

Clay-Mysten · 2022-03-04T22:55:42Z

doc/src/wallet.md

@@ -203,17 +205,242 @@ Created new keypair for address : 3F8962C87474F8FB8BFB99151D5F83E677062078

 #### Add existing accounts to `wallet.conf` manually.

-If you have existing key pair from an old wallet config, you can copy the account data manually to the new `wallet.conf`'s accounts section.
+If you have existing key pair from an old wallet config, you can copy the account 
+address manually to the new `wallet.conf`'s accounts section, and add the keypair to the keystore file, 


key pair (for consistency with previous references)

Throughout (three references are joined)

Clay-Mysten · 2022-03-04T22:57:20Z

doc/src/wallet.md

@@ -203,17 +205,242 @@ Created new keypair for address : 3F8962C87474F8FB8BFB99151D5F83E677062078

 #### Add existing accounts to `wallet.conf` manually.

-If you have existing key pair from an old wallet config, you can copy the account data manually to the new `wallet.conf`'s accounts section.
+If you have existing key pair from an old wallet config, you can copy the account 
+address manually to the new `wallet.conf`'s accounts section, and add the keypair to the keystore file, 


I would replace the comma at the end of this line with a semicolon (;) since the previous and next sentence are complete.

Also:

If you have an existing key pair

Clay-Mysten · 2022-03-04T22:58:44Z

doc/src/wallet.md

+
+FLAGS:
+    -h, --help       Prints help information
+        --json       Return command outputs in json format


JSON (in description/output)

Clay-Mysten · 2022-03-04T22:59:31Z

doc/src/wallet.md

+
+FLAGS:
+    -h, --help       Prints help information
+        --json       Return command outputs in json format


Returns (to agree with the previous and following Prints)

Clay-Mysten · 2022-03-04T23:00:56Z

doc/src/wallet.md

+    -V, --version    Prints version information
+
+OPTIONS:
+        --address <address>    Address owning the objects


I'm not sure what the
argument takes or does. What is the active verb that should proceed "Address owning the objects" here?

Clay-Mysten · 2022-03-04T23:01:37Z

doc/src/wallet.md

+OPTIONS:
+        --address <address>    Address owning the objects
+```
+To view the objects owned by the accounts created in genesis, run the following command in the Sui interactive console (substitute the address with one of the genesis address in your wallet): 


of the genesis addresses (plural)

Clay-Mysten · 2022-03-04T23:03:34Z

doc/src/wallet.md

+(9C7626A4CBFFCE894518B8A317F06D051597A378, SequenceNumber(0), o#10dd0b5cabf227952a6e731001d3b57039595225eb188ca6e7ac65bf55ac7c6f)
+(AFA6A58082E961E8706FFF48A0D531C2BED8A94D, SequenceNumber(0), o#2c4b5c7c8be3055287deb4d445c87cf02603d84155d761bcd71f0457d76254ad)
+```
+If you want to view more information about the objects, you can use the `object` command.


Why are we repeating this here when we have it at the beginning of this section ## View objects owned by the account?

Ah, we should centralize the generic flags (-h, --json, -V) at the beginning, before address and then focus upon only the new argument (address, object, etc.).

Clay-Mysten · 2022-03-04T23:07:06Z

doc/src/wallet.md

+```shell
+sui>-$ objects --address 0DC70EA8CA82FE6966900C697B73A235B8E2346F
+```
+The result should look similar to the following, which shows the object in the format of (`object_id`, `sequence_number`, `object_hash`).


Replace "look similar to" with "resemble" throughout

Clay-Mysten · 2022-03-04T23:09:53Z

doc/src/wallet.md

+Type: 0x2::Coin::Coin<0x2::GAS::GAS>
+```
+The result shows some basic information about the object, the owner, 
+version, id, if the object is immutable and the type of the object.


Clay-Mysten · 2022-03-04T23:10:09Z

doc/src/wallet.md

+The result shows some basic information about the object, the owner, 
+version, id, if the object is immutable and the type of the object.
+If you need a deeper look into the object, you can use the `--json`
+flag to view the raw json representation of the object.


Clay-Mysten · 2022-03-04T23:12:44Z

doc/src/wallet.md

+sui>-$ objects --address 830F66EA8EA867DDCA479535BC12CE2852E571F2
+Showing 0 results.
+```
+To add objects to the account, you can invoke a move function (see [Calling Move code](#calling-move-code) for more information), 


Optionally, simply make "invoke a move function" the link, like so:

you can invoke a move function, or you can transfer...

Clay-Mysten · 2022-03-04T23:12:58Z

doc/src/wallet.md

+Showing 0 results.
+```
+To add objects to the account, you can invoke a move function (see [Calling Move code](#calling-move-code) for more information), 
+or you can transfer one of the existing object from the genesis account to the new account. 


one of the existing objects

Clay-Mysten · 2022-03-04T23:13:56Z

doc/src/wallet.md

+        --to <to>                  Recipient address
+```
+To transfer an object to a recipient, you will need the recipient's address, 
+the object id of the object that you want to transfer, 


Clay-Mysten · 2022-03-04T23:14:16Z

doc/src/wallet.md

+```
+To transfer an object to a recipient, you will need the recipient's address, 
+the object id of the object that you want to transfer, 
+and the gas object' id for the transaction fee payment.


gas object's ID

lxfind · 2022-03-04T23:15:24Z

Hi @Clay-Mysten , I think you could use the "Start a review" button to batch review them, instead of adding single comment on each review :)

Clay-Mysten · 2022-03-04T23:15:32Z

doc/src/wallet.md

+the object id of the object that you want to transfer, 
+and the gas object' id for the transaction fee payment.
+
+Here is an example of a transfer of object to account `830F66EA8EA867DDCA479535BC12CE2852E571F2`.


Here is an example transfer of an object to account...

Clay-Mysten · 2022-03-04T23:16:51Z

doc/src/wallet.md

+```
+
+## Merging and splitting coin objects
+Overtime, the account might receive coins from other account and will become unmanageable when 


from other accounts (plural)

Clay-Mysten · 2022-03-04T23:17:19Z

doc/src/wallet.md

+
+## Merging and splitting coin objects
+Overtime, the account might receive coins from other account and will become unmanageable when 
+the number of coins grows; contrarily the account might need to split the coin for payment or 


contrarily,

Clay-Mysten · 2022-03-04T23:17:35Z

doc/src/wallet.md

+
+## Merging and splitting coin objects
+Overtime, the account might receive coins from other account and will become unmanageable when 
+the number of coins grows; contrarily the account might need to split the coin for payment or 


split the coins (plural?)

Clay-Mysten · 2022-03-04T23:18:04Z

doc/src/wallet.md

+## Merging and splitting coin objects
+Overtime, the account might receive coins from other account and will become unmanageable when 
+the number of coins grows; contrarily the account might need to split the coin for payment or 
+for transfer to other account.


to another account.

Clay-Mysten · 2022-03-04T23:18:39Z

doc/src/wallet.md

+the number of coins grows; contrarily the account might need to split the coin for payment or 
+for transfer to other account.
+
+We can use the `merge-coin` command and `split-coin` command to consolidate or split coins.


or split coins, respectively.

Clay-Mysten · 2022-03-04T23:19:49Z

doc/src/wallet.md

+        --gas-budget <gas-budget>          Gas budget for this call
+        --primary-coin <primary-coin>      Coin to merge into, in 20 bytes Hex string
+```
+Here is an example of how to merge coins, to merge coins, you will need at lease three coin objects - 


I would replace this first comma with a period, like so:

coins. To

Clay-Mysten · 2022-03-04T23:21:07Z

doc/src/wallet.md

+    -V, --version    Prints version information
+
+OPTIONS:
+        --amounts <amounts>...       Amount to split out from the coin


Why do we have ellipsis here:

...

this is generated by the code, it means the --amounts options can take multiple values

Clay-Mysten · 2022-03-04T23:22:44Z

doc/src/wallet.md

+Updated Gas : Coin { id: B7FD91A802DAF5523CAAB69FF4652FAFA6FF4ADF, value: 99996 }
+```
+
+### Split coin


Optionally, make plural:

Split coins

Clay-Mysten · 2022-03-04T23:23:38Z

doc/src/wallet.md

+For splitting coins, you will need at lease two coins to execute the `split-coin` command, 
+one coin to split, one for the gas payment.
+
+Here is an example of splitting coin, we are splitting out three new coins from the original coin, 


example of splitting coins (assuming we decided on plural; if not, not worries)

Clay-Mysten · 2022-03-04T23:24:38Z

doc/src/wallet.md

+one coin to split, one for the gas payment.
+
+Here is an example of splitting coin, we are splitting out three new coins from the original coin, 
+with values of 1000, 5000 and 3000 respectively, note that the `--amounts` accepts list of values.


with values of 1000, 5000 and 3000, respectively; note that the --amounts argument accepts a list of values.

This is an excellent point, by the way. I would have been confused seeing the command next without this. Thanks!

Clay-Mysten · 2022-03-04T23:26:52Z

doc/src/wallet.md

+(B7FD91A802DAF5523CAAB69FF4652FAFA6FF4ADF, SequenceNumber(3), o#839e98b33b3de62bc84c36c35b9fd6cdf3383f9d7c4f760c398cbbc7bef8c932)
+(BFA8BAB64ED8F74BA3731C9220FD7462456BC601, SequenceNumber(1), o#6748854128a8b4746fb5bd124eddafc4f9129bae36a8a34af85a8f29b07ee124)
+```
+From the result we can see three new coins got created in the transaction.


coins were created

Clay-Mysten

This is fantastic work, Patrick! An absolutely wonderful addition to our docs.

If you simply want to merge, I will gladly make these non-semantic changes myself. Thanks!

patrickkuo self-assigned this Mar 4, 2022

patrickkuo requested review from lxfind, awelc and oxade March 4, 2022 15:11

patrickkuo force-pushed the pat/wallet_merge_split_coin branch from ce19bef to dc83746 Compare March 4, 2022 19:09

patrickkuo marked this pull request as ready for review March 4, 2022 19:11

patrickkuo requested review from sblackshear and Clay-Mysten March 4, 2022 19:12

Clay-Mysten reviewed Mar 4, 2022

View reviewed changes

lxfind approved these changes Mar 4, 2022

View reviewed changes

Clay-Mysten reviewed Mar 4, 2022

View reviewed changes

Clay-Mysten approved these changes Mar 4, 2022

View reviewed changes

patrickkuo added 3 commits March 4, 2022 23:53

* added merge split coin to wallet

cdb6925

* added docs for merge and split coin

d016a5e

address PR comments

f75efb6

patrickkuo force-pushed the pat/wallet_merge_split_coin branch from dc83746 to f75efb6 Compare March 5, 2022 00:24

patrickkuo merged commit 2b5e4f7 into main Mar 5, 2022

patrickkuo deleted the pat/wallet_merge_split_coin branch March 5, 2022 00:49

patrickkuo linked an issue Mar 7, 2022 that may be closed by this pull request

[wallet] Add merge/split coins command to the wallet #655

Closed

[wallet] Add merge/split coins command to the wallet #657

[wallet] Add merge/split coins command to the wallet #657

Conversation

patrickkuo commented Mar 4, 2022 • edited Loading

patrickkuo commented Mar 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxfind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Clay-Mysten Mar 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxfind commented Mar 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Clay-Mysten left a comment

Choose a reason for hiding this comment

patrickkuo commented Mar 4, 2022 •

edited

Loading

Clay-Mysten Mar 4, 2022 •

edited

Loading