Skip to content

Commit

Permalink
[Issue-2438] Be more careful about disabling validator duties while "…
Browse files Browse the repository at this point in the history
…syncing" (#2555)
  • Loading branch information
mbaxter authored Aug 11, 2020
1 parent c272a47 commit b1b6390
Show file tree
Hide file tree
Showing 31 changed files with 261 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public Optional<BeaconHead> getBeaconHead() {
throw new ChainDataUnavailableException();
}

return recentChainData.getBestBlockAndState().map(BeaconHead::new);
return recentChainData.getHeadBlockAndState().map(BeaconHead::new);
}

public GetForkResponse getForkInfo() {
Expand All @@ -84,7 +84,7 @@ public SafeFuture<Optional<List<Committee>>> getCommitteesAtEpoch(final UInt64 e

final UInt64 earliestQueryableSlot =
CommitteeUtil.getEarliestQueryableSlotForTargetEpoch(epoch);
if (recentChainData.getBestSlot().isLessThan(earliestQueryableSlot)) {
if (recentChainData.getHeadSlot().isLessThan(earliestQueryableSlot)) {
return SafeFuture.completedFuture(Optional.empty());
}

Expand Down Expand Up @@ -184,7 +184,7 @@ public SafeFuture<Optional<BeaconValidators>> getValidatorsByValidatorsRequest(
() -> {
UInt64 slot =
request.epoch == null
? combinedChainDataClient.getBestSlot()
? combinedChainDataClient.getHeadSlot()
: BeaconStateUtil.compute_start_slot_at_epoch(request.epoch);

return combinedChainDataClient
Expand Down Expand Up @@ -212,6 +212,6 @@ public Optional<BeaconChainHead> getHeadState() {
if (!isStoreAvailable()) {
throw new ChainDataUnavailableException();
}
return recentChainData.getBestBlockAndState().map(BeaconChainHead::new);
return recentChainData.getHeadBlockAndState().map(BeaconChainHead::new);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public SafeFuture<Optional<BeaconBlock>> getUnsignedBeaconBlockAtSlot(
if (randao == null) {
throw new IllegalArgumentException(NO_RANDAO_PROVIDED);
}
UInt64 bestSlot = combinedChainDataClient.getBestSlot();
UInt64 bestSlot = combinedChainDataClient.getHeadSlot();
if (bestSlot.plus(SLOTS_PER_EPOCH).isLessThan(slot)) {
throw new IllegalArgumentException(CANNOT_PRODUCE_FAR_FUTURE_BLOCK);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void getCommitteeAssignmentAtEpoch_shouldReturnAListOfCommittees()
.thenReturn(Optional.of(bestBlock.getRoot()));
when(mockCombinedChainDataClient.getCommitteesFromState(any(), any()))
.thenReturn(committeeAssignments);
when(mockRecentChainData.getBestSlot()).thenReturn(bestBlock.getSlot());
when(mockRecentChainData.getHeadSlot()).thenReturn(bestBlock.getSlot());
when(mockCombinedChainDataClient.getCheckpointStateAtEpoch(any()))
.thenReturn(SafeFuture.completedFuture(Optional.of(checkpointState)));
final SafeFuture<Optional<List<Committee>>> future =
Expand Down Expand Up @@ -177,7 +177,7 @@ public void getBeaconHead_shouldReturnPopulatedBeaconHead() {
final BeaconHead head = optionalBeaconHead.get();
assertEquals(blockRoot, head.block_root);
assertEquals(beaconStateInternal.hash_tree_root(), head.state_root);
assertEquals(recentChainData.getBestSlot(), head.slot);
assertEquals(recentChainData.getHeadSlot(), head.slot);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ void getUnsignedBeaconBlockAtSlot_shouldThrowWithoutRandaoDefined() {

@Test
void getUnsignedBeaconBlockAtSlot_shouldThrowIfHistoricSlotRequested() {
when(combinedChainDataClient.getBestSlot()).thenReturn(ONE);
when(combinedChainDataClient.getHeadSlot()).thenReturn(ONE);

assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> provider.getUnsignedBeaconBlockAtSlot(ZERO, signature, Optional.empty()));
}

@Test
void getUnsignedBeaconBlockAtSlot_shouldThrowIfFarFutureSlotRequested() {
when(combinedChainDataClient.getBestSlot()).thenReturn(ONE);
when(combinedChainDataClient.getHeadSlot()).thenReturn(ONE);

assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(
Expand All @@ -113,7 +113,7 @@ void getUnsignedBeaconBlockAtSlot_shouldThrowIfFarFutureSlotRequested() {

@Test
void getUnsignedBeaconBlockAtSlot_shouldCreateAnUnsignedBlock() {
when(combinedChainDataClient.getBestSlot()).thenReturn(ZERO);
when(combinedChainDataClient.getHeadSlot()).thenReturn(ZERO);
when(validatorApiChannel.createUnsignedBlock(ONE, signatureInternal, Optional.empty()))
.thenReturn(SafeFuture.completedFuture(Optional.of(blockInternal)));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void generateBlocks() throws Exception {
localChain.initializeStorage();
AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);

UInt64 currentSlot = localStorage.getBestSlot();
UInt64 currentSlot = localStorage.getHeadSlot();
List<Attestation> attestations = Collections.emptyList();

String blocksFile =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private synchronized void processHead(Optional<UInt64> nodeSlot) {
justifiedCheckpointState.orElseThrow());
transaction.commit(() -> {}, "Failed to persist validator vote changes.");

recentChainData.updateBestBlock(
recentChainData.updateHead(
headBlockRoot,
nodeSlot.orElse(
forkChoiceStrategy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void importBlock_latestFinalizedBlock() throws Exception {
Constants.SLOTS_PER_EPOCH = 6;

final List<SignedBeaconBlock> blocks = new ArrayList<>();
UInt64 currentSlot = recentChainData.getBestSlot();
UInt64 currentSlot = recentChainData.getHeadSlot();
for (int i = 0; i < Constants.SLOTS_PER_EPOCH; i++) {
currentSlot = currentSlot.plus(UInt64.ONE);
final SignedBeaconBlock block = localChain.createAndImportBlockAtSlot(currentSlot);
Expand All @@ -167,7 +167,7 @@ public void importBlock_latestFinalizedBlock() throws Exception {
// Update finalized epoch
final StoreTransaction tx = recentChainData.startStoreTransaction();
final Bytes32 bestRoot = recentChainData.getBestBlockRoot().orElseThrow();
final UInt64 bestEpoch = compute_epoch_at_slot(recentChainData.getBestSlot());
final UInt64 bestEpoch = compute_epoch_at_slot(recentChainData.getHeadSlot());
assertThat(bestEpoch.longValue()).isEqualTo(Constants.GENESIS_EPOCH + 1L);
final Checkpoint finalized = new Checkpoint(bestEpoch, bestRoot);
tx.setFinalizedCheckpoint(finalized);
Expand All @@ -183,7 +183,7 @@ public void importBlock_knownBlockOlderThanLatestFinalized() throws Exception {
Constants.SLOTS_PER_EPOCH = 6;

final List<SignedBeaconBlock> blocks = new ArrayList<>();
UInt64 currentSlot = recentChainData.getBestSlot();
UInt64 currentSlot = recentChainData.getHeadSlot();
for (int i = 0; i < Constants.SLOTS_PER_EPOCH; i++) {
currentSlot = currentSlot.plus(UInt64.ONE);
final SignedBeaconBlock block = localChain.createAndImportBlockAtSlot(currentSlot);
Expand All @@ -193,7 +193,7 @@ public void importBlock_knownBlockOlderThanLatestFinalized() throws Exception {
// Update finalized epoch
final StoreTransaction tx = recentChainData.startStoreTransaction();
final Bytes32 bestRoot = recentChainData.getBestBlockRoot().orElseThrow();
final UInt64 bestEpoch = compute_epoch_at_slot(recentChainData.getBestSlot());
final UInt64 bestEpoch = compute_epoch_at_slot(recentChainData.getHeadSlot());
assertThat(bestEpoch.longValue()).isEqualTo(Constants.GENESIS_EPOCH + 1L);
final Checkpoint finalized = new Checkpoint(bestEpoch, bestRoot);
tx.setFinalizedCheckpoint(finalized);
Expand Down Expand Up @@ -263,7 +263,7 @@ public void importBlock_unknownParent() throws Exception {

@Test
public void importBlock_wrongChain() throws Exception {
UInt64 currentSlot = recentChainData.getBestSlot();
UInt64 currentSlot = recentChainData.getHeadSlot();
for (int i = 0; i < 3; i++) {
currentSlot = currentSlot.plus(UInt64.ONE);
localChain.createAndImportBlockAtSlot(currentSlot);
Expand All @@ -278,7 +278,7 @@ public void importBlock_wrongChain() throws Exception {

// Now create a new block that is not descendent from the finalized block
AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState blockAndState = otherStorage.getBestBlockAndState().orElseThrow();
final BeaconBlockAndState blockAndState = otherStorage.getHeadBlockAndState().orElseThrow();
final Attestation attestation = attestationGenerator.validAttestation(blockAndState);
final SignedBeaconBlock block =
otherChain.createAndImportBlockAtSlotWithAttestations(currentSlot, List.of(attestation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private SignedBlockAndState createBlockAndStateAtSlot(
withValidProposer || validatorKeys.size() > 1,
"Must have >1 validator in order to create a block from an invalid proposer.");
final BeaconBlockAndState bestBlockAndState =
recentChainData.getBestBlockAndState().orElseThrow();
recentChainData.getHeadBlockAndState().orElseThrow();
final Bytes32 bestBlockRoot = bestBlockAndState.getRoot();
final BeaconBlock bestBlock = bestBlockAndState.getBlock();
final BeaconState preState = bestBlockAndState.getState();
Expand All @@ -259,22 +259,22 @@ public void finalizeChainAtEpoch(final UInt64 epoch) throws Exception {
}

AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
createAndImportBlockAtSlot(recentChainData.getBestSlot().plus(MIN_ATTESTATION_INCLUSION_DELAY));
createAndImportBlockAtSlot(recentChainData.getHeadSlot().plus(MIN_ATTESTATION_INCLUSION_DELAY));

while (recentChainData.getStore().getFinalizedCheckpoint().getEpoch().compareTo(epoch) < 0) {

BeaconState headState =
recentChainData.getBestBlockAndState().map(BeaconBlockAndState::getState).orElseThrow();
recentChainData.getHeadBlockAndState().map(BeaconBlockAndState::getState).orElseThrow();
BeaconBlock headBlock =
recentChainData.getBestBlock().map(SignedBeaconBlock::getMessage).orElseThrow();
UInt64 slot = recentChainData.getBestSlot();
recentChainData.getHeadBlock().map(SignedBeaconBlock::getMessage).orElseThrow();
UInt64 slot = recentChainData.getHeadSlot();
SSZList<Attestation> currentSlotAssignments =
SSZList.createMutable(
attestationGenerator.getAttestationsForSlot(headState, headBlock, slot),
Constants.MAX_ATTESTATIONS,
Attestation.class);
createAndImportBlockAtSlot(
recentChainData.getBestSlot().plus(UInt64.ONE),
recentChainData.getHeadSlot().plus(UInt64.ONE),
Optional.of(currentSlotAssignments),
Optional.empty(),
Optional.empty(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ public void shouldSendEmptyResponseWhenNoBlocksAreAvailable() throws Exception {
public void shouldRespondWithBlocksFromCanonicalChain() throws Exception {
final SignedBeaconBlock block1 = beaconChainUtil.createAndImportBlockAtSlot(1);
final Bytes32 block1Root = block1.getMessage().hash_tree_root();
recentChainData1.updateBestBlock(block1Root, block1.getSlot());
recentChainData1.updateHead(block1Root, block1.getSlot());

final SignedBeaconBlock block2 = beaconChainUtil.createAndImportBlockAtSlot(2);
final Bytes32 block2Root = block2.getMessage().hash_tree_root();
recentChainData1.updateBestBlock(block2Root, block2.getSlot());
recentChainData1.updateHead(block2Root, block2.getSlot());

final List<SignedBeaconBlock> response = requestBlocks();
assertThat(response).containsExactly(block1, block2);
Expand All @@ -111,7 +111,7 @@ public void requestBlocksByRangeAfterPeerDisconnectedImmediately() throws Except
beaconChainUtil.createAndImportBlockAtSlot(1);
final SignedBeaconBlock block2 = beaconChainUtil.createAndImportBlockAtSlot(2);
final Bytes32 block2Root = block2.getMessage().hash_tree_root();
recentChainData1.updateBestBlock(block2Root, block2.getSlot());
recentChainData1.updateHead(block2Root, block2.getSlot());

peer1.disconnectImmediately(Optional.empty(), false);
final List<SignedBeaconBlock> blocks = new ArrayList<>();
Expand All @@ -131,7 +131,7 @@ public void requestBlocksByRangeAfterPeerDisconnected() throws Exception {
beaconChainUtil.createAndImportBlockAtSlot(1);
final SignedBeaconBlock block2 = beaconChainUtil.createAndImportBlockAtSlot(2);
final Bytes32 block2Root = block2.getMessage().hash_tree_root();
recentChainData1.updateBestBlock(block2Root, block2.getSlot());
recentChainData1.updateHead(block2Root, block2.getSlot());

peer1.disconnectCleanly(DisconnectReason.TOO_MANY_PEERS);
final List<SignedBeaconBlock> blocks = new ArrayList<>();
Expand All @@ -151,7 +151,7 @@ public void requestBlockBySlotAfterPeerDisconnectedImmediately() throws Exceptio
beaconChainUtil.createAndImportBlockAtSlot(1);
final SignedBeaconBlock block2 = beaconChainUtil.createAndImportBlockAtSlot(2);
final Bytes32 block2Root = block2.getMessage().hash_tree_root();
recentChainData1.updateBestBlock(block2Root, block2.getSlot());
recentChainData1.updateHead(block2Root, block2.getSlot());

peer1.disconnectImmediately(Optional.empty(), false);
final SafeFuture<SignedBeaconBlock> res = peer1.requestBlockBySlot(UInt64.ONE);
Expand All @@ -167,7 +167,7 @@ public void requestBlockByRootAfterPeerDisconnected() throws Exception {
beaconChainUtil.createAndImportBlockAtSlot(1);
final SignedBeaconBlock block2 = beaconChainUtil.createAndImportBlockAtSlot(2);
final Bytes32 block2Root = block2.getMessage().hash_tree_root();
recentChainData1.updateBestBlock(block2Root, block2.getSlot());
recentChainData1.updateHead(block2Root, block2.getSlot());

peer1.disconnectCleanly(DisconnectReason.TOO_MANY_PEERS);
final SafeFuture<SignedBeaconBlock> res = peer1.requestBlockBySlot(UInt64.ONE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void shouldNotGossipAttestationsAcrossPeersThatAreNotOnTheSameSubnet(
// Propagate attestation from network 1
AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState bestBlockAndState =
node1.storageClient().getBestBlockAndState().orElseThrow();
node1.storageClient().getHeadBlockAndState().orElseThrow();
Attestation validAttestation = attestationGenerator.validAttestation(bestBlockAndState);
processedAttestationSubscribers.forEach(
s -> s.accept(ValidateableAttestation.fromAttestation(validAttestation)));
Expand Down Expand Up @@ -236,7 +236,7 @@ public void shouldGossipAttestationsAcrossPeersThatAreOnTheSameSubnet(
// Propagate attestation from network 1
AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState bestBlockAndState =
node1.storageClient().getBestBlockAndState().orElseThrow();
node1.storageClient().getHeadBlockAndState().orElseThrow();
ValidateableAttestation validAttestation =
ValidateableAttestation.fromAttestation(
attestationGenerator.validAttestation(bestBlockAndState));
Expand Down Expand Up @@ -298,7 +298,7 @@ public void shouldNotGossipAttestationsWhenPeerDeregistersFromTopic(
// Propagate attestation from network 1
AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState bestBlockAndState =
node1.storageClient().getBestBlockAndState().orElseThrow();
node1.storageClient().getHeadBlockAndState().orElseThrow();
ValidateableAttestation validAttestation =
ValidateableAttestation.fromAttestation(
attestationGenerator.validAttestation(bestBlockAndState));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private void assertStatusMatchesStorage(
state.getFinalized_checkpoint().getRoot(),
state.getFinalized_checkpoint().getEpoch(),
storageClient.getBestBlockRoot().orElseThrow(),
storageClient.getBestSlot());
storageClient.getHeadSlot());
}

private void assertStatus(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public Optional<StatusMessage> createStatusMessage() {
}

final BeaconBlockAndState bestBlockAndState =
recentChainData.getBestBlockAndState().orElseThrow();
recentChainData.getHeadBlockAndState().orElseThrow();
final ForkInfo forkInfo = recentChainData.getForkInfoAtCurrentTime().orElseThrow();
final Checkpoint finalizedCheckpoint = bestBlockAndState.getState().getFinalized_checkpoint();
final BeaconBlock chainHead = bestBlockAndState.getBlock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void setup() {

@Test
public void handleMessage_validBlock() throws Exception {
final UInt64 nextSlot = recentChainData.getBestSlot().plus(UInt64.ONE);
final UInt64 nextSlot = recentChainData.getHeadSlot().plus(UInt64.ONE);
final SignedBeaconBlock block = beaconChainUtil.createBlockAtSlot(nextSlot);
Bytes serialized = gossipEncoding.encode(block);
beaconChainUtil.setSlot(nextSlot);
Expand All @@ -69,10 +69,10 @@ public void handleMessage_validBlock() throws Exception {

@Test
public void handleMessage_validFutureBlock() throws Exception {
final UInt64 nextSlot = recentChainData.getBestSlot().plus(UInt64.ONE);
final UInt64 nextSlot = recentChainData.getHeadSlot().plus(UInt64.ONE);
final SignedBeaconBlock block = beaconChainUtil.createBlockAtSlot(nextSlot);
Bytes serialized = gossipEncoding.encode(block);
beaconChainUtil.setSlot(recentChainData.getBestSlot());
beaconChainUtil.setSlot(recentChainData.getHeadSlot());

final ValidationResult result = topicHandler.handleMessage(serialized).join();
assertThat(result).isEqualTo(ValidationResult.Ignore);
Expand All @@ -99,7 +99,7 @@ public void handleMessage_invalidBlock_invalidSSZ() {

@Test
public void handleMessage_invalidBlock_wrongProposer() throws Exception {
final UInt64 nextSlot = recentChainData.getBestSlot().plus(UInt64.ONE);
final UInt64 nextSlot = recentChainData.getHeadSlot().plus(UInt64.ONE);
final SignedBeaconBlock block = beaconChainUtil.createBlockAtSlotFromInvalidProposer(nextSlot);
Bytes serialized = gossipEncoding.encode(block);
beaconChainUtil.setSlot(nextSlot);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void setup() {
@Test
public void handleMessage_valid() {
final AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState blockAndState = recentChainData.getBestBlockAndState().orElseThrow();
final BeaconBlockAndState blockAndState = recentChainData.getHeadBlockAndState().orElseThrow();
final ValidateableAttestation attestation =
ValidateableAttestation.fromAttestation(
attestationGenerator.validAttestation(blockAndState));
Expand All @@ -90,7 +90,7 @@ public void handleMessage_valid() {
@Test
public void handleMessage_ignored() {
final AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState blockAndState = recentChainData.getBestBlockAndState().orElseThrow();
final BeaconBlockAndState blockAndState = recentChainData.getHeadBlockAndState().orElseThrow();
final ValidateableAttestation attestation =
ValidateableAttestation.fromAttestation(
attestationGenerator.validAttestation(blockAndState));
Expand All @@ -106,7 +106,7 @@ public void handleMessage_ignored() {
@Test
public void handleMessage_saveForFuture() {
final AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState blockAndState = recentChainData.getBestBlockAndState().orElseThrow();
final BeaconBlockAndState blockAndState = recentChainData.getHeadBlockAndState().orElseThrow();
final ValidateableAttestation attestation =
ValidateableAttestation.fromAttestation(
attestationGenerator.validAttestation(blockAndState));
Expand All @@ -122,7 +122,7 @@ public void handleMessage_saveForFuture() {
@Test
public void handleMessage_invalid() {
final AttestationGenerator attestationGenerator = new AttestationGenerator(validatorKeys);
final BeaconBlockAndState blockAndState = recentChainData.getBestBlockAndState().orElseThrow();
final BeaconBlockAndState blockAndState = recentChainData.getHeadBlockAndState().orElseThrow();
final ValidateableAttestation attestation =
ValidateableAttestation.fromAttestation(
attestationGenerator.validAttestation(blockAndState));
Expand Down
Loading

0 comments on commit b1b6390

Please sign in to comment.