Apply proposer boost to ancestors correctly#2760
Conversation
| 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.
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 <[email protected]>
hwwhww
left a comment
There was a problem hiding this comment.
LGTM 👍
Thank @realbigsean for the find!
djrtwo
left a comment
There was a problem hiding this comment.
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.
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_scoreI 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.