Skip to content
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.

Sign up for GitHub

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

Use rounded derived height sync param #2364

Merged
merged 3 commits into from
Sep 21, 2023
Merged

Use rounded derived height sync param #2364

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b91b8bf
Use rounded derived height sync param
smk762 Sep 20, 2023
41ef0c2
avoid > 100%
smk762 Sep 20, 2023
771fe83
its not python mate
smk762 Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ FetchContent_Declare(
)

FetchContent_Declare(jl777-coins
URL https://github.com/KomodoPlatform/coins/archive/master.zip)
URL https://github.com/KomodoPlatform/coins/archive/ssl-only-coins-config.zip)

#FetchContent_Declare(adex-generics-coins
# URL https://github.com/KomodoPlatform/komodo-wallet-desktop/archive/main.zip)
Expand Down
11 changes: 5 additions & 6 deletions atomic_defi_design/Dex/Constants/General.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,13 @@ QtObject {

function zhtlcActivationProgress(activation_status, coin='ARRR')
{

const coin_info = API.app.portfolio_pg.global_cfg_mdl.get_coin_info(coin)
let block_offset = coin_info.checkpoint_block
let progress = 100
if (!activation_status.hasOwnProperty("result")) return progress
// console.log("["+coin+"] [zhtlcActivationProgress]: " + JSON.stringify(activation_status))
const coin_info = API.app.portfolio_pg.global_cfg_mdl.get_coin_info(coin)
let block_offset = coin_info.checkpoint_height
let status = activation_status.result.status
let details = activation_status.result.details
// console.log("["+coin+"] [zhtlcActivationProgress]: " + JSON.stringify(activation_status))
smk762 marked this conversation as resolved.
Show resolved Hide resolved

// use range from checkpoint block to present
if (!status)
Expand All @@ -145,14 +144,14 @@ QtObject {
block_offset = details.UpdatingBlocksCache.first_sync_block.actual
let n = details.UpdatingBlocksCache.current_scanned_block - block_offset
let d = details.UpdatingBlocksCache.latest_block - block_offset
progress = 5 + parseInt(n/d*40)
progress = 5 + parseInt(n/d*20)
}
else if (details.hasOwnProperty("BuildingWalletDb"))
{
block_offset = details.BuildingWalletDb.first_sync_block.actual
let n = details.BuildingWalletDb.current_scanned_block - block_offset
let d = details.BuildingWalletDb.latest_block - block_offset
progress = 45 + parseInt(n/d*40)
progress = 45 + parseInt(n/d*60)
}
else if (details.hasOwnProperty("RequestingBalance")) progress = 95
else if (details.hasOwnProperty("ActivatingCoin")) progress = 5
Expand Down
2 changes: 1 addition & 1 deletion src/core/atomicdex/api/mm2/rpc2.task.enable_z_coin.init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes right

Copy link
Collaborator Author

@smk762 smk762 Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

j["params"]["activation_params"]["mode"]["rpc_data"]["electrum_servers"] = request.servers;
j["params"]["activation_params"]["mode"]["rpc_data"]["light_wallet_d_servers"] = request.z_urls;
j["params"]["activation_params"]["scan_blocks_per_iteration"] = 5000;
Expand Down
2 changes: 1 addition & 1 deletion src/core/atomicdex/api/mm2/rpc2.task.enable_z_coin.init.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace atomic_dex::mm2
std::vector<atomic_dex::electrum_server> servers;
std::vector<std::string> z_urls;
CoinType coin_type;
int sync_date{1672531200}; // Jan 1st 2023
int sync_height{0};
smk762 marked this conversation as resolved.
Show resolved Hide resolved
bool is_testnet{false};
bool with_tx_history{false}; // Not yet in API
};
Expand Down
10 changes: 10 additions & 0 deletions src/core/atomicdex/config/coins.cfg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ namespace atomic_dex
cfg.wallet_only = is_wallet_only(cfg.ticker) ? is_wallet_only(cfg.ticker) : j.contains("wallet_only") ? j.at("wallet_only").get<bool>() : false;
cfg.default_coin = is_default_coin(cfg.ticker);
cfg.is_faucet_coin = is_faucet_coin(cfg.ticker);
cfg.checkpoint_height = 0;
cfg.checkpoint_blocktime = 0;
using namespace std::chrono;

if (j.contains("other_types"))
Expand Down Expand Up @@ -226,6 +228,14 @@ namespace atomic_dex
{
cfg.z_urls = j.at("light_wallet_d_servers").get<std::vector<std::string>>();
}
if (j.contains("checkpoint_blocktime"))
{
cfg.checkpoint_blocktime = j.at("checkpoint_blocktime").get<int>();
}
if (j.contains("checkpoint_height"))
{
cfg.checkpoint_height = j.at("checkpoint_height").get<int>();
}
if (j.contains("alias_ticker"))
{
cfg.alias_ticker = j.at("alias_ticker").get<std::string>();
Expand Down
2 changes: 2 additions & 0 deletions src/core/atomicdex/config/coins.cfg.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ namespace atomic_dex
std::string minimal_claim_amount{"0"};
CoinType coin_type;
nlohmann::json activation_status;
int checkpoint_height{0};
int checkpoint_blocktime{0};
bool segwit{false};
bool active{false};
bool checked{false};
Expand Down
2 changes: 2 additions & 0 deletions src/core/atomicdex/models/qt.global.coins.cfg.model.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ namespace
{"is_erc_family", coin.is_erc_family},
{"is_zhtlc_family", coin.is_zhtlc_family},
{"is_wallet_only", coin.wallet_only},
{"checkpoint_height", coin.checkpoint_height},
{"checkpoint_blocktime", coin.checkpoint_blocktime},
{"has_memos", coin.has_memos},
{"fees_ticker", QString::fromStdString(coin.fees_ticker)}};
return j;
Expand Down
23 changes: 23 additions & 0 deletions src/core/atomicdex/pages/qt.settings.page.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,29 @@ namespace atomic_dex
QSettings& settings = entity_registry_.ctx<QSettings>();
return settings.value("PirateSyncDate").toInt();
}
int settings_page::get_pirate_sync_height(int sync_date, int checkpoint_height, int checkpoint_blocktime) const
{
if (checkpoint_height == 0)
{
return 0;
}
int blocktime_estimate = 65; // Based on average block time between checkpoint block and block 2575600 + margin of error
SPDLOG_INFO("sync_date: {}", sync_date);
SPDLOG_INFO("checkpoint_height: {}", checkpoint_height);
SPDLOG_INFO("checkpoint_blocktime: {}", checkpoint_blocktime);
int time_delta = sync_date - checkpoint_blocktime;
SPDLOG_INFO("time_delta: {}", time_delta);
int block_delta = static_cast<int>(time_delta / blocktime_estimate);
SPDLOG_INFO("block_delta: {}", block_delta);
// As block time is variable, we round height to the nearest 1000 blocks
int height = checkpoint_height + static_cast<int>(block_delta / 1000) * 1000;
if (height < 0)
{
height = 0;
}
SPDLOG_INFO("height: {}", height);
return height;
smk762 marked this conversation as resolved.
Show resolved Hide resolved
}

void settings_page::set_pirate_sync_date(int new_timestamp)
{
Expand Down
1 change: 1 addition & 0 deletions src/core/atomicdex/pages/qt.settings.page.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ namespace atomic_dex
Q_INVOKABLE [[nodiscard]] bool is_this_ticker_present_in_normal_cfg(const QString& ticker) const;
Q_INVOKABLE [[nodiscard]] QString get_custom_coins_icons_path() const;
Q_INVOKABLE [[nodiscard]] int get_pirate_sync_date() const;
Q_INVOKABLE [[nodiscard]] int get_pirate_sync_height(int sync_date, int checkpoint_height, int checkpoint_blocktime) const;
Q_INVOKABLE void process_token_add(const QString& contract_address, const QString& coingecko_id, const QString& icon_filepath, CoinType coin_type);
Q_INVOKABLE void process_qrc_20_token_add(const QString& contract_address, const QString& coingecko_id, const QString& icon_filepath);
Q_INVOKABLE void submit();
Expand Down
5 changes: 3 additions & 2 deletions src/core/atomicdex/services/mm2/mm2.service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,14 +1403,15 @@ namespace atomic_dex
auto request_functor = [this](coin_config coin_info) -> std::pair<nlohmann::json, std::vector<std::string>>
{
const auto& settings_system = m_system_manager.get_system<settings_page>();
int sync_from_date = settings_system.get_pirate_sync_date();
int sync_date = settings_system.get_pirate_sync_date();
int sync_height = settings_system.get_pirate_sync_height(sync_date, coin_info.checkpoint_height, coin_info.checkpoint_blocktime);

t_enable_z_coin_request request{
.coin_name = coin_info.ticker,
.servers = coin_info.electrum_urls.value_or(get_electrum_server_from_token(coin_info.ticker)),
.z_urls = coin_info.z_urls.value_or(std::vector<std::string>{}),
.coin_type = coin_info.coin_type,
.sync_date = sync_from_date,
.sync_height = sync_height,
.is_testnet = coin_info.is_testnet.value_or(false),
.with_tx_history = false}; // Tx history not yet ready for ZHTLC

Expand Down