Skip to content

Transform FC module internals into DAG #3441

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

Merged
merged 4 commits into from
Jul 3, 2025
Merged

Transform FC module internals into DAG #3441

merged 4 commits into from
Jul 3, 2025

Conversation

jangko
Copy link
Contributor

@jangko jangko commented Jul 2, 2025

No more BranchRef, only BlockRef.
Simpler algorithm in many places.
Pruning leaves become more uniform among branches.

@jangko
Copy link
Contributor Author

jangko commented Jul 3, 2025

Tested on hoodi during syncing and chain following. No incident.

@jangko jangko merged commit 8a6877d into master Jul 3, 2025
5 checks passed
@jangko jangko deleted the blockref branch July 3, 2025 05:21
let branch = c.branches[i]
func reachable(head, fin: BlockRef): bool =
loopIt(head):
if it.colored:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this algorithm can be simplified based on the observation that block numbers are monotonically increasing - basically, when we've identified the finalized block, we walk the parent chain and compare block numbers - if the block number of the examined block is lower than that of the finalized block but is not the finalized block, we know that the branch is stale because the block where it meets the chain must be older than finalized.

proc isRelevantBranch(head, finalized): bool =
  var blk = head
  while blk != finalized:
    if blk.number < finalized.number:
      return false
    blk = blk.parent
  true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notably, we only need to run isRelevantBranch for branches that are not the canonical head right now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just tested, it does not work for branches with head lower than the new finalized.

e.g. old fin = A3. stale chain B5-B8. chain A continue from A4 to A10. new fin = A9.
The simplified algorithm basically not aware of B.

Copy link
Member

@arnetheduck arnetheduck Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your example, the starting point is the list of heads before fin was updated to A9, ie [A10, B8] .. then you do:

var toprune: seq[blockref]
for head in fc.heads:
  if head == canonical:
   continue
  if not isReachable(head, newfin):
    toprune.add(head)

b will hit the if blk.number < fin.number condition on first iteration and will be removed

Copy link
Contributor Author

@jangko jangko Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry. I mean not about the head. But how to stop the pruning? When we move backward from head of a stale branch, there is no marker of a junction/forking point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int eth1 we still need parent to calculate base movement. So there is no significant difference with color algorithm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, the base movement computation happens first - you establish the new base, then you clear the parents then you prune the branches - the difference is that you don't need a color field and you don't need to "uncolor" things after you're done - that's the simplification.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, clearing the parent has the added benefit that the GC can release the structure earlier as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not all finalized blocks are written to database,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aah, right - hm - I guess one could use a finalized bool to avoid the "uncoloring" in that case - while still a simplification, maybe that's not as attractive.

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

Successfully merging this pull request may close these issues.

2 participants