Switch to new sync target if it exceeds the td threshold (#1286)

Increase total difficulty threshold required to switch sync targets.
Increase threshold for changing sync target based on height to 200.

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
pull/2/head
Adrian Sutton 6 years ago committed by GitHub
parent 9fb04b44b7
commit cfaa83747a
  1. 8
      ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/SynchronizerConfiguration.java
  2. 63
      ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java
  3. 37
      ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncTargetManager.java
  4. 157
      ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluatorTest.java
  5. 15
      ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncChainDownloaderTest.java

@ -244,20 +244,20 @@ public class SynchronizerConfiguration {
@CommandLine.Option(
names = "--Xsynchronizer-downloader-change-target-threshold-by-height",
hidden = true,
defaultValue = "20",
defaultValue = "200",
paramLabel = "<LONG>",
description =
"Minimum height difference before switching fast sync download peers (default: ${DEFAULT-VALUE})")
private long downloaderChangeTargetThresholdByHeight = 20L;
private long downloaderChangeTargetThresholdByHeight = 200L;
@CommandLine.Option(
names = "--Xsynchronizer-downloader-change-target-threshold-by-td",
hidden = true,
defaultValue = "1000000000",
defaultValue = "1000000000000000000",
paramLabel = "<UINT256>",
description =
"Minimum total difficulty difference before switching fast sync download peers (default: ${DEFAULT-VALUE})")
private UInt256 downloaderChangeTargetThresholdByTd = UInt256.of(1_000_000_000L);
private UInt256 downloaderChangeTargetThresholdByTd = UInt256.of(1_000_000_000_000_000_000L);
@CommandLine.Option(
names = "--Xsynchronizer-downloader-header-request-size",

@ -0,0 +1,63 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.eth.sync.fullsync;
import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
import tech.pegasys.pantheon.util.uint.UInt256;
import java.util.Optional;
public class BetterSyncTargetEvaluator {
private final SynchronizerConfiguration config;
private final EthPeers ethPeers;
public BetterSyncTargetEvaluator(
final SynchronizerConfiguration config, final EthPeers ethPeers) {
this.config = config;
this.ethPeers = ethPeers;
}
public boolean shouldSwitchSyncTarget(final EthPeer currentSyncTarget) {
final ChainState currentPeerChainState = currentSyncTarget.chainState();
final Optional<EthPeer> maybeBestPeer = ethPeers.bestPeer();
return maybeBestPeer
.map(
bestPeer -> {
if (EthPeers.BEST_CHAIN.compare(bestPeer, currentSyncTarget) <= 0) {
// Our current target is better or equal to the best peer
return false;
}
// Require some threshold to be exceeded before switching targets to keep some
// stability when multiple peers are in range of each other
final ChainState bestPeerChainState = bestPeer.chainState();
final UInt256 tdDifference =
bestPeerChainState
.getBestBlock()
.getTotalDifficulty()
.minus(currentPeerChainState.getBestBlock().getTotalDifficulty());
if (tdDifference.compareTo(config.downloaderChangeTargetThresholdByTd()) > 0) {
return true;
}
final long heightDifference =
bestPeerChainState.getEstimatedHeight()
- currentPeerChainState.getEstimatedHeight();
return heightDifference > config.downloaderChangeTargetThresholdByHeight();
})
.orElse(false);
}
}

@ -17,10 +17,8 @@ import static java.util.concurrent.CompletableFuture.completedFuture;
import tech.pegasys.pantheon.ethereum.ProtocolContext;
import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain;
import tech.pegasys.pantheon.ethereum.core.BlockHeader;
import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
import tech.pegasys.pantheon.ethereum.eth.manager.EthContext;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
import tech.pegasys.pantheon.ethereum.eth.sync.SyncTargetManager;
import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncTarget;
@ -38,9 +36,9 @@ import org.apache.logging.log4j.Logger;
class FullSyncTargetManager<C> extends SyncTargetManager<C> {
private static final Logger LOG = LogManager.getLogger();
private final SynchronizerConfiguration config;
private final ProtocolContext<C> protocolContext;
private final EthContext ethContext;
private final BetterSyncTargetEvaluator betterSyncTargetEvaluator;
FullSyncTargetManager(
final SynchronizerConfiguration config,
@ -49,9 +47,9 @@ class FullSyncTargetManager<C> extends SyncTargetManager<C> {
final EthContext ethContext,
final MetricsSystem metricsSystem) {
super(config, protocolSchedule, protocolContext, ethContext, metricsSystem);
this.config = config;
this.protocolContext = protocolContext;
this.ethContext = ethContext;
betterSyncTargetEvaluator = new BetterSyncTargetEvaluator(config, ethContext.getEthPeers());
}
@Override
@ -100,36 +98,7 @@ class FullSyncTargetManager<C> extends SyncTargetManager<C> {
@Override
public boolean shouldSwitchSyncTarget(final SyncTarget currentTarget) {
final EthPeer currentPeer = currentTarget.peer();
final ChainState currentPeerChainState = currentPeer.chainState();
final Optional<EthPeer> maybeBestPeer = ethContext.getEthPeers().bestPeer();
return maybeBestPeer
.map(
bestPeer -> {
if (EthPeers.BEST_CHAIN.compare(bestPeer, currentPeer) <= 0) {
// Our current target is better or equal to the best peer
return false;
}
// Require some threshold to be exceeded before switching targets to keep some
// stability
// when multiple peers are in range of each other
final ChainState bestPeerChainState = bestPeer.chainState();
final long heightDifference =
bestPeerChainState.getEstimatedHeight()
- currentPeerChainState.getEstimatedHeight();
if (heightDifference == 0 && bestPeerChainState.getEstimatedHeight() == 0) {
// Only check td if we don't have a height metric
final UInt256 tdDifference =
bestPeerChainState
.getBestBlock()
.getTotalDifficulty()
.minus(currentPeerChainState.getBestBlock().getTotalDifficulty());
return tdDifference.compareTo(config.downloaderChangeTargetThresholdByTd()) > 0;
}
return heightDifference > config.downloaderChangeTargetThresholdByHeight();
})
.orElse(false);
return betterSyncTargetEvaluator.shouldSwitchSyncTarget(currentTarget.peer());
}
@Override

@ -0,0 +1,157 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.eth.sync.fullsync;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import tech.pegasys.pantheon.ethereum.core.Hash;
import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
import tech.pegasys.pantheon.util.uint.UInt256;
import java.util.Optional;
import org.junit.Test;
public class BetterSyncTargetEvaluatorTest {
private static final int CURRENT_TARGET_HEIGHT = 10;
private static final int CURRENT_TARGET_TD = 50;
private static final int HEIGHT_THRESHOLD = 100;
private static final int TD_THRESHOLD = 5;
private final EthPeers ethPeers = mock(EthPeers.class);
private final EthPeer currentTarget = peer(CURRENT_TARGET_HEIGHT, CURRENT_TARGET_TD);
private final BetterSyncTargetEvaluator evaluator =
new BetterSyncTargetEvaluator(
SynchronizerConfiguration.builder()
.downloaderChangeTargetThresholdByHeight(HEIGHT_THRESHOLD)
.downloaderChangeTargetThresholdByTd(UInt256.of(TD_THRESHOLD))
.build(),
ethPeers);
@Test
public void shouldNotSwitchTargetsIfNoBestPeerIsAvailable() {
when(ethPeers.bestPeer()).thenReturn(Optional.empty());
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchTargetWhenBestPeerHasLowerHeightAndDifficulty() {
bestPeerWithDelta(-1, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchTargetWhenBestPeerHasSameHeightAndLowerDifficulty() {
bestPeerWithDelta(0, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchTargetWhenBestPeerHasLowerHeightAndSameDifficulty() {
bestPeerWithDelta(-1, 0);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchTargetWhenBestPeerHasGreaterHeightAndLowerDifficulty() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchTargetWhenBestPeerHasEqualHeightAndDifficulty() {
bestPeerWithDelta(0, 0);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchWhenHeightAndTdHigherWithinThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD - 1, TD_THRESHOLD - 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldNotSwitchWhenHeightAndTdHigherEqualToThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD, TD_THRESHOLD);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldSwitchWhenHeightExceedsThresholdAndDifficultyEqual() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, 0);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
@Test
public void shouldSwitchWhenHeightExceedsThresholdAndDifficultyWithinThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, TD_THRESHOLD - 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
@Test
public void shouldSwitchWhenHeightAndDifficultyExceedThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
@Test
public void shouldNotSwitchWhenHeightExceedsThresholdButDifficultyIsLower() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}
@Test
public void shouldSwitchWhenDifficultyExceedsThresholdAndHeightIsEqual() {
bestPeerWithDelta(0, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
@Test
public void shouldSwitchWhenDifficultyExceedsThresholdAndHeightIsLower() {
bestPeerWithDelta(-1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
@Test
public void shouldSwitchWhenDifficultyExceedsThresholdAndHeightIsWithinThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD - 1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
@Test
public void shouldSwitchWhenHeightAndDifficultyExceedsThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}
private void bestPeerWithDelta(final long height, final long totalDifficulty) {
final EthPeer bestPeer =
peer(CURRENT_TARGET_HEIGHT + height, CURRENT_TARGET_TD + totalDifficulty);
when(ethPeers.bestPeer()).thenReturn(Optional.of(bestPeer));
}
private EthPeer peer(final long chainHeight, final long totalDifficulty) {
final EthPeer peer = mock(EthPeer.class);
final ChainState chainState = new ChainState();
chainState.updateHeightEstimate(chainHeight);
chainState.statusReceived(Hash.EMPTY, UInt256.of(totalDifficulty));
when(peer.chainState()).thenReturn(chainState);
return peer;
}
}

@ -26,6 +26,7 @@ import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain;
import tech.pegasys.pantheon.ethereum.core.Block;
import tech.pegasys.pantheon.ethereum.core.BlockBody;
import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator;
import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator.BlockOptions;
import tech.pegasys.pantheon.ethereum.core.BlockHeader;
import tech.pegasys.pantheon.ethereum.core.TransactionReceipt;
import tech.pegasys.pantheon.ethereum.eth.manager.EthContext;
@ -426,16 +427,10 @@ public class FullSyncChainDownloaderTest {
assertThat(syncState.syncTarget().get().peer()).isEqualTo(bestPeer.getEthPeer());
// Update otherPeer so that its a better target and send some responses to push logic forward
bestPeer
.getEthPeer()
.chainState()
.updateForAnnouncedBlock(
gen.header(1000), localBlockchain.getChainHead().getTotalDifficulty().plus(201));
otherPeer
.getEthPeer()
.chainState()
.updateForAnnouncedBlock(
gen.header(1000), localBlockchain.getChainHead().getTotalDifficulty().plus(300));
final BlockHeader newBestBlock =
gen.header(1000, new BlockOptions().setDifficulty(UInt256.ZERO));
bestPeer.getEthPeer().chainState().updateForAnnouncedBlock(newBestBlock, localTd.plus(201));
otherPeer.getEthPeer().chainState().updateForAnnouncedBlock(newBestBlock, localTd.plus(300));
// Process through first task cycle
final CompletableFuture<?> firstTask = downloader.getCurrentTask();

Loading…
Cancel
Save