Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Commit

Permalink
When a new block message is received, update the peer's chain head to…
Browse files Browse the repository at this point in the history 
… be the parent block, not the received block. (#819)
  • Loading branch information
@ajsutton
ajsutton committed Feb 11, 2019
1 parent 3fc31aa commit 91244f3
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 30 deletions.
16 changes: 10 additions & 6 deletions ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,18 @@ public void update(final BlockHeader header) {
}
}

public void update(final BlockHeader blockHeader, final UInt256 totalDifficulty) {
public void updateForAnnouncedBlock(
final BlockHeader blockHeader, final UInt256 totalDifficulty) {
synchronized (this) {
if (totalDifficulty.compareTo(bestBlock.totalDifficulty) >= 0) {
bestBlock.totalDifficulty = totalDifficulty;
bestBlock.hash = blockHeader.getHash();
bestBlock.number = blockHeader.getNumber();
// Blocks are announced before they're imported so their chain head must be the parent
final UInt256 parentTotalDifficulty = totalDifficulty.minus(blockHeader.getDifficulty());
final long parentBlockNumber = blockHeader.getNumber() - 1;
if (parentTotalDifficulty.compareTo(bestBlock.totalDifficulty) >= 0) {
bestBlock.totalDifficulty = parentTotalDifficulty;
bestBlock.hash = blockHeader.getParentHash();
bestBlock.number = parentBlockNumber;
}
updateHeightEstimate(blockHeader.getNumber());
updateHeightEstimate(parentBlockNumber);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...um/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/BlockPropagationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private void handleNewBlockFromNetwork(final EthMessage message) {
final Block block = newBlockMessage.block(protocolSchedule);
final UInt256 totalDifficulty = newBlockMessage.totalDifficulty(protocolSchedule);

message.getPeer().chainState().update(block.getHeader(), totalDifficulty);
message.getPeer().chainState().updateForAnnouncedBlock(block.getHeader(), totalDifficulty);

// Return early if we don't care about this block
final long localChainHeight = protocolContext.getBlockchain().getChainHeadBlockNumber();
Expand Down
34 changes: 17 additions & 17 deletions ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ChainStateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ public void updateBestBlockAndHeightFromBestBlockHeaderAndTd() {
assertThat(chainState.getEstimatedHeight()).isEqualTo(0L);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(0L);

chainState.update(bestBlockHeader, INITIAL_TOTAL_DIFFICULTY);
assertThat(chainState.getEstimatedHeight()).isEqualTo(blockNumber);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(blockNumber);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(bestBlockHeader.getHash());
chainState.updateForAnnouncedBlock(bestBlockHeader, INITIAL_TOTAL_DIFFICULTY);
assertThat(chainState.getEstimatedHeight()).isEqualTo(blockNumber - 1);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(blockNumber - 1);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(bestBlockHeader.getParentHash());
assertThat(chainState.getBestBlock().getTotalDifficulty()).isEqualTo(INITIAL_TOTAL_DIFFICULTY);
}

Expand All @@ -171,11 +171,11 @@ public void updateBestBlockAndHeightFromBetterBlockHeaderAndTd() {
final UInt256 betterTd = INITIAL_TOTAL_DIFFICULTY.plus(100L);
final BlockHeader betterBlock =
new BlockHeaderTestFixture().number(betterBlockNumber).buildHeader();
chainState.update(betterBlock, betterTd);
chainState.updateForAnnouncedBlock(betterBlock, betterTd);

assertThat(chainState.getEstimatedHeight()).isEqualTo(betterBlockNumber);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(betterBlockNumber);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(betterBlock.getHash());
assertThat(chainState.getEstimatedHeight()).isEqualTo(betterBlockNumber - 1);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(betterBlockNumber - 1);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(betterBlock.getParentHash());
assertThat(chainState.getBestBlock().getTotalDifficulty()).isEqualTo(betterTd);
}

Expand All @@ -192,9 +192,9 @@ public void updateHeightFromBlockHeaderAndTd() {
final UInt256 otherTd = INITIAL_TOTAL_DIFFICULTY.minus(100L);
final BlockHeader otherBlock =
new BlockHeaderTestFixture().number(otherBlockNumber).buildHeader();
chainState.update(otherBlock, otherTd);
chainState.updateForAnnouncedBlock(otherBlock, otherTd);

assertThat(chainState.getEstimatedHeight()).isEqualTo(otherBlockNumber);
assertThat(chainState.getEstimatedHeight()).isEqualTo(otherBlockNumber - 1);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(0L);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(bestBlockHeader.getHash());
assertThat(chainState.getBestBlock().getTotalDifficulty()).isEqualTo(INITIAL_TOTAL_DIFFICULTY);
Expand All @@ -209,17 +209,17 @@ public void doNotUpdateFromOldBlockHeaderAndTd() {
assertThat(chainState.getEstimatedHeight()).isEqualTo(0L);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(0L);

chainState.update(bestBlockHeader, INITIAL_TOTAL_DIFFICULTY);
chainState.updateForAnnouncedBlock(bestBlockHeader, INITIAL_TOTAL_DIFFICULTY);

final long otherBlockNumber = blockNumber - 2;
final UInt256 otherTd = INITIAL_TOTAL_DIFFICULTY.minus(100L);
final BlockHeader otherBlock =
new BlockHeaderTestFixture().number(otherBlockNumber).buildHeader();
chainState.update(otherBlock, otherTd);
chainState.updateForAnnouncedBlock(otherBlock, otherTd);

assertThat(chainState.getEstimatedHeight()).isEqualTo(blockNumber);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(blockNumber);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(bestBlockHeader.getHash());
assertThat(chainState.getEstimatedHeight()).isEqualTo(blockNumber - 1);
assertThat(chainState.getBestBlock().getNumber()).isEqualTo(blockNumber - 1);
assertThat(chainState.getBestBlock().getHash()).isEqualTo(bestBlockHeader.getParentHash());
assertThat(chainState.getBestBlock().getTotalDifficulty()).isEqualTo(INITIAL_TOTAL_DIFFICULTY);
}

Expand Down Expand Up @@ -265,8 +265,8 @@ public void observersInformedWhenHeightUpdatedViaHeaderAndTD() {
chainState.statusReceived(bestBlockHeader.getHash(), INITIAL_TOTAL_DIFFICULTY);
final EstimatedHeightListener listener = mock(EstimatedHeightListener.class);
chainState.addEstimatedHeightListener(listener);
chainState.update(bestBlockHeader, INITIAL_TOTAL_DIFFICULTY);
verify(listener).onEstimatedHeightChanged(blockNumber);
chainState.updateForAnnouncedBlock(bestBlockHeader, INITIAL_TOTAL_DIFFICULTY);
verify(listener).onEstimatedHeightChanged(blockNumber - 1);
}

@Test
Expand Down
8 changes: 5 additions & 3 deletions ...th/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/BlockPropagationManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,8 @@ public void updatesChainHeadWhenNewBlockMessageReceived() {

// Setup peer and messages
final RespondingEthPeer peer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 0);
final UInt256 parentTotalDifficulty =
fullBlockchain.getTotalDifficultyByHash(nextBlock.getHeader().getParentHash()).get();
final UInt256 totalDifficulty =
fullBlockchain.getTotalDifficultyByHash(nextBlock.getHash()).get();
final NewBlockMessage nextAnnouncement = NewBlockMessage.create(nextBlock, totalDifficulty);
Expand All @@ -533,11 +535,11 @@ public void updatesChainHeadWhenNewBlockMessageReceived() {
peer.respondWhile(responder, peer::hasOutstandingRequests);

assertThat(peer.getEthPeer().chainState().getBestBlock().getHash())
.isEqualTo(nextBlock.getHash());
.isEqualTo(nextBlock.getHeader().getParentHash());
assertThat(peer.getEthPeer().chainState().getEstimatedHeight())
.isEqualTo(nextBlock.getHeader().getNumber());
.isEqualTo(nextBlock.getHeader().getNumber() - 1);
assertThat(peer.getEthPeer().chainState().getBestBlock().getTotalDifficulty())
.isEqualTo(totalDifficulty);
.isEqualTo(parentTotalDifficulty);
}

@SuppressWarnings("unchecked")
Expand Down
6 changes: 3 additions & 3 deletions ...rc/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public void switchesSyncTarget_betterTd() {
peerB
.getEthPeer()
.chainState()
.update(gen.header(), syncState.chainHeadTotalDifficulty().plus(300));
.updateForAnnouncedBlock(gen.header(), syncState.chainHeadTotalDifficulty().plus(300));

// Process through first task cycle
final CompletableFuture<?> firstTask = downloader.getCurrentTask();
Expand Down Expand Up @@ -467,11 +467,11 @@ public void doesNotSwitchSyncTarget_betterTdUnderThreshold() {
bestPeer
.getEthPeer()
.chainState()
.update(gen.header(1000), syncState.chainHeadTotalDifficulty().plus(201));
.updateForAnnouncedBlock(gen.header(1000), syncState.chainHeadTotalDifficulty().plus(201));
otherPeer
.getEthPeer()
.chainState()
.update(gen.header(1000), syncState.chainHeadTotalDifficulty().plus(300));
.updateForAnnouncedBlock(gen.header(1000), syncState.chainHeadTotalDifficulty().plus(300));

// Process through first task cycle
final CompletableFuture<?> firstTask = downloader.getCurrentTask();
Expand Down

0 comments on commit 91244f3

Please sign in to comment.