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.

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

feat: index miners count and total rewards from fees #854

Merged
merged 4 commits into from
Aug 24, 2022
Merged

Conversation

sborrazas
Copy link
Contributor

@sborrazas sborrazas commented Aug 19, 2022

refs #844

Includes a migration that takes 15-30mins to index the already processed generation miners.

@sborrazas sborrazas self-assigned this Aug 19, 2022
Includes a migration that takes 15-30mins to index the already
processed generation miners.
@sborrazas sborrazas changed the title feat: index miners count and total rewards from fees (WIP) feat: index miners count and total rewards from fees Aug 23, 2022
@sborrazas sborrazas marked this pull request as ready for review August 23, 2022 16:47
The reason for this is that miner's pks always change, so for every
new key block there will almost always be a new miner.
Copy link
Member

@jyeshe jyeshe left a comment

Choose a reason for hiding this comment

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

No operational issues, only refactoring opportunities.

Comment on lines 25 to 36
case State.get(state, Model.Miner, beneficiary_pk) do
{:ok, Model.miner(total_reward: old_reward) = miner} ->
{State.put(state, Model.Miner, Model.miner(miner, total_reward: old_reward + reward)),
false}

:not_found ->
{State.put(
state,
Model.Miner,
Model.miner(index: beneficiary_pk, total_reward: reward)
), true}
end
Copy link
Member

Choose a reason for hiding this comment

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

It's already readable but might convey the idea at a glance by giving a name to this case block with a function (sth like increment_total_reward)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 39 to 45
State.update(state, Model.Stat, Stats.miners_count_key(), fn
Model.stat(payload: count) = stat ->
Model.stat(stat, payload: count + 1)

nil ->
Model.stat(index: Stats.miners_count_key(), payload: 1)
end)
Copy link
Member

Choose a reason for hiding this comment

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

same here with increment_payload

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

def fetch_miner!(state, miner_pk),
do: render_miner(State.fetch!(state, Model.Miner, miner_pk))

defp build_streamer(state, cursor), do: &Collection.stream(state, Model.Miner, &1, nil, cursor)
Copy link
Member

Choose a reason for hiding this comment

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

👍

def run(_from_start?) do
state = State.new()

case state |> Collection.stream(Model.DeltaStat, :backward, nil, nil) |> Enum.take(1) do
Copy link
Member

Choose a reason for hiding this comment

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

maybe Database.last_key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@sborrazas sborrazas requested a review from jyeshe August 24, 2022 10:32
@sborrazas sborrazas merged commit 725beb7 into master Aug 24, 2022
@sborrazas sborrazas deleted the miners-index branch August 24, 2022 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants