Skip to content
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

IF: Reject blocks with unsupported claims of last QC in block header extension #2071

Closed
Tracked by #2110
arhag opened this issue Jan 10, 2024 · 0 comments · Fixed by #2120 or #2139
Closed
Tracked by #2110

IF: Reject blocks with unsupported claims of last QC in block header extension #2071

arhag opened this issue Jan 10, 2024 · 0 comments · Fixed by #2120 or #2139
Assignees

Comments

@arhag
Copy link
Member

arhag commented Jan 10, 2024

The block header extension for instant finality makes a claim of the block number of the last block in its ancestry for which it has seen a QC. It also makes a claim as to whether this QC was strong or weak.

The node should not simply trust these claims. It needs to validate them somehow.

One way of validating this is if the claim is the same as the claim in the prior block that was already validated. This can be seen by looking at the header extension within the header stored in the block_header_state that the block builds upon.

But often the claim will be new. And in this case, to verify the claim, the node must verify that a valid QC (of the appropriate type) exists on the claimed block. Note on appropriate type: if the claim is that a strong QC exists, the node must verify it has a strong QC on the block; if the claim is that a weak QC exists, the node must verify it has some QC (strong or weak) on the block.

The block should include the necessary QC in its block extension. So one way to carry out this verification process is to first integrate the QC provided from that block (after checking it is valid) into the block_state associated with the claimed block. This should be done anyway because the QC may be valuable information to keep around even if the received (new) block is abandoned anyway due to a fork reorganization or because we later realize the received block is invalid and needs to be thrown away.

Note: When integrating the QC into the block_state of the claimed block, it may provide the information/proof needed to allow LIB to advance. LIB should be advanced at this point (or shortly after) without having to wait for a new block to be built/completed or for a new vote message to be received.

Then validating the claim in the received block always involves looking up the block_state for the claimed block and ensuring it has a valid QC on it of the appropriate type.

A note on integrating the QC into the block_state:
The block_state may have a pending quorum certificate that the node is collecting votes for. This should not be touched when integrating a received QC into the block_state; there is a separate optional field for the valid_qc. The received QC should be placed in the field for the valid_qc but only if is better than any existing one in valid_qc. A strong valid QC is better than a weak valid QC. For now, do not bother comparing two different valid QC of the same type; they can be treated as equally good.
When observing the QC available for a particular block, we want to pick the best one available. This means that an accessor function get_best_qc should compare the pending quorum certificate (assuming it is valid) against the valid_qc and choose the better of the two (again better in this sense can just consider strong vs weak).

@enf-ci-bot enf-ci-bot moved this to Todo in Team Backlog Jan 10, 2024
@arhag arhag added 👍 lgtm and removed triage labels Jan 10, 2024
@linh2931 linh2931 self-assigned this Jan 15, 2024
@linh2931 linh2931 moved this from Todo to In Progress in Team Backlog Jan 16, 2024
@arhag arhag changed the title IF: Unification: Reject blocks with unsupported claims of last QC in block header extension IF: Reject blocks with unsupported claims of last QC in block header extension Jan 19, 2024
@linh2931 linh2931 moved this from In Progress to Awaiting Review in Team Backlog Jan 22, 2024
@linh2931 linh2931 added this to the Leap v6.0.0-rc1 milestone Jan 22, 2024
@linh2931 linh2931 moved this from Awaiting Review to Done in Team Backlog Jan 24, 2024
@arhag arhag reopened this Jan 24, 2024
@github-project-automation github-project-automation bot moved this from Done to Todo in Team Backlog Jan 24, 2024
@heifner heifner linked a pull request Jan 25, 2024 that will close this issue
@BenjaminGormanPMP BenjaminGormanPMP moved this from Todo to Awaiting Review in Team Backlog Jan 25, 2024
@linh2931 linh2931 moved this from Awaiting Review to Done in Team Backlog Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment