Skip to content

Need to upgrade FC module to handle very long branch or non finalized branch #3002

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

Closed
3 tasks
jangko opened this issue Jan 15, 2025 · 3 comments · Fixed by #3191
Closed
3 tasks

Need to upgrade FC module to handle very long branch or non finalized branch #3002

jangko opened this issue Jan 15, 2025 · 3 comments · Fixed by #3191
Assignees
Labels

Comments

@jangko
Copy link
Contributor

jangko commented Jan 15, 2025

This feature is needed for #2995

@jangko
Copy link
Contributor Author

jangko commented Mar 27, 2025

Copied from discord discussion:

basically, the first step that is needed is to keep track of the latest arrived fcu in forkedchain .. there are two cases:
we know the header -> we can store the "latest finalized block number" and use that in logic surrounding base updates and other things
we don't know the header -> we store the hash in a "pendingFcu" field - then, when we receive any header (either via newpayload or from the sync), can simply inform FC about it and FC can check if it's in the "pendingFCu" entry and promote it to to latestFinalizedNumber, then continue the above logic

in particular, there's no "long async queue" involved here.
once that's done, we can start reasoning about storing the "header chain" directly in the database instead of using the intermediate table that is used now
also - cc @mjfh - we need to get rid of those out-of-band persist calls - the syncer must not interfere with the base flow - if need be, it can even have its own column family which is a lot easier now the backend has been cleaned of excessive abstraction

the other critical change needed to FC is to remove all writes to baseTxFrame essentially, such as here:

c.baseTxFrame.setHead(head.branch.headHeader,

that head update, it should not be written to the base txframe but rather to the txframe of the block being selected has head - same for finalized etc, they should always go into the same frame as the block that gets selected so that when we write the base to disk, we don't accidentally write a head hash that is not yet on disk

this "lastestFinalizedNumber" is suffcient to implement the correct "base update" logic after importBlock, ie block arrives, we check against latest known finalized number, and if the finalized number is far in the future, we can update the base (with some lag - ie the base "target" becomes roundDown32(min(max(head-distance, lastFinalizedNumber)) where head is the latest block we imported, lastestFinalizedNumber is the current finality point according to the CL (ie it can be both higher and lower than head depending on whether we're in sync or not)
the syncer has the right idea here, ie it does something similar - it's just implemented in the wrong place, ie

finNumber: BlockNumber # number of final block

@jangko jangko self-assigned this Mar 27, 2025
@arnetheduck
Copy link
Member

arnetheduck commented Mar 27, 2025

Regarding handling of block bodies/headers in non-final branches, after the above changes have been done, the steps are:

  • introduce a list of heads - these are the head blocks of each known chain branch, one of which is the canonical head - this is analogous to to ChainDagRef.heads in eth2 and can be kept updated the same way
  • store this list of heads in the database and load it on startup, together with all other "candidate chains" (this step does not exist in eth2, but should eventually be added)
  • store blocks that are parents of heads in the database - right now, we only store blocks earlier than base, but if we store the heads we can also store head ancestors
  • when pruning the list of heads as finality happens, also prune the blocks in the database

Likely, we will keep the dag outline -(hash, parent,number) tuples - in memory for all non-final blocks - the rest can be persisted eagerly

@jangko
Copy link
Contributor Author

jangko commented Mar 27, 2025

Regarding handling of block bodies/headers in non-final branches, after the above changes have been done, the steps are:

We have separate issue open for this #2888

jangko added a commit that referenced this issue Mar 31, 2025
A preparation for #3002
jangko added a commit that referenced this issue Mar 31, 2025
A preparation for #3002

This is a piece of break up from bigger FC module fix. Easier for reviewer. Basically this is a fix based on this paragraph:

> that head update, it should not be written to the base txframe but rather to the txframe of the block being selected has head - same for finalized etc, they should always go into the same frame as the block that gets selected so that when we write the base to disk, we don't accidentally write a head hash that is not yet on disk
tersec pushed a commit that referenced this issue Mar 31, 2025
A preparation for #3002

This is a piece of break up from bigger FC module fix. Easier for reviewer. Basically this is a fix based on this paragraph:

> that head update, it should not be written to the base txframe but rather to the txframe of the block being selected has head - same for finalized etc, they should always go into the same frame as the block that gets selected so that when we write the base to disk, we don't accidentally write a head hash that is not yet on disk
jangko added a commit that referenced this issue Apr 1, 2025
This is the second piece of preparation for #3002
jangko added a commit that referenced this issue Apr 1, 2025
This is the second piece of preparation for #3002
jangko added a commit that referenced this issue Apr 1, 2025
This is the second piece of preparation for #3002
jangko added a commit that referenced this issue Apr 2, 2025
* FC header cache get its own column family

This is the second piece of preparation for #3002

* Decouple temporary header storage from CoreDb

* synchronizerKvt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants