diff --git a/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/ForkChoiceTestExecutor.java b/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/ForkChoiceTestExecutor.java index 5ba20ccd6e1..058caca916e 100644 --- a/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/ForkChoiceTestExecutor.java +++ b/eth-reference-tests/src/referenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/ForkChoiceTestExecutor.java @@ -464,14 +464,10 @@ private void applyChecks( case "get_proposer_head" -> { final Bytes32 expectedProposerHead = getBytes32(checks, checkType); - final Optional boostedRoot = recentChainData.getBestBlockRoot(); - if (expectedProposerHead.isZero()) { - assertThat(boostedRoot).describedAs("get_proposer_head").isEmpty(); - } else { - assertThat(boostedRoot) - .describedAs("get_proposer_head") - .contains(expectedProposerHead); - } + final Bytes32 root = + recentChainData.getProposerHead( + expectedProposerHead, recentChainData.getHeadSlot().increment()); + assertThat(root).describedAs("get_proposer_head").isEqualTo(expectedProposerHead); } case "should_override_forkchoice_update" -> { diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/forkchoice/ReadOnlyStore.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/forkchoice/ReadOnlyStore.java index 9e07bf30e31..5d9080c089c 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/forkchoice/ReadOnlyStore.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/forkchoice/ReadOnlyStore.java @@ -136,10 +136,10 @@ SafeFuture> retrieveCheckpointState( Checkpoint checkpoint, BeaconState latestStateAtEpoch); // implements is_head_weak from fork-choice Consensus Spec - SafeFuture> isHeadWeak(final Bytes32 root); + boolean isHeadWeak(BeaconState justifiedState, Bytes32 root); // implements is_parent_strong from fork-choice Consensus Spec - SafeFuture> isParentStrong(final Bytes32 parentRoot); + boolean isParentStrong(BeaconState justifiedState, Bytes32 parentRoot); // implements is_ffg_competitive from Consensus Spec Optional isFfgCompetitive(final Bytes32 headRoot, final Bytes32 parentRoot); diff --git a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/datastructures/forkchoice/TestStoreImpl.java b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/datastructures/forkchoice/TestStoreImpl.java index 574e0149882..3d6fe727c85 100644 --- a/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/datastructures/forkchoice/TestStoreImpl.java +++ b/ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/datastructures/forkchoice/TestStoreImpl.java @@ -247,13 +247,13 @@ public SafeFuture> retrieveCheckpointState( } @Override - public SafeFuture> isHeadWeak(Bytes32 root) { - return SafeFuture.completedFuture(Optional.empty()); + public boolean isHeadWeak(BeaconState justifiedState, Bytes32 root) { + return false; } @Override - public SafeFuture> isParentStrong(Bytes32 parentRoot) { - return SafeFuture.completedFuture(Optional.empty()); + public boolean isParentStrong(BeaconState justifiedState, Bytes32 parentRoot) { + return false; } @Override diff --git a/storage/src/main/java/tech/pegasys/teku/storage/client/RecentChainData.java b/storage/src/main/java/tech/pegasys/teku/storage/client/RecentChainData.java index 27654860a93..c76c02fc245 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/client/RecentChainData.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/client/RecentChainData.java @@ -21,6 +21,7 @@ import java.util.NavigableMap; import java.util.Optional; import java.util.TreeMap; +import java.util.concurrent.CancellationException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.logging.log4j.LogManager; @@ -619,6 +620,74 @@ public List getAllBlockRootsAtSlot(final UInt64 slot) { .orElse(Collections.emptyList()); } + public Bytes32 getProposerHead(final Bytes32 headRoot, final UInt64 slot) { + // if proposer boost is still active, don't attempt to override head + final boolean isProposerBoostActive = + store.getProposerBoostRoot().map(root -> !root.equals(headRoot)).orElse(false); + final boolean isShufflingStable = spec.atSlot(slot).getForkChoiceUtil().isShufflingStable(slot); + final boolean isFinalizationOk = + spec.atSlot(slot).getForkChoiceUtil().isFinalizationOk(store, slot); + final boolean isProposingOnTime = isProposingOnTime(slot); + final boolean isHeadLate = isBlockLate(headRoot); + final Optional maybeHead = store.getBlockIfAvailable(headRoot); + + // to return parent root, we need all of (isHeadLate, isShufflingStable, isFfgCompetetive, + // isFinalizationOk, isProposingOnTime, isSingleSlotReorg, isHeadWeak, isParentStrong) + + // cheap checks of that list: + // (isHeadLate, isShufflingStable, isFinalizationOk, isProposingOnTime); + // and isProposerBoostActive (assert condition); + // finally need head block to make further checks + if (!isHeadLate + || !isShufflingStable + || !isFinalizationOk + || !isProposingOnTime + || isProposerBoostActive + || maybeHead.isEmpty()) { + return headRoot; + } + + final SignedBeaconBlock head = maybeHead.get(); + final boolean isFfgCompetitive = + store.isFfgCompetitive(headRoot, head.getParentRoot()).orElse(false); + + final Optional maybeParentSlot = getSlotForBlockRoot(head.getParentRoot()); + final boolean isParentSlotOk = + maybeParentSlot.map(uInt64 -> uInt64.increment().equals(head.getSlot())).orElse(false); + final boolean isCurrentTimeOk = head.getSlot().increment().equals(slot); + final boolean isSingleSlotReorg = isParentSlotOk && isCurrentTimeOk; + + // from the initial list, check + // isFfgCompetitive, isSingleSlotReorg + if (!isFfgCompetitive || !isSingleSlotReorg) { + return headRoot; + } + + final SafeFuture> future = + store.retrieveCheckpointState(store.getJustifiedCheckpoint()); + try { + final Optional maybeJustifiedState = future.join(); + // to make further checks, we would need the justified state, return headRoot if we don't have + // it. + if (maybeJustifiedState.isEmpty()) { + return headRoot; + } + final boolean isHeadWeak = store.isHeadWeak(maybeJustifiedState.get(), headRoot); + final boolean isParentStrong = + store.isParentStrong(maybeJustifiedState.get(), head.getParentRoot()); + // finally, the parent must be strong, and the current head must be weak. + if (isHeadWeak && isParentStrong) { + return head.getParentRoot(); + } + } catch (Exception exception) { + if (!(exception instanceof CancellationException)) { + LOG.error("Failed to get justified checkpoint", exception); + } + } + + return headRoot; + } + public void setBlockTimelinessFromArrivalTime( final SignedBeaconBlock block, final UInt64 arrivalTime) { blockTimelinessTracker.setBlockTimelinessFromArrivalTime(block, arrivalTime); diff --git a/storage/src/main/java/tech/pegasys/teku/storage/store/Store.java b/storage/src/main/java/tech/pegasys/teku/storage/store/Store.java index e9ae37ef04a..d424072b521 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/store/Store.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/store/Store.java @@ -535,94 +535,55 @@ public Optional getBlockIfAvailable(final Bytes32 blockRoot) } @Override - public SafeFuture> isHeadWeak(final Bytes32 root) { - + public boolean isHeadWeak(final BeaconState justifiedState, final Bytes32 root) { final Optional maybeBlockData = getBlockDataFromForkChoiceStrategy(root); - if (maybeBlockData.isEmpty()) { - LOG.trace("isHeadWeak {}: no block data in protoArray, returning false", root); - return SafeFuture.completedFuture(Optional.empty()); - } - - final UInt64 headWeight = maybeBlockData.get().getWeight(); - - final SafeFuture> futureMaybeJustifiedState = - retrieveCheckpointState(justifiedCheckpoint); - - return futureMaybeJustifiedState - .thenApply( - maybeJustifiedState -> - maybeJustifiedState.map( - justifiedState -> { - final SpecVersion specVersion = spec.atSlot(justifiedState.getSlot()); - final BeaconStateAccessors beaconStateAccessors = - specVersion.beaconStateAccessors(); - final UInt64 reorgThreashold = - beaconStateAccessors.calculateCommitteeFraction( - justifiedState, - specVersion.getConfig().getReorgHeadWeightThreshold()); - final boolean result = headWeight.isLessThan(reorgThreashold); - - LOG.trace( - "isHeadWeak {}: headWeight: {}, reorgThreshold: {}, result: {}", - root, - headWeight, - reorgThreashold, - result); - return result; - })) - .exceptionallyCompose( - err -> { - LOG.error( - "Failed to retrieve justified state when checking head weight for block {}.", + return maybeBlockData + .map( + blockData -> { + final UInt64 headWeight = blockData.getWeight(); + + final SpecVersion specVersion = spec.atSlot(justifiedState.getSlot()); + final BeaconStateAccessors beaconStateAccessors = specVersion.beaconStateAccessors(); + final UInt64 reorgThreshold = + beaconStateAccessors.calculateCommitteeFraction( + justifiedState, specVersion.getConfig().getReorgHeadWeightThreshold()); + final boolean result = headWeight.isLessThan(reorgThreshold); + + LOG.trace( + "isHeadWeak {}: headWeight: {}, reorgThreshold: {}, result: {}", root, - err); - return SafeFuture.completedFuture(Optional.empty()); - }); + headWeight, + reorgThreshold, + result); + return result; + }) + .orElse(false); } @Override - public SafeFuture> isParentStrong(final Bytes32 parentRoot) { + public boolean isParentStrong(final BeaconState justifiedState, final Bytes32 parentRoot) { final Optional maybeBlockData = getBlockDataFromForkChoiceStrategy(parentRoot); - if (maybeBlockData.isEmpty()) { - LOG.trace("isParentStrong {}: no block data in protoArray, returning false", parentRoot); - return SafeFuture.completedFuture(Optional.empty()); - } - - final UInt64 parentWeight = maybeBlockData.get().getWeight(); - - final SafeFuture> futureMaybeJustifiedState = - retrieveCheckpointState(justifiedCheckpoint); - - return futureMaybeJustifiedState - .thenApply( - maybeJustifiedState -> - maybeJustifiedState.map( - justifiedState -> { - final SpecVersion specVersion = spec.atSlot(justifiedState.getSlot()); - final BeaconStateAccessors beaconStateAccessors = - specVersion.beaconStateAccessors(); - final UInt64 parentThreshold = - beaconStateAccessors.calculateCommitteeFraction( - justifiedState, - specVersion.getConfig().getReorgParentWeightThreshold()); - final boolean result = parentWeight.isGreaterThan(parentThreshold); - - LOG.trace( - "isParentStrong {}: parentWeight: {}, parentThreshold: {}, result: {}", - parentRoot, - parentWeight, - parentThreshold, - result); - return result; - })) - .exceptionallyCompose( - err -> { - LOG.error( - "Failed to retrieve justified state when checking weight for parent {}.", + return maybeBlockData + .map( + blockData -> { + final UInt64 parentWeight = blockData.getWeight(); + + final SpecVersion specVersion = spec.atSlot(justifiedState.getSlot()); + final BeaconStateAccessors beaconStateAccessors = specVersion.beaconStateAccessors(); + final UInt64 parentThreshold = + beaconStateAccessors.calculateCommitteeFraction( + justifiedState, specVersion.getConfig().getReorgParentWeightThreshold()); + final boolean result = parentWeight.isGreaterThan(parentThreshold); + + LOG.debug( + "isParentStrong {}: parentWeight: {}, parentThreshold: {}, result: {}", parentRoot, - err); - return SafeFuture.completedFuture(Optional.empty()); - }); + parentWeight, + parentThreshold, + result); + return result; + }) + .orElse(false); } @Override diff --git a/storage/src/main/java/tech/pegasys/teku/storage/store/StoreTransaction.java b/storage/src/main/java/tech/pegasys/teku/storage/store/StoreTransaction.java index 595e502b1f5..51d5286bbbd 100644 --- a/storage/src/main/java/tech/pegasys/teku/storage/store/StoreTransaction.java +++ b/storage/src/main/java/tech/pegasys/teku/storage/store/StoreTransaction.java @@ -452,13 +452,13 @@ public SafeFuture> retrieveCheckpointState( } @Override - public SafeFuture> isHeadWeak(Bytes32 root) { - return store.isHeadWeak(root); + public boolean isHeadWeak(BeaconState justifiedState, Bytes32 root) { + return store.isHeadWeak(justifiedState, root); } @Override - public SafeFuture> isParentStrong(Bytes32 parentRoot) { - return store.isParentStrong(parentRoot); + public boolean isParentStrong(BeaconState justifiedState, Bytes32 parentRoot) { + return store.isParentStrong(justifiedState, parentRoot); } @Override diff --git a/storage/src/test/java/tech/pegasys/teku/storage/store/StoreTest.java b/storage/src/test/java/tech/pegasys/teku/storage/store/StoreTest.java index 3e2170c0458..cdb241b071d 100644 --- a/storage/src/test/java/tech/pegasys/teku/storage/store/StoreTest.java +++ b/storage/src/test/java/tech/pegasys/teku/storage/store/StoreTest.java @@ -100,8 +100,7 @@ public void isHeadWeak_withoutNodeData() { processChainHeadWithMockForkChoiceStrategy( (store, blockAndState) -> { final Bytes32 root = blockAndState.getRoot(); - final SafeFuture> isHeadWeakFuture = store.isHeadWeak(root); - assertThat(safeJoin(isHeadWeakFuture)).isEmpty(); + assertThat(store.isHeadWeak(justifiedState(store), root)).isFalse(); }); } @@ -112,8 +111,7 @@ public void isHeadWeak_withSufficientWeightIsFalse() { final Bytes32 root = blockAndState.getRoot(); setProtoNodeDataForBlock(blockAndState, UInt64.valueOf("2400000001"), UInt64.MAX_VALUE); - final SafeFuture> isHeadWeakFuture = store.isHeadWeak(root); - assertThat(safeJoin(isHeadWeakFuture)).contains(false); + assertThat(store.isHeadWeak(justifiedState(store), root)).isFalse(); }); } @@ -124,8 +122,7 @@ public void isHeadWeak_Boundary() { final Bytes32 root = blockAndState.getRoot(); setProtoNodeDataForBlock(blockAndState, UInt64.valueOf("2399999999"), UInt64.MAX_VALUE); - final SafeFuture> isHeadWeakFuture = store.isHeadWeak(root); - assertThat(safeJoin(isHeadWeakFuture)).contains(true); + assertThat(store.isHeadWeak(justifiedState(store), root)).isTrue(); }); } @@ -136,8 +133,7 @@ public void isHeadWeak_withLowWeightIsTrue() { final Bytes32 root = blockAndState.getRoot(); setProtoNodeDataForBlock(blockAndState, UInt64.valueOf("1000000000"), UInt64.MAX_VALUE); - final SafeFuture> isHeadWeakFuture = store.isHeadWeak(root); - assertThat(safeJoin(isHeadWeakFuture)).contains(true); + assertThat(store.isHeadWeak(justifiedState(store), root)).isTrue(); }); } @@ -146,8 +142,7 @@ public void isParentStrong_withoutNodeData() { processChainHeadWithMockForkChoiceStrategy( (store, blockAndState) -> { final Bytes32 root = blockAndState.getBlock().getParentRoot(); - final SafeFuture> isParentStrongFuture = store.isParentStrong(root); - assertThat(safeJoin(isParentStrongFuture)).isEmpty(); + assertThat(store.isParentStrong(justifiedState(store), root)).isFalse(); }); } @@ -157,8 +152,7 @@ public void isParentStrong_withSufficientWeight() { (store, blockAndState) -> { final Bytes32 root = blockAndState.getBlock().getParentRoot(); setProtoNodeDataForBlock(blockAndState, UInt64.ZERO, UInt64.valueOf("19200000001")); - final SafeFuture> isParentStrongFuture = store.isParentStrong(root); - assertThat(safeJoin(isParentStrongFuture)).contains(true); + assertThat(store.isParentStrong(justifiedState(store), root)).isTrue(); }); } @@ -168,8 +162,7 @@ public void isParentStrong_wityBoundaryWeight() { (store, blockAndState) -> { final Bytes32 root = blockAndState.getBlock().getParentRoot(); setProtoNodeDataForBlock(blockAndState, UInt64.ZERO, UInt64.valueOf("19200000000")); - final SafeFuture> isParentStrongFuture = store.isParentStrong(root); - assertThat(safeJoin(isParentStrongFuture)).contains(false); + assertThat(store.isParentStrong(justifiedState(store), root)).isFalse(); }); } @@ -179,8 +172,7 @@ public void isParentStrong_wityZeroWeight() { (store, blockAndState) -> { final Bytes32 root = blockAndState.getBlock().getParentRoot(); setProtoNodeDataForBlock(blockAndState, UInt64.ZERO, UInt64.ZERO); - final SafeFuture> isParentStrongFuture = store.isParentStrong(root); - assertThat(safeJoin(isParentStrongFuture)).contains(false); + assertThat(store.isParentStrong(justifiedState(store), root)).isFalse(); }); } @@ -525,6 +517,10 @@ public void shouldKeepOnlyMostRecentBlocksInBlockCache() { } } + private BeaconState justifiedState(final UpdatableStore store) { + return safeJoin(store.retrieveCheckpointState(store.getJustifiedCheckpoint())).orElseThrow(); + } + private void testApplyChangesWhenTransactionCommits(final boolean withInterleavedTransaction) { final UpdatableStore store = createGenesisStore(); final UInt64 epoch3 = UInt64.valueOf(4);