komodo-wallet-desktop
komodo-wallet-desktop copied to clipboard
Pirate (ARRR) integration
Closes #1188
- [x] Enable ZHTLC coins
- [x] Disable ZHTLC coins
- [x] Withdraw ZHTLC coins
- [x] Swap ZHTLC coins
- [x] Update Bestorders to v2 RPC
- [x] Update Orderbook to v2 RPC
TODO:
- [ ] enabling and withdrawing are a 2 part process. at the moment there is a loop while awaiting step one to be ready for step 2 which exits after 50 tries. If error or success, operation will continue, but if operation not "ready" we might have to check on it gain later (e.g. additional loop at step-off frequency) or force cancel the operation.
- [x] Implement transaction history for ZHTLC coins. Uses
z_coin_tx_historyv2 RPC
This PR is currently using non-standard mm2 and coins files for testing. This mm2 will not work on Ubuntu 18.04 due to GCLIB version
ZOMBIE and ARRR are marked as "wallet only" but you can remove that manually in local coins.json file to allow for swapping. ARRR electrums are down at the moment, but I can supply some ZOMBIE (or put up a trade for you) on request for testing.
PoC steps for delayed actions:
- [ ] when action initiates, push notification
{method} initiated. task id: {task_id} - [ ] when action status changes to "Ready" or "failed", push notification with result
- [ ] clicking on notification before "Ready" shows modal with current status message (e.g. sync pct enabling)
Will work towards applying the above to init_z_coin / inti_zcoin_status and init_withdraw / withdraw_status methods - @SylEze @tonymorony if you have other ideas for how to better communicate this in GUI, please let me know.
@smk762 could you please sync this branch with dev?
Right now it's not building since mm2 artefact linked in it not available anymore
for me nothing works atm @smk762 so even not sure from where to start. could you please check on your side?
For activation while waiting:
- [ ] Remove from enable coins menu
- [ ] Display in portfolio and wallet page (along with enable progress)
- [ ] Notification?
for me nothing works atm @smk762 so even not sure from where to start. could you please check on your side?
There appears to be an issue with the "get_balance" response parsing for ZHTLC coins so address/balance is not displaying correctly. Debugging now.
for me nothing works atm @smk762 so even not sure from where to start. could you please check on your side?
There appears to be an issue with the "get_balance" response parsing for ZHTLC coins so address/balance is not displaying correctly. Debugging now.
Issue is related to a misconfiguration, not sure if coins file or lightwallet_d server yet. Awaiting feedback from API team.
ZOMBIE enabling takes around 3 minutes (varies based on history tho), see: https://github.com/KomodoPlatform/atomicDEX-Desktop/issues/1188#issuecomment-1144422272
Most of this time is the BuildingWalletDb state, which offers no progress feedback, so we need a "please wait" message for gui users. Something like the swap progress modal might be one way to track enabling.
Proposed flow:
- user selects to enable HTLC coin.
- coin enters enabling state, no longer visible on coin activation menu.
- coin entry appears in portfolio (greyscale?) with "enabling" tag. Clicking on this coin's row opens the "Enabling progress" modal
- On wallet page, coin is visible in sidebar menu, along with "hourglass" icon. Clicking on it in the sidebar opens the "Enabling progress" modal
Progress Steps:
- ActivatingCoin (5 sec)
- UpdatingBlocksCache (first time depends on block height, subsequent runs faster if recently sync'd. rpc data returns values to derive % sync)
- BuildingWalletDb (minutes/hours. No indication of progress yet available).
- RequestingWalletBalance (5 sec)
- Ready (final step, exposes address / balance)
Currently ZHTLC enabling progress is wedged in among standard enabling, with a loop to request task_id status 500 times (50 was not enough!). There is potential case where this loop exits before enabling is complete (the coin will enable anyway later if no error). This loop might be better as a service, which will need to keep track of incomplete task_ids and the operation they represent (which coin, which method). A task_id tracking service could be useful for other methods later also.
This could be overkill for now though - once reaching the BuildingWalletDb there is not really any relevant RPC response info until Ready. A Ready state can be assumed once balance/address are available without needing to check the task_id status (which will return NoSuchTask a second after Ready.
Issue is related to a misconfiguration, not sure if coins file or lightwallet_d server yet. Awaiting feedback from API team.
ZOMBIE enabling takes around 3 minutes (varies based on history tho), see: https://github.com/KomodoPlatform/atomicDEX-Desktop/issues/1188#issuecomment-1144422272
I've found a problem: Sapling activation height is ignored when blocks scanning starts, so MM2 attempts to build the cache from 0 block, which is not actually supported. Fixing it now.
Also, as PIRATE is quite a big chain already, its activation time takes even more than one hour, also consuming around 2Gb of hard drive space. Since all ARRR addresses in AtomicDEX will be actually newly generated, we can choose a checkpoint block and start scanning only from it (e.g., 2000000). But if we plan to support import of already existing and long-time used addresses, users will have to wait for this scanning process to complete anyway (and it will take even longer and longer as the chain advances).
So requesting operation status only 500 times is surely not enough. This process should run until Ready status is received. I've also discovered that I can provide more info on BuildingWalletDb status about the progress. Will do it this week.
@smk762 @tonymorony
@artemii235 could "scan_from_height" be a user configurable param? This way we could use a default "scan from" block, with option for user to override this in their coins file if needed for more mature addresses.
could "scan_from_height" be a user configurable param? This way we could use a default "scan from" block, with option for user to override this in their coins file if needed for more mature addresses.
@smk762 Yes, but we will need this to be implemented from komodod side: https://github.com/KomodoPlatform/komodo/issues/558.
500 loop was with 1 second sleep to allow closer log monitoring during testing, but I can increase the sleep to "step off" during the BuildingWalletDb stage so it is still monitored enough for feedback but without moving the loop counter up to quickly. Would 3 hours be a good upper limit on how long at max enabling would be expected to take?
Issue is related to a misconfiguration, not sure if coins file or lightwallet_d server yet. Awaiting feedback from API team.
ZOMBIE enabling takes around 3 minutes (varies based on history tho), see: #1188 (comment)
I've found a problem: Sapling activation height is ignored when blocks scanning starts, so MM2 attempts to build the cache from 0 block, which is not actually supported. Fixing it now.
Also, as PIRATE is quite a big chain already, its activation time takes even more than one hour, also consuming around 2Gb of hard drive space. Since all ARRR addresses in AtomicDEX will be actually newly generated, we can choose a checkpoint block and start scanning only from it (e.g., 2000000). But if we plan to support import of already existing and long-time used addresses, users will have to wait for this scanning process to complete anyway (and it will take even longer and longer as the chain advances).
So requesting operation status only 500 times is surely not enough. This process should run until
Readystatus is received. I've also discovered that I can provide more info onBuildingWalletDbstatus about the progress. Will do it this week.@smk762 @tonymorony
Thanks for the info!
Thats good idea with checkpoint. Since chain is moving I do propose not fixed height so but delta from current height e.g. 777777 blocks to the past (depends on how fast this process is to reach some satisfying UX).
UPD: just read messages above about addition into komodod
And to cover other cases add configurable scan delta param to change. Also, maybe on GUI side we'll also need to detect if user created new seed and activating ARRR or he imported the seed and activating ARRR and treat these cases differently
Would 3 hours be a good upper limit on how long at max enabling would be expected to take?
I'm afraid that in some cases even 3 hours might be not enough for full sync :slightly_smiling_face: It takes more than one hour on decent hardware and internet connection, hard to predict how long it will take on mobile devices.
unattached reviewers for now @smk762 please re-request review when it'll be possible to send/receive/trade ARRR and see tx history (even if it'll look not fancy / POC)
Since chain is moving I do propose not fixed height so but delta from current height e.g. 777777 blocks to the past (depends on how fast this process is to reach some satisfying UX).
@tonymorony
If we release at e.g. 2000000 block, at some point current - delta will be ahead of 2000000 and some users might stop seeing correct balance if they HODL for all this time. Also, 777777 is too large, wallet DB generation takes like 8-9 seconds per 1000 blocks. I have to wait for over 15 minutes even with 1900000 block as checkpoint (and I have no transactions on the address).
@smk762 I have added check point feature and more info to BuildingWalletDb status, please try adding the following to the PIRATE conf: https://github.com/KomodoPlatform/atomicDEX-API/blob/626624c96c5469fabc7cd6edf49b131aa1cd9148/mm2src/mm2_test_helpers/src/for_tests.rs#L216
BuildingWalletDbStatus status is changed to:
{
"mmrpc":"2.0",
"result":{
"status":"InProgress",
"details":{
"BuildingWalletDb":{
"current_scanned_block":1000,
"latest_block":215326
}
}
},
"id":null
}
I have to wait for over 15 minutes even with 1900000 block as checkpoint (and I have no transactions on the address).
I will also check if this can be optimized.
The code is in the https://github.com/KomodoPlatform/atomicDEX-API/tree/arrr-activation-hotfix branch. CI will publish binaries to http://195.201.0.6/arrr-activation-hotfix/ soon.
UPD: binaries uploaded.
Thanks @artemii235 I can enable ARRR now. Will test it further this weekend.
@SylEze here is a visual indication of progress (and what's missing)
https://user-images.githubusercontent.com/35845239/183118936-21104473-ad8c-4eb0-a446-9b6074243a66.mp4
- ZHTLC Coins can be enabled, but it takes a while.
- Activation progress is tracked in logs, but we need to do it for front end too (e.g. hourglass next to coin in portfolio, click on to get progressbar / event).
- Currently while activating, coin stays on enable menu and doesn't appear in portfolio. This is governed by the boolean
m_coins_informations[tickers[idx]].currently_enabled. We need to add a third state which covers "enabling in progress" so enable menu and portfolio display is sane. - Transaction history display is implemented, though I have seen some errors on one of my testing wallets with a bit more history
error: Response(zombie.sirseven.me:10033, Object({\"code\": Number(-32600), \"message\": String(\"response too large (over 1,000,000 bytes\")}))so we need to handle this.
https://user-images.githubusercontent.com/35845239/183121329-3f332af0-2786-4234-ae55-39be28079cfd.mp4


- Sending funds is implemented.
- Swaps should be working, but I have not re-confirmed since re-aligning code with dev. I need to source some ARRR, but I can send you some ZOMBIE for testing if needed, just DM me an address.
@tonymorony The current coin icon image for zombie is just a random placeholder I found online somewhere. Can our artists create something? Or could we have a community competition / bounty for creating / selecting the image?
I've imported and merged in @Milerius 's SLP code and enabling is now functional - but is failing to enable tokens if BCH/tBCH is not already enabled. So we need to handle the same way as other platform parent / token enabling logic.
For testing, here is a working faucet: https://tbch.googol.cash/ or from a telegram bot at https://t.me/tbch_bot SLP token explorer: https://simpleledger.info/
Transaction history display is implemented, though I have seen some errors on one of my testing wallets with a bit more history error: Response(zombie.sirseven.me:10033, Object({"code": Number(-32600), "message": String("response too large (over 1,000,000 bytes")})) so we need to handle this.
I think the easiest workaround is to increase it in Electrum's config @tonymorony @SirSevenG
But since z_coin_tx_history requests transaction data from electrums in batch, certain response size will be surely reached at the certain limit. However, I wouldn't recommend to use big limit for any of calls in general. It can allocate a lot of RAM, which might not be freed for entire MM2 execution.
fwiw, this only happened on my test wallet with lots of tx - reduced limit to 50 in gui code and it is ok now (though other coins seem to handle more). Max I got returned without error was around 90.
ARRR activation fix, TemporaryError RPC status and check point block feature is released https://github.com/KomodoPlatform/atomicDEX-API/releases/tag/beta-2.1.7404 @smk762 @tonymorony
Wow, great progress @smk762 !
Noticed minor issue:
Opening such link:
https://pirate.explorer.dexstats.info/address/Invalid%20Ticker
It would be great to not show this link for pirate
There seems to be no option to generate/get zcash params in app. So trying to activate coins leads to error:
{
"id": null,
"mmrpc": "2.0",
"result": {
"details": {
"error": "Error on platform coin ZOMBIE creation: ZCashParamsNotFound",
"error_data": {
"error": "ZCashParamsNotFound",
"ticker": "ZOMBIE"
},
"error_path": "lib.z_coin_activation.z_coin",
"error_trace": "lib:93] z_coin_activation:191] z_coin:781]",
"error_type": "CoinCreationError"
},
"status": "Ready"
}
}
I don't see most users using Adex-Desktop on the same device running full node, so that will be a big issue.
Nice catch @SirSevenG :) Most of the users never setup a full node so it's a blocker and we should detect this error and handle for the user params downloading (prob we can just execute https://github.com/KomodoPlatform/komodo/blob/master/zcutil/fetch-params-alt.bat and https://github.com/KomodoPlatform/komodo/blob/master/zcutil/fetch-params-alt.sh)
Zombie page has a faucet button thinking
I've setup a couple of Zombie nodes to send faucet transactions from, and modified the faucet code to handle ZOMBIE with the alternative z_ methods for sending and address validation.
Tested myself and should be working. I can't check the balances of requesting addresses tho, so individual rate limiting is more difficult. Fortunately I can mine 100k zombie overnight :laughing:
There are 2 issues with zparams download i think should be addressed:
Partial downloading support / downloading resume is not trivial thing and overkill for this task imo
I've since confirmed with Artem also that the sprout params are not needed, which significantly reduces download time.
There are 2 issues with zparams download i think should be addressed:
- Continue download if interrupted
This was not implemented (ref Tony's comment), but I've removed the sprout params from download so it's really quick.
- try to activate any of Z-coins
- See zparams not found error
- Nothing happens
- try to activate any of Z-coins again
- Popup for downloading zparams opens
- Start download
It's quite confusing for the user. )
I think what happened here might be a click outside the modal as it was loading, so it auto-closed. I've added NoAutoClose policy to the modal, so now it will only close if use clicks the close button, or once the download completes.
On completion of downloads, modal automatically closes, and previously selected ZHTLC coins start the activation process again.
