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

[PAN-2325] Fix IndexOutOfBoundsException in DetermineCommonAncestorTask #1038

Merged
merged 9 commits into from
Mar 5, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,19 @@ default Block getChainHeadBlock() {
return new Block(header, body);
}

default Block getGenesisBlock() {
final Hash genesisHash =
getBlockHashByNumber(BlockHeader.GENESIS_BLOCK_NUMBER)
.orElseThrow(() -> new IllegalStateException("Missing genesis block."));
final BlockHeader header =
getBlockHeader(genesisHash)
.orElseThrow(() -> new IllegalStateException("Missing genesis block."));
final BlockBody body =
getBlockBody(genesisHash)
.orElseThrow(() -> new IllegalStateException("Missing genesis block."));
return new Block(header, body);
}

/**
* Checks whether the block corresponding to the given hash is on the canonical chain.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ public List<Block> blockSequence(final int count) {
return blockSequence(count, worldState, Collections.emptyList(), Collections.emptyList());
}

public List<Block> blockSequence(final Block previousBlock, final int count) {
final WorldStateArchive worldState = createInMemoryWorldStateArchive();
Hash parentHash = previousBlock.getHeader().getHash();
long blockNumber = previousBlock.getHeader().getNumber() + 1;
return blockSequence(
count,
blockNumber,
parentHash,
worldState,
Collections.emptyList(),
Collections.emptyList());
}

public List<Block> blockSequence(
final int count,
final WorldStateArchive worldStateArchive,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public static class Builder {
private Range<Long> blockPropagationRange = Range.closed(-10L, 30L);
private long downloaderChangeTargetThresholdByHeight = 20L;
private UInt256 downloaderChangeTargetThresholdByTd = UInt256.of(1_000_000_000L);
private int downloaderHeaderRequestSize = 10;
private int downloaderHeaderRequestSize = 200;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat unrelated, but I see no reason not to bump up this value.

This will mean we request larger chunks of headers when running DetermineCommonAncestorTask. We'll also request a larger number of checkpoint headers when downloading.

private int downloaderCheckpointTimeoutsPermitted = 5;
private int downloaderChainSegmentTimeoutsPermitted = 5;
private int downloaderChainSegmentSize = 200;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,17 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class DetermineCommonAncestorTask<C> extends AbstractEthTask<BlockHeader> {
/**
* Finds the common ancestor with the given peer. It is assumed that the peer will at least share
* the same genesis block with this node. Running this task against a peer with a non-matching
* genesis block will result in undefined behavior: the task may complete exceptionally or in some
* cases this node's genesis block will be returned.
*/
public class DetermineCommonAncestorTask extends AbstractEthTask<BlockHeader> {
private static final Logger LOG = LogManager.getLogger();
private final EthContext ethContext;
private final ProtocolSchedule<C> protocolSchedule;
private final ProtocolContext<C> protocolContext;
private final ProtocolSchedule<?> protocolSchedule;
private final ProtocolContext<?> protocolContext;
private final EthPeer peer;
private final int headerRequestSize;
private final MetricsSystem metricsSystem;
Expand All @@ -46,8 +52,8 @@ public class DetermineCommonAncestorTask<C> extends AbstractEthTask<BlockHeader>
private boolean initialQuery = true;

private DetermineCommonAncestorTask(
final ProtocolSchedule<C> protocolSchedule,
final ProtocolContext<C> protocolContext,
final ProtocolSchedule<?> protocolSchedule,
final ProtocolContext<?> protocolContext,
final EthContext ethContext,
final EthPeer peer,
final int headerRequestSize,
Expand All @@ -66,14 +72,14 @@ private DetermineCommonAncestorTask(
protocolContext.getBlockchain().getBlockHeader(BlockHeader.GENESIS_BLOCK_NUMBER).get();
}

public static <C> DetermineCommonAncestorTask<C> create(
final ProtocolSchedule<C> protocolSchedule,
final ProtocolContext<C> protocolContext,
public static DetermineCommonAncestorTask create(
final ProtocolSchedule<?> protocolSchedule,
final ProtocolContext<?> protocolContext,
final EthContext ethContext,
final EthPeer peer,
final int headerRequestSize,
final MetricsSystem metricsSystem) {
return new DetermineCommonAncestorTask<>(
return new DetermineCommonAncestorTask(
protocolSchedule, protocolContext, ethContext, peer, headerRequestSize, metricsSystem);
}

Expand Down Expand Up @@ -144,6 +150,10 @@ private CompletableFuture<Void> processHeaders(
final AbstractPeerTask.PeerTaskResult<List<BlockHeader>> headersResult) {
initialQuery = false;
final List<BlockHeader> headers = headersResult.getResult();
if (headers.isEmpty()) {
// Nothing to do
return CompletableFuture.completedFuture(null);
}

final OptionalInt maybeAncestorNumber =
BlockchainUtil.findHighestKnownBlockIndex(protocolContext.getBlockchain(), headers, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import tech.pegasys.pantheon.ethereum.p2p.api.MessageData;
import tech.pegasys.pantheon.ethereum.p2p.wire.Capability;
import tech.pegasys.pantheon.ethereum.p2p.wire.DefaultMessage;
import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason;
import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
import tech.pegasys.pantheon.util.bytes.BytesValue;
import tech.pegasys.pantheon.util.uint.UInt256;
Expand Down Expand Up @@ -82,6 +83,16 @@ public static void respondOnce(final Responder responder, final RespondingEthPee
respondOnce(responder, Arrays.asList(peers));
}

public boolean disconnect(final DisconnectReason reason) {
if (ethPeer.isDisconnected()) {
return false;
}

ethPeer.disconnect(reason);
ethProtocolManager.handleDisconnect(getPeerConnection(), reason, true);
return true;
}

public MockPeerConnection getPeerConnection() {
return peerConnection;
}
Expand Down
Loading