Skip to content

fix: recompute head was taking more than 20s in mainnet #1380

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

Merged
merged 5 commits into from
Feb 6, 2025
Merged

fix: recompute head was taking more than 20s in mainnet #1380

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2da6d91
initial test
rodrigo-o Feb 5, 2025
f38ab61
remove uove unneded diffs and format
rodrigo-o Feb 5, 2025
25dd37e
Fixed an issue and readded sort to maintain the previous logic
rodrigo-o Feb 5, 2025
ad99991
remove testing time and added a comment
rodrigo-o Feb 5, 2025
cb10958
remove unneded require
rodrigo-o Feb 5, 2025
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
44 changes: 28 additions & 16 deletions lib/lambda_ethereum_consensus/fork_choice/head.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,38 @@ defmodule LambdaEthereumConsensus.ForkChoice.Head do
@spec get_head(Store.t()) :: {:ok, Types.root()} | {:error, any}
def get_head(%Store{} = store) do
# Get filtered block tree that only includes viable branches
blocks = get_filtered_block_tree(store)
filtered_blocks = get_filtered_block_tree(store)
# Execute the LMD-GHOST fork choice
head = store.justified_checkpoint.root

{_store, %BeaconState{} = justified_state} =
Store.get_checkpoint_state(store, store.justified_checkpoint)

# PERF: return just the parent root and the block root in `get_filtered_block_tree`
Stream.cycle([nil])
|> Enum.reduce_while(head, fn nil, head ->
blocks
|> Stream.filter(fn {_, block} -> block.parent_root == head end)
|> Stream.map(fn {root, _} -> root end)
# Ties broken by favoring block with lexicographically higher root
|> Enum.sort(:desc)
|> then(fn
[] -> {:halt, head}
c -> {:cont, Enum.max_by(c, &get_weight(store, &1, justified_state))}
Comment on lines -29 to -31
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As we discussed with @ElFantasma this could have been just 1 line change like:

  |> then(fn
    [] -> {:halt, head}
    [single_child] -> {:cont, single_child}
    c -> {:cont, Enum.max_by(c, &get_weight(store, &1, justified_state))}

most of the changes are just for readability

end)
end)
|> then(&{:ok, &1})
head = compute_head(store, filtered_blocks, head, justified_state)
{:ok, head}
end

defp compute_head(store, blocks, current_root, justified_state) do
children = for {root, parent_root} <- blocks, parent_root == current_root, do: root

case children do
[] ->
current_root

[only_child] ->
# Directly continue without a max_by call
compute_head(store, blocks, only_child, justified_state)

candidates ->
# Choose the candidate with the maximal weight according to get_weight/3
best_child =
candidates
# Ties broken by favoring block with lexicographically higher root
|> Enum.sort(:desc)
|> Enum.max_by(&get_weight(store, &1, justified_state))

compute_head(store, blocks, best_child, justified_state)
end
end

defp get_weight(%Store{} = store, root, state) do
Expand Down Expand Up @@ -79,11 +90,12 @@ defmodule LambdaEthereumConsensus.ForkChoice.Head do

# Retrieve a filtered block tree from ``store``, only returning branches
# whose leaf state's justified/finalized info agrees with that in ``store``.
# Only return the roots and their parent roots.
defp get_filtered_block_tree(%Store{} = store) do
base = store.justified_checkpoint.root
block = Blocks.get_block!(base)
{_, blocks} = filter_block_tree(store, base, block, %{})
blocks
Enum.map(blocks, fn {root, block} -> {root, block.parent_root} end)
end

defp filter_block_tree(%Store{} = store, block_root, block, blocks) do
Expand Down
Loading