diff --git a/CHANGELOG.md b/CHANGELOG.md index b593e3a07da..e3faafd3f42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ For information on changes in released versions of Teku, see the [releases page] This simplifies using the standard REST API to retrieve the initial state as just the base URL can be specified (e.g. `--initial-state https:// finalizeCurrentChain() { + final UInt64 chainHeadSlot = getLatestSlot(); + final UInt64 finalizeEpoch = spec.computeEpochAtSlot(chainHeadSlot).max(2); + final UInt64 finalHeadEpoch = finalizeEpoch.plus(3); + final UInt64 finalHeadSlot = spec.computeStartSlotAtEpoch(finalHeadEpoch); + + final List addedBlockAndStates = new ArrayList<>(); + SignedBlockAndState newChainHead = null; + for (UInt64 slot = chainHeadSlot.plus(1); + slot.isLessThan(finalHeadSlot); + slot = slot.increment()) { + final BlockOptions blockOptions = BlockOptions.create(); + streamValidAttestationsForBlockAtSlot(slot).forEach(blockOptions::addAttestation); + newChainHead = generateBlockAtSlot(slot, blockOptions); + addedBlockAndStates.add(newChainHead); + } + final Checkpoint finalizedCheckpoint = newChainHead.getState().getFinalizedCheckpoint(); + assertThat(finalizedCheckpoint.getEpoch()) + .describedAs("Failed to finalize epoch %s", finalizeEpoch) + .isEqualTo(finalizeEpoch); + assertThat(finalizedCheckpoint.getRoot()) + .describedAs("Failed to finalize epoch %s", finalizeEpoch) + .isNotEqualTo(Bytes32.ZERO); + return addedBlockAndStates; + } + /** * Utility for streaming valid attestations available for inclusion at the given slot. This * utility can be used to assign valid attestations to a generated block. diff --git a/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategy.java b/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategy.java index e716620cff5..de5d4511ffb 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategy.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategy.java @@ -200,7 +200,7 @@ public ForkChoiceState getForkChoiceState( headExecutionBlockHash, justifiedExecutionHash, finalizedExecutionHash, - headNode.isOptimistic()); + headNode.isOptimistic() || !protoArray.nodeIsViableForHead(headNode)); } finally { protoArrayLock.readLock().unlock(); } diff --git a/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoArray.java b/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoArray.java index af4d7c92462..5352773e4d4 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoArray.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoArray.java @@ -241,7 +241,8 @@ private Optional findHead( // Perform a sanity check that the node is indeed valid to be the head. if (!nodeIsViableForHead(bestNode) && !bestNode.equals(justifiedNode)) { - throw new RuntimeException("ProtoArray: Best node is not viable for head"); + throw new IllegalStateException( + "ProtoArray: Best node " + bestNode.toLogString() + " is not viable for head"); } return Optional.of(bestNode); } diff --git a/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoNode.java b/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoNode.java index afc9ac685fb..5f7c5a031a5 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoNode.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/protoarray/ProtoNode.java @@ -21,6 +21,7 @@ import java.util.Objects; import java.util.Optional; import org.apache.tuweni.bytes.Bytes32; +import tech.pegasys.teku.infrastructure.logging.LogFormatter; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.datastructures.blocks.BlockCheckpoints; import tech.pegasys.teku.spec.datastructures.forkchoice.ProtoNodeData; @@ -270,4 +271,8 @@ public String toString() { .add("validationStatus", validationStatus) .toString(); } + + public String toLogString() { + return LogFormatter.formatBlock(blockSlot, blockRoot); + } } diff --git a/storage/src/test/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategyTest.java b/storage/src/test/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategyTest.java index 8596b6ce2c1..b4661c4c81a 100644 --- a/storage/src/test/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategyTest.java +++ b/storage/src/test/java/tech/pegasys/teku/storage/protoarray/ForkChoiceStrategyTest.java @@ -38,6 +38,7 @@ import tech.pegasys.teku.spec.datastructures.blocks.Eth1Data; import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; import tech.pegasys.teku.spec.datastructures.blocks.SignedBlockAndState; +import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; import tech.pegasys.teku.spec.datastructures.forkchoice.ProtoNodeData; import tech.pegasys.teku.spec.datastructures.forkchoice.ReadOnlyForkChoiceStrategy; import tech.pegasys.teku.spec.datastructures.forkchoice.TestStoreFactory; @@ -47,10 +48,13 @@ import tech.pegasys.teku.spec.datastructures.state.AnchorPoint; import tech.pegasys.teku.spec.datastructures.state.Checkpoint; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; +import tech.pegasys.teku.spec.executionlayer.ForkChoiceState; import tech.pegasys.teku.spec.executionlayer.PayloadStatus; import tech.pegasys.teku.spec.generator.ChainBuilder; import tech.pegasys.teku.spec.generator.ChainBuilder.BlockOptions; import tech.pegasys.teku.spec.util.DataStructureUtil; +import tech.pegasys.teku.storage.client.ChainUpdater; +import tech.pegasys.teku.storage.client.RecentChainData; import tech.pegasys.teku.storage.storageSystem.InMemoryStorageSystemBuilder; import tech.pegasys.teku.storage.storageSystem.StorageSystem; @@ -453,6 +457,64 @@ void applyPendingVotes_shouldMarkEquivocation() { assertThat(transaction3.getVote(ZERO).getNextRoot()).isEqualTo(block1.getRoot()); } + @Test + void shouldConsiderHeadOptimisticWhenItIsNotViable() { + // If we optimistically import blocks which include enough attestations to update the justified + // checkpoint, then later invalidate some non-justified blocks such that the head block no + // longer includes enough attestations to support that justification, then none of our blocks + // will be viable because they won't have the same checkpoints as our store. + // In that case we will use the justified checkpoint as chain head and should remain in + // optimistic sync mode + + final StorageSystem storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec); + final ChainUpdater chainUpdater = storageSystem.chainUpdater(); + chainUpdater.initializeGenesisWithPayload(true); + final ForkChoiceStrategy protoArray = getProtoArray(storageSystem); + final RecentChainData recentChainData = storageSystem.recentChainData(); + final SignedBlockAndState optimisticHead = chainUpdater.finalizeCurrentChainOptimistically(); + ProtoNodeData firstJustifiedData = + protoArray.getBlockData(optimisticHead.getRoot()).orElseThrow(); + ProtoNodeData lastUnjustifiedData = + protoArray.getBlockData(firstJustifiedData.getParentRoot()).orElseThrow(); + final Checkpoint currentJustified = recentChainData.getJustifiedCheckpoint().orElseThrow(); + while (lastUnjustifiedData.getCheckpoints().getJustifiedCheckpoint().equals(currentJustified)) { + firstJustifiedData = lastUnjustifiedData; + lastUnjustifiedData = + protoArray.getBlockData(lastUnjustifiedData.getParentRoot()).orElseThrow(); + } + assertThat(firstJustifiedData.getCheckpoints().getJustifiedCheckpoint()) + .isEqualTo(currentJustified); + assertThat(lastUnjustifiedData.getCheckpoints().getJustifiedCheckpoint()) + .isNotEqualTo(currentJustified); + + // Invalidate blocks that updated the justified checkpoint + protoArray.onExecutionPayloadResult( + firstJustifiedData.getRoot(), + PayloadStatus.invalid(Optional.empty(), Optional.of("No good, very bad block")), + true); + // And validate the justified checkpoint + protoArray.onExecutionPayloadResult(currentJustified.getRoot(), PayloadStatus.VALID, false); + + // Find new chain head + final SlotAndBlockRoot revertHead = + protoArray.findHead( + recentChainData.getJustifiedCheckpoint().orElseThrow(), + recentChainData.getFinalizedCheckpoint().orElseThrow()); + recentChainData.updateHead(revertHead.getBlockRoot(), optimisticHead.getSlot()); + + final ForkChoiceState forkChoiceState = + protoArray.getForkChoiceState( + recentChainData.getJustifiedCheckpoint().orElseThrow(), + recentChainData.getFinalizedCheckpoint().orElseThrow()); + + // Should have reverted to the justified checkpoint as head + assertThat(forkChoiceState.getHeadBlockRoot()).isEqualTo(currentJustified.getRoot()); + // The current head block itself is fully validated + assertThat(protoArray.isFullyValidated(currentJustified.getRoot())).isTrue(); + // But we consider the chain head optimistic because of the updated justified checkpoint + assertThat(forkChoiceState.isHeadOptimistic()).isTrue(); + } + private StorageSystem initStorageSystem() { return initStorageSystem(spec); } diff --git a/storage/src/testFixtures/java/tech/pegasys/teku/storage/client/ChainUpdater.java b/storage/src/testFixtures/java/tech/pegasys/teku/storage/client/ChainUpdater.java index bbf7c97d3a9..d5addfaaed0 100644 --- a/storage/src/testFixtures/java/tech/pegasys/teku/storage/client/ChainUpdater.java +++ b/storage/src/testFixtures/java/tech/pegasys/teku/storage/client/ChainUpdater.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static tech.pegasys.teku.infrastructure.time.TimeUtilities.secondsToMillis; +import java.util.List; import java.util.Optional; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; @@ -30,7 +31,6 @@ import tech.pegasys.teku.spec.datastructures.state.Checkpoint; import tech.pegasys.teku.spec.executionlayer.PayloadStatus; import tech.pegasys.teku.spec.generator.ChainBuilder; -import tech.pegasys.teku.spec.generator.ChainBuilder.BlockOptions; import tech.pegasys.teku.spec.schemas.SchemaDefinitionsBellatrix; import tech.pegasys.teku.storage.store.UpdatableStore.StoreTransaction; @@ -143,31 +143,19 @@ public SignedBlockAndState finalizeEpoch(final UInt64 epoch) { } public SignedBlockAndState finalizeCurrentChain() { - final UInt64 chainHeadSlot = recentChainData.getHeadSlot(); - final UInt64 finalizeEpoch = spec.computeEpochAtSlot(chainHeadSlot).max(2); - final UInt64 finalHeadEpoch = finalizeEpoch.plus(3); - final UInt64 finalHeadSlot = spec.computeStartSlotAtEpoch(finalHeadEpoch); - - SignedBlockAndState newChainHead = null; - for (UInt64 slot = chainHeadSlot.plus(1); - slot.isLessThan(finalHeadSlot); - slot = slot.increment()) { - final BlockOptions blockOptions = BlockOptions.create(); - chainBuilder - .streamValidAttestationsForBlockAtSlot(slot) - .forEach(blockOptions::addAttestation); - newChainHead = chainBuilder.generateBlockAtSlot(slot, blockOptions); - saveBlock(newChainHead); - updateBestBlock(newChainHead); - } - final Checkpoint finalizedCheckpoint = newChainHead.getState().getFinalizedCheckpoint(); - assertThat(finalizedCheckpoint.getEpoch()) - .describedAs("Failed to finalize epoch %s", finalizeEpoch) - .isEqualTo(finalizeEpoch); - assertThat(finalizedCheckpoint.getRoot()) - .describedAs("Failed to finalize epoch %s", finalizeEpoch) - .isNotEqualTo(Bytes32.ZERO); - return newChainHead; + final List newBlocks = chainBuilder.finalizeCurrentChain(); + newBlocks.forEach(this::saveBlock); + final SignedBlockAndState newHead = newBlocks.get(newBlocks.size() - 1); + updateBestBlock(newHead); + return newHead; + } + + public SignedBlockAndState finalizeCurrentChainOptimistically() { + final List newBlocks = chainBuilder.finalizeCurrentChain(); + newBlocks.forEach(this::saveOptimisticBlock); + final SignedBlockAndState newHead = newBlocks.get(newBlocks.size() - 1); + updateBestBlock(newHead); + return newHead; } public SignedBlockAndState justifyEpoch(final long epoch) {