Skip to content

Commit

Permalink
Implemented get_proposer_head for late block reorg (Consensys#7753)
Browse files Browse the repository at this point in the history 
Implemented get_proposer_head function, and tested results via the reftests.

It's going to be more efficient to get the justified state once, so have refactored isHeadWeak and isParentStrong.

The ordering of checks was cheapest -> most expensive, we can avoid fetching state if any of the lighter tests fail.

partially addresses Consensys#6595

Signed-off-by: Paul Harris <[email protected]>
  • Loading branch information
@rolfyone
rolfyone authored Nov 27, 2023
1 parent 992b224 commit a51b243
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 115 deletions.
12 changes: 4 additions & 8 deletions ...erenceTest/java/tech/pegasys/teku/reference/phase0/forkchoice/ForkChoiceTestExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,10 @@ private void applyChecks(

case "get_proposer_head" -> {
final Bytes32 expectedProposerHead = getBytes32(checks, checkType);
final Optional<Bytes32> 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" -> {
Expand Down
4 changes: 2 additions & 2 deletions ...um/spec/src/main/java/tech/pegasys/teku/spec/datastructures/forkchoice/ReadOnlyStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ SafeFuture<Optional<BeaconState>> retrieveCheckpointState(
Checkpoint checkpoint, BeaconState latestStateAtEpoch);

// implements is_head_weak from fork-choice Consensus Spec
SafeFuture<Optional<Boolean>> isHeadWeak(final Bytes32 root);
boolean isHeadWeak(BeaconState justifiedState, Bytes32 root);

// implements is_parent_strong from fork-choice Consensus Spec
SafeFuture<Optional<Boolean>> isParentStrong(final Bytes32 parentRoot);
boolean isParentStrong(BeaconState justifiedState, Bytes32 parentRoot);

// implements is_ffg_competitive from Consensus Spec
Optional<Boolean> isFfgCompetitive(final Bytes32 headRoot, final Bytes32 parentRoot);
Expand Down
8 changes: 4 additions & 4 deletions ...src/testFixtures/java/tech/pegasys/teku/spec/datastructures/forkchoice/TestStoreImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,13 @@ public SafeFuture<Optional<BeaconState>> retrieveCheckpointState(
}

@Override
public SafeFuture<Optional<Boolean>> isHeadWeak(Bytes32 root) {
return SafeFuture.completedFuture(Optional.empty());
public boolean isHeadWeak(BeaconState justifiedState, Bytes32 root) {
return false;
}

@Override
public SafeFuture<Optional<Boolean>> isParentStrong(Bytes32 parentRoot) {
return SafeFuture.completedFuture(Optional.empty());
public boolean isParentStrong(BeaconState justifiedState, Bytes32 parentRoot) {
return false;
}

@Override
Expand Down
69 changes: 69 additions & 0 deletions storage/src/main/java/tech/pegasys/teku/storage/client/RecentChainData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -619,6 +620,74 @@ public List<Bytes32> 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<SignedBeaconBlock> 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<UInt64> 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<Optional<BeaconState>> future =
store.retrieveCheckpointState(store.getJustifiedCheckpoint());
try {
final Optional<BeaconState> 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);
Expand Down
123 changes: 42 additions & 81 deletions storage/src/main/java/tech/pegasys/teku/storage/store/Store.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,94 +535,55 @@ public Optional<SignedBeaconBlock> getBlockIfAvailable(final Bytes32 blockRoot)
}

@Override
public SafeFuture<Optional<Boolean>> isHeadWeak(final Bytes32 root) {

public boolean isHeadWeak(final BeaconState justifiedState, final Bytes32 root) {
final Optional<ProtoNodeData> 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<Optional<BeaconState>> 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<Optional<Boolean>> isParentStrong(final Bytes32 parentRoot) {
public boolean isParentStrong(final BeaconState justifiedState, final Bytes32 parentRoot) {
final Optional<ProtoNodeData> 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<Optional<BeaconState>> 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
Expand Down
8 changes: 4 additions & 4 deletions storage/src/main/java/tech/pegasys/teku/storage/store/StoreTransaction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,13 @@ public SafeFuture<Optional<BeaconState>> retrieveCheckpointState(
}

@Override
public SafeFuture<Optional<Boolean>> isHeadWeak(Bytes32 root) {
return store.isHeadWeak(root);
public boolean isHeadWeak(BeaconState justifiedState, Bytes32 root) {
return store.isHeadWeak(justifiedState, root);
}

@Override
public SafeFuture<Optional<Boolean>> isParentStrong(Bytes32 parentRoot) {
return store.isParentStrong(parentRoot);
public boolean isParentStrong(BeaconState justifiedState, Bytes32 parentRoot) {
return store.isParentStrong(justifiedState, parentRoot);
}

@Override
Expand Down
28 changes: 12 additions & 16 deletions storage/src/test/java/tech/pegasys/teku/storage/store/StoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ public void isHeadWeak_withoutNodeData() {
processChainHeadWithMockForkChoiceStrategy(
(store, blockAndState) -> {
final Bytes32 root = blockAndState.getRoot();
final SafeFuture<Optional<Boolean>> isHeadWeakFuture = store.isHeadWeak(root);
assertThat(safeJoin(isHeadWeakFuture)).isEmpty();
assertThat(store.isHeadWeak(justifiedState(store), root)).isFalse();
});
}

Expand All @@ -112,8 +111,7 @@ public void isHeadWeak_withSufficientWeightIsFalse() {
final Bytes32 root = blockAndState.getRoot();
setProtoNodeDataForBlock(blockAndState, UInt64.valueOf("2400000001"), UInt64.MAX_VALUE);

final SafeFuture<Optional<Boolean>> isHeadWeakFuture = store.isHeadWeak(root);
assertThat(safeJoin(isHeadWeakFuture)).contains(false);
assertThat(store.isHeadWeak(justifiedState(store), root)).isFalse();
});
}

Expand All @@ -124,8 +122,7 @@ public void isHeadWeak_Boundary() {
final Bytes32 root = blockAndState.getRoot();
setProtoNodeDataForBlock(blockAndState, UInt64.valueOf("2399999999"), UInt64.MAX_VALUE);

final SafeFuture<Optional<Boolean>> isHeadWeakFuture = store.isHeadWeak(root);
assertThat(safeJoin(isHeadWeakFuture)).contains(true);
assertThat(store.isHeadWeak(justifiedState(store), root)).isTrue();
});
}

Expand All @@ -136,8 +133,7 @@ public void isHeadWeak_withLowWeightIsTrue() {
final Bytes32 root = blockAndState.getRoot();
setProtoNodeDataForBlock(blockAndState, UInt64.valueOf("1000000000"), UInt64.MAX_VALUE);

final SafeFuture<Optional<Boolean>> isHeadWeakFuture = store.isHeadWeak(root);
assertThat(safeJoin(isHeadWeakFuture)).contains(true);
assertThat(store.isHeadWeak(justifiedState(store), root)).isTrue();
});
}

Expand All @@ -146,8 +142,7 @@ public void isParentStrong_withoutNodeData() {
processChainHeadWithMockForkChoiceStrategy(
(store, blockAndState) -> {
final Bytes32 root = blockAndState.getBlock().getParentRoot();
final SafeFuture<Optional<Boolean>> isParentStrongFuture = store.isParentStrong(root);
assertThat(safeJoin(isParentStrongFuture)).isEmpty();
assertThat(store.isParentStrong(justifiedState(store), root)).isFalse();
});
}

Expand All @@ -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<Optional<Boolean>> isParentStrongFuture = store.isParentStrong(root);
assertThat(safeJoin(isParentStrongFuture)).contains(true);
assertThat(store.isParentStrong(justifiedState(store), root)).isTrue();
});
}

Expand All @@ -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<Optional<Boolean>> isParentStrongFuture = store.isParentStrong(root);
assertThat(safeJoin(isParentStrongFuture)).contains(false);
assertThat(store.isParentStrong(justifiedState(store), root)).isFalse();
});
}

Expand All @@ -179,8 +172,7 @@ public void isParentStrong_wityZeroWeight() {
(store, blockAndState) -> {
final Bytes32 root = blockAndState.getBlock().getParentRoot();
setProtoNodeDataForBlock(blockAndState, UInt64.ZERO, UInt64.ZERO);
final SafeFuture<Optional<Boolean>> isParentStrongFuture = store.isParentStrong(root);
assertThat(safeJoin(isParentStrongFuture)).contains(false);
assertThat(store.isParentStrong(justifiedState(store), root)).isFalse();
});
}

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a51b243

Please sign in to comment.