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

refactor: remove forkchoice casts #1126

Merged
merged 14 commits into from
Jun 4, 2024
Merged

Conversation

avilagaston9
Copy link
Contributor

@avilagaston9 avilagaston9 commented May 30, 2024

This PR removes all GenServer casts from ForkChoice.

Casts were replaced by function calls from PendingBlocks.

Also, makes recompute_head synchronous adding grafana metrics:

image

Closes #1118

Base automatically changed from persist-forkchoice-store to main June 3, 2024 15:05
@avilagaston9 avilagaston9 marked this pull request as ready for review June 3, 2024 16:03
@avilagaston9 avilagaston9 requested a review from a team as a code owner June 3, 2024 16:03
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, Gaston! I added some comments.


{:error, reason} ->
Logger.error("[Fork choice] Failed to add block: #{reason}", slot: slot, root: block_root)
GenServer.cast(from, {:block_processed, block_root, false})
:error
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should return the error with its reason and let the caller handle it.

Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

LGTM with two small comments.

@avilagaston9 avilagaston9 merged commit 203aace into main Jun 4, 2024
19 checks passed
@avilagaston9 avilagaston9 deleted the refactor-forkchoice-casts branch June 4, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[Refactor Fork Choice] Delete casts
2 participants