Implement should_override_forkchoice_update #11981
Conversation
potuz
added
Ready For Review
There was a problem hiding this comment.
Looks reasonable to me. This is blocked by #11964. A little hard to review until that is merged
potuz
force-pushed
the
proposer-head-v2
branch
from
7cc1144 to
5762405
Compare
How? this doesn't have any runtime changes a feature flag to gate what? |
| // orphanLateBlockFirstThreshold is the number of seconds after which we | ||
| // consider a block to be late, and thus a candidate to being reorged. | ||
| const orphanLateBlockFirstThreshold = 4 | ||
|
|
||
| // processAttestationsThreshold is the number of seconds after which we | ||
| // process attestations for the current slot | ||
| const processAttestationsThreshold = 10 |
There was a problem hiding this comment.
any thoughts on putting these as flags with default to 4 / 10 or we simply do not want ppl to change these
There was a problem hiding this comment.
I rather users not mess with these since that can cause more trouble than what we are trying to fix.
| @@ -104,14 +104,12 @@ func (s *Store) pullTips(state state.BeaconState, node *Node, jc, fc *ethpb.Chec | |||
| return jc, fc | |||
| } | |||
|
|
|||
| ab, uj, uf, err := precompute.UnrealizedCheckpoints(state) | |||
| _, uj, uf, err := precompute.UnrealizedCheckpoints(state) | |||
There was a problem hiding this comment.
Let's just remove activeBalance from UnrealizedCheckpoints. It's cleaner
beacon-chain/forkchoice/doubly-linked-tree/reorg_late_blocks.go
Outdated
Show resolved
Hide resolved
beacon-chain/forkchoice/doubly-linked-tree/reorg_late_blocks.go
Outdated
Show resolved
Hide resolved
beacon-chain/forkchoice/doubly-linked-tree/reorg_late_blocks.go
Outdated
Show resolved
Hide resolved
| // the engine's view of head with the parent block or the incoming block. It | ||
| // does not guarantee an attempted reorg. This will only be decided later at | ||
| // proposal time by calling GetProposerHead. | ||
| func (f *ForkChoice) ShouldOverrideFCU() (override bool) { |
There was a problem hiding this comment.
I think we are missing this:
# Only re-org if we are proposing on-time.
time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT
proposing_on_time = time_into_slot <= SECONDS_PER_SLOT // INTERVALS_PER_SLOT // 2
There was a problem hiding this comment.
Yeah, that can't really happen, we will only call this function after receiving a block and we are proposing during the next slot. The spec implementation allows you to reorg a late block that was supposed to be ready during N and that arrives during N+1. We will always reorg such blocks anyway.
| "github.com/prysmaticlabs/prysm/v3/testing/require" | ||
| ) | ||
|
|
||
| func TestForkChoice_ShouldOverrideFCU(t *testing.T) { |
There was a problem hiding this comment.
We are missing an important test case where head arrives early
Part of implementing honest validators orphaning late blocks.