Use rounded derived height sync param #2364

smk762 · 2023-09-20T08:26:08Z

This PR is a further improvement on Z coin activation. Previously we used the "date" sync param, which would be effectively changed in the API to a derived height based on the average block time.
As blocktime varies, this potentially resulted in the derived height not being a constant value, and in cases where it did not return the same height as calculated during a prior sync, could force a long resync.
The simplest mitigation for this was to use the height param in the RPC instead, derived by the GUI and rounding the height to the nearest 1000 so that the same date selection would be much more likely to return the same value as in the past.

To Test:

Use the debug build of the app from CI to see the relevant log entries
Activate ARRR with a date set last month. Check the logs to find "requested":2448000 value in logs on lines like below:

[14:55:02] [info] [mm2.service.cpp:1495] [2605136]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":{"UpdatingBlocksCache":{"current_scanned_block":2521275,"first_sync_block":{"actual":2448000,"is_pre_sapling":false,"requested":2448000},"latest_block":2575728}},"status":"InProgress"}}

the requested value should be divisible by 1000.

activate ARRR again, using same date. You should see the same value for requested.

Note: This relies on updated data in coins_config.json, so please delete your local %appdata%/0.6.1 folder and the coins file used for your wallet before launch

borngraced · 2023-09-20T08:34:31Z

Great! Thanks for the quick resolution @smk762

src/core/atomicdex/pages/qt.settings.page.cpp

atomic_defi_design/Dex/Constants/General.qml

src/core/atomicdex/api/mm2/rpc2.task.enable_z_coin.init.hpp

kivqa · 2023-09-20T12:05:08Z

@smk762 I did installation from 0 (with removing old app data). But for some reason Asset list is not displaying:

In case click on DEX button app crashed.

Logs file contains only one exception:
[json.exception.parse_error.101] parse error at line 44053, column 3: syntax error while parsing object key - unexpected number literal; expected string literal

14:59:35] [info] [mm2.service.cpp:205] [43926]: Retrieving Wallet information of /Users/ivankiseleiv/Library/Application Support/Komodo Wallet/config/0.6.1-coins.imp2.json
[14:59:35] [error] [mm2.service.cpp:232] [43926]: exception caught: [json.exception.parse_error.101] parse error at line 44053, column 3: syntax error while parsing object key - unexpected number literal; expected string literal
[14:59:35] [debug] [mm2.service.cpp:235] [43926]: Coins file does not exist!
[14:59:35] [debug] [mm2.service.cpp:235] [43926]: Coins file does not exist!
[14:59:35] [info] [qt.global.coins.cfg.model.cpp:213] [43926]: Initializing global coin cfg model with size 1
[14:59:35] [info] [global.provider.cpp:418] [43926]: Forcing update providers
[14:59:35] [info] [qt.addressbook.page.cpp:66] [43926]: post_login: filling addressbook from cfg
[14:59:35] [warning] [main.prerequisites.hpp:93] [43926]: QIODevice::read (QFile, "/Users/ivankiseleiv/Library/Application Support/Komodo Wallet/addressbook/imp2"): device not open
[14:59:35] [warning] [addressbook.cfg.cpp:48] [43926]: Addressbook config file was invalid, use empty configuration: [json.exception.parse_error.101] parse error at line 1, column 1: syntax error while parsing value - unexpected end of input; expected '[', '{', or a literal. Content was:

kivqa · 2023-09-20T12:25:43Z

@smk762 Problem releated to issue in this PR: KomodoPlatform/coins#821
I fixed it locally and do retest for ARRR activation

cipig · 2023-09-20T13:15:01Z

src/core/atomicdex/api/mm2/rpc2.task.enable_z_coin.init.cpp

@@ -28,7 +28,7 @@ namespace atomic_dex::mm2
    {
        j["params"]["ticker"]                                                          = request.coin_name;
        j["params"]["activation_params"]["mode"]["rpc"]                                = "Light";
-        j["params"]["activation_params"]["mode"]["rpc_data"]["sync_params"]["date"]    = request.sync_date;
+        j["params"]["activation_params"]["mode"]["rpc_data"]["sync_params"]["height"]  = request.sync_height;


afaik, as soon as sync params are present, mm2 will start syncing from scratch from the specified date/height, mo matter what it's in the DB

only if height != db_height
see https://github.com/KomodoPlatform/komodo-defi-framework/pull/1922/files#diff-878c719c6e82f1f4290cc74719c8a7de8e3ef2d5927ebbee5925e3dd54655cfcR381-R382 and https://github.com/KomodoPlatform/komodo-defi-framework/pull/1922/files#diff-878c719c6e82f1f4290cc74719c8a7de8e3ef2d5927ebbee5925e3dd54655cfcR487

@borngraced can you please confirm?

yes only if derived starting_height != to the minimum height in db

can't follow the links to the code, it's not jumping to the specified line, so i will just trust you :-)
ok, so we rely now on some height calculation based on a 60s blocktime which can vary a lot on ARRR
does it mean there is still a chance that my app will start syncing from scratch if the calc is off eg because blocktime was 120s in the last 2 weeks? is there a way to avoid this 100%? it takes 2 days to sync from scratch and i want to avoid this at all costs, so i rather prefer to remove sync_params completely from my repo then to risk syncing from scratch

ok, guess if i have the date set at 2 days and the DB is 2 years old, it should never happen... just need to think to reset the date back to 2 days ago after a complete resync from 2 years ago

https://github.com/KomodoPlatform/komodo-wallet-desktop/pull/2367 has been prepared for the next release (0.6.2) after current API dev branch is merged.

so we rely now on some height calculation based on a 60s blocktime

The calculated avg block time between the checkpoint block (19000000) and the current height was a little over 63 seconds. In the GUI side date to height calc, I've assumed a 65 sec avg blocktime (to avoid recent dates potentially returning a block_height > tip) calculated as an offset from the checkpoint block (middle out reduces the cumulative overage compared to edges in).

This calc will generally return a height for a date which errors on the side of being earlier than actual, which is better than later than actual, for example if user knows the date of a tx and syncs from there but the calc sets it ahead a day, the sync excludes the tx. If set earlier, it is sure to be included.

The real problem though that this PR aims to fix is the potential for the date to height calc to return a different value when run again later, due to the rate of block production between the first and second runs. This changed block height may force a rescan even if selected date remains unchanged, and the odds this will happen increases as the time between first and second run increases. By rounding the result to the nearest 1000, it is much less likely for this to occur, and in a large majority of cases, the returned height will be the same so that no resync is triggered and the sync will continue from the prior cache height.

If we get reports that rounding to 1000 still results in cases of forced resync, we can change the rounding factor to a higher value, though a value of 1000 should be enough to overcome most periods of excessively high or low block production unless they are uncharacteristically extended before reverting back to the mean.

kivqa · 2023-09-20T13:15:57Z

Test 1: Activation ARRR coin with default Activation date - 2 days in the past
During activation of ARRR coin I observed such logs:
ActivatingCoin stage:

[15:24:32] [info] [mm2.service.cpp:1479] [76356]: ARRR enable_z_coin Task ID: 0
[15:24:32] [info] [mm2.service.cpp:1495] [76356]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":"ActivatingCoin","status":"InProgress"}}
[15:24:32] [info] [mm2.service.cpp:1555] [76356]: Waiting for ARRR to enable [InProgress: ActivatingCoin]...

UpdatingBlocksCache stage:

[15:24:42] [info] [mm2.service.cpp:1495] [76356]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":{"UpdatingBlocksCache":{"current_scanned_block":2556581,"first_sync_block":{"actual":2554000,"is_pre_sapling":false,"requested":2554000},"latest_block":2576044}},"status":"InProgress"}}

BuildingWalletDb stage:

[15:25:07] [info] [mm2.service.cpp:1495] [76356]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":{"BuildingWalletDb":{"current_scanned_block":2554000,"first_sync_block":{"actual":2554000,"is_pre_sapling":false,"requested":2554000},"latest_block":2576044}},"status":"InProgress"}}

RequestingWalletBalance stage:

[15:28:23] [info] [mm2.service.cpp:1495] [76356]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":"RequestingWalletBalance","status":"InProgress"}}
[15:28:28] [info] [mm2.service.cpp:1495] [76356]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":{"current_block":2576046,"first_sync_block":{"actual":2554000,"is_pre_sapling":false,"requested":2554000},"ticker":"ARRR","wallet_balance":{"address":"zs1gn3mxk493yyvyu8ffsgj6kt3luajdezwr6t7lzzfjf34u4nzztze3r44wplm0ry95tp4uang9yg","balance":{"spendable":"0.998969","unspendable":"0"},"wallet_type":"Iguana"}},"status":"Ok"}}

After app restart ARRR coin activation takes about 1 minute. Activation started from RequestingWalletBalance stage:

 [15:48:34] [info] [mm2.service.cpp:1495] [104360]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":"RequestingWalletBalance","status":"InProgress"}}
[15:48:34] [info] [app.cpp:808] [104244]: Application inactive
[15:48:39] [info] [mm2.service.cpp:1495] [104360]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":{"current_block":2576090,"first_sync_block":{"actual":2554000,"is_pre_sapling":false,"requested":2554000},"ticker":"ARRR","wallet_balance":{"address":"zs1gn3mxk493yyvyu8ffsgj6kt3luajdezwr6t7lzzfjf34u4nzztze3r44wplm0ry95tp4uang9yg","balance":{"spendable":"0.998969","unspendable":"0"},"wallet_type":"Iguana"}},"status":"Ok"}}

kivqa · 2023-09-20T14:00:55Z

Test 2: Activate ARRR using exact Activation date. I selected 8/30/2023
ARRR coin was successfully activated. Balance and transactions are correct.
Logs example:

[16:54:26] [info] [mm2.service.cpp:1495] [172348]: ARRR Activation Status: {"id":null,"mmrpc":"2.0","result":{"details":{"BuildingWalletDb":{"current_scanned_block":2573000,"first_sync_block":{"actual":2528000,"is_pre_sapling":false,"requested":2528000},"latest_block":2576128}},"status":"InProgress"}}

After app restart ARRR activation takes less 1 minute.

NOTE: There is incorrect percentage calculation, probably it's related to rounded height

kivqa

Approved.
ARRR coin activation works. On app restart ARRR activation takes less 1 minute.
Percentage displayed correctly. 95% is max value that displayed.

Use rounded derived height sync param

b91b8bf

smk762 requested review from cipig, borngraced and kivqa September 20, 2023 08:26

borngraced suggested changes Sep 20, 2023

View reviewed changes

src/core/atomicdex/pages/qt.settings.page.cpp Show resolved Hide resolved

atomic_defi_design/Dex/Constants/General.qml Outdated Show resolved Hide resolved

src/core/atomicdex/api/mm2/rpc2.task.enable_z_coin.init.hpp Show resolved Hide resolved

cipig reviewed Sep 20, 2023

View reviewed changes

smk762 and others added 2 commits September 20, 2023 22:23

avoid > 100%

41ef0c2

its not python mate

771fe83

kivqa approved these changes Sep 20, 2023

View reviewed changes

borngraced approved these changes Sep 21, 2023

View reviewed changes

smk762 merged commit 30b4fbe into dev Sep 21, 2023

smk762 mentioned this pull request Dec 20, 2023

0.7.0 Release #2396

Merged

ca333 deleted the z-sync-rounded branch October 23, 2024 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use rounded derived height sync param #2364

Use rounded derived height sync param #2364

smk762 commented Sep 20, 2023 •

edited

Loading

borngraced commented Sep 20, 2023

kivqa commented Sep 20, 2023

kivqa commented Sep 20, 2023

cipig Sep 20, 2023

smk762 Sep 20, 2023

borngraced Sep 20, 2023

cipig Sep 20, 2023

cipig Sep 20, 2023

borngraced Sep 20, 2023

smk762 Sep 21, 2023 •

edited

Loading

smk762 Sep 21, 2023

kivqa commented Sep 20, 2023

kivqa commented Sep 20, 2023 •

edited

Loading

kivqa left a comment

Use rounded derived height sync param #2364

Use rounded derived height sync param #2364

Conversation

smk762 commented Sep 20, 2023 • edited Loading

borngraced commented Sep 20, 2023

kivqa commented Sep 20, 2023

kivqa commented Sep 20, 2023

cipig Sep 20, 2023

Choose a reason for hiding this comment

smk762 Sep 20, 2023

Choose a reason for hiding this comment

borngraced Sep 20, 2023

Choose a reason for hiding this comment

cipig Sep 20, 2023

Choose a reason for hiding this comment

cipig Sep 20, 2023

Choose a reason for hiding this comment

borngraced Sep 20, 2023

Choose a reason for hiding this comment

smk762 Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

smk762 Sep 21, 2023

Choose a reason for hiding this comment

kivqa commented Sep 20, 2023

kivqa commented Sep 20, 2023 • edited Loading

kivqa left a comment

Choose a reason for hiding this comment

smk762 commented Sep 20, 2023 •

edited

Loading

smk762 Sep 21, 2023 •

edited

Loading

kivqa commented Sep 20, 2023 •

edited

Loading