-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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.
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
Add autoswitching from eth_subscribe to eth_blockNumber in Staking DApp #3585
Add autoswitching from eth_subscribe to eth_blockNumber in Staking DApp #3585
Conversation
9be082b
to
7f2c4b9
Compare
Pull Request Test Coverage Report for Build 820a586ee1c376efe1cd60e45bdcb8b7e4d546e5-PR-3585
💛 - Coveralls |
7f2c4b9
to
5fdaad5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@varasev 👍
In overall looks good. I left some nitpicks to increase the manageability of the feature and code readability.
@token_renew_frequency 10 | ||
|
||
# eth_subscribe max delay to switch to eth_blockNumber, in seconds | ||
@eth_subscribe_max_delay 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add manageability of this param through environment variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, added it in 00f3943
) | ||
|
||
# sleep up to 500 ms before the next eth_blockNumber request | ||
Process.send_after(self(), :eth_subscribe_stopped, max(500 - round(microseconds / 1000), 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's define 500 ms as a module parameter. Ideally, to make it manageable via Environment variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, added it in 00f3943
@@ -117,6 +121,7 @@ defmodule Explorer.Staking.ContractState do | |||
block_reward_contract: %{abi: block_reward_abi, address: block_reward_contract_address}, | |||
is_snapshotting: false, | |||
last_change_block: 0, | |||
seen_block: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this parameters order change needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During the work on the module, I moved seen_block
from state
to ETS since in one of my local versions there was a risk of race condition, but later I removed excess code which could lead to that, so overall the seen_block
could stay in the state, but I propose to leave it as it is.
end | ||
|
||
# handles new block and decides to fetch fresh chain info | ||
# credo:disable-for-next-line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest refactoring fetch_state
function in order to eliminate mix credo warning. Refactoring can look like this:
defp fetch_state(state, block_number) do
if block_number <= get(:seen_block) do
state
else
fetch_state_internal(state, block_number)
end
end
defp fetch_state_internal(state, block_number) do
# read general info from the contracts (including pool list and validator list)
global_responses =
ContractReader.perform_requests(ContractReader.global_requests(block_number), state.contracts, state.abi)
epoch_very_beginning = global_responses.epoch_start_block == block_number + 1
start_snapshotting = start_snapshotting?(global_responses)
active_pools_length = Enum.count(global_responses.active_pools)
# determine if something changed in contracts state since the previous seen block.
# if something changed or the `fetch_state` function is called for the first time
# or we are at the beginning of staking epoch or snapshotting recently finished
# then we should update database
last_change_block =
max(global_responses.staking_last_change_block, global_responses.validator_set_last_change_block)
should_update_db = should_update_db?(state, start_snapshotting, last_change_block)
# save the general info to ETS (excluding pool list and validator list)
save_settings(
global_responses,
state,
block_number,
epoch_very_beginning,
start_snapshotting,
active_pools_length,
last_change_block
)
# we should update database as something changed in contracts state
if should_update_db do
update_database(
epoch_very_beginning,
start_snapshotting,
global_responses,
state.contracts,
state.abi,
block_number
)
end
# notify the UI about a new block
data = %{
active_pools_length: active_pools_length,
block_number: block_number,
epoch_end_block: global_responses.epoch_end_block,
epoch_number: global_responses.epoch_number,
max_candidates: global_responses.max_candidates,
staking_allowed: global_responses.staking_allowed,
staking_token_defined: get(:token, nil) != nil,
validator_set_apply_block: global_responses.validator_set_apply_block
}
Publisher.broadcast([{:staking_update, data}], :realtime)
if state.snapshotting_finished do
%{state | snapshotting_finished: false}
else
state
end
end
defp save_settings(
global_responses,
state,
block_number,
epoch_very_beginning,
start_snapshotting,
active_pools_length,
last_change_block
) do
validator_min_reward_percent =
get_validator_min_reward_percent(global_responses.epoch_number, block_number, state.contracts, state.abi)
settings =
global_responses
|> get_settings(validator_min_reward_percent, block_number)
|> Enum.concat(active_pools_length: active_pools_length)
|> Enum.concat(last_change_block: last_change_block)
|> Enum.concat(seen_block: block_number)
:ets.insert(@table_name, settings)
if epoch_very_beginning or start_snapshotting do
# if the block_number is the latest block of the finished staking epoch
# or we are starting Blockscout server, the BlockRewardAuRa contract balance
# could increase before (without Mint/Transfer events),
# so we need to update its balance in database
update_block_reward_balance(block_number)
end
end
defp start_snapshotting?(global_responses) do
global_responses.epoch_number > get(:snapshotted_epoch_number) && global_responses.epoch_number > 0 &&
not get(:is_snapshotting)
end
defp should_update_db?(state, start_snapshotting, last_change_block) do
first_fetch = get(:epoch_end_block, 0) == 0
start_snapshotting or state.snapshotting_finished or first_fetch or last_change_block > get(:last_change_block)
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 00f3943
Motivation
Currently, Staking DApp only receives a new block number from
eth_subscribe
connected to WebSocket node. This PR allows the App to additionally useeth_blockNumber
wheneth_subscribe
stops sending notifications.If
eth_subscribe
stops working, the DApp waits for 60 seconds and then activateseth_blockNumber
requests which are sent every 500 ms. Once the new notification frometh_subscribe
is received,eth_blockNumber
requests stop and the DApp switches back toeth_subscribe
.Also, this PR makes a couple of minor fixes:
Likelihood of Becoming a Validator on the Next Epoch
now should display correct value in the pool info popup: if there are less than 20 candidates, the likelihood is 100%. Otherwise, it is calculated as before.Potential Reward Percent
(Current Reward Percent
) in delegator list popup renamed toPotential Reward Share
(Current Reward Share
)Checklist for your Pull Request (PR)
CHANGELOG.md
with this PRmaster
in the Version column. Changes will be reflected in this table: https://docs.blockscout.com/for-developers/information-and-settings/env-variables.