Skip to content

Fork choice specs and implementation issues: blocks #191

@ericsson49

Description

@ericsson49

List of differences, that I found, between Fork choice specs and our implementation. This doesn't necessarily mean it's a bug.

1 Blocks are not delayed and time check is different
Spec says:

# Blocks cannot be in the future. If they are, their consideration must be delayed until the are in the past.
assert store.time >= pre_state.genesis_time + block.slot * SECONDS_PER_SLOT

However, DefaultBeaconChain::insert simply rejects blocks too far the future, using a bit different time check:
assert block.slot <= get_current_slot(store.time) + 1

2 Blocks are not checked against finalized checkpoint
Spec says:

# Check block is a descendant of the finalized block

And also

# Check that block is later than the finalized epoch slot

The time check seems to be redundant, given the block passes the descendance check.
Our implementation performs a similar check against justified checkpoint LMDGhostProcessor::isJustifiedAncestor.
The other issue is that it's performed after the block is imported.
While, according to the spec, it's not imported if the finality-descendance check is failed.

3 Finality updates

Spec says:

# Update justified checkpoint
if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch:
    store.justified_checkpoint = state.current_justified_checkpoint
# Update finalized checkpoint
if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch:
    store.finalized_checkpoint = state.finalized_checkpoint

However, DefaultBeaconChain::updateFinality implementation is somewhat different:

    if (!previous.getFinalizedCheckpoint().equals(current.getFinalizedCheckpoint())) {
      chainStorage.getFinalizedStorage().set(current.getFinalizedCheckpoint());
      finalizedCheckpointStream.onNext(current.getFinalizedCheckpoint());
    }
    if (!previous.getCurrentJustifiedCheckpoint().equals(current.getCurrentJustifiedCheckpoint())) {
      // store new justified checkpoint if its epoch greater than previous one
      if (current
          .getCurrentJustifiedCheckpoint()
          .getEpoch()
          .greater(fetchJustifiedCheckpoint().getEpoch())) {
        chainStorage.getJustifiedStorage().set(current.getCurrentJustifiedCheckpoint());
      }

While the justified checkpoint update looks like giving an equivalent result, I'm not so sure about the finalized checkpoint update.

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requestedspecChanges related to spec

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions