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

[Merged by Bors] - ForkChoice: remove head_block_root field #4590

Closed

Conversation

zhiqiangxu
Copy link
Contributor

@zhiqiangxu zhiqiangxu commented Aug 9, 2023

This field is redundant with ForkchoiceUpdateParameters.head_root, the same head_root is assigned to both fields in get_head.

@zhiqiangxu zhiqiangxu force-pushed the rm_ForkChoice.head_block_root branch from fe97dbd to ee98b41 Compare August 9, 2023 11:20
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Nice simplification, thanks!

I'll get someone else to double-check it, just because fork choice is a very sensitive component.

@michaelsproul michaelsproul added code-quality consensus An issue/PR that touches consensus code, such as state_processing or block verification. v4.4.1 ETA August 2023 under-review A reviewer has only partially completed a review. labels Aug 11, 2023
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Very nice, thanks!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed under-review A reviewer has only partially completed a review. labels Aug 14, 2023
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 14, 2023
This field is redundant with `ForkchoiceUpdateParameters.head_root`, the same `head_root` is assigned to both fields in [`get_head`](https://github.com/sigp/lighthouse/blob/dfcb3363c757671eb19d5f8e519b4b94ac74677a/consensus/fork_choice/src/fork_choice.rs#L508-L523).
@bors
Copy link

bors bot commented Aug 14, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Aug 14, 2023
This field is redundant with `ForkchoiceUpdateParameters.head_root`, the same `head_root` is assigned to both fields in [`get_head`](https://github.com/sigp/lighthouse/blob/dfcb3363c757671eb19d5f8e519b4b94ac74677a/consensus/fork_choice/src/fork_choice.rs#L508-L523).
@bors
Copy link

bors bot commented Aug 14, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title ForkChoice: remove head_block_root field [Merged by Bors] - ForkChoice: remove head_block_root field Aug 14, 2023
@bors bors bot closed this Aug 14, 2023
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
This field is redundant with `ForkchoiceUpdateParameters.head_root`, the same `head_root` is assigned to both fields in [`get_head`](https://github.com/sigp/lighthouse/blob/dfcb3363c757671eb19d5f8e519b4b94ac74677a/consensus/fork_choice/src/fork_choice.rs#L508-L523).
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
This field is redundant with `ForkchoiceUpdateParameters.head_root`, the same `head_root` is assigned to both fields in [`get_head`](https://github.com/sigp/lighthouse/blob/dfcb3363c757671eb19d5f8e519b4b94ac74677a/consensus/fork_choice/src/fork_choice.rs#L508-L523).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality consensus An issue/PR that touches consensus code, such as state_processing or block verification. ready-for-merge This PR is ready to merge. v4.4.1 ETA August 2023
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants