Fork choice specs and implementation issues: blocks

[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.