Beacon sync revisting fc automove base2 #2995
Conversation
details: This is basically a tweaked version of Andri's function `importBlockBlindly()` from the draft PR mentioned above.
why: Not needed anymore, `forkChoice()` as has been integrated into `importBlock()`.
| if distanceFromBase < c.baseDistance + hysteresis: | ||
| return ok() | ||
|
|
||
| # Move the base forward and stay away `baseDistance` blocks from |
There was a problem hiding this comment.
Why is this necessarily valid? For example, a chain might never have finalized.
There was a problem hiding this comment.
Needs to be discussed/re-visited, probably. Was ported from an earlier PR.
|
Note: This PR is postponed until FC module is ready to handle non-finalized branch or a very long branch yet to be finalized. Per 14 Jan 2025 nimbus-eth1 weekly meeting discussion. |
| @@ -462,6 +462,50 @@ proc updateHeadIfNecessary(c: ForkedChainRef, pvarc: PivotArc) = | |||
|
|
|||
| c.setHead(pvarc) | |||
|
|
|||
| proc autoUpdateBase(c: ForkedChainRef): Result[void, string] = | |||
There was a problem hiding this comment.
btw, this function is a step in a good direction: the responsibility for updating the base should indeed rest firmly with FC.
However, there's nothing "special" with this flow at all, like this documentation makes it seem - instead, it should be considered a normal part of the importBlock flow and of the fCU flow.
There are two things that can cause the base to be updated:
- we received new blocks (either from RPC or the syncer)
- we move the finalization point (via fCU)
Both of these should update base potentially and both flows should call the same function to do so, also so that the hysteresis computation below applies to both "sources" of base updates equally. This means that this same function should be called for all base updates that happen in FC.
Long-range syncing, ie syncing blocks that are older than finality, is a special case of the above logic because FC already knows that they form a linear history, but for the updateBase logic it does not matter: the execution head is what determines base in this case, that and the execution head in the case of long-range syncing is simply a block on the DAG branch that leads to finality and eventually the consensus head.
No description provided.