add enable / disable / status methods for z coins #395

smk762 · 2022-09-12T09:40:41Z

closes #391

Enabling
Withdraw
Sidebar entries
Converted to draft until updated method names are merged in API repo.

laruh

Looks good!
I have one note

laruh · 2022-11-10T10:32:36Z

docs/basic-docs/atomicdex-api-20-dev/enable_z_coin_init.md

+| ----------------------------------------- | --------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| ticker                                    | string          | Ticker of coin to activate                                                                                                                                           |
+| activation_params                         | object          | List of [Electrum servers](https://github.com/KomodoPlatform/coins/tree/master/electrums)                                                                            |
+| activation_params.mode.zcash_params_path  | string          | Optional. Path to folder containing [Zcash parameters](https://z.cash/technology/paramgen/). Defaults to standard location as defined in [this guide](https://forum.komodoplatform.com/t/installing-zcash-params/603) |


We should provide a description of zcash_params_path after activation_params.mode, as its optional parameter and its not a part of mode. zcash_params_path is one of activation_params arguments.
You can expand the view till activation_params here
In addition, you can see, how zcoin activation params look in API code

pub struct ZcoinActivationParams { pub mode: ZcoinRpcMode, pub required_confirmations: Option<u64>, pub requires_notarization: Option<bool>, pub zcash_params_path: Option<String>, }

Where ZcoinRpcModecould be Native or Light with electrum_servers and with light_wallet_d_servers.

laruh

Thanks for the fixes!
Next review iteration :)

laruh · 2022-12-29T11:20:47Z

docs/basic-docs/atomicdex-api-20-dev/enable_z_coin_init.md

+## task::enable_z_coin::init
+
+Z coins, like Pirate (ARRR) and the test coin ZOMBIE take a little longer to enable, and use a new two step method to enable. Activation can take a little while the first time, as we need to download some block cache data, and build a wallet database. Subsequent enabling will be faster, but still take a bit longer than other coins. The second step for activation is optional, but allows us to check the status of the activation process.
+<b>To enable Z coins you also need to [install some Zcash Params](https://forum.komodoplatform.com/t/installing-zcash-params/603)</b>


Could you, please, add additional links to alternative fetch-params-alt scripts in your guide?
I had situations that some files from https://z.cash/downloads were unavailable, so I had Failed to fetch the Zcash zkSNARK parameters! error.
You left the link to zcutil with all scripts, but I think some users may not notice the alternative scripts in the list.

updated in https://forum.komodoplatform.com/t/installing-zcash-params/603 👍

laruh · 2022-12-29T12:05:35Z

docs/basic-docs/atomicdex-api-20-dev/enable_z_coin_init.md

+| ..rpc_data.electrum_servers.protocol      | string          | Transport protocol used by AtomicDEX API to connect to the electrum server (`TCP` or `SSL`). Optional, defaults to `TCP`                                             |
+| ..rpc_data.electrum_servers.urls          | string          | Urls which are hosting electrum servers                                                                                                                              |
+| ...electrum_servers.disable_cert_verification | boolean     | If `true`, this disables server SSL/TLS certificate verification (e.g. to use self-signed certificate). <b>Use at your own risk</b>                                  |


We dont have urls in electrum_servers.
We have list of ElectrumRpcRequest with parameters: url(one string), protocol and disable_cert_verification. ElectrumRpcRequest is structure in our code, we dont use its name in curl, we just use arguments of this structure in curly brackets.

If user wants to use one url and put true in disable_cert_verification, list will look like:
\"electrum_servers\": [{\"url\":\"zombie.sirseven.me:10033\", \"disable_cert_verification\":true}],.

Several urls in electrum_servers will look like:
\"electrum_servers\": [{\"url\":\"zombie.sirseven.me:10033\"}, {\"url\":\"zombie.sirseven.me:10034\"}],.

So basically electrum_servers has one parameter, which is a list of {url, protocol, disable_cert_verification}

@artemii235 am I right?

Updated the table, thanks for pointing it out.

smk762 · 2023-01-04T07:11:05Z

closes #391

SirSevenG · 2023-01-04T10:34:16Z

docs/basic-docs/atomicdex-api-20-dev/enable_z_coin_status.md

+    \"method\": \"task::enable_z_coin::status\",
+    \"mmrpc\": \"2.0\",
+    \"params\": {
+        \"task_id\": $1


"task_id": $1,
Missing comma here and extra trailing one below

thanks,fixed.

laruh

Thanks for the fixes!
Next review iteration. I have couple notes and one question :)

laruh · 2023-01-06T06:44:30Z

docs/basic-docs/atomicdex-api-20-dev/enable_z_coin_init.md

+| ..rpc_data.electrum_servers                   | list of objects | Contains additional details about a coins electrum servers.                                                                                                          |
+| ...electrum_servers.protocol                  | string          | Transport protocol used by AtomicDEX API to connect to the electrum server (`TCP` or `SSL`). Optional, defaults to `TCP`                                             |
+| ...electrum_servers.url                       | string          | The URL and port of an electrum server                                                                                                                               |
+| ...electrum_servers.disable_cert_verification | boolean         | If `true`, this disables server SSL/TLS certificate verification (e.g. to use self-signed certificate). <b>Use at your own risk</b>                                  |


Please, add note for disable_cert_verification: Optional, defaults to false
:)

laruh · 2023-01-06T09:18:34Z

docs/basic-docs/atomicdex-api-20-dev/withdraw_init.md

@@ -0,0 +1,63 @@
+# withdraw_init
+
+The `withdraw_init` method generates, signs, and returns a transaction that transfers the `amount` of `coin` to the address indicated in the `to` argument. It is only used for z coins like ZOMBIE, which may take some time to complete. The status of this method can be queried via the [withdraw_status](withdraw_status.html) method, or cancelled with [withdraw_status](withdraw_cancel.html).


typo cancelled with [withdraw_status](withdraw_cancel.html). - I think this should be cancelled with [withdraw_cancel](withdraw_cancel.html).

Could you tell me am I right or wrong? Right now links for withdraw_status and withdraw_cancel are invalid. They lead to our github docs project pages. But when this branch will be merged into master, links with withdraw_status.html and withdraw_cancel.html will exist, so everything will be fine.

It is only used for z coins like ZOMBIE - as I can see in our code the method task::withdraw::init is used not only for ZCoin, but also for UtxoCoin, QtumCoin. In the case of other coins there will be an error CoinDoesntSupportInitWithdraw

Thanks - I've reflowed these docs into two separate docs with subsections, and the sidebar accordingly

Any z coin specific text was removed from the withdraw tasks doc, with the z coin doc noting that the withdraw tasks are required for those coins and a link to the withdraw task doc.

The links will work once deployed (you can build & test locally to confirm. They don't work in github because the md / html extension change when built.

laruh · 2023-01-06T09:40:42Z

docs/basic-docs/atomicdex-api-20-dev/withdraw_init.md

+| amount        | string (numeric) | the amount the user desires to withdraw, ignored when `max=true`                                                                          |
+| max           | bool             | withdraw the maximum available amount                                                                                                     |
+| fee.type      | string           | type of transaction fee; possible values: `UtxoFixed` or `UtxoPerKbyte`                                                                   |
+| fee.amount    | string (numeric) | fee amount in coin units, used only when type is `UtxoFixed` (fixed amount not depending on tx size) or `UtxoPerKbyte` (amount per Kbyte) |


We also have arguments memo (optional and should be used carefully, as Artem suggested), and from (optional). As I can see, we use from for WithdrawRequest in trezor integration, in the case of Iguana there will be an error 'from' is not supported if the coin is initialized with an Iguana private key

Ain't 100% sure, but memo sounds like tendermint-only thing here.

Ain't 100% sure, but memo sounds like tendermint-only thing here.

We merged the PR related to memo use in ARRR withdraw KomodoPlatform/komodo-defi-framework#1574

Memo tested with ARR successfully (tho not seen in tx_history output as "error_type":"NotSupportedFor","error_data":"ARRR")
Added to the doc along with example to satisfy KomodoPlatform/komodo-defi-framework#1589 (comment)

tho not seen in tx_history output as "error_type":"NotSupportedFor","error_data":"ARRR"

Yep, thats correct, my_tx_history_v2_rpc now works only for Bch, SlpToken, UtxoCoin, QtumCoin, Tendermint and TendermintToken

laruh · 2023-01-09T07:19:15Z

docs/basic-docs/atomicdex-api-20-dev/withdraw_tasks.md

+| amount        | string (numeric) | the amount the user desires to withdraw, ignored when `max=true`                                                                          |
+| memo          | string           | Optional, used for ZHTLC and Tendermint coins only. Attaches a memo to the transaction.                                                   |
+| from          | string           | Optional, used only for transactions using a hardware wallet. For more information, see the [Trezor Integration guide](trezor_integration.html) |
+| max           | bool             | withdraw the maximum available amount                                                                                                     |


Please, add Optional, defaults to false for max.

laruh · 2023-01-09T07:20:38Z

docs/basic-docs/atomicdex-api-20-dev/withdraw_tasks.md

+| memo          | string           | Optional, used for ZHTLC and Tendermint coins only. Attaches a memo to the transaction.                                                   |
+| from          | string           | Optional, used only for transactions using a hardware wallet. For more information, see the [Trezor Integration guide](trezor_integration.html) |
+| max           | bool             | withdraw the maximum available amount                                                                                                     |
+| fee.type      | string           | type of transaction fee; possible values: `UtxoFixed` or `UtxoPerKbyte`                                                                   |


fee param is Optional

laruh · 2023-01-09T08:08:02Z

docs/basic-docs/atomicdex-api-20-dev/withdraw_init.md

+| amount        | string (numeric) | the amount the user desires to withdraw, ignored when `max=true`                                                                          |
+| max           | bool             | withdraw the maximum available amount                                                                                                     |
+| fee.type      | string           | type of transaction fee; possible values: `UtxoFixed` or `UtxoPerKbyte`                                                                   |
+| fee.amount    | string (numeric) | fee amount in coin units, used only when type is `UtxoFixed` (fixed amount not depending on tx size) or `UtxoPerKbyte` (amount per Kbyte) |


tho not seen in tx_history output as "error_type":"NotSupportedFor","error_data":"ARRR"

Yep, thats correct, my_tx_history_v2_rpc now works only for Bch, SlpToken, UtxoCoin, QtumCoin, Tendermint and TendermintToken

laruh

Excellent!
Couple more non critical notes

upd: I see everything is done :D

docs/basic-docs/atomicdex-api-20/my_tx_history.md

SirSevenG

lgtm

laruh

Awesome! 🔥

add enable / disable / status methods for z coins

cfa4f14

smk762 requested review from gcharang and sergeyboyko0791 September 12, 2022 09:40

smk762 marked this pull request as draft September 12, 2022 09:40

add withdraw docs, sidebar entries

92630e3

smk762 added the awaiting API label Nov 1, 2022

add zcash_params_path

f932e80

smk762 marked this pull request as ready for review November 10, 2022 09:54

smk762 removed the awaiting API label Nov 10, 2022

laruh reviewed Nov 10, 2022

View reviewed changes

tweak init params

e882267

smk762 assigned gcharang and smk762 Dec 2, 2022

smk762 added 2 commits December 6, 2022 14:37

rm gas

6fffb38

add conf/nota to init

e2e0b69

smk762 requested review from laruh, tonymorony and SirSevenG and removed request for laruh December 6, 2022 06:49

smk762 requested a review from laruh December 13, 2022 08:32

add forget_if_false param

308a746

laruh suggested changes Dec 29, 2022

View reviewed changes

smk762 and others added 2 commits January 4, 2023 15:12

Merge branch 'master' into zhtlc_docs

3ffd98a

update params table

5d67712

smk762 requested review from laruh and Canialon and removed request for laruh January 4, 2023 07:43

smk762 requested a review from laruh January 4, 2023 07:44

SirSevenG reviewed Jan 4, 2023

View reviewed changes

fix comma

550fd97

smk762 requested review from SirSevenG and laruh and removed request for laruh January 4, 2023 10:37

laruh suggested changes Jan 6, 2023

View reviewed changes

smk762 added 2 commits January 6, 2023 21:12

separate withdraw and z coin docs

8d6dbb7

add from and memo info

e3315d4

smk762 requested a review from laruh January 7, 2023 07:54

smk762 mentioned this pull request Jan 7, 2023

Document Trezor Methods #403

Merged

14 tasks

fix comma in withdraw_status

2e09f64

laruh reviewed Jan 9, 2023

View reviewed changes

add notes about optional params, update tx_history_v2

62a2c9c

smk762 requested a review from laruh January 9, 2023 08:27

laruh reviewed Jan 9, 2023

View reviewed changes

docs/basic-docs/atomicdex-api-20/my_tx_history.md Outdated Show resolved Hide resolved

update text about tx_history support

0772d6c

smk762 requested a review from laruh January 9, 2023 11:07

add history method

03d6ffd

SirSevenG approved these changes Jan 10, 2023

View reviewed changes

add note about memos in tx_history

abd1e4c

laruh approved these changes Jan 10, 2023

View reviewed changes

smk762 merged commit 1ab1544 into master Jan 10, 2023

smk762 deleted the zhtlc_docs branch January 10, 2023 11:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add enable / disable / status methods for z coins #395

add enable / disable / status methods for z coins #395

smk762 commented Sep 12, 2022 •

edited

Loading

laruh left a comment

laruh Nov 10, 2022

laruh left a comment

laruh Dec 29, 2022

smk762 Jan 4, 2023

laruh Dec 29, 2022 •

edited

Loading

smk762 Jan 4, 2023

smk762 commented Jan 4, 2023

SirSevenG Jan 4, 2023

smk762 Jan 4, 2023

laruh left a comment

laruh Jan 6, 2023

smk762 Jan 6, 2023

laruh Jan 6, 2023 •

edited

Loading

smk762 Jan 6, 2023

laruh Jan 6, 2023

SirSevenG Jan 6, 2023

laruh Jan 6, 2023

smk762 Jan 6, 2023

laruh Jan 9, 2023

laruh Jan 9, 2023 •

edited

Loading

smk762 Jan 9, 2023

laruh Jan 9, 2023

smk762 Jan 9, 2023

laruh Jan 9, 2023

laruh left a comment •

edited

Loading

SirSevenG left a comment

laruh left a comment

		@@ -0,0 +1,63 @@
		# withdraw_init

		The `withdraw_init` method generates, signs, and returns a transaction that transfers the `amount` of `coin` to the address indicated in the `to` argument. It is only used for z coins like ZOMBIE, which may take some time to complete. The status of this method can be queried via the [withdraw_status](withdraw_status.html) method, or cancelled with [withdraw_status](withdraw_cancel.html).

add enable / disable / status methods for z coins #395

add enable / disable / status methods for z coins #395

Conversation

smk762 commented Sep 12, 2022 • edited Loading

laruh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laruh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laruh Dec 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smk762 commented Jan 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laruh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laruh Jan 6, 2023 • 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

laruh Jan 9, 2023 • 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

laruh left a comment • edited Loading

Choose a reason for hiding this comment

SirSevenG left a comment

Choose a reason for hiding this comment

laruh left a comment

Choose a reason for hiding this comment

smk762 commented Sep 12, 2022 •

edited

Loading

laruh Dec 29, 2022 •

edited

Loading

laruh Jan 6, 2023 •

edited

Loading

laruh Jan 9, 2023 •

edited

Loading

laruh left a comment •

edited

Loading