From c110679920c85a4792e3de3baf4de74fea6ef11d Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Wed, 20 Feb 2019 18:31:17 -0500 Subject: [PATCH 1/5] DeconstructedGetNodeDataFromPeerTaskTest --- ...onstructedGetNodeDataFromPeerTaskTest.java | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java new file mode 100644 index 0000000000..2bae199055 --- /dev/null +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java @@ -0,0 +1,96 @@ +/* + * 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.manager.task; + +import static java.util.stream.Collectors.toList; +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.ProtocolContext; +import tech.pegasys.pantheon.ethereum.chain.Blockchain; +import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.eth.manager.EthContext; +import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManager; +import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManagerTestUtil; +import tech.pegasys.pantheon.ethereum.eth.manager.RespondingEthPeer; +import tech.pegasys.pantheon.ethereum.eth.manager.ethtaskutils.BlockchainSetupUtil; +import tech.pegasys.pantheon.metrics.LabelledMetric; +import tech.pegasys.pantheon.metrics.OperationTimer; +import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.Test; + +public class DeconstructedGetNodeDataFromPeerTaskTest { + + @Test + public void getNodeDataFromPeerTaskTest() { + final BlockchainSetupUtil blockchainSetupUtil = BlockchainSetupUtil.forTesting(); + blockchainSetupUtil.importAllBlocks(); + final Blockchain blockchain = blockchainSetupUtil.getBlockchain(); + final ProtocolContext protocolContext = blockchainSetupUtil.getProtocolContext(); + final LabelledMetric ethTasksTimer = NoOpMetricsSystem.NO_OP_LABELLED_TIMER; + assertThat(blockchainSetupUtil.getMaxBlockNumber() >= 20L).isTrue(); + + final AtomicBoolean peersDoTimeout = new AtomicBoolean(false); + final AtomicInteger peerCountToTimeout = new AtomicInteger(0); + final EthProtocolManager ethProtocolManager = + EthProtocolManagerTestUtil.create( + blockchain, + protocolContext.getWorldStateArchive(), + () -> peerCountToTimeout.getAndDecrement() > 0 || peersDoTimeout.get()); + final EthContext ethContext = ethProtocolManager.ethContext(); + + // Setup a responsive peer + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder(blockchain, protocolContext.getWorldStateArchive()); + final RespondingEthPeer respondingPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); + + // Setup data to be requested and expected response + final List requestedData = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + final BlockHeader blockHeader = blockchain.getBlockHeader(10 + i).get(); + requestedData.add( + protocolContext.getWorldStateArchive().getNodeData(blockHeader.getStateRoot()).get()); + } + + // Execute task and wait for response + final AtomicReference>> actualResult = + new AtomicReference<>(); + final AtomicBoolean done = new AtomicBoolean(false); + + final List hashes = requestedData.stream().map(Hash::hash).collect(toList()); + final EthTask>> task = + GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer); + + final CompletableFuture>> future = task.run(); + respondingPeer.respondWhile(responder, () -> !future.isDone()); + future.whenComplete( + (result, error) -> { + actualResult.set(result); + done.compareAndSet(false, true); + }); + + assertThat(done).isTrue(); + assertThat(actualResult.get().getResult()).containsExactlyInAnyOrderElementsOf(requestedData); + assertThat(actualResult.get().getPeer()).isEqualTo(respondingPeer.getEthPeer()); + } +} From 4786df4aac101f56290944b21a175ee4ff1f6675 Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Thu, 21 Feb 2019 12:41:19 -0500 Subject: [PATCH 2/5] GetNodeDataFromPeerTaskTest --- .../manager/task/GetNodeDataFromPeerTask.java | 25 ++++++++++----- .../sync/worldstate/WorldStateDownloader.java | 9 +++--- ...onstructedGetNodeDataFromPeerTaskTest.java | 13 +++++--- .../task/GetNodeDataFromPeerTaskTest.java | 31 ++++++++++++------- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java index c9582af02e..a4087c1d10 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.eth.manager.task; -import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.eth.manager.EthContext; @@ -27,15 +27,17 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -public class GetNodeDataFromPeerTask extends AbstractPeerRequestTask> { +public class GetNodeDataFromPeerTask extends AbstractPeerRequestTask> { private static final Logger LOG = LogManager.getLogger(); @@ -63,18 +65,20 @@ protected ResponseStream sendRequest(final EthPeer peer) throws PeerNotConnected } @Override - protected Optional> processResponse( + protected Optional> processResponse( final boolean streamClosed, final MessageData message, final EthPeer peer) { if (streamClosed) { // We don't record this as a useless response because it's impossible to know if a peer has // the data we're requesting. - return Optional.of(emptyList()); + return Optional.of(emptyMap()); } final NodeDataMessage nodeDataMessage = NodeDataMessage.readFrom(message); final List nodeData = nodeDataMessage.nodeData(); - if (nodeData.isEmpty()) { + final Map nodeDataByHash = mapNodeDataByHash(nodeData); + + if (nodeDataByHash.isEmpty()) { return Optional.empty(); - } else if (nodeData.size() > hashes.size()) { + } else if (nodeDataByHash.entrySet().size() > hashes.size()) { // Can't be the response to our request return Optional.empty(); } @@ -83,6 +87,13 @@ protected Optional> processResponse( // Message contains unrequested data, must not be the response to our request. return Optional.empty(); } - return Optional.of(nodeData); + return Optional.of(nodeDataByHash); + } + + private Map mapNodeDataByHash(final List data) { + // Map data by hash + final Map dataByHash = new HashMap<>(); + data.forEach(d -> dataByHash.put(Hash.hash(d), d)); + return dataByHash; } } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java index e7786c3140..4c31e7fdd4 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java @@ -240,7 +240,7 @@ private CompletableFuture waitForNewPeer() { .timeout(WaitForPeerTask.create(ethContext, ethTasksTimer), Duration.ofSeconds(5)); } - private CompletableFuture>> sendAndProcessRequests( + private CompletableFuture>> sendAndProcessRequests( final EthPeer peer, final List> requestTasks, final BlockHeader blockHeader) { @@ -250,13 +250,12 @@ private CompletableFuture>> sendAndProcessRequ .map(NodeDataRequest::getHash) .distinct() .collect(Collectors.toList()); - final AbstractPeerTask> ethTask = + final AbstractPeerTask> ethTask = GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer).assignPeer(peer); outstandingRequests.add(ethTask); return ethTask .run() .thenApply(PeerTaskResult::getResult) - .thenApply(this::mapNodeDataByHash) .exceptionally( error -> { final Throwable rootCause = ExceptionUtils.rootCause(error); @@ -276,10 +275,10 @@ private CompletableFuture>> sendAndProcessRequ () -> storeData(requestTasks, blockHeader, ethTask, data))); } - private CompletableFuture>> storeData( + private CompletableFuture>> storeData( final List> requestTasks, final BlockHeader blockHeader, - final AbstractPeerTask> ethTask, + final AbstractPeerTask> ethTask, final Map data) { final Updater storageUpdater = worldStateStorage.updater(); for (final Task task : requestTasks) { diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java index 2bae199055..0cfd136f93 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java @@ -31,6 +31,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -73,15 +74,16 @@ public void getNodeDataFromPeerTaskTest() { } // Execute task and wait for response - final AtomicReference>> actualResult = + final AtomicReference>> actualResult = new AtomicReference<>(); final AtomicBoolean done = new AtomicBoolean(false); final List hashes = requestedData.stream().map(Hash::hash).collect(toList()); - final EthTask>> task = + final EthTask>> task = GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer); - final CompletableFuture>> future = task.run(); + final CompletableFuture>> future = + task.run(); respondingPeer.respondWhile(responder, () -> !future.isDone()); future.whenComplete( (result, error) -> { @@ -90,7 +92,10 @@ public void getNodeDataFromPeerTaskTest() { }); assertThat(done).isTrue(); - assertThat(actualResult.get().getResult()).containsExactlyInAnyOrderElementsOf(requestedData); + + final List resultData = new ArrayList<>(actualResult.get().getResult().values()); + + assertThat(resultData).containsExactlyInAnyOrderElementsOf(requestedData); assertThat(actualResult.get().getPeer()).isEqualTo(respondingPeer.getEthPeer()); } } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java index c22988d8a6..438049ad4f 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java @@ -23,42 +23,49 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; -public class GetNodeDataFromPeerTaskTest extends PeerMessageTaskTest> { +public class GetNodeDataFromPeerTaskTest extends PeerMessageTaskTest> { @Override - protected List generateDataToBeRequested() { - final List requestedData = new ArrayList<>(); + protected Map generateDataToBeRequested() { + final Map requestedData = new HashMap<>(); for (int i = 0; i < 3; i++) { final BlockHeader blockHeader = blockchain.getBlockHeader(10 + i).get(); - requestedData.add( + requestedData.put( + Hash.fromHexStringLenient(Integer.toHexString(i)), protocolContext.getWorldStateArchive().getNodeData(blockHeader.getStateRoot()).get()); } return requestedData; } @Override - protected EthTask>> createTask( - final List requestedData) { - final List hashes = requestedData.stream().map(Hash::hash).collect(toList()); + protected EthTask>> createTask( + final Map requestedData) { + final List hashes = requestedData.values().stream().map(Hash::hash).collect(toList()); return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer); } @Override protected void assertPartialResultMatchesExpectation( - final List requestedData, final List partialResponse) { + final Map requestedData, final Map partialResponse) { assertThat(partialResponse.size()).isLessThanOrEqualTo(requestedData.size()); assertThat(partialResponse.size()).isGreaterThan(0); - assertThat(requestedData).containsAll(partialResponse); + final List requestData = new ArrayList<>(requestedData.values()); + final List resultData = new ArrayList<>(partialResponse.values()); + assertThat(requestData).containsAll(resultData); } @Override protected void assertResultMatchesExpectation( - final List requestedData, - final PeerTaskResult> response, + final Map requestedData, + final PeerTaskResult> response, final EthPeer respondingPeer) { - assertThat(response.getResult()).containsExactlyInAnyOrderElementsOf(requestedData); + final List requestData = new ArrayList<>(requestedData.values()); + final List resultData = new ArrayList<>(response.getResult().values()); + assertThat(resultData).containsExactlyInAnyOrderElementsOf(requestData); assertThat(response.getPeer()).isEqualTo(respondingPeer); } } From fa15b64098bbb24feeae6241cbe39f036cd75ee0 Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Thu, 21 Feb 2019 12:43:09 -0500 Subject: [PATCH 3/5] rm DeconstructedGetNodeDataFromPeerTaskTest --- ...onstructedGetNodeDataFromPeerTaskTest.java | 101 ------------------ 1 file changed, 101 deletions(-) delete mode 100644 ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java deleted file mode 100644 index 0cfd136f93..0000000000 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/DeconstructedGetNodeDataFromPeerTaskTest.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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.manager.task; - -import static java.util.stream.Collectors.toList; -import static org.assertj.core.api.Assertions.assertThat; - -import tech.pegasys.pantheon.ethereum.ProtocolContext; -import tech.pegasys.pantheon.ethereum.chain.Blockchain; -import tech.pegasys.pantheon.ethereum.core.BlockHeader; -import tech.pegasys.pantheon.ethereum.core.Hash; -import tech.pegasys.pantheon.ethereum.eth.manager.EthContext; -import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManager; -import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManagerTestUtil; -import tech.pegasys.pantheon.ethereum.eth.manager.RespondingEthPeer; -import tech.pegasys.pantheon.ethereum.eth.manager.ethtaskutils.BlockchainSetupUtil; -import tech.pegasys.pantheon.metrics.LabelledMetric; -import tech.pegasys.pantheon.metrics.OperationTimer; -import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; -import tech.pegasys.pantheon.util.bytes.BytesValue; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; - -import org.junit.Test; - -public class DeconstructedGetNodeDataFromPeerTaskTest { - - @Test - public void getNodeDataFromPeerTaskTest() { - final BlockchainSetupUtil blockchainSetupUtil = BlockchainSetupUtil.forTesting(); - blockchainSetupUtil.importAllBlocks(); - final Blockchain blockchain = blockchainSetupUtil.getBlockchain(); - final ProtocolContext protocolContext = blockchainSetupUtil.getProtocolContext(); - final LabelledMetric ethTasksTimer = NoOpMetricsSystem.NO_OP_LABELLED_TIMER; - assertThat(blockchainSetupUtil.getMaxBlockNumber() >= 20L).isTrue(); - - final AtomicBoolean peersDoTimeout = new AtomicBoolean(false); - final AtomicInteger peerCountToTimeout = new AtomicInteger(0); - final EthProtocolManager ethProtocolManager = - EthProtocolManagerTestUtil.create( - blockchain, - protocolContext.getWorldStateArchive(), - () -> peerCountToTimeout.getAndDecrement() > 0 || peersDoTimeout.get()); - final EthContext ethContext = ethProtocolManager.ethContext(); - - // Setup a responsive peer - final RespondingEthPeer.Responder responder = - RespondingEthPeer.blockchainResponder(blockchain, protocolContext.getWorldStateArchive()); - final RespondingEthPeer respondingPeer = - EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1000); - - // Setup data to be requested and expected response - final List requestedData = new ArrayList<>(); - for (int i = 0; i < 3; i++) { - final BlockHeader blockHeader = blockchain.getBlockHeader(10 + i).get(); - requestedData.add( - protocolContext.getWorldStateArchive().getNodeData(blockHeader.getStateRoot()).get()); - } - - // Execute task and wait for response - final AtomicReference>> actualResult = - new AtomicReference<>(); - final AtomicBoolean done = new AtomicBoolean(false); - - final List hashes = requestedData.stream().map(Hash::hash).collect(toList()); - final EthTask>> task = - GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer); - - final CompletableFuture>> future = - task.run(); - respondingPeer.respondWhile(responder, () -> !future.isDone()); - future.whenComplete( - (result, error) -> { - actualResult.set(result); - done.compareAndSet(false, true); - }); - - assertThat(done).isTrue(); - - final List resultData = new ArrayList<>(actualResult.get().getResult().values()); - - assertThat(resultData).containsExactlyInAnyOrderElementsOf(requestedData); - assertThat(actualResult.get().getPeer()).isEqualTo(respondingPeer.getEthPeer()); - } -} From 4ca7c136557924e9823f9f250010bf3efe21958e Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Fri, 22 Feb 2019 11:11:00 -0500 Subject: [PATCH 4/5] update --- .../manager/task/GetNodeDataFromPeerTask.java | 28 +++++++------------ .../sync/worldstate/WorldStateDownloader.java | 8 ------ 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java index a4087c1d10..8745be7d1b 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java @@ -74,26 +74,18 @@ protected Optional> processResponse( } final NodeDataMessage nodeDataMessage = NodeDataMessage.readFrom(message); final List nodeData = nodeDataMessage.nodeData(); - final Map nodeDataByHash = mapNodeDataByHash(nodeData); - - if (nodeDataByHash.isEmpty()) { - return Optional.empty(); - } else if (nodeDataByHash.entrySet().size() > hashes.size()) { - // Can't be the response to our request - return Optional.empty(); - } + return mapNodeDataByHash(nodeData); + } - if (nodeData.stream().anyMatch(data -> !hashes.contains(Hash.hash(data)))) { - // Message contains unrequested data, must not be the response to our request. - return Optional.empty(); + private Optional> mapNodeDataByHash(final List nodeData) { + final Map nodeDataByHash = new HashMap<>(); + for (BytesValue data : nodeData) { + final Hash hash = Hash.hash(data); + if (!hashes.contains(hash)) { + return Optional.empty(); + } + nodeDataByHash.put(hash, data); } return Optional.of(nodeDataByHash); } - - private Map mapNodeDataByHash(final List data) { - // Map data by hash - final Map dataByHash = new HashMap<>(); - data.forEach(d -> dataByHash.put(Hash.hash(d), d)); - return dataByHash; - } } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java index 4c31e7fdd4..a60778c4a4 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java @@ -37,7 +37,6 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -377,11 +376,4 @@ private synchronized void markDone() { private boolean isRootState(final BlockHeader blockHeader, final NodeDataRequest request) { return request.getHash().equals(blockHeader.getStateRoot()); } - - private Map mapNodeDataByHash(final List data) { - // Map data by hash - final Map dataByHash = new HashMap<>(); - data.forEach(d -> dataByHash.put(Hash.hash(d), d)); - return dataByHash; - } } From 74ddc7880b77852e2a8705091f46ccd9fb817981 Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Fri, 22 Feb 2019 12:23:27 -0500 Subject: [PATCH 5/5] update II --- .../manager/task/GetNodeDataFromPeerTask.java | 4 ++++ .../task/GetNodeDataFromPeerTaskTest.java | 23 ++++++++++--------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java index 8745be7d1b..25f507afb7 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTask.java @@ -74,6 +74,10 @@ protected Optional> processResponse( } final NodeDataMessage nodeDataMessage = NodeDataMessage.readFrom(message); final List nodeData = nodeDataMessage.nodeData(); + if (nodeData.size() > hashes.size()) { + // Can't be the response to our request + return Optional.empty(); + } return mapNodeDataByHash(nodeData); } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java index 438049ad4f..f0b0e23128 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/task/GetNodeDataFromPeerTaskTest.java @@ -12,7 +12,6 @@ */ package tech.pegasys.pantheon.ethereum.eth.manager.task; -import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import tech.pegasys.pantheon.ethereum.core.BlockHeader; @@ -22,11 +21,12 @@ import tech.pegasys.pantheon.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import tech.pegasys.pantheon.util.bytes.BytesValue; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import com.google.common.collect.Lists; + public class GetNodeDataFromPeerTaskTest extends PeerMessageTaskTest> { @Override @@ -35,7 +35,8 @@ protected Map generateDataToBeRequested() { for (int i = 0; i < 3; i++) { final BlockHeader blockHeader = blockchain.getBlockHeader(10 + i).get(); requestedData.put( - Hash.fromHexStringLenient(Integer.toHexString(i)), + Hash.hash( + protocolContext.getWorldStateArchive().getNodeData(blockHeader.getStateRoot()).get()), protocolContext.getWorldStateArchive().getNodeData(blockHeader.getStateRoot()).get()); } return requestedData; @@ -44,7 +45,7 @@ protected Map generateDataToBeRequested() { @Override protected EthTask>> createTask( final Map requestedData) { - final List hashes = requestedData.values().stream().map(Hash::hash).collect(toList()); + final List hashes = Lists.newArrayList(requestedData.keySet()); return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer); } @@ -53,9 +54,9 @@ protected void assertPartialResultMatchesExpectation( final Map requestedData, final Map partialResponse) { assertThat(partialResponse.size()).isLessThanOrEqualTo(requestedData.size()); assertThat(partialResponse.size()).isGreaterThan(0); - final List requestData = new ArrayList<>(requestedData.values()); - final List resultData = new ArrayList<>(partialResponse.values()); - assertThat(requestData).containsAll(resultData); + for (Map.Entry data : partialResponse.entrySet()) { + assertThat(requestedData.get(data.getKey())).isEqualTo(data.getValue()); + } } @Override @@ -63,9 +64,9 @@ protected void assertResultMatchesExpectation( final Map requestedData, final PeerTaskResult> response, final EthPeer respondingPeer) { - final List requestData = new ArrayList<>(requestedData.values()); - final List resultData = new ArrayList<>(response.getResult().values()); - assertThat(resultData).containsExactlyInAnyOrderElementsOf(requestData); - assertThat(response.getPeer()).isEqualTo(respondingPeer); + assertThat(response.getResult().size()).isEqualTo(requestedData.size()); + for (Map.Entry data : response.getResult().entrySet()) { + assertThat(requestedData.get(data.getKey())).isEqualTo(data.getValue()); + } } }