-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Apply proposer boost to ancestors correctly #2760
Conversation
@@ -182,12 +182,13 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: | |||
and get_ancestor(store, store.latest_messages[i].root, store.blocks[root].slot) == root) | |||
)) | |||
proposer_score = Gwei(0) | |||
if store.proposer_boost_root != Root() and root == store.proposer_boost_root: |
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.
This conditional implies that get_latest_attesting_balance
applies the proposer score boost only to the proposer's block, but not its ancestors. This is not the correct method to apply the proposer score boost.
The boost should be applied like usual LMD votes (i.e., store.latest_messages
) - all ancestors of store.proposer_boost_root
should get the additional weight.
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
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.
LGTM 👍
Thank @realbigsean for the find!
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.
logic looks correct. some suggestions for alternative formating
specs/phase0/fork-choice.md
Outdated
store.proposer_boost_root != Root() | ||
and get_ancestor(store, store.proposer_boost_root, store.blocks[root].slot) == root |
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.
generally should avoid multi-line ifs. This is also a bit secretly too pythonic (it assumes that if the first bool fails, the second is not executed, avoiding the undefined behavior if Root()
being passed into get_ancestor
suggest somthing like the following
if store.proposer_boost_root != Root():
if get_ancestor():
...
An alternative, I personally prefer this:
if store.proposer_boost_root == Root():
return attestation_score
proposer_score = Gwei(0)
if get_ancestor():
...
return attestation_score + proposer_score
I think that very nicely reads as "If proposer_boost is not enabled, just return the attestation_score, otherwise calculate the proposer score and return the sum"
Fixes #2757 by correctly applying proposer score boost to ancestors of the boosted block.
This removes the simplification changes from #2753.