Skip to content

Conversation

@mjfh
Copy link
Contributor

@mjfh mjfh commented Oct 23, 2025

No description provided.

why
  Setting this field before running `finalizedBlockHash()` will
  lead to mismatched number/hash information in the `FC`
  descriptor if the current finalised block already registered
  is child of the one to used for updating.

  This happens because the function `finalizedBlockHash()` will
  end up calling `tryUpdatePendingFCU()` which has an effect only
  if the update is child on the already stored register.

  In case that the update is parent when `tryUpdatePendingFCU()` has
  no effect, before this PR one ended up with a (new) hash of the
  parent header, and an already registered block number of the child.

how to reproduce
  + Use the latest manual finalised sync target while the `CL` is
    still catching up syncing.
  + Switch between `CL`s with different sync states or run them
    simultaneously.
@mjfh mjfh requested review from bhartnett and jangko October 23, 2025 14:16
@arnetheduck
Copy link
Member

It is normal that we don't know the number of the fcu and that there's a mismatch - I'm not sure why this is being removed - the idea is that when the hash becomes known, the number is updated accordingly.

@mjfh
Copy link
Contributor Author

mjfh commented Oct 23, 2025

Problem appears when hash and block number already exist and a parent block hash is added by overwriting the pendingFCU register. In that case the resolved block number is still present and registered as latestFinalizedBlockNumber which apparently does not match the hash, any more.

This is all handled correctly by tryUpdatePendingFCU() which is implicitly called by resolveFinHash().

I saw your PR #3257 from 5/5/25 but there was no comment why this statement (now to be removed) was added.

The effect of this mismatch was that the base will not be properly updated, any more.

But I see that a solution might also be that the FC module should handle this hash/block number mismatch -- either way.

@mjfh
Copy link
Contributor Author

mjfh commented Oct 23, 2025

Thinking of it, using a second register latestFinalizedBlockHash and clearing the pendingFCU when it is not pending any more might cover all edge cases.

@arnetheduck
Copy link
Member

a second register

you can just look up the block by number and take the hash from there - pendingFCU is indeed only supposed to be used for resolving a fin block number after which it's no longer useful - the second important point being that the finalized block number only increases monotonically, so if pendingFCU resolves to an older block, it should ignore that update and warn that it was attempted.

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