From 9593d1f4cd514a6151d5af1d50df59d6553f3e45 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Mon, 21 Jan 2019 11:01:04 -0500 Subject: [PATCH 01/23] In progress - start writing WorldStateDownloader --- .../KeyValueStorageWorldStateStorage.java | 4 +- .../worldstate/WorldStateStorage.java | 6 +- ethereum/eth/build.gradle | 1 + .../eth/sync/worldstate/NodeData.java | 96 +++++++++++++ .../sync/worldstate/WorldStateDownloader.java | 130 ++++++++++++++++++ services/queue/build.gradle | 38 +++++ .../services/queue/InMemoryQueue.java | 35 +++++ .../pantheon/services/queue/Queue.java | 35 +++++ services/queue/src/main/resources/log4j2.xml | 16 +++ settings.gradle | 1 + 10 files changed, 359 insertions(+), 3 deletions(-) create mode 100644 ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java create mode 100644 ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java create mode 100644 services/queue/build.gradle create mode 100644 services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java create mode 100644 services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java create mode 100644 services/queue/src/main/resources/log4j2.xml diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java index b300ece3b0..225c40e56f 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java @@ -62,8 +62,8 @@ public Updater(final KeyValueStorage.Transaction transaction) { } @Override - public void putCode(final BytesValue code) { - transaction.put(Hash.hash(code), code); + public void putCode(final Bytes32 codeHash, final BytesValue code) { + transaction.put(codeHash, code); } @Override diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java index ac2a30af90..cd3fe93140 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java @@ -32,7 +32,11 @@ public interface WorldStateStorage { interface Updater { - void putCode(BytesValue code); + void putCode(Bytes32 nodeHash, BytesValue code); + + default void putCode(final BytesValue code) { + putCode(Hash.hash(code), code); + } void putAccountStateTrieNode(Bytes32 nodeHash, BytesValue node); diff --git a/ethereum/eth/build.gradle b/ethereum/eth/build.gradle index 931817b3b7..af1c81bb95 100644 --- a/ethereum/eth/build.gradle +++ b/ethereum/eth/build.gradle @@ -32,6 +32,7 @@ dependencies { implementation project(':ethereum:permissioning') implementation project(':metrics') implementation project(':services:kvstore') + implementation project(':services:queue') implementation 'io.vertx:vertx-core' implementation 'com.google.guava:guava' diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java new file mode 100644 index 0000000000..68ab6916c6 --- /dev/null +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java @@ -0,0 +1,96 @@ +package tech.pegasys.pantheon.ethereum.eth.sync.worldstate; + +import static com.google.common.base.Preconditions.checkNotNull; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage.Updater; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +public abstract class NodeData { + public enum Kind { + ACCOUNT_TRIE_NODE, + STORAGE_TRIE_NODE, + CODE + } + + private final Kind kind; + private final Hash hash; + private BytesValue data; + + protected NodeData(Kind kind, Hash hash) { + this.kind = kind; + this.hash = hash; + } + + public static AccountTrieNodeData createAccountTrieNode(Hash hash) { + return new AccountTrieNodeData(hash); + } + + public static StorageTrieNodeData createStorageTrieNode(Hash hash) { + return new StorageTrieNodeData(hash); + } + + public static CodeNodeData createCodeNode(Hash hash) { + return new CodeNodeData(hash); + } + + public Kind getKind() { + return kind; + } + + public Hash getHash() { + return hash; + } + + public BytesValue getData() { + return data; + } + + public NodeData setData(BytesValue data) { + this.data = data; + return this; + } + + abstract void persist(WorldStateStorage.Updater updater); + + + public static class AccountTrieNodeData extends NodeData { + + public AccountTrieNodeData(Hash hash) { + super(Kind.ACCOUNT_TRIE_NODE, hash); + } + + @Override + void persist(Updater updater) { + checkNotNull(getData(), "Must set data before node can be persisted."); + updater.putAccountStateTrieNode(getHash(), getData()); + } + } + + public static class StorageTrieNodeData extends NodeData { + + public StorageTrieNodeData(Hash hash) { + super(Kind.STORAGE_TRIE_NODE, hash); + } + + @Override + void persist(Updater updater) { + checkNotNull(getData(), "Must set data before node can be persisted."); + updater.putAccountStorageTrieNode(getHash(), getData()); + } + } + + public static class CodeNodeData extends NodeData { + + public CodeNodeData(Hash hash) { + super(Kind.CODE, hash); + } + + @Override + void persist(Updater updater) { + checkNotNull(getData(), "Must set data before node can be persisted."); + updater.putCode(getHash(), getData()); + } + } +} 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 new file mode 100644 index 0000000000..01b1ebdeb1 --- /dev/null +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloader.java @@ -0,0 +1,130 @@ +package tech.pegasys.pantheon.ethereum.eth.sync.worldstate; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.eth.manager.AbstractPeerTask.PeerTaskResult; +import tech.pegasys.pantheon.ethereum.eth.manager.EthContext; +import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer; +import tech.pegasys.pantheon.ethereum.eth.sync.tasks.GetNodeDataFromPeerTask; +import tech.pegasys.pantheon.ethereum.eth.sync.tasks.WaitForPeerTask; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; +import tech.pegasys.pantheon.metrics.LabelledMetric; +import tech.pegasys.pantheon.metrics.OperationTimer; +import tech.pegasys.pantheon.services.queue.Queue; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +public class WorldStateDownloader { + + private final EthContext ethContext; + // The target header for which we want to retrieve world state + private final BlockHeader header; + private final Queue pendingNodeQueue; + private final WorldStateStorage.Updater worldStateStorageUpdater; + private final int hashCountPerRequest = 50; + private final int maxOutstandingRequests = 50; + private final AtomicInteger outstandingRequests = new AtomicInteger(0); + private final LabelledMetric ethTasksTimer; + private Optional> future = Optional.empty(); + + public WorldStateDownloader(EthContext ethContext, WorldStateStorage worldStateStorage, BlockHeader header, Queue pendingNodeQueue, final LabelledMetric ethTasksTimer) { + this.ethContext = ethContext; + this.header = header; + this.pendingNodeQueue = pendingNodeQueue; + this.ethTasksTimer = ethTasksTimer; + // TODO: construct an updater that will commit changes periodically as updates accumulate + this.worldStateStorageUpdater = worldStateStorage.updater(); + + pendingNodeQueue.enqueue(NodeData.createAccountTrieNode(header.getStateRoot())); + } + + public CompletableFuture run() { + if (future.isPresent() && !future.get().isDone()) { + return future.get(); + } + future = Optional.of(new CompletableFuture<>()); + requestNodeData(); + + // TODO: Complete exceptionally on timeout / stalled download + return future.get(); + } + + private void requestNodeData() { + while (outstandingRequests.get() < maxOutstandingRequests && !pendingNodeQueue.isEmpty()) { + // Find available peer + Optional maybePeer = ethContext.getEthPeers().idlePeer(header.getNumber()); + + if (!maybePeer.isPresent()) { + // If no peer is available, wait and try again + waitForNewPeer() + .whenComplete((r,t) -> { + requestNodeData(); + }); + return; + } + EthPeer peer = maybePeer.get(); + + // Collect data to be requested + List toRequest = new ArrayList<>(); + for(int i = 0; i < hashCountPerRequest; i++) { + if (pendingNodeQueue.isEmpty()) { + break; + } + toRequest.add(pendingNodeQueue.dequeue()); + } + + // Request and process node data + outstandingRequests.incrementAndGet(); + sendAndProcessNodeDataRequest(peer, toRequest) + .thenAccept((res) -> { + if (outstandingRequests.decrementAndGet() == 0 && pendingNodeQueue.isEmpty()) { + // We're done + future.get().complete(null); + } + }); + } + } + + private CompletableFuture waitForNewPeer() { + return ethContext + .getScheduler() + .timeout(WaitForPeerTask.create(ethContext, ethTasksTimer), Duration.ofSeconds(5)); + } + + private CompletableFuture sendAndProcessNodeDataRequest(EthPeer peer, List nodeData) { + List hashes = nodeData.stream().map(NodeData::getHash).collect(Collectors.toList()); + return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer).assignPeer(peer).run() + .thenApply(PeerTaskResult::getResult) + // TODO: Update task to return this mapping + .thenApply(this::mapNodeDataByHash) + .thenAccept(data -> { + for (NodeData nodeDatum : nodeData) { + BytesValue matchingData = data.get(nodeDatum.getHash()); + if (matchingData == null) { + pendingNodeQueue.enqueue(nodeDatum); + } else { + // Persist node + nodeDatum.setData(matchingData); + nodeDatum.persist(worldStateStorageUpdater); + // TODO: Process node value and/or children + } + } + }); + } + + private Map mapNodeDataByHash(List data) { + // Map data by hash + Map dataByHash = new HashMap<>(); + data.stream() + .forEach(d -> dataByHash.put(Hash.hash(d), d)); + return dataByHash; + } +} diff --git a/services/queue/build.gradle b/services/queue/build.gradle new file mode 100644 index 0000000000..616f648e2f --- /dev/null +++ b/services/queue/build.gradle @@ -0,0 +1,38 @@ +/* + * Copyright 2018 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. + */ + +apply plugin: 'java-library' + +jar { + baseName 'pantheon-queue' + manifest { + attributes( + 'Specification-Title': baseName, + 'Specification-Version': project.version, + 'Implementation-Title': baseName, + 'Implementation-Version': calculateVersion() + ) + } +} + +dependencies { + api project(':util') + implementation project(':metrics') + + implementation 'org.apache.logging.log4j:log4j-api' + implementation 'com.google.guava:guava' + + runtime 'org.apache.logging.log4j:log4j-core' + + testImplementation 'junit:junit' +} diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java new file mode 100644 index 0000000000..cd4d778054 --- /dev/null +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java @@ -0,0 +1,35 @@ +package tech.pegasys.pantheon.services.queue; + +import java.io.IOException; +import java.util.ArrayDeque; +import java.util.Deque; + +public class InMemoryQueue implements Queue { + private long size = 0; + private Deque internalQueue = new ArrayDeque(); + + @Override + public void enqueue(T value) { + size += 1; + internalQueue.addFirst(value); + } + + @Override + public T dequeue() { + if (size == 0) { + return null; + } + size -= 1; + return internalQueue.removeLast(); + } + + @Override + public long size() { + return size; + } + + @Override + public void close() throws IOException { + internalQueue.clear(); + } +} diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java new file mode 100644 index 0000000000..4b50898dfa --- /dev/null +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java @@ -0,0 +1,35 @@ +/* + * Copyright 2018 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.services.queue; + +import static com.google.common.base.Preconditions.checkState; + +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.io.Closeable; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Stream; + +public interface Queue extends Closeable { + + void enqueue(T value); + + T dequeue(); + + long size(); + + default boolean isEmpty() { + return size() == 0; + } +} diff --git a/services/queue/src/main/resources/log4j2.xml b/services/queue/src/main/resources/log4j2.xml new file mode 100644 index 0000000000..1d05234b62 --- /dev/null +++ b/services/queue/src/main/resources/log4j2.xml @@ -0,0 +1,16 @@ + + + + INFO + + + + + + + + + + + + \ No newline at end of file diff --git a/settings.gradle b/settings.gradle index bbfac97692..42fc2c1ec5 100644 --- a/settings.gradle +++ b/settings.gradle @@ -33,6 +33,7 @@ include 'ethereum:trie' include 'metrics' include 'pantheon' include 'services:kvstore' +include 'services:queue' include 'testutil' include 'util' include 'errorprone-checks' From fad8ce1503e09dd241ce96cdc76f359c1a448626 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Mon, 21 Jan 2019 14:03:33 -0500 Subject: [PATCH 02/23] Queue additonal requests as node data comes back from the network --- .../pantheon/ethereum/core/AccountTuple.java | 92 +++++++++++++ .../worldstate/DefaultMutableWorldState.java | 46 ++----- .../worldstate/WorldStateStorage.java | 5 + ethereum/eth/build.gradle | 1 + .../sync/worldstate/AccountTrieNodeData.java | 58 ++++++++ .../eth/sync/worldstate/CodeNodeData.java | 39 ++++++ .../eth/sync/worldstate/NodeData.java | 71 +++------- .../sync/worldstate/StorageTrieNodeData.java | 46 +++++++ .../eth/sync/worldstate/TrieNodeData.java | 67 +++++++++ .../sync/worldstate/WorldStateDownloader.java | 129 +++++++++++------- .../pantheon/ethereum/trie/BranchNode.java | 9 +- .../pantheon/ethereum/trie/ExtensionNode.java | 11 +- .../pantheon/ethereum/trie/LeafNode.java | 8 +- .../pegasys/pantheon/ethereum/trie/Node.java | 17 ++- .../pantheon/ethereum/trie/NullNode.java | 6 + .../pantheon/ethereum/trie/StoredNode.java | 17 ++- .../ethereum/trie/StoredNodeFactory.java | 16 ++- .../services/queue/InMemoryQueue.java | 12 ++ .../pantheon/services/queue/Queue.java | 7 - 19 files changed, 510 insertions(+), 147 deletions(-) create mode 100644 ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java create mode 100644 ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java create mode 100644 ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java create mode 100644 ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java create mode 100644 ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java new file mode 100644 index 0000000000..6b73babe30 --- /dev/null +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java @@ -0,0 +1,92 @@ +/* + * 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.core; + +import tech.pegasys.pantheon.ethereum.rlp.RLPInput; +import tech.pegasys.pantheon.ethereum.rlp.RLPOutput; + +/** Represents the raw values associated with an account in the world state trie. */ +public class AccountTuple { + + private final long nonce; + private final Wei balance; + private final Hash storageRoot; + private final Hash codeHash; + + public AccountTuple(long nonce, Wei balance, Hash storageRoot, Hash codeHash) { + this.nonce = nonce; + this.balance = balance; + this.storageRoot = storageRoot; + this.codeHash = codeHash; + } + + /** + * The account nonce, that is the number of transactions sent from that account. + * + * @return the account nonce. + */ + public long getNonce() { + return nonce; + } + + /** + * The available balance of that account. + * + * @return the balance, in Wei, of the account. + */ + public Wei getBalance() { + return balance; + } + + /** + * The hash of the root of the storage trie associated with this account. + * + * @return the hash of the root node of the storage trie. + */ + public Hash getStorageRoot() { + return storageRoot; + } + + /** + * The hash of the EVM bytecode associated with this account. + * + * @return the hash of the account code (which may be {@link Hash#EMPTY}. + */ + public Hash getCodeHash() { + return codeHash; + } + + public void writeTo(final RLPOutput out) { + out.startList(); + + out.writeLongScalar(nonce); + out.writeUInt256Scalar(balance); + out.writeBytesValue(storageRoot); + out.writeBytesValue(codeHash); + + out.endList(); + } + + public static AccountTuple readFrom(final RLPInput in) { + in.enterList(); + + final long nonce = in.readLongScalar(); + final Wei balance = in.readUInt256Scalar(Wei::wrap); + final Hash storageRoot = Hash.wrap(in.readBytes32()); + final Hash codeHash = Hash.wrap(in.readBytes32()); + + in.leaveList(); + + return new AccountTuple(nonce, balance, storageRoot, codeHash); + } +} diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java index b3c87fbd42..4cf31e0d05 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java @@ -14,6 +14,7 @@ import tech.pegasys.pantheon.ethereum.core.AbstractWorldUpdater; import tech.pegasys.pantheon.ethereum.core.Account; +import tech.pegasys.pantheon.ethereum.core.AccountTuple; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.core.MutableWorldState; @@ -103,31 +104,14 @@ public Account get(final Address address) { private AccountState deserializeAccount( final Address address, final Hash addressHash, final BytesValue encoded) throws RLPException { final RLPInput in = RLP.input(encoded); - in.enterList(); - - final long nonce = in.readLongScalar(); - final Wei balance = in.readUInt256Scalar(Wei::wrap); - final Hash storageRoot = Hash.wrap(in.readBytes32()); - final Hash codeHash = Hash.wrap(in.readBytes32()); - - in.leaveList(); - - return new AccountState(address, addressHash, nonce, balance, storageRoot, codeHash); + AccountTuple tuple = AccountTuple.readFrom(in); + return new AccountState(address, addressHash, tuple); } private static BytesValue serializeAccount( - final long nonce, final Wei balance, final Hash codeHash, final Hash storageRoot) { - return RLP.encode( - out -> { - out.startList(); - - out.writeLongScalar(nonce); - out.writeUInt256Scalar(balance); - out.writeBytesValue(storageRoot); - out.writeBytesValue(codeHash); - - out.endList(); - }); + final long nonce, final Wei balance, final Hash storageRoot, final Hash codeHash) { + AccountTuple tuple = new AccountTuple(nonce, balance, storageRoot, codeHash); + return RLP.encode(tuple::writeTo); } @Override @@ -195,20 +179,14 @@ protected class AccountState implements Account { // Lazily initialized since we don't always access storage. private volatile MerklePatriciaTrie storageTrie; - private AccountState( - final Address address, - final Hash addressHash, - final long nonce, - final Wei balance, - final Hash storageRoot, - final Hash codeHash) { + private AccountState(final Address address, final Hash addressHash, final AccountTuple tuple) { this.address = address; this.addressHash = addressHash; - this.nonce = nonce; - this.balance = balance; - this.storageRoot = storageRoot; - this.codeHash = codeHash; + this.nonce = tuple.getNonce(); + this.balance = tuple.getBalance(); + this.storageRoot = tuple.getStorageRoot(); + this.codeHash = tuple.getCodeHash(); } private MerklePatriciaTrie storageTrie() { @@ -386,7 +364,7 @@ public void commit() { // Lastly, save the new account. final BytesValue account = - serializeAccount(updated.getNonce(), updated.getBalance(), codeHash, storageRoot); + serializeAccount(updated.getNonce(), updated.getBalance(), storageRoot, codeHash); wrapped.accountStateTrie.put(updated.getAddressHash(), account); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java index cd3fe93140..25871ce7ba 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java @@ -28,6 +28,11 @@ public interface WorldStateStorage { Optional getNodeData(Hash hash); + // TODO: look into optimizing this call in implementing classes + default boolean contains(Hash hash) { + return getNodeData(hash).isPresent(); + } + Updater updater(); interface Updater { diff --git a/ethereum/eth/build.gradle b/ethereum/eth/build.gradle index af1c81bb95..68e5f7cffc 100644 --- a/ethereum/eth/build.gradle +++ b/ethereum/eth/build.gradle @@ -29,6 +29,7 @@ dependencies { implementation project(':ethereum:core') implementation project(':ethereum:p2p') implementation project(':ethereum:rlp') + implementation project(':ethereum:trie') implementation project(':ethereum:permissioning') implementation project(':metrics') implementation project(':services:kvstore') diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java new file mode 100644 index 0000000000..9b89956a48 --- /dev/null +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java @@ -0,0 +1,58 @@ +/* + * 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.worldstate; + +import static com.google.common.base.Preconditions.checkNotNull; + +import tech.pegasys.pantheon.ethereum.core.AccountTuple; +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.rlp.RLP; +import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage.Updater; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.ArrayList; +import java.util.List; + +class AccountTrieNodeData extends TrieNodeData { + + AccountTrieNodeData(final Hash hash) { + super(Kind.ACCOUNT_TRIE_NODE, hash); + } + + @Override + void persist(final Updater updater) { + checkNotNull(getData(), "Must set data before node can be persisted."); + updater.putAccountStateTrieNode(getHash(), getData()); + } + + @Override + protected NodeData createTrieChildNodeData(final Hash childHash) { + return NodeData.createAccountTrieNode(childHash); + } + + @Override + protected List getNodeDataFromTrieNodeValue(final BytesValue value) { + List nodeData = new ArrayList<>(2); + AccountTuple accountTuple = AccountTuple.readFrom(RLP.input(value)); + // Add code + nodeData.add(NodeData.createCodeNode(accountTuple.getCodeHash())); + // Add storage, if appropriate + if (!accountTuple.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { + // If storage is non-empty queue download + NodeData storageNode = NodeData.createStorageTrieNode(accountTuple.getStorageRoot()); + nodeData.add(storageNode); + } + return nodeData; + } +} diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java new file mode 100644 index 0000000000..17a8920bda --- /dev/null +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java @@ -0,0 +1,39 @@ +/* + * 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.worldstate; + +import static com.google.common.base.Preconditions.checkNotNull; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage.Updater; + +import java.util.stream.Stream; + +class CodeNodeData extends NodeData { + + CodeNodeData(final Hash hash) { + super(Kind.CODE, hash); + } + + @Override + void persist(final Updater updater) { + checkNotNull(getData(), "Must set data before node can be persisted."); + updater.putCode(getHash(), getData()); + } + + @Override + Stream getChildNodeData() { + // Code nodes have nothing further to download + return Stream.empty(); + } +} diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java index 68ab6916c6..3e24142528 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java @@ -1,13 +1,24 @@ +/* + * 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.worldstate; -import static com.google.common.base.Preconditions.checkNotNull; - import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; -import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage.Updater; import tech.pegasys.pantheon.util.bytes.BytesValue; -public abstract class NodeData { +import java.util.stream.Stream; + +abstract class NodeData { public enum Kind { ACCOUNT_TRIE_NODE, STORAGE_TRIE_NODE, @@ -18,20 +29,20 @@ public enum Kind { private final Hash hash; private BytesValue data; - protected NodeData(Kind kind, Hash hash) { + protected NodeData(final Kind kind, final Hash hash) { this.kind = kind; this.hash = hash; } - public static AccountTrieNodeData createAccountTrieNode(Hash hash) { + public static AccountTrieNodeData createAccountTrieNode(final Hash hash) { return new AccountTrieNodeData(hash); } - public static StorageTrieNodeData createStorageTrieNode(Hash hash) { + public static StorageTrieNodeData createStorageTrieNode(final Hash hash) { return new StorageTrieNodeData(hash); } - public static CodeNodeData createCodeNode(Hash hash) { + public static CodeNodeData createCodeNode(final Hash hash) { return new CodeNodeData(hash); } @@ -47,50 +58,12 @@ public BytesValue getData() { return data; } - public NodeData setData(BytesValue data) { + public NodeData setData(final BytesValue data) { this.data = data; return this; } - abstract void persist(WorldStateStorage.Updater updater); - - - public static class AccountTrieNodeData extends NodeData { - - public AccountTrieNodeData(Hash hash) { - super(Kind.ACCOUNT_TRIE_NODE, hash); - } - - @Override - void persist(Updater updater) { - checkNotNull(getData(), "Must set data before node can be persisted."); - updater.putAccountStateTrieNode(getHash(), getData()); - } - } - - public static class StorageTrieNodeData extends NodeData { - - public StorageTrieNodeData(Hash hash) { - super(Kind.STORAGE_TRIE_NODE, hash); - } - - @Override - void persist(Updater updater) { - checkNotNull(getData(), "Must set data before node can be persisted."); - updater.putAccountStorageTrieNode(getHash(), getData()); - } - } + abstract void persist(final WorldStateStorage.Updater updater); - public static class CodeNodeData extends NodeData { - - public CodeNodeData(Hash hash) { - super(Kind.CODE, hash); - } - - @Override - void persist(Updater updater) { - checkNotNull(getData(), "Must set data before node can be persisted."); - updater.putCode(getHash(), getData()); - } - } + abstract Stream getChildNodeData(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java new file mode 100644 index 0000000000..1b251bae26 --- /dev/null +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java @@ -0,0 +1,46 @@ +/* + * 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.worldstate; + +import static com.google.common.base.Preconditions.checkNotNull; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage.Updater; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.Collections; +import java.util.List; + +class StorageTrieNodeData extends TrieNodeData { + + StorageTrieNodeData(final Hash hash) { + super(Kind.STORAGE_TRIE_NODE, hash); + } + + @Override + void persist(final Updater updater) { + checkNotNull(getData(), "Must set data before node can be persisted."); + updater.putAccountStorageTrieNode(getHash(), getData()); + } + + @Override + protected NodeData createTrieChildNodeData(final Hash childHash) { + return NodeData.createStorageTrieNode(childHash); + } + + @Override + protected List getNodeDataFromTrieNodeValue(final BytesValue value) { + // Nothing to do for terminal storage node + return Collections.emptyList(); + } +} diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java new file mode 100644 index 0000000000..ab887e31aa --- /dev/null +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java @@ -0,0 +1,67 @@ +/* + * 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.worldstate; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.trie.Node; +import tech.pegasys.pantheon.ethereum.trie.StoredNode; +import tech.pegasys.pantheon.ethereum.trie.StoredNodeFactory; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.List; +import java.util.stream.Stream; + +abstract class TrieNodeData extends NodeData { + + private static StoredNodeFactory nodeFactory = StoredNodeFactory.create(); + + TrieNodeData(Kind kind, Hash hash) { + super(kind, hash); + } + + @Override + Stream getChildNodeData() { + if (getData() == null) { + // If this node hasn't been downloaded yet, we can't return any child data + return Stream.empty(); + } + + final Node node = nodeFactory.decode(getData()); + return processTrieNode(node); + } + + private Stream processTrieNode(final Node trieNode) { + if (trieNode instanceof StoredNode && !((StoredNode) trieNode).isLoaded()) { + // Stored nodes represent nodes that are referenced by hash (and therefore must be downloaded) + NodeData req = createTrieChildNodeData(Hash.wrap(trieNode.getHash())); + return Stream.of(req); + } + // Process this child's children + final Stream childRequests = + trieNode + .getChildren() + .map(List::stream) + .map(s -> s.flatMap(this::processTrieNode)) + .orElse(Stream.of()); + + // Process value at this node, if present + return trieNode + .getValue() + .map(v -> Stream.concat(childRequests, (getNodeDataFromTrieNodeValue(v).stream()))) + .orElse(childRequests); + } + + protected abstract NodeData createTrieChildNodeData(final Hash childHash); + + protected abstract List getNodeDataFromTrieNodeValue(final BytesValue value); +} 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 01b1ebdeb1..374f688527 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 @@ -1,14 +1,17 @@ +/* + * 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.worldstate; -import java.time.Duration; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.eth.manager.AbstractPeerTask.PeerTaskResult; @@ -22,6 +25,17 @@ import tech.pegasys.pantheon.services.queue.Queue; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.time.Duration; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; + public class WorldStateDownloader { private final EthContext ethContext; @@ -33,10 +47,18 @@ public class WorldStateDownloader { private final int maxOutstandingRequests = 50; private final AtomicInteger outstandingRequests = new AtomicInteger(0); private final LabelledMetric ethTasksTimer; - private Optional> future = Optional.empty(); + private final WorldStateStorage worldStateStorage; + private final AtomicReference> future = + new AtomicReference>(null); - public WorldStateDownloader(EthContext ethContext, WorldStateStorage worldStateStorage, BlockHeader header, Queue pendingNodeQueue, final LabelledMetric ethTasksTimer) { + public WorldStateDownloader( + EthContext ethContext, + WorldStateStorage worldStateStorage, + BlockHeader header, + Queue pendingNodeQueue, + final LabelledMetric ethTasksTimer) { this.ethContext = ethContext; + this.worldStateStorage = worldStateStorage; this.header = header; this.pendingNodeQueue = pendingNodeQueue; this.ethTasksTimer = ethTasksTimer; @@ -47,10 +69,10 @@ public WorldStateDownloader(EthContext ethContext, WorldStateStorage worldStateS } public CompletableFuture run() { - if (future.isPresent() && !future.get().isDone()) { + if (future.get() != null && !future.get().isDone()) { return future.get(); } - future = Optional.of(new CompletableFuture<>()); + future.set(new CompletableFuture<>()); requestNodeData(); // TODO: Complete exceptionally on timeout / stalled download @@ -58,23 +80,26 @@ public CompletableFuture run() { } private void requestNodeData() { - while (outstandingRequests.get() < maxOutstandingRequests && !pendingNodeQueue.isEmpty()) { + while (!future.get().isDone() + && outstandingRequests.get() < maxOutstandingRequests + && !pendingNodeQueue.isEmpty()) { // Find available peer Optional maybePeer = ethContext.getEthPeers().idlePeer(header.getNumber()); if (!maybePeer.isPresent()) { // If no peer is available, wait and try again waitForNewPeer() - .whenComplete((r,t) -> { - requestNodeData(); - }); + .whenComplete( + (r, t) -> { + requestNodeData(); + }); return; } EthPeer peer = maybePeer.get(); // Collect data to be requested List toRequest = new ArrayList<>(); - for(int i = 0; i < hashCountPerRequest; i++) { + for (int i = 0; i < hashCountPerRequest; i++) { if (pendingNodeQueue.isEmpty()) { break; } @@ -84,47 +109,59 @@ private void requestNodeData() { // Request and process node data outstandingRequests.incrementAndGet(); sendAndProcessNodeDataRequest(peer, toRequest) - .thenAccept((res) -> { - if (outstandingRequests.decrementAndGet() == 0 && pendingNodeQueue.isEmpty()) { - // We're done - future.get().complete(null); - } - }); + .whenComplete( + (res, error) -> { + if (outstandingRequests.decrementAndGet() == 0 && pendingNodeQueue.isEmpty()) { + // We're done + future.get().complete(null); + } else { + // Send out additional requests + requestNodeData(); + } + }); } } private CompletableFuture waitForNewPeer() { return ethContext - .getScheduler() - .timeout(WaitForPeerTask.create(ethContext, ethTasksTimer), Duration.ofSeconds(5)); + .getScheduler() + .timeout(WaitForPeerTask.create(ethContext, ethTasksTimer), Duration.ofSeconds(5)); } - private CompletableFuture sendAndProcessNodeDataRequest(EthPeer peer, List nodeData) { + private CompletableFuture sendAndProcessNodeDataRequest( + EthPeer peer, List nodeData) { List hashes = nodeData.stream().map(NodeData::getHash).collect(Collectors.toList()); - return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer).assignPeer(peer).run() - .thenApply(PeerTaskResult::getResult) - // TODO: Update task to return this mapping - .thenApply(this::mapNodeDataByHash) - .thenAccept(data -> { - for (NodeData nodeDatum : nodeData) { - BytesValue matchingData = data.get(nodeDatum.getHash()); - if (matchingData == null) { - pendingNodeQueue.enqueue(nodeDatum); - } else { - // Persist node - nodeDatum.setData(matchingData); - nodeDatum.persist(worldStateStorageUpdater); - // TODO: Process node value and/or children - } - } - }); + return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer) + .assignPeer(peer) + .run() + .thenApply(PeerTaskResult::getResult) + // TODO: Update task to return this mapping + .thenApply(this::mapNodeDataByHash) + .thenAccept( + data -> { + for (NodeData nodeDatum : nodeData) { + BytesValue matchingData = data.get(nodeDatum.getHash()); + if (matchingData == null) { + pendingNodeQueue.enqueue(nodeDatum); + } else { + // Persist node + nodeDatum.setData(matchingData); + nodeDatum.persist(worldStateStorageUpdater); + + // Queue child requests + nodeDatum + .getChildNodeData() + .filter(n -> !worldStateStorage.contains(n.getHash())) + .forEach(pendingNodeQueue::enqueue); + } + } + }); } private Map mapNodeDataByHash(List data) { // Map data by hash Map dataByHash = new HashMap<>(); - data.stream() - .forEach(d -> dataByHash.put(Hash.hash(d), d)); + data.stream().forEach(d -> dataByHash.put(Hash.hash(d), d)); return dataByHash; } } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java index f3b02ebf1b..803a44ed00 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java @@ -23,6 +23,8 @@ import java.lang.ref.SoftReference; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.function.Function; @@ -73,6 +75,11 @@ public Optional getValue() { return value; } + @Override + public Optional>> getChildren() { + return Optional.of(Collections.unmodifiableList(children)); + } + public Node child(final byte index) { return children.get(index); } @@ -104,7 +111,7 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { final BytesValue rlp = getRlp(); - if (rlp.size() < 32) { + if (shouldBeInlined()) { return rlp; } else { return RLP.encodeOne(getHash()); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java index 7a8be5d591..d5755b4f8a 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java @@ -22,6 +22,8 @@ import java.lang.ref.SoftReference; import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.List; import java.util.Optional; class ExtensionNode implements Node { @@ -58,7 +60,12 @@ public BytesValue getPath() { @Override public Optional getValue() { - throw new UnsupportedOperationException(); + return Optional.empty(); + } + + @Override + public Optional>> getChildren() { + return Optional.of(Collections.singletonList(child)); } public Node getChild() { @@ -86,7 +93,7 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { final BytesValue rlp = getRlp(); - if (rlp.size() < 32) { + if (shouldBeInlined()) { return rlp; } else { return RLP.encodeOne(getHash()); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java index 07ed18fc5d..cffc008f45 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java @@ -21,6 +21,7 @@ import java.lang.ref.SoftReference; import java.lang.ref.WeakReference; +import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -64,6 +65,11 @@ public Optional getValue() { return Optional.of(value); } + @Override + public Optional>> getChildren() { + return Optional.empty(); + } + @Override public BytesValue getRlp() { if (rlp != null) { @@ -86,7 +92,7 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { final BytesValue rlp = getRlp(); - if (rlp.size() < 32) { + if (shouldBeInlined()) { return rlp; } else { return RLP.encodeOne(getHash()); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java index 4d8b62e1ce..135a837720 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java @@ -15,9 +15,10 @@ import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.util.List; import java.util.Optional; -interface Node { +public interface Node { Node accept(PathNodeVisitor visitor, BytesValue path); @@ -27,10 +28,24 @@ interface Node { Optional getValue(); + Optional>> getChildren(); + BytesValue getRlp(); BytesValue getRlpRef(); + /** + * Whether a reference to this node should be inlined (the rlp stored directly in the parent + * node). If true, rlp is included in parent directly. If false, a hash of the rlp is stored in + * the parent node as a reference. + * + * @return + */ + default boolean shouldBeInlined() { + BytesValue rlp = getRlp(); + return rlp.size() >= 32; + } + Bytes32 getHash(); Node replacePath(BytesValue path); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java index 322e6240e6..1adca25d6d 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java @@ -18,6 +18,7 @@ import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.util.List; import java.util.Optional; class NullNode implements Node { @@ -53,6 +54,11 @@ public Optional getValue() { return Optional.empty(); } + @Override + public Optional>> getChildren() { + return Optional.empty(); + } + @Override public BytesValue getRlp() { return RLP.NULL; diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java index bd80594f90..861fcdcf8a 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java @@ -16,9 +16,10 @@ import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.util.List; import java.util.Optional; -class StoredNode implements Node { +public class StoredNode implements Node { private final StoredNodeFactory nodeFactory; private final Bytes32 hash; private Node loaded; @@ -28,6 +29,10 @@ class StoredNode implements Node { this.hash = hash; } + public boolean isLoaded() { + return loaded != null; + } + /** @return True if the node needs to be persisted. */ @Override public boolean isDirty() { @@ -63,6 +68,11 @@ public Optional getValue() { return load().getValue(); } + @Override + public Optional>> getChildren() { + return load().getChildren(); + } + @Override public BytesValue getRlp() { // Getting the rlp representation is only needed when persisting a concrete node @@ -87,7 +97,10 @@ public Node replacePath(final BytesValue path) { private Node load() { if (loaded == null) { - loaded = nodeFactory.retrieve(hash); + loaded = + nodeFactory + .retrieve(hash) + .orElseThrow(() -> new MerkleStorageException("Missing value for hash " + hash)); } return loaded; diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java index 5268022fe2..9bfc5cd9fe 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java @@ -26,7 +26,7 @@ import java.util.function.Function; import java.util.function.Supplier; -class StoredNodeFactory implements NodeFactory { +public class StoredNodeFactory implements NodeFactory { @SuppressWarnings("rawtypes") private static final NullNode NULL_NODE = NullNode.instance(); @@ -43,6 +43,11 @@ class StoredNodeFactory implements NodeFactory { this.valueDeserializer = valueDeserializer; } + public static StoredNodeFactory create() { + return new StoredNodeFactory<>( + (h) -> Optional.empty(), Function.identity(), Function.identity()); + } + @Override public Node createExtension(final BytesValue path, final Node child) { return handleNewNode(new ExtensionNode<>(path, child, this)); @@ -87,7 +92,7 @@ private Node handleNewNode(final Node node) { return node; } - public Node retrieve(final Bytes32 hash) throws MerkleStorageException { + public Optional> retrieve(final Bytes32 hash) throws MerkleStorageException { return nodeLoader .getNode(hash) .map( @@ -97,8 +102,11 @@ public Node retrieve(final Bytes32 hash) throws MerkleStorageException { assert (hash.equals(node.getHash())) : "Node hash " + node.getHash() + " not equal to expected " + hash; return node; - }) - .orElseThrow(() -> new MerkleStorageException("Missing value for hash " + hash)); + }); + } + + public Node decode(final BytesValue rlp) { + return decode(rlp, () -> String.format("Failed to decode value %s", rlp.toString())); } private Node decode(final BytesValue rlp, final Supplier errMessage) diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java index cd4d778054..00f8613da6 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java @@ -1,3 +1,15 @@ +/* + * 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.services.queue; import java.io.IOException; diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java index 4b50898dfa..fdc3592a1c 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java @@ -12,14 +12,7 @@ */ package tech.pegasys.pantheon.services.queue; -import static com.google.common.base.Preconditions.checkState; - -import tech.pegasys.pantheon.util.bytes.BytesValue; - import java.io.Closeable; -import java.util.Objects; -import java.util.Optional; -import java.util.stream.Stream; public interface Queue extends Closeable { From e1178b3eb4c11662d0ee9f038bf8967ccf4970a5 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Wed, 23 Jan 2019 12:29:53 -0500 Subject: [PATCH 03/23] Fix bug --- .../java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java | 3 +-- .../tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java | 3 +-- .../java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java | 3 +-- .../main/java/tech/pegasys/pantheon/ethereum/trie/Node.java | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java index 803a44ed00..24cb398030 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java @@ -110,9 +110,8 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { - final BytesValue rlp = getRlp(); if (shouldBeInlined()) { - return rlp; + return getRlp(); } else { return RLP.encodeOne(getHash()); } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java index d5755b4f8a..f99af8c2a6 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java @@ -92,9 +92,8 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { - final BytesValue rlp = getRlp(); if (shouldBeInlined()) { - return rlp; + return getRlp(); } else { return RLP.encodeOne(getHash()); } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java index cffc008f45..fd4bf9a36a 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java @@ -91,9 +91,8 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { - final BytesValue rlp = getRlp(); if (shouldBeInlined()) { - return rlp; + return getRlp(); } else { return RLP.encodeOne(getHash()); } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java index 135a837720..436c5daf72 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java @@ -42,8 +42,7 @@ public interface Node { * @return */ default boolean shouldBeInlined() { - BytesValue rlp = getRlp(); - return rlp.size() >= 32; + return getRlp().size() < 32; } Bytes32 getHash(); From 1b2dff64e93ddbd0b3e8a38c8d3fbe031c85a07d Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Wed, 23 Jan 2019 13:44:43 -0500 Subject: [PATCH 04/23] Fix some warnings, start adding tests --- .../eth/sync/worldstate/TrieNodeData.java | 2 +- .../sync/worldstate/WorldStateDownloader.java | 5 +- .../ethtaskutils/BlockchainSetupUtil.java | 2 +- .../worldstate/WorldStateDownloaderTest.java | 84 +++++++++++++++++++ .../services/queue/InMemoryQueue.java | 4 +- 5 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java index ab887e31aa..3d112cd229 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java @@ -23,7 +23,7 @@ abstract class TrieNodeData extends NodeData { - private static StoredNodeFactory nodeFactory = StoredNodeFactory.create(); + private static final StoredNodeFactory nodeFactory = StoredNodeFactory.create(); TrieNodeData(Kind kind, Hash hash) { super(kind, hash); 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 374f688527..670f69eb6d 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 @@ -49,13 +49,13 @@ public class WorldStateDownloader { private final LabelledMetric ethTasksTimer; private final WorldStateStorage worldStateStorage; private final AtomicReference> future = - new AtomicReference>(null); + new AtomicReference<>(null); public WorldStateDownloader( EthContext ethContext, WorldStateStorage worldStateStorage, BlockHeader header, - Queue pendingNodeQueue, + Queue pendingNodeQueue, final LabelledMetric ethTasksTimer) { this.ethContext = ethContext; this.worldStateStorage = worldStateStorage; @@ -113,6 +113,7 @@ private void requestNodeData() { (res, error) -> { if (outstandingRequests.decrementAndGet() == 0 && pendingNodeQueue.isEmpty()) { // We're done + worldStateStorageUpdater.commit(); future.get().complete(null); } else { // Send out additional requests diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ethtaskutils/BlockchainSetupUtil.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ethtaskutils/BlockchainSetupUtil.java index 61abd6d906..f6db36633a 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ethtaskutils/BlockchainSetupUtil.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/ethtaskutils/BlockchainSetupUtil.java @@ -54,7 +54,7 @@ public class BlockchainSetupUtil { private final List blocks; private long maxBlockNumber; - public BlockchainSetupUtil( + private BlockchainSetupUtil( final GenesisState genesisState, final MutableBlockchain blockchain, final ProtocolContext protocolContext, diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java new file mode 100644 index 0000000000..631f319bae --- /dev/null +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -0,0 +1,84 @@ +package tech.pegasys.pantheon.ethereum.eth.sync.worldstate; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.junit.BeforeClass; +import org.junit.Test; +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.EthProtocolManager; +import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManagerTestUtil; +import tech.pegasys.pantheon.ethereum.eth.manager.RespondingEthPeer; +import tech.pegasys.pantheon.ethereum.eth.manager.RespondingEthPeer.Responder; +import tech.pegasys.pantheon.ethereum.eth.manager.ethtaskutils.BlockchainSetupUtil; +import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; +import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueStorageWorldStateStorage; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; +import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; +import tech.pegasys.pantheon.metrics.LabelledMetric; +import tech.pegasys.pantheon.metrics.OperationTimer; +import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; +import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; +import tech.pegasys.pantheon.services.queue.InMemoryQueue; +import tech.pegasys.pantheon.services.queue.Queue; + +public class WorldStateDownloaderTest { + + private static WorldStateArchive remoteWorldArchive; + private static MutableBlockchain remoteBlockchain; + private static ProtocolSchedule protocolSchedule; + private static ProtocolContext protocolContext; + private static LabelledMetric ethTasksTimer; + private static EthProtocolManager ethProtocolManager; + + @BeforeClass + public static void setup() { + final BlockchainSetupUtil blockchainSetupUtil = BlockchainSetupUtil.forTesting(); + blockchainSetupUtil.importAllBlocks(); + remoteBlockchain = blockchainSetupUtil.getBlockchain(); + remoteWorldArchive = blockchainSetupUtil.getWorldArchive(); + protocolSchedule = blockchainSetupUtil.getProtocolSchedule(); + protocolContext = blockchainSetupUtil.getProtocolContext(); + ethTasksTimer = NoOpMetricsSystem.NO_OP_LABELLED_TIMER; + ethProtocolManager = EthProtocolManagerTestUtil.create(); + } + + @Test + public void downloadAvailableWorldStateFromPeers() { + // Pull chain head and a prior header with a different state root + BlockHeader chainHead = remoteBlockchain.getChainHeadHeader(); + BlockHeader header = remoteBlockchain.getBlockHeader(chainHead.getNumber() - 1).get(); + assertThat(chainHead.getStateRoot()).isNotEqualTo(header.getStateRoot()); + + Queue queue = new InMemoryQueue<>(); + WorldStateStorage stateStorage = new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + WorldStateDownloader downloader = new WorldStateDownloader(ethProtocolManager.ethContext(), stateStorage, header, queue, ethTasksTimer); + + // Create some peers that can respond + int peerCount = 5; + List peers = Stream.generate(() -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, remoteBlockchain)) + .limit(peerCount) + .collect(Collectors.toList()); + + // Start downloader + CompletableFuture result = downloader.run(); + + // Respond to node data requests + Responder responder = RespondingEthPeer.blockchainResponder(remoteBlockchain, remoteWorldArchive); + while (!result.isDone()) { + for (RespondingEthPeer peer : peers) { + peer.respond(responder); + } + } + + assertThat(result).isDone(); + assertThat(stateStorage.contains(chainHead.getStateRoot())).isFalse(); + } + + +} diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java index 00f8613da6..36ea10e5dd 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java @@ -18,10 +18,10 @@ public class InMemoryQueue implements Queue { private long size = 0; - private Deque internalQueue = new ArrayDeque(); + private final Deque internalQueue = new ArrayDeque(); @Override - public void enqueue(T value) { + public void enqueue(final T value) { size += 1; internalQueue.addFirst(value); } From 17114a8f6d5273f281c0e8980e656171625acfac Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Wed, 23 Jan 2019 17:17:22 -0500 Subject: [PATCH 05/23] Finish first downloader test, fix some issues related codeless accounts Add some small optimizations related to persisting empty code. --- .../KeyValueStorageWorldStateStorage.java | 7 ++ .../worldstate/WorldStateArchive.java | 4 + .../worldstate/WorldStateStorage.java | 4 +- .../ethereum/eth/manager/EthServer.java | 7 +- .../sync/worldstate/AccountTrieNodeData.java | 6 +- .../sync/worldstate/WorldStateDownloader.java | 3 +- .../ethereum/eth/manager/EthServerTest.java | 8 ++ .../worldstate/WorldStateDownloaderTest.java | 107 ++++++++++++++---- .../trie/StoredMerklePatriciaTrie.java | 6 + 9 files changed, 127 insertions(+), 25 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java index 225c40e56f..a8fa6e40fc 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java @@ -30,6 +30,9 @@ public KeyValueStorageWorldStateStorage(final KeyValueStorage keyValueStorage) { @Override public Optional getCode(final Hash codeHash) { + if (codeHash.equals(Hash.EMPTY)) { + return Optional.of(BytesValue.EMPTY); + } return keyValueStorage.get(codeHash); } @@ -63,6 +66,10 @@ public Updater(final KeyValueStorage.Transaction transaction) { @Override public void putCode(final Bytes32 codeHash, final BytesValue code) { + if (code.size() == 0) { + // Don't save empty values + return; + } transaction.put(codeHash, code); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java index 215fc61385..2fedbf0f98 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java @@ -47,4 +47,8 @@ public MutableWorldState getMutable() { public Optional getNodeData(final Hash hash) { return storage.getNodeData(hash); } + + public WorldStateStorage getStorage() { + return storage; + } } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java index 25871ce7ba..f6feb08bf6 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java @@ -40,7 +40,9 @@ interface Updater { void putCode(Bytes32 nodeHash, BytesValue code); default void putCode(final BytesValue code) { - putCode(Hash.hash(code), code); + // Skip the hash calculation for empty code + Hash codeHash = code.size() == 0 ? Hash.EMPTY : Hash.hash(code); + putCode(codeHash, code); } void putAccountStateTrieNode(Bytes32 nodeHash, BytesValue node); diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java index d631a13d17..7b3ef049d6 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java @@ -217,7 +217,12 @@ static MessageData constructGetNodeDataResponse( } count++; - worldStateArchive.getNodeData(hash).ifPresent(nodeData::add); + if (hash.equals(Hash.EMPTY)) { + // No need to go to the archive for an empty value + nodeData.add(BytesValue.EMPTY); + } else { + worldStateArchive.getNodeData(hash).ifPresent(nodeData::add); + } } return NodeDataMessage.create(nodeData); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java index 9b89956a48..63ff1e18be 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java @@ -45,8 +45,10 @@ protected NodeData createTrieChildNodeData(final Hash childHash) { protected List getNodeDataFromTrieNodeValue(final BytesValue value) { List nodeData = new ArrayList<>(2); AccountTuple accountTuple = AccountTuple.readFrom(RLP.input(value)); - // Add code - nodeData.add(NodeData.createCodeNode(accountTuple.getCodeHash())); + // Add code, if appropriate + if (!accountTuple.getCodeHash().equals(Hash.EMPTY)) { + nodeData.add(NodeData.createCodeNode(accountTuple.getCodeHash())); + } // Add storage, if appropriate if (!accountTuple.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { // If storage is non-empty queue download 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 670f69eb6d..8843128dc7 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 @@ -48,8 +48,7 @@ public class WorldStateDownloader { private final AtomicInteger outstandingRequests = new AtomicInteger(0); private final LabelledMetric ethTasksTimer; private final WorldStateStorage worldStateStorage; - private final AtomicReference> future = - new AtomicReference<>(null); + private final AtomicReference> future = new AtomicReference<>(null); public WorldStateDownloader( EthContext ethContext, diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java index 43eddf2397..6ba0ad4b60 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java @@ -57,6 +57,14 @@ public void shouldRespondToNodeDataRequests() throws Exception { verify(ethPeer).send(NodeDataMessage.create(asList(VALUE1, VALUE2))); } + @Test + public void shouldRespondToNodeDataRequestsForEmptyValues() throws Exception { + when(worldStateArchive.getNodeData(Hash.EMPTY)).thenReturn(Optional.empty()); + ethMessages.dispatch(new EthMessage(ethPeer, GetNodeDataMessage.create(asList(Hash.EMPTY)))); + + verify(ethPeer).send(NodeDataMessage.create(asList(BytesValue.EMPTY))); + } + @Test public void shouldHandleDataBeingUnavailableWhenRespondingToNodeDataRequests() throws Exception { when(worldStateArchive.getNodeData(HASH1)).thenReturn(Optional.of(VALUE1)); diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index 631f319bae..75f0691af2 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -1,23 +1,32 @@ +/* + * 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.worldstate; import static org.assertj.core.api.Assertions.assertThat; -import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import org.junit.BeforeClass; -import org.junit.Test; -import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; +import tech.pegasys.pantheon.ethereum.core.AccountTuple; import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.Hash; 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.RespondingEthPeer.Responder; import tech.pegasys.pantheon.ethereum.eth.manager.ethtaskutils.BlockchainSetupUtil; -import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; +import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueStorageWorldStateStorage; +import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; +import tech.pegasys.pantheon.ethereum.trie.StoredMerklePatriciaTrie; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; import tech.pegasys.pantheon.metrics.LabelledMetric; @@ -26,13 +35,23 @@ import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; import tech.pegasys.pantheon.services.queue.InMemoryQueue; import tech.pegasys.pantheon.services.queue.Queue; +import tech.pegasys.pantheon.util.bytes.Bytes32; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.BeforeClass; +import org.junit.Test; public class WorldStateDownloaderTest { private static WorldStateArchive remoteWorldArchive; + private static WorldStateStorage remoteWorldStorage; private static MutableBlockchain remoteBlockchain; - private static ProtocolSchedule protocolSchedule; - private static ProtocolContext protocolContext; private static LabelledMetric ethTasksTimer; private static EthProtocolManager ethProtocolManager; @@ -42,8 +61,7 @@ public static void setup() { blockchainSetupUtil.importAllBlocks(); remoteBlockchain = blockchainSetupUtil.getBlockchain(); remoteWorldArchive = blockchainSetupUtil.getWorldArchive(); - protocolSchedule = blockchainSetupUtil.getProtocolSchedule(); - protocolContext = blockchainSetupUtil.getProtocolContext(); + remoteWorldStorage = remoteWorldArchive.getStorage(); ethTasksTimer = NoOpMetricsSystem.NO_OP_LABELLED_TIMER; ethProtocolManager = EthProtocolManagerTestUtil.create(); } @@ -56,20 +74,26 @@ public void downloadAvailableWorldStateFromPeers() { assertThat(chainHead.getStateRoot()).isNotEqualTo(header.getStateRoot()); Queue queue = new InMemoryQueue<>(); - WorldStateStorage stateStorage = new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); - WorldStateDownloader downloader = new WorldStateDownloader(ethProtocolManager.ethContext(), stateStorage, header, queue, ethTasksTimer); + WorldStateStorage localStorage = + new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + WorldStateDownloader downloader = + new WorldStateDownloader( + ethProtocolManager.ethContext(), localStorage, header, queue, ethTasksTimer); // Create some peers that can respond int peerCount = 5; - List peers = Stream.generate(() -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, remoteBlockchain)) - .limit(peerCount) - .collect(Collectors.toList()); + List peers = + Stream.generate( + () -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, remoteBlockchain)) + .limit(peerCount) + .collect(Collectors.toList()); // Start downloader CompletableFuture result = downloader.run(); // Respond to node data requests - Responder responder = RespondingEthPeer.blockchainResponder(remoteBlockchain, remoteWorldArchive); + Responder responder = + RespondingEthPeer.blockchainResponder(remoteBlockchain, remoteWorldArchive); while (!result.isDone()) { for (RespondingEthPeer peer : peers) { peer.respond(responder); @@ -77,8 +101,53 @@ public void downloadAvailableWorldStateFromPeers() { } assertThat(result).isDone(); - assertThat(stateStorage.contains(chainHead.getStateRoot())).isFalse(); + assertStorageForWorldStateMatchesExpectation( + header.getStateRoot(), remoteWorldStorage, localStorage); + // We shouldn't have the chain head data, since we downloaded from a prior block + assertThat(localStorage.contains(chainHead.getStateRoot())).isFalse(); } + protected void assertStorageForWorldStateMatchesExpectation( + Hash stateRoot, WorldStateStorage expectedStorage, WorldStateStorage actualStorage) { + // Get account entries + final MerklePatriciaTrie expectedWorldStateTrie = + StoredMerklePatriciaTrie.create(expectedStorage::getAccountStorageTrieNode, stateRoot); + final Map expectedAccountEntries = + expectedWorldStateTrie.entriesFrom(Bytes32.ZERO, 500); + final MerklePatriciaTrie actualWorldStateTrie = + StoredMerklePatriciaTrie.create(actualStorage::getAccountStorageTrieNode, stateRoot); + final Map actualAccountEntries = + actualWorldStateTrie.entriesFrom(Bytes32.ZERO, 500); + // Verify account entries + assertThat(expectedAccountEntries).isNotEmpty(); // Sanity check + assertThat(actualAccountEntries).isEqualTo(expectedAccountEntries); + // Extract account tuples + List accounts = + expectedAccountEntries + .entrySet() + .stream() + .map(e -> AccountTuple.readFrom(RLP.input(e.getValue()))) + .collect(Collectors.toList()); + + // Verify each account + for (AccountTuple account : accounts) { + // Verify storage + final MerklePatriciaTrie expectedStorageTrie = + StoredMerklePatriciaTrie.create( + expectedStorage::getAccountStorageTrieNode, account.getStorageRoot()); + final Map expectedStorageEntries = + expectedStorageTrie.entriesFrom(Bytes32.ZERO, 500); + final MerklePatriciaTrie actualStorageTrie = + StoredMerklePatriciaTrie.create( + actualStorage::getAccountStorageTrieNode, account.getStorageRoot()); + final Map actualStorageEntries = + actualStorageTrie.entriesFrom(Bytes32.ZERO, 500); + assertThat(actualStorageEntries).isEqualTo(expectedStorageEntries); + + // Verify code is stored + Hash codeHash = account.getCodeHash(); + assertThat(actualStorage.getCode(codeHash)).isEqualTo(expectedStorage.getCode(codeHash)); + } + } } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java index b7f820c692..2175e285a2 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java @@ -48,6 +48,12 @@ public StoredMerklePatriciaTrie( this(nodeLoader, MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, valueSerializer, valueDeserializer); } + public static StoredMerklePatriciaTrie create( + NodeLoader nodeLoader, Bytes32 rootHash) { + return new StoredMerklePatriciaTrie<>( + nodeLoader, rootHash, Function.identity(), Function.identity()); + } + /** * Create a trie. * From ac572f34a5b0197f94ee93e28382724d0b908ce0 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Wed, 23 Jan 2019 17:34:08 -0500 Subject: [PATCH 06/23] Add final's --- .../pegasys/pantheon/ethereum/core/AccountTuple.java | 3 ++- .../ethereum/worldstate/WorldStateStorage.java | 2 +- .../ethereum/eth/sync/worldstate/TrieNodeData.java | 2 +- .../eth/sync/worldstate/WorldStateDownloader.java | 12 ++++++------ .../sync/worldstate/WorldStateDownloaderTest.java | 4 +++- .../ethereum/trie/StoredMerklePatriciaTrie.java | 2 +- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java index 6b73babe30..eb6c442932 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java @@ -23,7 +23,8 @@ public class AccountTuple { private final Hash storageRoot; private final Hash codeHash; - public AccountTuple(long nonce, Wei balance, Hash storageRoot, Hash codeHash) { + public AccountTuple( + final long nonce, final Wei balance, final Hash storageRoot, final Hash codeHash) { this.nonce = nonce; this.balance = balance; this.storageRoot = storageRoot; diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java index f6feb08bf6..dda7b3ab5e 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java @@ -29,7 +29,7 @@ public interface WorldStateStorage { Optional getNodeData(Hash hash); // TODO: look into optimizing this call in implementing classes - default boolean contains(Hash hash) { + default boolean contains(final Hash hash) { return getNodeData(hash).isPresent(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java index 3d112cd229..53dd257332 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java @@ -25,7 +25,7 @@ abstract class TrieNodeData extends NodeData { private static final StoredNodeFactory nodeFactory = StoredNodeFactory.create(); - TrieNodeData(Kind kind, Hash hash) { + TrieNodeData(final Kind kind, final Hash hash) { super(kind, hash); } 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 8843128dc7..446cc0892d 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 @@ -51,10 +51,10 @@ public class WorldStateDownloader { private final AtomicReference> future = new AtomicReference<>(null); public WorldStateDownloader( - EthContext ethContext, - WorldStateStorage worldStateStorage, - BlockHeader header, - Queue pendingNodeQueue, + final EthContext ethContext, + final WorldStateStorage worldStateStorage, + final BlockHeader header, + final Queue pendingNodeQueue, final LabelledMetric ethTasksTimer) { this.ethContext = ethContext; this.worldStateStorage = worldStateStorage; @@ -129,7 +129,7 @@ private CompletableFuture waitForNewPeer() { } private CompletableFuture sendAndProcessNodeDataRequest( - EthPeer peer, List nodeData) { + final EthPeer peer, final List nodeData) { List hashes = nodeData.stream().map(NodeData::getHash).collect(Collectors.toList()); return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer) .assignPeer(peer) @@ -158,7 +158,7 @@ private CompletableFuture sendAndProcessNodeDataRequest( }); } - private Map mapNodeDataByHash(List data) { + private Map mapNodeDataByHash(final List data) { // Map data by hash Map dataByHash = new HashMap<>(); data.stream().forEach(d -> dataByHash.put(Hash.hash(d), d)); diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index 75f0691af2..e2a3463f6d 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -108,7 +108,9 @@ public void downloadAvailableWorldStateFromPeers() { } protected void assertStorageForWorldStateMatchesExpectation( - Hash stateRoot, WorldStateStorage expectedStorage, WorldStateStorage actualStorage) { + final Hash stateRoot, + final WorldStateStorage expectedStorage, + final WorldStateStorage actualStorage) { // Get account entries final MerklePatriciaTrie expectedWorldStateTrie = StoredMerklePatriciaTrie.create(expectedStorage::getAccountStorageTrieNode, stateRoot); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java index 2175e285a2..f38604d3eb 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java @@ -49,7 +49,7 @@ public StoredMerklePatriciaTrie( } public static StoredMerklePatriciaTrie create( - NodeLoader nodeLoader, Bytes32 rootHash) { + final NodeLoader nodeLoader, final Bytes32 rootHash) { return new StoredMerklePatriciaTrie<>( nodeLoader, rootHash, Function.identity(), Function.identity()); } From 2ef2c6aa1aa9813b58d30bcc5805f3b5bf165149 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Wed, 23 Jan 2019 17:54:54 -0500 Subject: [PATCH 07/23] Rename some classes and variable for clarity --- ...a.java => AccountTrieNodeDataRequest.java} | 17 +++---- ...NodeData.java => CodeNodeDataRequest.java} | 6 +-- .../{NodeData.java => NodeDataRequest.java} | 20 ++++---- ...a.java => StorageTrieNodeDataRequest.java} | 10 ++-- ...NodeData.java => TrieNodeDataRequest.java} | 22 ++++----- .../sync/worldstate/WorldStateDownloader.java | 47 ++++++++++--------- .../worldstate/WorldStateDownloaderTest.java | 6 +-- .../queue/{Queue.java => BigQueue.java} | 7 ++- ...MemoryQueue.java => InMemoryBigQueue.java} | 2 +- 9 files changed, 72 insertions(+), 65 deletions(-) rename ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/{AccountTrieNodeData.java => AccountTrieNodeDataRequest.java} (75%) rename ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/{CodeNodeData.java => CodeNodeDataRequest.java} (89%) rename ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/{NodeData.java => NodeDataRequest.java} (69%) rename ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/{StorageTrieNodeData.java => StorageTrieNodeDataRequest.java} (79%) rename ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/{TrieNodeData.java => TrieNodeDataRequest.java} (71%) rename services/queue/src/main/java/tech/pegasys/pantheon/services/queue/{Queue.java => BigQueue.java} (84%) rename services/queue/src/main/java/tech/pegasys/pantheon/services/queue/{InMemoryQueue.java => InMemoryBigQueue.java} (95%) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java similarity index 75% rename from ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java rename to ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java index 63ff1e18be..77de4b904b 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java @@ -24,9 +24,9 @@ import java.util.ArrayList; import java.util.List; -class AccountTrieNodeData extends TrieNodeData { +class AccountTrieNodeDataRequest extends TrieNodeDataRequest { - AccountTrieNodeData(final Hash hash) { + AccountTrieNodeDataRequest(final Hash hash) { super(Kind.ACCOUNT_TRIE_NODE, hash); } @@ -37,22 +37,23 @@ void persist(final Updater updater) { } @Override - protected NodeData createTrieChildNodeData(final Hash childHash) { - return NodeData.createAccountTrieNode(childHash); + protected NodeDataRequest createTrieChildNodeData(final Hash childHash) { + return NodeDataRequest.createAccountTrieNode(childHash); } @Override - protected List getNodeDataFromTrieNodeValue(final BytesValue value) { - List nodeData = new ArrayList<>(2); + protected List getRequestsFromTrieNodeValue(final BytesValue value) { + List nodeData = new ArrayList<>(2); AccountTuple accountTuple = AccountTuple.readFrom(RLP.input(value)); // Add code, if appropriate if (!accountTuple.getCodeHash().equals(Hash.EMPTY)) { - nodeData.add(NodeData.createCodeNode(accountTuple.getCodeHash())); + nodeData.add(NodeDataRequest.createCodeNode(accountTuple.getCodeHash())); } // Add storage, if appropriate if (!accountTuple.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { // If storage is non-empty queue download - NodeData storageNode = NodeData.createStorageTrieNode(accountTuple.getStorageRoot()); + NodeDataRequest storageNode = + NodeDataRequest.createStorageTrieNode(accountTuple.getStorageRoot()); nodeData.add(storageNode); } return nodeData; diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java similarity index 89% rename from ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java rename to ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java index 17a8920bda..eee77db011 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java @@ -19,9 +19,9 @@ import java.util.stream.Stream; -class CodeNodeData extends NodeData { +class CodeNodeDataRequest extends NodeDataRequest { - CodeNodeData(final Hash hash) { + CodeNodeDataRequest(final Hash hash) { super(Kind.CODE, hash); } @@ -32,7 +32,7 @@ void persist(final Updater updater) { } @Override - Stream getChildNodeData() { + Stream getChildRequests() { // Code nodes have nothing further to download return Stream.empty(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java similarity index 69% rename from ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java rename to ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java index 3e24142528..6b817c030b 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java @@ -18,7 +18,7 @@ import java.util.stream.Stream; -abstract class NodeData { +abstract class NodeDataRequest { public enum Kind { ACCOUNT_TRIE_NODE, STORAGE_TRIE_NODE, @@ -29,21 +29,21 @@ public enum Kind { private final Hash hash; private BytesValue data; - protected NodeData(final Kind kind, final Hash hash) { + protected NodeDataRequest(final Kind kind, final Hash hash) { this.kind = kind; this.hash = hash; } - public static AccountTrieNodeData createAccountTrieNode(final Hash hash) { - return new AccountTrieNodeData(hash); + public static AccountTrieNodeDataRequest createAccountTrieNode(final Hash hash) { + return new AccountTrieNodeDataRequest(hash); } - public static StorageTrieNodeData createStorageTrieNode(final Hash hash) { - return new StorageTrieNodeData(hash); + public static StorageTrieNodeDataRequest createStorageTrieNode(final Hash hash) { + return new StorageTrieNodeDataRequest(hash); } - public static CodeNodeData createCodeNode(final Hash hash) { - return new CodeNodeData(hash); + public static CodeNodeDataRequest createCodeNode(final Hash hash) { + return new CodeNodeDataRequest(hash); } public Kind getKind() { @@ -58,12 +58,12 @@ public BytesValue getData() { return data; } - public NodeData setData(final BytesValue data) { + public NodeDataRequest setData(final BytesValue data) { this.data = data; return this; } abstract void persist(final WorldStateStorage.Updater updater); - abstract Stream getChildNodeData(); + abstract Stream getChildRequests(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java similarity index 79% rename from ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java rename to ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java index 1b251bae26..0b91e751b1 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java @@ -21,9 +21,9 @@ import java.util.Collections; import java.util.List; -class StorageTrieNodeData extends TrieNodeData { +class StorageTrieNodeDataRequest extends TrieNodeDataRequest { - StorageTrieNodeData(final Hash hash) { + StorageTrieNodeDataRequest(final Hash hash) { super(Kind.STORAGE_TRIE_NODE, hash); } @@ -34,12 +34,12 @@ void persist(final Updater updater) { } @Override - protected NodeData createTrieChildNodeData(final Hash childHash) { - return NodeData.createStorageTrieNode(childHash); + protected NodeDataRequest createTrieChildNodeData(final Hash childHash) { + return NodeDataRequest.createStorageTrieNode(childHash); } @Override - protected List getNodeDataFromTrieNodeValue(final BytesValue value) { + protected List getRequestsFromTrieNodeValue(final BytesValue value) { // Nothing to do for terminal storage node return Collections.emptyList(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java similarity index 71% rename from ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java rename to ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java index 53dd257332..5dcb2b85b1 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeData.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java @@ -21,47 +21,47 @@ import java.util.List; import java.util.stream.Stream; -abstract class TrieNodeData extends NodeData { +abstract class TrieNodeDataRequest extends NodeDataRequest { private static final StoredNodeFactory nodeFactory = StoredNodeFactory.create(); - TrieNodeData(final Kind kind, final Hash hash) { + TrieNodeDataRequest(final Kind kind, final Hash hash) { super(kind, hash); } @Override - Stream getChildNodeData() { + Stream getChildRequests() { if (getData() == null) { // If this node hasn't been downloaded yet, we can't return any child data return Stream.empty(); } final Node node = nodeFactory.decode(getData()); - return processTrieNode(node); + return getRequestsFromTrieNode(node); } - private Stream processTrieNode(final Node trieNode) { + private Stream getRequestsFromTrieNode(final Node trieNode) { if (trieNode instanceof StoredNode && !((StoredNode) trieNode).isLoaded()) { // Stored nodes represent nodes that are referenced by hash (and therefore must be downloaded) - NodeData req = createTrieChildNodeData(Hash.wrap(trieNode.getHash())); + NodeDataRequest req = createTrieChildNodeData(Hash.wrap(trieNode.getHash())); return Stream.of(req); } // Process this child's children - final Stream childRequests = + final Stream childRequests = trieNode .getChildren() .map(List::stream) - .map(s -> s.flatMap(this::processTrieNode)) + .map(s -> s.flatMap(this::getRequestsFromTrieNode)) .orElse(Stream.of()); // Process value at this node, if present return trieNode .getValue() - .map(v -> Stream.concat(childRequests, (getNodeDataFromTrieNodeValue(v).stream()))) + .map(v -> Stream.concat(childRequests, (getRequestsFromTrieNodeValue(v).stream()))) .orElse(childRequests); } - protected abstract NodeData createTrieChildNodeData(final Hash childHash); + protected abstract NodeDataRequest createTrieChildNodeData(final Hash childHash); - protected abstract List getNodeDataFromTrieNodeValue(final BytesValue value); + protected abstract List getRequestsFromTrieNodeValue(final BytesValue value); } 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 446cc0892d..cb8d826963 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 @@ -22,7 +22,7 @@ import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; import tech.pegasys.pantheon.metrics.LabelledMetric; import tech.pegasys.pantheon.metrics.OperationTimer; -import tech.pegasys.pantheon.services.queue.Queue; +import tech.pegasys.pantheon.services.queue.BigQueue; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.time.Duration; @@ -41,7 +41,7 @@ public class WorldStateDownloader { private final EthContext ethContext; // The target header for which we want to retrieve world state private final BlockHeader header; - private final Queue pendingNodeQueue; + private final BigQueue pendingRequests; private final WorldStateStorage.Updater worldStateStorageUpdater; private final int hashCountPerRequest = 50; private final int maxOutstandingRequests = 50; @@ -54,17 +54,17 @@ public WorldStateDownloader( final EthContext ethContext, final WorldStateStorage worldStateStorage, final BlockHeader header, - final Queue pendingNodeQueue, + final BigQueue pendingRequests, final LabelledMetric ethTasksTimer) { this.ethContext = ethContext; this.worldStateStorage = worldStateStorage; this.header = header; - this.pendingNodeQueue = pendingNodeQueue; + this.pendingRequests = pendingRequests; this.ethTasksTimer = ethTasksTimer; // TODO: construct an updater that will commit changes periodically as updates accumulate this.worldStateStorageUpdater = worldStateStorage.updater(); - pendingNodeQueue.enqueue(NodeData.createAccountTrieNode(header.getStateRoot())); + pendingRequests.enqueue(NodeDataRequest.createAccountTrieNode(header.getStateRoot())); } public CompletableFuture run() { @@ -81,7 +81,7 @@ public CompletableFuture run() { private void requestNodeData() { while (!future.get().isDone() && outstandingRequests.get() < maxOutstandingRequests - && !pendingNodeQueue.isEmpty()) { + && !pendingRequests.isEmpty()) { // Find available peer Optional maybePeer = ethContext.getEthPeers().idlePeer(header.getNumber()); @@ -97,20 +97,20 @@ private void requestNodeData() { EthPeer peer = maybePeer.get(); // Collect data to be requested - List toRequest = new ArrayList<>(); + List toRequest = new ArrayList<>(); for (int i = 0; i < hashCountPerRequest; i++) { - if (pendingNodeQueue.isEmpty()) { + if (pendingRequests.isEmpty()) { break; } - toRequest.add(pendingNodeQueue.dequeue()); + toRequest.add(pendingRequests.dequeue()); } // Request and process node data outstandingRequests.incrementAndGet(); - sendAndProcessNodeDataRequest(peer, toRequest) + sendAndProcessRequests(peer, toRequest) .whenComplete( (res, error) -> { - if (outstandingRequests.decrementAndGet() == 0 && pendingNodeQueue.isEmpty()) { + if (outstandingRequests.decrementAndGet() == 0 && pendingRequests.isEmpty()) { // We're done worldStateStorageUpdater.commit(); future.get().complete(null); @@ -128,9 +128,10 @@ private CompletableFuture waitForNewPeer() { .timeout(WaitForPeerTask.create(ethContext, ethTasksTimer), Duration.ofSeconds(5)); } - private CompletableFuture sendAndProcessNodeDataRequest( - final EthPeer peer, final List nodeData) { - List hashes = nodeData.stream().map(NodeData::getHash).collect(Collectors.toList()); + private CompletableFuture sendAndProcessRequests( + final EthPeer peer, final List requests) { + List hashes = + requests.stream().map(NodeDataRequest::getHash).collect(Collectors.toList()); return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer) .assignPeer(peer) .run() @@ -139,20 +140,20 @@ private CompletableFuture sendAndProcessNodeDataRequest( .thenApply(this::mapNodeDataByHash) .thenAccept( data -> { - for (NodeData nodeDatum : nodeData) { - BytesValue matchingData = data.get(nodeDatum.getHash()); + for (NodeDataRequest request : requests) { + BytesValue matchingData = data.get(request.getHash()); if (matchingData == null) { - pendingNodeQueue.enqueue(nodeDatum); + pendingRequests.enqueue(request); } else { - // Persist node - nodeDatum.setData(matchingData); - nodeDatum.persist(worldStateStorageUpdater); + // Persist request data + request.setData(matchingData); + request.persist(worldStateStorageUpdater); // Queue child requests - nodeDatum - .getChildNodeData() + request + .getChildRequests() .filter(n -> !worldStateStorage.contains(n.getHash())) - .forEach(pendingNodeQueue::enqueue); + .forEach(pendingRequests::enqueue); } } }); diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index e2a3463f6d..a9aa8591b2 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -33,8 +33,8 @@ import tech.pegasys.pantheon.metrics.OperationTimer; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; -import tech.pegasys.pantheon.services.queue.InMemoryQueue; -import tech.pegasys.pantheon.services.queue.Queue; +import tech.pegasys.pantheon.services.queue.BigQueue; +import tech.pegasys.pantheon.services.queue.InMemoryBigQueue; import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -73,7 +73,7 @@ public void downloadAvailableWorldStateFromPeers() { BlockHeader header = remoteBlockchain.getBlockHeader(chainHead.getNumber() - 1).get(); assertThat(chainHead.getStateRoot()).isNotEqualTo(header.getStateRoot()); - Queue queue = new InMemoryQueue<>(); + BigQueue queue = new InMemoryBigQueue<>(); WorldStateStorage localStorage = new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); WorldStateDownloader downloader = diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java similarity index 84% rename from services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java rename to services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java index fdc3592a1c..2eca9b5e77 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/Queue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java @@ -14,7 +14,12 @@ import java.io.Closeable; -public interface Queue extends Closeable { +/** + * Represents a very large queue that may exceed memory limits. + * + * @param + */ +public interface BigQueue extends Closeable { void enqueue(T value); diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java similarity index 95% rename from services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java rename to services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java index 36ea10e5dd..cc19e6c3e4 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java @@ -16,7 +16,7 @@ import java.util.ArrayDeque; import java.util.Deque; -public class InMemoryQueue implements Queue { +public class InMemoryBigQueue implements BigQueue { private long size = 0; private final Deque internalQueue = new ArrayDeque(); From e205567c5c2caf5669553deac28a4e29647ba510 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Thu, 24 Jan 2019 12:32:54 -0500 Subject: [PATCH 08/23] Rework downloader w thread-safety in mind --- .../sync/worldstate/WorldStateDownloader.java | 128 +++++++++++------- .../worldstate/WorldStateDownloaderTest.java | 2 +- .../pantheon/services/queue/BigQueue.java | 2 +- .../services/queue/InMemoryBigQueue.java | 26 ++-- 4 files changed, 94 insertions(+), 64 deletions(-) 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 cb8d826963..d3c9f49c0a 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 @@ -19,6 +19,7 @@ import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer; import tech.pegasys.pantheon.ethereum.eth.sync.tasks.GetNodeDataFromPeerTask; import tech.pegasys.pantheon.ethereum.eth.sync.tasks.WaitForPeerTask; +import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; import tech.pegasys.pantheon.metrics.LabelledMetric; import tech.pegasys.pantheon.metrics.OperationTimer; @@ -32,96 +33,125 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; public class WorldStateDownloader { + private enum Status { + IDLE, + RUNNING, + DONE + } + private final EthContext ethContext; // The target header for which we want to retrieve world state private final BlockHeader header; private final BigQueue pendingRequests; private final WorldStateStorage.Updater worldStateStorageUpdater; - private final int hashCountPerRequest = 50; - private final int maxOutstandingRequests = 50; + private final int hashCountPerRequest; + private final int maxOutstandingRequests; private final AtomicInteger outstandingRequests = new AtomicInteger(0); private final LabelledMetric ethTasksTimer; private final WorldStateStorage worldStateStorage; - private final AtomicReference> future = new AtomicReference<>(null); + private final AtomicBoolean sendingRequests = new AtomicBoolean(false); + private volatile CompletableFuture future; + private volatile Status status = Status.IDLE; public WorldStateDownloader( final EthContext ethContext, final WorldStateStorage worldStateStorage, final BlockHeader header, final BigQueue pendingRequests, + final int hashCountPerRequest, + final int maxOutstandingRequests, final LabelledMetric ethTasksTimer) { this.ethContext = ethContext; this.worldStateStorage = worldStateStorage; this.header = header; this.pendingRequests = pendingRequests; + this.hashCountPerRequest = hashCountPerRequest; + this.maxOutstandingRequests = maxOutstandingRequests; this.ethTasksTimer = ethTasksTimer; // TODO: construct an updater that will commit changes periodically as updates accumulate this.worldStateStorageUpdater = worldStateStorage.updater(); - pendingRequests.enqueue(NodeDataRequest.createAccountTrieNode(header.getStateRoot())); + Hash stateRoot = header.getStateRoot(); + if (stateRoot.equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { + // If we're requesting data for an empty world state, we're already done + future = CompletableFuture.completedFuture(null); + status = Status.DONE; + } else { + pendingRequests.enqueue(NodeDataRequest.createAccountTrieNode(header.getStateRoot())); + } } public CompletableFuture run() { - if (future.get() != null && !future.get().isDone()) { - return future.get(); + synchronized (this) { + if (status == Status.DONE || status == Status.RUNNING) { + return future; + } + status = Status.RUNNING; + future = new CompletableFuture<>(); } - future.set(new CompletableFuture<>()); - requestNodeData(); + requestNodeData(); // TODO: Complete exceptionally on timeout / stalled download - return future.get(); + return future; } private void requestNodeData() { - while (!future.get().isDone() - && outstandingRequests.get() < maxOutstandingRequests - && !pendingRequests.isEmpty()) { - // Find available peer - Optional maybePeer = ethContext.getEthPeers().idlePeer(header.getNumber()); - - if (!maybePeer.isPresent()) { - // If no peer is available, wait and try again - waitForNewPeer() - .whenComplete( - (r, t) -> { - requestNodeData(); - }); - return; - } - EthPeer peer = maybePeer.get(); - - // Collect data to be requested - List toRequest = new ArrayList<>(); - for (int i = 0; i < hashCountPerRequest; i++) { - if (pendingRequests.isEmpty()) { - break; + if (sendingRequests.compareAndSet(false, true)) { + while (shouldRequestNodeData()) { + Optional maybePeer = ethContext.getEthPeers().idlePeer(header.getNumber()); + + if (!maybePeer.isPresent()) { + // If no peer is available, wait and try again + waitForNewPeer() + .whenComplete( + (r, t) -> { + requestNodeData(); + }); + } else { + EthPeer peer = maybePeer.get(); + + // Collect data to be requested + List toRequest = new ArrayList<>(); + for (int i = 0; i < hashCountPerRequest; i++) { + if (pendingRequests.isEmpty()) { + break; + } + toRequest.add(pendingRequests.dequeue()); + } + + // Request and process node data + outstandingRequests.incrementAndGet(); + sendAndProcessRequests(peer, toRequest) + .whenComplete( + (res, error) -> { + if (outstandingRequests.decrementAndGet() == 0 && pendingRequests.isEmpty()) { + // We're done + worldStateStorageUpdater.commit(); + future.complete(null); + } else { + // Send out additional requests + requestNodeData(); + } + }); } - toRequest.add(pendingRequests.dequeue()); - } - // Request and process node data - outstandingRequests.incrementAndGet(); - sendAndProcessRequests(peer, toRequest) - .whenComplete( - (res, error) -> { - if (outstandingRequests.decrementAndGet() == 0 && pendingRequests.isEmpty()) { - // We're done - worldStateStorageUpdater.commit(); - future.get().complete(null); - } else { - // Send out additional requests - requestNodeData(); - } - }); + } + sendingRequests.set(false); } } + private boolean shouldRequestNodeData() { + return !future.isDone() + && outstandingRequests.get() < maxOutstandingRequests + && !pendingRequests.isEmpty(); + } + private CompletableFuture waitForNewPeer() { return ethContext .getScheduler() @@ -131,7 +161,7 @@ private CompletableFuture waitForNewPeer() { private CompletableFuture sendAndProcessRequests( final EthPeer peer, final List requests) { List hashes = - requests.stream().map(NodeDataRequest::getHash).collect(Collectors.toList()); + requests.stream().map(NodeDataRequest::getHash).distinct().collect(Collectors.toList()); return GetNodeDataFromPeerTask.forHashes(ethContext, hashes, ethTasksTimer) .assignPeer(peer) .run() diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index a9aa8591b2..cd22be5ec4 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -78,7 +78,7 @@ public void downloadAvailableWorldStateFromPeers() { new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); WorldStateDownloader downloader = new WorldStateDownloader( - ethProtocolManager.ethContext(), localStorage, header, queue, ethTasksTimer); + ethProtocolManager.ethContext(), localStorage, header, queue, 50, 50, ethTasksTimer); // Create some peers that can respond int peerCount = 5; diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java index 2eca9b5e77..887feed38c 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java @@ -15,7 +15,7 @@ import java.io.Closeable; /** - * Represents a very large queue that may exceed memory limits. + * Represents a very large thread-safe queue that may exceed memory limits. * * @param */ diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java index cc19e6c3e4..623adc4e5c 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java @@ -12,36 +12,36 @@ */ package tech.pegasys.pantheon.services.queue; -import java.io.IOException; -import java.util.ArrayDeque; -import java.util.Deque; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicLong; public class InMemoryBigQueue implements BigQueue { - private long size = 0; - private final Deque internalQueue = new ArrayDeque(); + private final AtomicLong size = new AtomicLong(0); + private final Queue internalQueue = new ConcurrentLinkedQueue<>(); @Override public void enqueue(final T value) { - size += 1; - internalQueue.addFirst(value); + internalQueue.add(value); + size.incrementAndGet(); } @Override public T dequeue() { - if (size == 0) { - return null; + T result = internalQueue.poll(); + if (result != null) { + size.decrementAndGet(); } - size -= 1; - return internalQueue.removeLast(); + return result; } @Override public long size() { - return size; + return size.get(); } @Override - public void close() throws IOException { + public void close() { internalQueue.clear(); } } From 91d7f6d8f6cb0fd484cc719cfc359cf1c453a005 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Thu, 24 Jan 2019 15:21:02 -0500 Subject: [PATCH 09/23] Clarify some method names --- .../eth/sync/worldstate/AccountTrieNodeDataRequest.java | 8 ++++---- .../ethereum/eth/sync/worldstate/NodeDataRequest.java | 6 +++--- .../eth/sync/worldstate/StorageTrieNodeDataRequest.java | 4 ++-- .../ethereum/eth/sync/worldstate/TrieNodeDataRequest.java | 4 ++-- .../eth/sync/worldstate/WorldStateDownloader.java | 3 +-- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java index 77de4b904b..f6f82e52f6 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java @@ -37,8 +37,8 @@ void persist(final Updater updater) { } @Override - protected NodeDataRequest createTrieChildNodeData(final Hash childHash) { - return NodeDataRequest.createAccountTrieNode(childHash); + protected NodeDataRequest createChildNodeDataRequest(final Hash childHash) { + return NodeDataRequest.createAccountDataRequest(childHash); } @Override @@ -47,13 +47,13 @@ protected List getRequestsFromTrieNodeValue(final BytesValue va AccountTuple accountTuple = AccountTuple.readFrom(RLP.input(value)); // Add code, if appropriate if (!accountTuple.getCodeHash().equals(Hash.EMPTY)) { - nodeData.add(NodeDataRequest.createCodeNode(accountTuple.getCodeHash())); + nodeData.add(NodeDataRequest.createCodeRequest(accountTuple.getCodeHash())); } // Add storage, if appropriate if (!accountTuple.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { // If storage is non-empty queue download NodeDataRequest storageNode = - NodeDataRequest.createStorageTrieNode(accountTuple.getStorageRoot()); + NodeDataRequest.createStorageDataRequest(accountTuple.getStorageRoot()); nodeData.add(storageNode); } return nodeData; diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java index 6b817c030b..3e821c1c1e 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java @@ -34,15 +34,15 @@ protected NodeDataRequest(final Kind kind, final Hash hash) { this.hash = hash; } - public static AccountTrieNodeDataRequest createAccountTrieNode(final Hash hash) { + public static AccountTrieNodeDataRequest createAccountDataRequest(final Hash hash) { return new AccountTrieNodeDataRequest(hash); } - public static StorageTrieNodeDataRequest createStorageTrieNode(final Hash hash) { + public static StorageTrieNodeDataRequest createStorageDataRequest(final Hash hash) { return new StorageTrieNodeDataRequest(hash); } - public static CodeNodeDataRequest createCodeNode(final Hash hash) { + public static CodeNodeDataRequest createCodeRequest(final Hash hash) { return new CodeNodeDataRequest(hash); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java index 0b91e751b1..b28aec1dbf 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java @@ -34,8 +34,8 @@ void persist(final Updater updater) { } @Override - protected NodeDataRequest createTrieChildNodeData(final Hash childHash) { - return NodeDataRequest.createStorageTrieNode(childHash); + protected NodeDataRequest createChildNodeDataRequest(final Hash childHash) { + return NodeDataRequest.createStorageDataRequest(childHash); } @Override diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java index 5dcb2b85b1..e192919bc5 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java @@ -43,7 +43,7 @@ Stream getChildRequests() { private Stream getRequestsFromTrieNode(final Node trieNode) { if (trieNode instanceof StoredNode && !((StoredNode) trieNode).isLoaded()) { // Stored nodes represent nodes that are referenced by hash (and therefore must be downloaded) - NodeDataRequest req = createTrieChildNodeData(Hash.wrap(trieNode.getHash())); + NodeDataRequest req = createChildNodeDataRequest(Hash.wrap(trieNode.getHash())); return Stream.of(req); } // Process this child's children @@ -61,7 +61,7 @@ private Stream getRequestsFromTrieNode(final Node t .orElse(childRequests); } - protected abstract NodeDataRequest createTrieChildNodeData(final Hash childHash); + protected abstract NodeDataRequest createChildNodeDataRequest(final Hash childHash); protected abstract List getRequestsFromTrieNodeValue(final BytesValue value); } 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 d3c9f49c0a..5c8861b3db 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 @@ -83,7 +83,7 @@ public WorldStateDownloader( future = CompletableFuture.completedFuture(null); status = Status.DONE; } else { - pendingRequests.enqueue(NodeDataRequest.createAccountTrieNode(header.getStateRoot())); + pendingRequests.enqueue(NodeDataRequest.createAccountDataRequest(header.getStateRoot())); } } @@ -140,7 +140,6 @@ private void requestNodeData() { } }); } - } sendingRequests.set(false); } From 4928b414a48209356624e63b05fbc63bd797bf13 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Thu, 24 Jan 2019 15:27:18 -0500 Subject: [PATCH 10/23] Fix comment --- .../ethereum/eth/sync/worldstate/TrieNodeDataRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java index e192919bc5..a9ba06b18b 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java @@ -46,7 +46,7 @@ private Stream getRequestsFromTrieNode(final Node t NodeDataRequest req = createChildNodeDataRequest(Hash.wrap(trieNode.getHash())); return Stream.of(req); } - // Process this child's children + // Process this node's children final Stream childRequests = trieNode .getChildren() From 3af28bdabef8f4f9f9b7ad50428f58355d9aaec5 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Thu, 24 Jan 2019 16:20:30 -0500 Subject: [PATCH 11/23] Fix bug - add missing break statement --- .../ethereum/eth/sync/worldstate/WorldStateDownloader.java | 1 + 1 file changed, 1 insertion(+) 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 5c8861b3db..9130213e20 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 @@ -113,6 +113,7 @@ private void requestNodeData() { (r, t) -> { requestNodeData(); }); + break; } else { EthPeer peer = maybePeer.get(); From 98d808d5ff7f35ab1cc433b3acd6c91c66d7eac2 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Thu, 24 Jan 2019 18:27:13 -0500 Subject: [PATCH 12/23] Add more tests --- .../ethereum/core/BlockDataGenerator.java | 27 ++ .../worldstate/WorldStateDownloaderTest.java | 234 ++++++++++++------ 2 files changed, 179 insertions(+), 82 deletions(-) diff --git a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java index a20175cd8f..6bd10ef6b3 100644 --- a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java +++ b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java @@ -95,6 +95,33 @@ private List blockSequence( return seq; } + public List createRandomAccounts(final MutableWorldState worldState, final int count) { + WorldUpdater updater = worldState.updater(); + List accounts = new ArrayList<>(count); + for (int i = 0; i < count; i++) { + MutableAccount account = updater.getOrCreate(this.address()); + // Make some accounts contract accounts + if (random.nextFloat() < .5) { + // Subset of random accounts are contract accounts + account.setCode(this.bytesValue(5, 50)); + if (random.nextFloat() < .75) { + // Add some storage for most contract accounts + int storageValues = random.nextInt(20) + 10; + for (int j = 0; j < storageValues; j++) { + account.setStorageValue(uint256(), uint256()); + } + } + } + account.setNonce(random.nextInt(10)); + account.setBalance(Wei.of(positiveLong())); + + accounts.add(account); + } + updater.commit(); + worldState.persist(); + return accounts; + } + public List blockSequence(final int count) { final WorldStateArchive worldState = createInMemoryWorldStateArchive(); return blockSequence(count, worldState, Collections.emptyList(), Collections.emptyList()); diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index cd22be5ec4..918a912d7e 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -13,30 +13,30 @@ package tech.pegasys.pantheon.ethereum.eth.sync.worldstate; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; -import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; -import tech.pegasys.pantheon.ethereum.core.AccountTuple; +import tech.pegasys.pantheon.ethereum.chain.Blockchain; +import tech.pegasys.pantheon.ethereum.core.Account; +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.Hash; +import tech.pegasys.pantheon.ethereum.core.MutableWorldState; +import tech.pegasys.pantheon.ethereum.core.WorldState; 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.RespondingEthPeer.Responder; -import tech.pegasys.pantheon.ethereum.eth.manager.ethtaskutils.BlockchainSetupUtil; -import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueStorageWorldStateStorage; import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; -import tech.pegasys.pantheon.ethereum.trie.StoredMerklePatriciaTrie; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; -import tech.pegasys.pantheon.metrics.LabelledMetric; -import tech.pegasys.pantheon.metrics.OperationTimer; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; import tech.pegasys.pantheon.services.queue.BigQueue; import tech.pegasys.pantheon.services.queue.InMemoryBigQueue; import tech.pegasys.pantheon.util.bytes.Bytes32; -import tech.pegasys.pantheon.util.bytes.BytesValue; +import tech.pegasys.pantheon.util.uint.UInt256; import java.util.List; import java.util.Map; @@ -44,112 +44,182 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import org.junit.BeforeClass; import org.junit.Test; public class WorldStateDownloaderTest { - private static WorldStateArchive remoteWorldArchive; - private static WorldStateStorage remoteWorldStorage; - private static MutableBlockchain remoteBlockchain; - private static LabelledMetric ethTasksTimer; - private static EthProtocolManager ethProtocolManager; - - @BeforeClass - public static void setup() { - final BlockchainSetupUtil blockchainSetupUtil = BlockchainSetupUtil.forTesting(); - blockchainSetupUtil.importAllBlocks(); - remoteBlockchain = blockchainSetupUtil.getBlockchain(); - remoteWorldArchive = blockchainSetupUtil.getWorldArchive(); - remoteWorldStorage = remoteWorldArchive.getStorage(); - ethTasksTimer = NoOpMetricsSystem.NO_OP_LABELLED_TIMER; - ethProtocolManager = EthProtocolManagerTestUtil.create(); + private static final Hash EMPTY_TRIE_ROOT = Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH); + + @Test + public void downloadWorldStateFromPeers_onePeerOneWithManyRequestsOneAtATime() { + downloadAvailableWorldStateFromPeers(1, 50, 1, 1); + } + + @Test + public void downloadWorldStateFromPeers_onePeerOneWithManyRequests() { + downloadAvailableWorldStateFromPeers(1, 50, 1, 10); + } + + @Test + public void downloadWorldStateFromPeers_onePeerWithSingleRequest() { + downloadAvailableWorldStateFromPeers(1, 1, 100, 10); + } + + @Test + public void downloadWorldStateFromPeers_largeStateFromMultiplePeers() { + downloadAvailableWorldStateFromPeers(5, 100, 10, 10); + } + + @Test + public void downloadWorldStateFromPeers_smallStateFromMultiplePeers() { + downloadAvailableWorldStateFromPeers(5, 5, 1, 10); + } + + @Test + public void downloadWorldStateFromPeers_singleRequestWithMultiplePeers() { + downloadAvailableWorldStateFromPeers(5, 1, 50, 50); } @Test - public void downloadAvailableWorldStateFromPeers() { - // Pull chain head and a prior header with a different state root - BlockHeader chainHead = remoteBlockchain.getChainHeadHeader(); - BlockHeader header = remoteBlockchain.getBlockHeader(chainHead.getNumber() - 1).get(); - assertThat(chainHead.getStateRoot()).isNotEqualTo(header.getStateRoot()); + public void downloadEmptyWorldState() { + BlockDataGenerator dataGen = new BlockDataGenerator(1); + final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create(); + BlockHeader header = + dataGen + .block(BlockOptions.create().setStateRoot(EMPTY_TRIE_ROOT).setBlockNumber(10)) + .getHeader(); + + // Create some peers + List peers = + Stream.generate( + () -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, header.getNumber())) + .limit(5) + .collect(Collectors.toList()); BigQueue queue = new InMemoryBigQueue<>(); WorldStateStorage localStorage = new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); WorldStateDownloader downloader = new WorldStateDownloader( - ethProtocolManager.ethContext(), localStorage, header, queue, 50, 50, ethTasksTimer); + ethProtocolManager.ethContext(), + localStorage, + header, + queue, + 10, + 10, + NoOpMetricsSystem.NO_OP_LABELLED_TIMER); + + CompletableFuture future = downloader.run(); + assertThat(future).isDone(); + + // Peers should not have been queried + for (RespondingEthPeer peer : peers) { + assertThat(peer.hasOutstandingRequests()).isFalse(); + } + } + + private void downloadAvailableWorldStateFromPeers( + final int peerCount, + final int accountCount, + final int hashesPerRequest, + final int maxOutstandingRequests) { + final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create(); + final int trailingPeerCount = 5; + BlockDataGenerator dataGen = new BlockDataGenerator(1); + + // Setup "remote" state + final WorldStateStorage remoteStorage = + new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + final WorldStateArchive remoteWorldStateArchive = new WorldStateArchive(remoteStorage); + final MutableWorldState remoteWorldState = remoteWorldStateArchive.getMutable(); + + // Generate accounts and save corresponding state root + final List accounts = dataGen.createRandomAccounts(remoteWorldState, accountCount); + final Hash stateRoot = remoteWorldState.rootHash(); + assertThat(stateRoot).isNotEqualTo(EMPTY_TRIE_ROOT); // Sanity check + BlockHeader header = + dataGen.block(BlockOptions.create().setStateRoot(stateRoot).setBlockNumber(10)).getHeader(); + + // Generate more data that should not be downloaded + final List otherAccounts = dataGen.createRandomAccounts(remoteWorldState, 5); + Hash otherStateRoot = remoteWorldState.rootHash(); + BlockHeader otherHeader = + dataGen + .block(BlockOptions.create().setStateRoot(otherStateRoot).setBlockNumber(11)) + .getHeader(); + assertThat(otherStateRoot).isNotEqualTo(stateRoot); // Sanity check + + BigQueue queue = new InMemoryBigQueue<>(); + WorldStateStorage localStorage = + new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + WorldStateArchive localWorldStateArchive = new WorldStateArchive(localStorage); + WorldStateDownloader downloader = + new WorldStateDownloader( + ethProtocolManager.ethContext(), + localStorage, + header, + queue, + hashesPerRequest, + maxOutstandingRequests, + NoOpMetricsSystem.NO_OP_LABELLED_TIMER); // Create some peers that can respond - int peerCount = 5; - List peers = + List usefulPeers = Stream.generate( - () -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, remoteBlockchain)) + () -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, header.getNumber())) .limit(peerCount) .collect(Collectors.toList()); + // And some irrelevant peers + List trailingPeers = + Stream.generate( + () -> + EthProtocolManagerTestUtil.createPeer( + ethProtocolManager, header.getNumber() - 1L)) + .limit(trailingPeerCount) + .collect(Collectors.toList()); // Start downloader CompletableFuture result = downloader.run(); // Respond to node data requests Responder responder = - RespondingEthPeer.blockchainResponder(remoteBlockchain, remoteWorldArchive); + RespondingEthPeer.blockchainResponder(mock(Blockchain.class), remoteWorldStateArchive); while (!result.isDone()) { - for (RespondingEthPeer peer : peers) { + for (RespondingEthPeer peer : usefulPeers) { peer.respond(responder); } } + // Check that trailing peers were not queried for data + for (RespondingEthPeer trailingPeer : trailingPeers) { + assertThat(trailingPeer.hasOutstandingRequests()).isFalse(); + } + + // Check that all expected account data was downloaded + final WorldState localWorldState = localWorldStateArchive.get(stateRoot); assertThat(result).isDone(); - assertStorageForWorldStateMatchesExpectation( - header.getStateRoot(), remoteWorldStorage, localStorage); - // We shouldn't have the chain head data, since we downloaded from a prior block - assertThat(localStorage.contains(chainHead.getStateRoot())).isFalse(); - } + assertAccountsMatch(localWorldState, accounts); - protected void assertStorageForWorldStateMatchesExpectation( - final Hash stateRoot, - final WorldStateStorage expectedStorage, - final WorldStateStorage actualStorage) { - // Get account entries - final MerklePatriciaTrie expectedWorldStateTrie = - StoredMerklePatriciaTrie.create(expectedStorage::getAccountStorageTrieNode, stateRoot); - final Map expectedAccountEntries = - expectedWorldStateTrie.entriesFrom(Bytes32.ZERO, 500); - final MerklePatriciaTrie actualWorldStateTrie = - StoredMerklePatriciaTrie.create(actualStorage::getAccountStorageTrieNode, stateRoot); - final Map actualAccountEntries = - actualWorldStateTrie.entriesFrom(Bytes32.ZERO, 500); - // Verify account entries - assertThat(expectedAccountEntries).isNotEmpty(); // Sanity check - assertThat(actualAccountEntries).isEqualTo(expectedAccountEntries); - - // Extract account tuples - List accounts = - expectedAccountEntries - .entrySet() - .stream() - .map(e -> AccountTuple.readFrom(RLP.input(e.getValue()))) - .collect(Collectors.toList()); + // We shouldn't have any extra data locally + assertThat(localStorage.contains(otherHeader.getStateRoot())).isFalse(); + for (Account otherAccount : otherAccounts) { + assertThat(localWorldState.get(otherAccount.getAddress())).isNull(); + } + } - // Verify each account - for (AccountTuple account : accounts) { - // Verify storage - final MerklePatriciaTrie expectedStorageTrie = - StoredMerklePatriciaTrie.create( - expectedStorage::getAccountStorageTrieNode, account.getStorageRoot()); - final Map expectedStorageEntries = - expectedStorageTrie.entriesFrom(Bytes32.ZERO, 500); - final MerklePatriciaTrie actualStorageTrie = - StoredMerklePatriciaTrie.create( - actualStorage::getAccountStorageTrieNode, account.getStorageRoot()); - final Map actualStorageEntries = - actualStorageTrie.entriesFrom(Bytes32.ZERO, 500); - assertThat(actualStorageEntries).isEqualTo(expectedStorageEntries); - - // Verify code is stored - Hash codeHash = account.getCodeHash(); - assertThat(actualStorage.getCode(codeHash)).isEqualTo(expectedStorage.getCode(codeHash)); + private void assertAccountsMatch( + final WorldState worldState, final List expectedAccounts) { + for (Account expectedAccount : expectedAccounts) { + Account actualAccount = worldState.get(expectedAccount.getAddress()); + assertThat(actualAccount).isNotNull(); + // Check each field + assertThat(actualAccount.getNonce()).isEqualTo(expectedAccount.getNonce()); + assertThat(actualAccount.getCode()).isEqualTo(expectedAccount.getCode()); + assertThat(actualAccount.getBalance()).isEqualTo(expectedAccount.getBalance()); + + Map actualStorage = actualAccount.storageEntriesFrom(Bytes32.ZERO, 500); + Map expectedStorage = expectedAccount.storageEntriesFrom(Bytes32.ZERO, 500); + assertThat(actualStorage).isEqualTo(expectedStorage); } } } From cce5e7e27551909bd447ad6a264c5ba23987e2f2 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Thu, 24 Jan 2019 18:46:58 -0500 Subject: [PATCH 13/23] Minor cleanup --- .../pegasys/pantheon/ethereum/core/BlockDataGenerator.java | 4 ++-- .../ethereum/eth/sync/worldstate/WorldStateDownloader.java | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java index 6bd10ef6b3..d8b3ae2f07 100644 --- a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java +++ b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java @@ -99,11 +99,11 @@ public List createRandomAccounts(final MutableWorldState worldState, fi WorldUpdater updater = worldState.updater(); List accounts = new ArrayList<>(count); for (int i = 0; i < count; i++) { - MutableAccount account = updater.getOrCreate(this.address()); + MutableAccount account = updater.getOrCreate(address()); // Make some accounts contract accounts if (random.nextFloat() < .5) { // Subset of random accounts are contract accounts - account.setCode(this.bytesValue(5, 50)); + account.setCode(bytesValue(5, 50)); if (random.nextFloat() < .75) { // Add some storage for most contract accounts int storageValues = random.nextInt(20) + 10; 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 9130213e20..5480c63159 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 @@ -87,6 +87,7 @@ public WorldStateDownloader( } } + // TODO: add a cancel method public CompletableFuture run() { synchronized (this) { if (status == Status.DONE || status == Status.RUNNING) { From 365c32de82cb489a0d704518bb2be1a6b21bfa08 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 11:11:08 -0500 Subject: [PATCH 14/23] Rename AccountTuple and move it into the worldstate package Clean up AccountState handling of account value --- .../worldstate/DefaultMutableWorldState.java | 42 +++++++++---------- .../StateTrieAccountValue.java} | 12 +++--- .../AccountTrieNodeDataRequest.java | 12 +++--- 3 files changed, 33 insertions(+), 33 deletions(-) rename ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/{core/AccountTuple.java => worldstate/StateTrieAccountValue.java} (86%) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java index 4cf31e0d05..58ef798940 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java @@ -14,7 +14,6 @@ import tech.pegasys.pantheon.ethereum.core.AbstractWorldUpdater; import tech.pegasys.pantheon.ethereum.core.Account; -import tech.pegasys.pantheon.ethereum.core.AccountTuple; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.core.MutableWorldState; @@ -104,14 +103,14 @@ public Account get(final Address address) { private AccountState deserializeAccount( final Address address, final Hash addressHash, final BytesValue encoded) throws RLPException { final RLPInput in = RLP.input(encoded); - AccountTuple tuple = AccountTuple.readFrom(in); - return new AccountState(address, addressHash, tuple); + StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(in); + return new AccountState(address, addressHash, accountValue); } private static BytesValue serializeAccount( final long nonce, final Wei balance, final Hash storageRoot, final Hash codeHash) { - AccountTuple tuple = new AccountTuple(nonce, balance, storageRoot, codeHash); - return RLP.encode(tuple::writeTo); + StateTrieAccountValue accountValue = new StateTrieAccountValue(nonce, balance, storageRoot, codeHash); + return RLP.encode(accountValue::writeTo); } @Override @@ -171,22 +170,16 @@ protected class AccountState implements Account { private final Address address; private final Hash addressHash; - private final long nonce; - private final Wei balance; - private final Hash storageRoot; - private final Hash codeHash; + final StateTrieAccountValue accountValue; // Lazily initialized since we don't always access storage. private volatile MerklePatriciaTrie storageTrie; - private AccountState(final Address address, final Hash addressHash, final AccountTuple tuple) { + private AccountState(final Address address, final Hash addressHash, final StateTrieAccountValue accountValue) { this.address = address; this.addressHash = addressHash; - this.nonce = tuple.getNonce(); - this.balance = tuple.getBalance(); - this.storageRoot = tuple.getStorageRoot(); - this.codeHash = tuple.getCodeHash(); + this.accountValue = accountValue; } private MerklePatriciaTrie storageTrie() { @@ -195,7 +188,7 @@ private MerklePatriciaTrie storageTrie() { storageTrie = updatedTrie; } if (storageTrie == null) { - storageTrie = newAccountStorageTrie(storageRoot); + storageTrie = newAccountStorageTrie(getStorageRoot()); } return storageTrie; } @@ -212,12 +205,16 @@ public Hash getAddressHash() { @Override public long getNonce() { - return nonce; + return accountValue.getNonce(); } @Override public Wei getBalance() { - return balance; + return accountValue.getBalance(); + } + + Hash getStorageRoot() { + return accountValue.getStorageRoot(); } @Override @@ -227,6 +224,7 @@ public BytesValue getCode() { return updatedCode; } // No code is common, save the KV-store lookup. + Hash codeHash = getCodeHash(); if (codeHash.equals(Hash.EMPTY)) { return BytesValue.EMPTY; } @@ -240,7 +238,7 @@ public boolean hasCode() { @Override public Hash getCodeHash() { - return codeHash; + return accountValue.getCodeHash(); } @Override @@ -281,8 +279,8 @@ public String toString() { builder.append("address=").append(getAddress()).append(", "); builder.append("nonce=").append(getNonce()).append(", "); builder.append("balance=").append(getBalance()).append(", "); - builder.append("storageRoot=").append(storageRoot).append(", "); - builder.append("codeHash=").append(codeHash); + builder.append("storageRoot=").append(getStorageRoot()).append(", "); + builder.append("codeHash=").append(getCodeHash()); return builder.append("}").toString(); } } @@ -331,14 +329,14 @@ public void commit() { final AccountState origin = updated.getWrappedAccount(); // Save the code in key-value storage ... - Hash codeHash = origin == null ? Hash.EMPTY : origin.codeHash; + Hash codeHash = origin == null ? Hash.EMPTY : origin.getCodeHash(); if (updated.codeWasUpdated()) { codeHash = Hash.hash(updated.getCode()); wrapped.updatedAccountCode.put(updated.getAddress(), updated.getCode()); } // ...and storage in the account trie first. final boolean freshState = origin == null || updated.getStorageWasCleared(); - Hash storageRoot = freshState ? Hash.EMPTY_TRIE_HASH : origin.storageRoot; + Hash storageRoot = freshState ? Hash.EMPTY_TRIE_HASH : origin.getStorageRoot(); if (freshState) { wrapped.updatedStorageTries.remove(updated.getAddress()); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/StateTrieAccountValue.java similarity index 86% rename from ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java rename to ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/StateTrieAccountValue.java index eb6c442932..57e5c11c4b 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountTuple.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/StateTrieAccountValue.java @@ -10,20 +10,22 @@ * 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.core; +package tech.pegasys.pantheon.ethereum.worldstate; +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.core.Wei; import tech.pegasys.pantheon.ethereum.rlp.RLPInput; import tech.pegasys.pantheon.ethereum.rlp.RLPOutput; /** Represents the raw values associated with an account in the world state trie. */ -public class AccountTuple { +public class StateTrieAccountValue { private final long nonce; private final Wei balance; private final Hash storageRoot; private final Hash codeHash; - public AccountTuple( + public StateTrieAccountValue( final long nonce, final Wei balance, final Hash storageRoot, final Hash codeHash) { this.nonce = nonce; this.balance = balance; @@ -78,7 +80,7 @@ public void writeTo(final RLPOutput out) { out.endList(); } - public static AccountTuple readFrom(final RLPInput in) { + public static StateTrieAccountValue readFrom(final RLPInput in) { in.enterList(); final long nonce = in.readLongScalar(); @@ -88,6 +90,6 @@ public static AccountTuple readFrom(final RLPInput in) { in.leaveList(); - return new AccountTuple(nonce, balance, storageRoot, codeHash); + return new StateTrieAccountValue(nonce, balance, storageRoot, codeHash); } } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java index f6f82e52f6..1da78cc4eb 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java @@ -14,7 +14,7 @@ import static com.google.common.base.Preconditions.checkNotNull; -import tech.pegasys.pantheon.ethereum.core.AccountTuple; +import tech.pegasys.pantheon.ethereum.worldstate.StateTrieAccountValue; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; @@ -44,16 +44,16 @@ protected NodeDataRequest createChildNodeDataRequest(final Hash childHash) { @Override protected List getRequestsFromTrieNodeValue(final BytesValue value) { List nodeData = new ArrayList<>(2); - AccountTuple accountTuple = AccountTuple.readFrom(RLP.input(value)); + StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(RLP.input(value)); // Add code, if appropriate - if (!accountTuple.getCodeHash().equals(Hash.EMPTY)) { - nodeData.add(NodeDataRequest.createCodeRequest(accountTuple.getCodeHash())); + if (!accountValue.getCodeHash().equals(Hash.EMPTY)) { + nodeData.add(NodeDataRequest.createCodeRequest(accountValue.getCodeHash())); } // Add storage, if appropriate - if (!accountTuple.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { + if (!accountValue.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { // If storage is non-empty queue download NodeDataRequest storageNode = - NodeDataRequest.createStorageDataRequest(accountTuple.getStorageRoot()); + NodeDataRequest.createStorageDataRequest(accountValue.getStorageRoot()); nodeData.add(storageNode); } return nodeData; From 6e1a67cd1ddae89b0923201149e408ec732a61a3 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 11:48:28 -0500 Subject: [PATCH 15/23] Remove TODO comments --- .../pantheon/ethereum/worldstate/WorldStateStorage.java | 1 - .../ethereum/eth/sync/worldstate/WorldStateDownloader.java | 4 ---- 2 files changed, 5 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java index dda7b3ab5e..9432dbbf6c 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java @@ -28,7 +28,6 @@ public interface WorldStateStorage { Optional getNodeData(Hash hash); - // TODO: look into optimizing this call in implementing classes default boolean contains(final Hash hash) { return getNodeData(hash).isPresent(); } 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 5480c63159..46559de9a4 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 @@ -74,7 +74,6 @@ public WorldStateDownloader( this.hashCountPerRequest = hashCountPerRequest; this.maxOutstandingRequests = maxOutstandingRequests; this.ethTasksTimer = ethTasksTimer; - // TODO: construct an updater that will commit changes periodically as updates accumulate this.worldStateStorageUpdater = worldStateStorage.updater(); Hash stateRoot = header.getStateRoot(); @@ -87,7 +86,6 @@ public WorldStateDownloader( } } - // TODO: add a cancel method public CompletableFuture run() { synchronized (this) { if (status == Status.DONE || status == Status.RUNNING) { @@ -98,7 +96,6 @@ public CompletableFuture run() { } requestNodeData(); - // TODO: Complete exceptionally on timeout / stalled download return future; } @@ -167,7 +164,6 @@ private CompletableFuture sendAndProcessRequests( .assignPeer(peer) .run() .thenApply(PeerTaskResult::getResult) - // TODO: Update task to return this mapping .thenApply(this::mapNodeDataByHash) .thenAccept( data -> { From 03018c54ab95874d529ba59fa5262377aabee289 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 12:58:32 -0500 Subject: [PATCH 16/23] Skip storage lookup for empty code and empty trie nodes Fully implement this optimization for world state archive and storage. Make world storage api consistent. --- .../KeyValueStorageWorldStateStorage.java | 60 +++++-- .../worldstate/DefaultMutableWorldState.java | 8 +- .../worldstate/WorldStateArchive.java | 12 +- .../worldstate/WorldStateStorage.java | 16 +- .../ethereum/core/BlockDataGenerator.java | 2 +- .../KeyValueStorageWorldStateStorageTest.java | 157 ++++++++++++++++++ .../DefaultMutableWorldStateTest.java | 22 +-- .../worldstate/WorldStateArchiveTest.java | 44 +++++ .../ethereum/eth/manager/EthServer.java | 7 +- .../AccountTrieNodeDataRequest.java | 4 +- .../sync/worldstate/WorldStateDownloader.java | 2 +- .../worldstate/WorldStateDownloaderTest.java | 2 +- .../ethereum/trie/MerklePatriciaTrie.java | 4 +- .../pantheon/ethereum/trie/NullNode.java | 11 +- .../trie/StoredMerklePatriciaTrie.java | 6 +- 15 files changed, 297 insertions(+), 60 deletions(-) create mode 100644 ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java create mode 100644 ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java index a8fa6e40fc..e136fb80b9 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java @@ -13,12 +13,15 @@ package tech.pegasys.pantheon.ethereum.storage.keyvalue; import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.rlp.RLP; +import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; import tech.pegasys.pantheon.services.kvstore.KeyValueStorage; import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.Optional; +import java.util.function.Function; public class KeyValueStorageWorldStateStorage implements WorldStateStorage { @@ -29,26 +32,46 @@ public KeyValueStorageWorldStateStorage(final KeyValueStorage keyValueStorage) { } @Override - public Optional getCode(final Hash codeHash) { - if (codeHash.equals(Hash.EMPTY)) { - return Optional.of(BytesValue.EMPTY); - } - return keyValueStorage.get(codeHash); + public Optional getCode(final Bytes32 codeHash) { + return getCodeValue(codeHash, keyValueStorage::get); } @Override public Optional getAccountStateTrieNode(final Bytes32 nodeHash) { - return keyValueStorage.get(nodeHash); + return getTrieValue(nodeHash, keyValueStorage::get); } @Override public Optional getAccountStorageTrieNode(final Bytes32 nodeHash) { - return keyValueStorage.get(nodeHash); + return getTrieValue(nodeHash, keyValueStorage::get); } @Override - public Optional getNodeData(final Hash hash) { - return keyValueStorage.get(hash); + public Optional getNodeData(final Bytes32 hash) { + return getValue(hash, keyValueStorage::get); + } + + private Optional getTrieValue( + final Bytes32 hash, final Function> getter) { + if (hash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { + return Optional.of(RLP.NULL); + } else { + return getter.apply(hash); + } + } + + private Optional getCodeValue( + final Bytes32 hash, final Function> getter) { + if (hash.equals(Hash.EMPTY)) { + return Optional.of(BytesValue.EMPTY); + } else { + return getter.apply(hash); + } + } + + private Optional getValue( + final Bytes32 hash, final Function> getter) { + return getTrieValue(hash, (h) -> getCodeValue(h, getter)); } @Override @@ -65,22 +88,33 @@ public Updater(final KeyValueStorage.Transaction transaction) { } @Override - public void putCode(final Bytes32 codeHash, final BytesValue code) { + public Updater putCode(final Bytes32 codeHash, final BytesValue code) { if (code.size() == 0) { // Don't save empty values - return; + return this; } transaction.put(codeHash, code); + return this; } @Override - public void putAccountStateTrieNode(final Bytes32 nodeHash, final BytesValue node) { + public Updater putAccountStateTrieNode(final Bytes32 nodeHash, final BytesValue node) { + if (nodeHash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { + // Don't save empty nodes + return this; + } transaction.put(nodeHash, node); + return this; } @Override - public void putAccountStorageTrieNode(final Bytes32 nodeHash, final BytesValue node) { + public Updater putAccountStorageTrieNode(final Bytes32 nodeHash, final BytesValue node) { + if (nodeHash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { + // Don't save empty nodes + return this; + } transaction.put(nodeHash, node); + return this; } @Override diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java index 58ef798940..f83d0f4e3d 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java @@ -49,7 +49,7 @@ public class DefaultMutableWorldState implements MutableWorldState { private final WorldStateStorage worldStateStorage; public DefaultMutableWorldState(final WorldStateStorage storage) { - this(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, storage); + this(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, storage); } public DefaultMutableWorldState( @@ -109,7 +109,8 @@ private AccountState deserializeAccount( private static BytesValue serializeAccount( final long nonce, final Wei balance, final Hash storageRoot, final Hash codeHash) { - StateTrieAccountValue accountValue = new StateTrieAccountValue(nonce, balance, storageRoot, codeHash); + StateTrieAccountValue accountValue = + new StateTrieAccountValue(nonce, balance, storageRoot, codeHash); return RLP.encode(accountValue::writeTo); } @@ -175,7 +176,8 @@ protected class AccountState implements Account { // Lazily initialized since we don't always access storage. private volatile MerklePatriciaTrie storageTrie; - private AccountState(final Address address, final Hash addressHash, final StateTrieAccountValue accountValue) { + private AccountState( + final Address address, final Hash addressHash, final StateTrieAccountValue accountValue) { this.address = address; this.addressHash = addressHash; diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java index 2fedbf0f98..dcaa93d6e7 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java @@ -22,7 +22,7 @@ public class WorldStateArchive { private final WorldStateStorage storage; - private static final Hash EMPTY_ROOT_HASH = Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH); + private static final Hash EMPTY_ROOT_HASH = Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH); public WorldStateArchive(final WorldStateStorage storage) { this.storage = storage; @@ -45,7 +45,15 @@ public MutableWorldState getMutable() { } public Optional getNodeData(final Hash hash) { - return storage.getNodeData(hash); + if (hash.equals(Hash.EMPTY)) { + // No need to go to storage for an empty value + return Optional.of(BytesValue.EMPTY); + } else if (hash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { + // No need to go to storage for an trie node + return Optional.of(MerklePatriciaTrie.EMPTY_TRIE_NODE); + } else { + return storage.getNodeData(hash); + } } public WorldStateStorage getStorage() { diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java index 9432dbbf6c..2b147cf2ce 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateStorage.java @@ -20,15 +20,15 @@ public interface WorldStateStorage { - Optional getCode(Hash codeHash); + Optional getCode(Bytes32 codeHash); Optional getAccountStateTrieNode(Bytes32 nodeHash); Optional getAccountStorageTrieNode(Bytes32 nodeHash); - Optional getNodeData(Hash hash); + Optional getNodeData(Bytes32 hash); - default boolean contains(final Hash hash) { + default boolean contains(final Bytes32 hash) { return getNodeData(hash).isPresent(); } @@ -36,17 +36,17 @@ default boolean contains(final Hash hash) { interface Updater { - void putCode(Bytes32 nodeHash, BytesValue code); + Updater putCode(Bytes32 nodeHash, BytesValue code); - default void putCode(final BytesValue code) { + default Updater putCode(final BytesValue code) { // Skip the hash calculation for empty code Hash codeHash = code.size() == 0 ? Hash.EMPTY : Hash.hash(code); - putCode(codeHash, code); + return putCode(codeHash, code); } - void putAccountStateTrieNode(Bytes32 nodeHash, BytesValue node); + Updater putAccountStateTrieNode(Bytes32 nodeHash, BytesValue node); - void putAccountStorageTrieNode(Bytes32 nodeHash, BytesValue node); + Updater putAccountStorageTrieNode(Bytes32 nodeHash, BytesValue node); void commit(); diff --git a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java index d8b3ae2f07..efdee7741d 100644 --- a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java +++ b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java @@ -58,7 +58,7 @@ private List blockSequence( final List seq = new ArrayList<>(count); final MutableWorldState worldState = - worldStateArchive.getMutable(Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)); + worldStateArchive.getMutable(Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)); long nextBlockNumber = nextBlock; Hash parentHash = parent; diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java new file mode 100644 index 0000000000..7132e5471b --- /dev/null +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java @@ -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.storage.keyvalue; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; +import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import org.junit.Test; + +public class KeyValueStorageWorldStateStorageTest { + + @Test + public void getsCode_returnsEmpty() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + assertThat(storage.getCode(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getAccountStateTrieNode_returnsNodeNode() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + assertThat(storage.getAccountStateTrieNode(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + } + + @Test + public void getAccountStorageTrieNode_returnsEmptyNode() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + assertThat(storage.getAccountStorageTrieNode(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + } + + @Test + public void getNodeData_returnsEmptyValue() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + assertThat(storage.getNodeData(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getNodeData_returnsEmptyNode() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + assertThat(storage.getNodeData(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + } + + @Test + public void getCode_saveAndGetSpecialValues() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage + .updater() + .putCode(MerklePatriciaTrie.EMPTY_TRIE_NODE) + .putCode(BytesValue.EMPTY) + .commit(); + + assertThat(storage.getCode(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + assertThat(storage.getCode(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getCode_saveAndGetRegularValue() { + BytesValue bytes = BytesValue.fromHexString("0x123456"); + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage.updater().putCode(bytes).commit(); + + assertThat(storage.getCode(Hash.hash(bytes))).contains(bytes); + } + + @Test + public void getAccountStateTrieNode_saveAndGetSpecialValues() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage + .updater() + .putAccountStateTrieNode( + Hash.hash(MerklePatriciaTrie.EMPTY_TRIE_NODE), MerklePatriciaTrie.EMPTY_TRIE_NODE) + .putAccountStateTrieNode(Hash.hash(BytesValue.EMPTY), BytesValue.EMPTY) + .commit(); + + assertThat(storage.getAccountStateTrieNode(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + assertThat(storage.getAccountStateTrieNode(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getAccountStateTrieNode_saveAndGetRegularValue() { + BytesValue bytes = BytesValue.fromHexString("0x123456"); + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage.updater().putAccountStateTrieNode(Hash.hash(bytes), bytes).commit(); + + assertThat(storage.getAccountStateTrieNode(Hash.hash(bytes))).contains(bytes); + } + + @Test + public void getAccountStorageTrieNode_saveAndGetSpecialValues() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage + .updater() + .putAccountStorageTrieNode( + Hash.hash(MerklePatriciaTrie.EMPTY_TRIE_NODE), MerklePatriciaTrie.EMPTY_TRIE_NODE) + .putAccountStorageTrieNode(Hash.hash(BytesValue.EMPTY), BytesValue.EMPTY) + .commit(); + + assertThat(storage.getAccountStorageTrieNode(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + assertThat(storage.getAccountStorageTrieNode(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getAccountStorageTrieNode_saveAndGetRegularValue() { + BytesValue bytes = BytesValue.fromHexString("0x123456"); + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage.updater().putAccountStorageTrieNode(Hash.hash(bytes), bytes).commit(); + + assertThat(storage.getAccountStateTrieNode(Hash.hash(bytes))).contains(bytes); + } + + @Test + public void getNodeData_saveAndGetSpecialValues() { + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage + .updater() + .putAccountStorageTrieNode( + Hash.hash(MerklePatriciaTrie.EMPTY_TRIE_NODE), MerklePatriciaTrie.EMPTY_TRIE_NODE) + .putAccountStorageTrieNode(Hash.hash(BytesValue.EMPTY), BytesValue.EMPTY) + .commit(); + + assertThat(storage.getNodeData(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + assertThat(storage.getNodeData(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getNodeData_saveAndGetRegularValue() { + BytesValue bytes = BytesValue.fromHexString("0x123456"); + KeyValueStorageWorldStateStorage storage = emptyStorage(); + storage.updater().putAccountStorageTrieNode(Hash.hash(bytes), bytes).commit(); + + assertThat(storage.getNodeData(Hash.hash(bytes))).contains(bytes); + } + + private KeyValueStorageWorldStateStorage emptyStorage() { + return new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + } +} diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldStateTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldStateTest.java index 4fcfa153b3..7f4beb028d 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldStateTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldStateTest.java @@ -58,10 +58,10 @@ private static MutableWorldState createEmpty() { @Test public void rootHash_Empty() { final MutableWorldState worldState = createEmpty(); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); worldState.persist(); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); } @Test @@ -88,10 +88,10 @@ public void removeAccount_AccountDoesNotExist() { final WorldUpdater updater = worldState.updater(); updater.deleteAccount(ADDRESS); updater.commit(); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); worldState.persist(); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); } @Test @@ -101,10 +101,10 @@ public void removeAccount_UpdatedAccount() { updater.createAccount(ADDRESS).setBalance(Wei.of(100000)); updater.deleteAccount(ADDRESS); updater.commit(); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); worldState.persist(); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); } @Test @@ -115,7 +115,7 @@ public void removeAccount_AccountExists() { updater.createAccount(ADDRESS).setBalance(Wei.of(100000)); updater.commit(); assertNotNull(worldState.get(ADDRESS)); - assertNotEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertNotEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); // Delete account updater = worldState.updater(); @@ -125,7 +125,7 @@ public void removeAccount_AccountExists() { updater.commit(); assertNull(updater.get(ADDRESS)); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); } @Test @@ -137,7 +137,7 @@ public void removeAccount_AccountExistsAndIsPersisted() { updater.commit(); worldState.persist(); assertNotNull(worldState.get(ADDRESS)); - assertNotEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertNotEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); // Delete account updater = worldState.updater(); @@ -151,7 +151,7 @@ public void removeAccount_AccountExistsAndIsPersisted() { worldState.persist(); assertNull(updater.get(ADDRESS)); - assertEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); } @Test @@ -377,7 +377,7 @@ public void clearStorage_AfterPersisting() { updater.commit(); worldState.persist(); assertNotNull(worldState.get(ADDRESS)); - assertNotEquals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, worldState.rootHash()); + assertNotEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); // Clear storage account = updater.getMutable(ADDRESS); diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java new file mode 100644 index 0000000000..22e1db22aa --- /dev/null +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java @@ -0,0 +1,44 @@ +/* + * 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.worldstate; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueStorageWorldStateStorage; +import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; +import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import org.junit.Test; + +public class WorldStateArchiveTest { + + @Test + public void getNodeData_whenEmptyReturnsRequestForEmptyValue() { + WorldStateArchive archive = emptyArchive(); + assertThat(archive.getNodeData(Hash.EMPTY)).contains(BytesValue.EMPTY); + } + + @Test + public void getNodeData_whenEmptyReturnsRequestForEmptyTrieNode() { + WorldStateArchive archive = emptyArchive(); + assertThat(archive.getNodeData(Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH))) + .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); + } + + private WorldStateArchive emptyArchive() { + WorldStateStorage storage = new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + return new WorldStateArchive(storage); + } +} diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java index 7b3ef049d6..d631a13d17 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServer.java @@ -217,12 +217,7 @@ static MessageData constructGetNodeDataResponse( } count++; - if (hash.equals(Hash.EMPTY)) { - // No need to go to the archive for an empty value - nodeData.add(BytesValue.EMPTY); - } else { - worldStateArchive.getNodeData(hash).ifPresent(nodeData::add); - } + worldStateArchive.getNodeData(hash).ifPresent(nodeData::add); } return NodeDataMessage.create(nodeData); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java index 1da78cc4eb..3e48ba7d4a 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java @@ -14,10 +14,10 @@ import static com.google.common.base.Preconditions.checkNotNull; -import tech.pegasys.pantheon.ethereum.worldstate.StateTrieAccountValue; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; +import tech.pegasys.pantheon.ethereum.worldstate.StateTrieAccountValue; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage.Updater; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -50,7 +50,7 @@ protected List getRequestsFromTrieNodeValue(final BytesValue va nodeData.add(NodeDataRequest.createCodeRequest(accountValue.getCodeHash())); } // Add storage, if appropriate - if (!accountValue.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { + if (!accountValue.getStorageRoot().equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { // If storage is non-empty queue download NodeDataRequest storageNode = NodeDataRequest.createStorageDataRequest(accountValue.getStorageRoot()); 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 46559de9a4..5f57602030 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 @@ -77,7 +77,7 @@ public WorldStateDownloader( this.worldStateStorageUpdater = worldStateStorage.updater(); Hash stateRoot = header.getStateRoot(); - if (stateRoot.equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH)) { + if (stateRoot.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { // If we're requesting data for an empty world state, we're already done future = CompletableFuture.completedFuture(null); status = Status.DONE; diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index 918a912d7e..54796fde3b 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -48,7 +48,7 @@ public class WorldStateDownloaderTest { - private static final Hash EMPTY_TRIE_ROOT = Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH); + private static final Hash EMPTY_TRIE_ROOT = Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH); @Test public void downloadWorldStateFromPeers_onePeerOneWithManyRequestsOneAtATime() { diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerklePatriciaTrie.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerklePatriciaTrie.java index 0cb31da680..1b869ada5e 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerklePatriciaTrie.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerklePatriciaTrie.java @@ -16,6 +16,7 @@ import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.Bytes32; +import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.Map; import java.util.Optional; @@ -23,7 +24,8 @@ /** An Merkle Patricial Trie. */ public interface MerklePatriciaTrie { - Bytes32 EMPTY_TRIE_ROOT_HASH = keccak256(RLP.NULL); + BytesValue EMPTY_TRIE_NODE = RLP.NULL; + Bytes32 EMPTY_TRIE_NODE_HASH = keccak256(EMPTY_TRIE_NODE); /** * Returns an {@code Optional} of value mapped to the hash if it exists; otherwise empty. diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java index 1adca25d6d..e6996facab 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/NullNode.java @@ -12,9 +12,6 @@ */ package tech.pegasys.pantheon.ethereum.trie; -import static tech.pegasys.pantheon.crypto.Hash.keccak256; - -import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -22,8 +19,6 @@ import java.util.Optional; class NullNode implements Node { - private static final Bytes32 HASH = keccak256(RLP.NULL); - @SuppressWarnings("rawtypes") private static final NullNode instance = new NullNode(); @@ -61,17 +56,17 @@ public Optional>> getChildren() { @Override public BytesValue getRlp() { - return RLP.NULL; + return MerklePatriciaTrie.EMPTY_TRIE_NODE; } @Override public BytesValue getRlpRef() { - return RLP.NULL; + return MerklePatriciaTrie.EMPTY_TRIE_NODE; } @Override public Bytes32 getHash() { - return HASH; + return MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH; } @Override diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java index f38604d3eb..fbd565515c 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java @@ -45,7 +45,7 @@ public StoredMerklePatriciaTrie( final NodeLoader nodeLoader, final Function valueSerializer, final Function valueDeserializer) { - this(nodeLoader, MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH, valueSerializer, valueDeserializer); + this(nodeLoader, MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, valueSerializer, valueDeserializer); } public static StoredMerklePatriciaTrie create( @@ -70,7 +70,7 @@ public StoredMerklePatriciaTrie( final Function valueDeserializer) { this.nodeFactory = new StoredNodeFactory<>(nodeLoader, valueSerializer, valueDeserializer); this.root = - rootHash.equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH) + rootHash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH) ? NullNode.instance() : new StoredNode<>(nodeFactory, rootHash); } @@ -105,7 +105,7 @@ public void commit(final NodeUpdater nodeUpdater) { // Reset root so dirty nodes can be garbage collected final Bytes32 rootHash = root.getHash(); this.root = - rootHash.equals(MerklePatriciaTrie.EMPTY_TRIE_ROOT_HASH) + rootHash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH) ? NullNode.instance() : new StoredNode<>(nodeFactory, rootHash); } From ac98c31ef3fdd4ff88a6c46594d9b1ad97315dae Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 14:59:47 -0500 Subject: [PATCH 17/23] Fix some issues related to thread-safety, fix error-handling bug We should requeue requests when an error is encountered. --- .../sync/worldstate/WorldStateDownloader.java | 32 +++++----- .../manager/DeterministicEthScheduler.java | 16 ++++- .../manager/EthProtocolManagerTestUtil.java | 8 ++- .../worldstate/WorldStateDownloaderTest.java | 58 +++++++++++++++++++ .../services/queue/InMemoryBigQueue.java | 11 +--- 5 files changed, 100 insertions(+), 25 deletions(-) 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 5f57602030..06afeed700 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 @@ -79,8 +79,7 @@ public WorldStateDownloader( Hash stateRoot = header.getStateRoot(); if (stateRoot.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { // If we're requesting data for an empty world state, we're already done - future = CompletableFuture.completedFuture(null); - status = Status.DONE; + markDone(); } else { pendingRequests.enqueue(NodeDataRequest.createAccountDataRequest(header.getStateRoot())); } @@ -106,11 +105,7 @@ private void requestNodeData() { if (!maybePeer.isPresent()) { // If no peer is available, wait and try again - waitForNewPeer() - .whenComplete( - (r, t) -> { - requestNodeData(); - }); + waitForNewPeer().whenComplete((r, t) -> requestNodeData()); break; } else { EthPeer peer = maybePeer.get(); @@ -118,10 +113,11 @@ private void requestNodeData() { // Collect data to be requested List toRequest = new ArrayList<>(); for (int i = 0; i < hashCountPerRequest; i++) { - if (pendingRequests.isEmpty()) { + NodeDataRequest pendingRequest = pendingRequests.dequeue(); + if (pendingRequest == null) { break; } - toRequest.add(pendingRequests.dequeue()); + toRequest.add(pendingRequest); } // Request and process node data @@ -132,7 +128,7 @@ private void requestNodeData() { if (outstandingRequests.decrementAndGet() == 0 && pendingRequests.isEmpty()) { // We're done worldStateStorageUpdater.commit(); - future.complete(null); + markDone(); } else { // Send out additional requests requestNodeData(); @@ -144,6 +140,15 @@ private void requestNodeData() { } } + private synchronized void markDone() { + if (future == null) { + future = CompletableFuture.completedFuture(null); + } else { + future.complete(null); + } + status = Status.DONE; + } + private boolean shouldRequestNodeData() { return !future.isDone() && outstandingRequests.get() < maxOutstandingRequests @@ -165,10 +170,11 @@ private CompletableFuture sendAndProcessRequests( .run() .thenApply(PeerTaskResult::getResult) .thenApply(this::mapNodeDataByHash) - .thenAccept( - data -> { + .whenComplete( + (data, err) -> { + boolean requestFailed = err != null; for (NodeDataRequest request : requests) { - BytesValue matchingData = data.get(request.getHash()); + BytesValue matchingData = requestFailed ? null : data.get(request.getHash()); if (matchingData == null) { pendingRequests.enqueue(request); } else { diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/DeterministicEthScheduler.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/DeterministicEthScheduler.java index dea698c3d8..49c7629c2e 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/DeterministicEthScheduler.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/DeterministicEthScheduler.java @@ -16,6 +16,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; /** Schedules tasks that run immediately and synchronously for testing. */ public class DeterministicEthScheduler extends EthScheduler { @@ -23,7 +24,7 @@ public class DeterministicEthScheduler extends EthScheduler { private final TimeoutPolicy timeoutPolicy; DeterministicEthScheduler() { - this(() -> false); + this(TimeoutPolicy.NEVER); } DeterministicEthScheduler(final TimeoutPolicy timeoutPolicy) { @@ -55,6 +56,19 @@ public void failAfterTimeout(final CompletableFuture promise, final Durat @FunctionalInterface public interface TimeoutPolicy { + TimeoutPolicy NEVER = () -> false; + boolean shouldTimeout(); + + static TimeoutPolicy timeoutXTimes(final int times) { + final AtomicInteger timeouts = new AtomicInteger(times); + return () -> { + if (timeouts.get() <= 0) { + return false; + } + timeouts.decrementAndGet(); + return true; + }; + } } } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManagerTestUtil.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManagerTestUtil.java index a2ae437c19..ff523b04f8 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManagerTestUtil.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthProtocolManagerTestUtil.java @@ -47,16 +47,20 @@ public static EthProtocolManager create( public static EthProtocolManager create( final Blockchain blockchain, final WorldStateArchive worldStateArchive) { - return create(blockchain, worldStateArchive, () -> false); + return create(blockchain, worldStateArchive, TimeoutPolicy.NEVER); } public static EthProtocolManager create() { + return create(TimeoutPolicy.NEVER); + } + + public static EthProtocolManager create(final TimeoutPolicy timeoutPolicy) { final ProtocolSchedule protocolSchedule = MainnetProtocolSchedule.create(); final GenesisConfigFile config = GenesisConfigFile.mainnet(); final GenesisState genesisState = GenesisState.fromConfig(config, protocolSchedule); final Blockchain blockchain = createInMemoryBlockchain(genesisState.getBlock()); final WorldStateArchive worldStateArchive = createInMemoryWorldStateArchive(); - return create(blockchain, worldStateArchive); + return create(blockchain, worldStateArchive, timeoutPolicy); } public static void broadcastMessage( diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java index 54796fde3b..a66e559598 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/WorldStateDownloaderTest.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.core.MutableWorldState; import tech.pegasys.pantheon.ethereum.core.WorldState; +import tech.pegasys.pantheon.ethereum.eth.manager.DeterministicEthScheduler.TimeoutPolicy; import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManager; import tech.pegasys.pantheon.ethereum.eth.manager.EthProtocolManagerTestUtil; import tech.pegasys.pantheon.ethereum.eth.manager.RespondingEthPeer; @@ -118,6 +119,63 @@ public void downloadEmptyWorldState() { } } + @Test + public void canRecoverFromTimeouts() { + BlockDataGenerator dataGen = new BlockDataGenerator(1); + TimeoutPolicy timeoutPolicy = TimeoutPolicy.timeoutXTimes(2); + final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create(timeoutPolicy); + + // Setup "remote" state + final WorldStateStorage remoteStorage = + new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + final WorldStateArchive remoteWorldStateArchive = new WorldStateArchive(remoteStorage); + final MutableWorldState remoteWorldState = remoteWorldStateArchive.getMutable(); + + // Generate accounts and save corresponding state root + final List accounts = dataGen.createRandomAccounts(remoteWorldState, 20); + final Hash stateRoot = remoteWorldState.rootHash(); + assertThat(stateRoot).isNotEqualTo(EMPTY_TRIE_ROOT); // Sanity check + BlockHeader header = + dataGen.block(BlockOptions.create().setStateRoot(stateRoot).setBlockNumber(10)).getHeader(); + + // Create some peers + List peers = + Stream.generate( + () -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, header.getNumber())) + .limit(5) + .collect(Collectors.toList()); + + BigQueue queue = new InMemoryBigQueue<>(); + WorldStateStorage localStorage = + new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); + WorldStateDownloader downloader = + new WorldStateDownloader( + ethProtocolManager.ethContext(), + localStorage, + header, + queue, + 10, + 10, + NoOpMetricsSystem.NO_OP_LABELLED_TIMER); + + CompletableFuture result = downloader.run(); + + // Respond to node data requests + Responder responder = + RespondingEthPeer.blockchainResponder(mock(Blockchain.class), remoteWorldStateArchive); + while (!result.isDone()) { + for (RespondingEthPeer peer : peers) { + peer.respond(responder); + } + } + + // Check that all expected account data was downloaded + WorldStateArchive localWorldStateArchive = new WorldStateArchive(localStorage); + final WorldState localWorldState = localWorldStateArchive.get(stateRoot); + assertThat(result).isDone(); + assertAccountsMatch(localWorldState, accounts); + } + private void downloadAvailableWorldStateFromPeers( final int peerCount, final int accountCount, diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java index 623adc4e5c..b40b4ea186 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/InMemoryBigQueue.java @@ -14,30 +14,23 @@ import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.atomic.AtomicLong; public class InMemoryBigQueue implements BigQueue { - private final AtomicLong size = new AtomicLong(0); private final Queue internalQueue = new ConcurrentLinkedQueue<>(); @Override public void enqueue(final T value) { internalQueue.add(value); - size.incrementAndGet(); } @Override public T dequeue() { - T result = internalQueue.poll(); - if (result != null) { - size.decrementAndGet(); - } - return result; + return internalQueue.poll(); } @Override public long size() { - return size.get(); + return internalQueue.size(); } @Override From af5ff059bf7a70bd46bfb509588d8c74750f4704 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 15:31:34 -0500 Subject: [PATCH 18/23] Cut obsolete test --- .../pantheon/ethereum/eth/manager/EthServerTest.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java index 6ba0ad4b60..43eddf2397 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/EthServerTest.java @@ -57,14 +57,6 @@ public void shouldRespondToNodeDataRequests() throws Exception { verify(ethPeer).send(NodeDataMessage.create(asList(VALUE1, VALUE2))); } - @Test - public void shouldRespondToNodeDataRequestsForEmptyValues() throws Exception { - when(worldStateArchive.getNodeData(Hash.EMPTY)).thenReturn(Optional.empty()); - ethMessages.dispatch(new EthMessage(ethPeer, GetNodeDataMessage.create(asList(Hash.EMPTY)))); - - verify(ethPeer).send(NodeDataMessage.create(asList(BytesValue.EMPTY))); - } - @Test public void shouldHandleDataBeingUnavailableWhenRespondingToNodeDataRequests() throws Exception { when(worldStateArchive.getNodeData(HASH1)).thenReturn(Optional.of(VALUE1)); From c57737a3120cac021840989541383dc75baa2a81 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 16:01:47 -0500 Subject: [PATCH 19/23] Clean up trie node handling --- .../AccountTrieNodeDataRequest.java | 2 +- .../sync/worldstate/CodeNodeDataRequest.java | 4 ++-- .../eth/sync/worldstate/NodeDataRequest.java | 4 ++-- .../StorageTrieNodeDataRequest.java | 2 +- .../sync/worldstate/TrieNodeDataRequest.java | 24 +++++++++++-------- .../pantheon/ethereum/trie/BranchNode.java | 6 ++--- .../pantheon/ethereum/trie/ExtensionNode.java | 6 ++--- .../pantheon/ethereum/trie/LeafNode.java | 6 ++--- .../pegasys/pantheon/ethereum/trie/Node.java | 12 +++++----- .../pantheon/ethereum/trie/StoredNode.java | 22 ++++++++++------- 10 files changed, 48 insertions(+), 40 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java index 3e48ba7d4a..fcdb9df8e1 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/AccountTrieNodeDataRequest.java @@ -31,7 +31,7 @@ class AccountTrieNodeDataRequest extends TrieNodeDataRequest { } @Override - void persist(final Updater updater) { + public void persist(final Updater updater) { checkNotNull(getData(), "Must set data before node can be persisted."); updater.putAccountStateTrieNode(getHash(), getData()); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java index eee77db011..0466252281 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/CodeNodeDataRequest.java @@ -26,13 +26,13 @@ class CodeNodeDataRequest extends NodeDataRequest { } @Override - void persist(final Updater updater) { + public void persist(final Updater updater) { checkNotNull(getData(), "Must set data before node can be persisted."); updater.putCode(getHash(), getData()); } @Override - Stream getChildRequests() { + public Stream getChildRequests() { // Code nodes have nothing further to download return Stream.empty(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java index 3e821c1c1e..67b7a63ae5 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/NodeDataRequest.java @@ -63,7 +63,7 @@ public NodeDataRequest setData(final BytesValue data) { return this; } - abstract void persist(final WorldStateStorage.Updater updater); + public abstract void persist(final WorldStateStorage.Updater updater); - abstract Stream getChildRequests(); + public abstract Stream getChildRequests(); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java index b28aec1dbf..d61292a066 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/StorageTrieNodeDataRequest.java @@ -28,7 +28,7 @@ class StorageTrieNodeDataRequest extends TrieNodeDataRequest { } @Override - void persist(final Updater updater) { + public void persist(final Updater updater) { checkNotNull(getData(), "Must set data before node can be persisted."); updater.putAccountStorageTrieNode(getHash(), getData()); } diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java index a9ba06b18b..ba3afb7989 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java @@ -14,7 +14,6 @@ import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.trie.Node; -import tech.pegasys.pantheon.ethereum.trie.StoredNode; import tech.pegasys.pantheon.ethereum.trie.StoredNodeFactory; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -30,28 +29,23 @@ abstract class TrieNodeDataRequest extends NodeDataRequest { } @Override - Stream getChildRequests() { + public Stream getChildRequests() { if (getData() == null) { // If this node hasn't been downloaded yet, we can't return any child data return Stream.empty(); } final Node node = nodeFactory.decode(getData()); - return getRequestsFromTrieNode(node); + return getRequestsFromLoadedTrieNode(node); } - private Stream getRequestsFromTrieNode(final Node trieNode) { - if (trieNode instanceof StoredNode && !((StoredNode) trieNode).isLoaded()) { - // Stored nodes represent nodes that are referenced by hash (and therefore must be downloaded) - NodeDataRequest req = createChildNodeDataRequest(Hash.wrap(trieNode.getHash())); - return Stream.of(req); - } + private Stream getRequestsFromLoadedTrieNode(final Node trieNode) { // Process this node's children final Stream childRequests = trieNode .getChildren() .map(List::stream) - .map(s -> s.flatMap(this::getRequestsFromTrieNode)) + .map(s -> s.flatMap(this::getRequestsFromChildTrieNode)) .orElse(Stream.of()); // Process value at this node, if present @@ -61,6 +55,16 @@ private Stream getRequestsFromTrieNode(final Node t .orElse(childRequests); } + private Stream getRequestsFromChildTrieNode(final Node trieNode) { + if (trieNode.isReferencedByHash()) { + // If child nodes are reference by hash, we need to download them + NodeDataRequest req = createChildNodeDataRequest(Hash.wrap(trieNode.getHash())); + return Stream.of(req); + } + // Otherwise if the child's value has been inlined we can go ahead and process it + return getRequestsFromLoadedTrieNode(trieNode); + } + protected abstract NodeDataRequest createChildNodeDataRequest(final Hash childHash); protected abstract List getRequestsFromTrieNodeValue(final BytesValue value); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java index 24cb398030..2899d4a14d 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/BranchNode.java @@ -110,10 +110,10 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { - if (shouldBeInlined()) { - return getRlp(); - } else { + if (isReferencedByHash()) { return RLP.encodeOne(getHash()); + } else { + return getRlp(); } } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java index f99af8c2a6..41c0909581 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/ExtensionNode.java @@ -92,10 +92,10 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { - if (shouldBeInlined()) { - return getRlp(); - } else { + if (isReferencedByHash()) { return RLP.encodeOne(getHash()); + } else { + return getRlp(); } } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java index fd4bf9a36a..45e72e6923 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/LeafNode.java @@ -91,10 +91,10 @@ public BytesValue getRlp() { @Override public BytesValue getRlpRef() { - if (shouldBeInlined()) { - return getRlp(); - } else { + if (isReferencedByHash()) { return RLP.encodeOne(getHash()); + } else { + return getRlp(); } } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java index 436c5daf72..8f35692d20 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/Node.java @@ -35,14 +35,14 @@ public interface Node { BytesValue getRlpRef(); /** - * Whether a reference to this node should be inlined (the rlp stored directly in the parent - * node). If true, rlp is included in parent directly. If false, a hash of the rlp is stored in - * the parent node as a reference. + * Whether a reference to this node should be represented as a hash of the rlp, or the node rlp + * itself should be inlined (the rlp stored directly in the parent node). If true, the node is + * referenced by hash. If false, the node is referenced by its rlp-encoded value. * - * @return + * @return true if this node should be referenced by hash */ - default boolean shouldBeInlined() { - return getRlp().size() < 32; + default boolean isReferencedByHash() { + return getRlp().size() >= 32; } Bytes32 getHash(); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java index 861fcdcf8a..54137ea427 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java @@ -19,7 +19,7 @@ import java.util.List; import java.util.Optional; -public class StoredNode implements Node { +class StoredNode implements Node { private final StoredNodeFactory nodeFactory; private final Bytes32 hash; private Node loaded; @@ -29,10 +29,6 @@ public class StoredNode implements Node { this.hash = hash; } - public boolean isLoaded() { - return loaded != null; - } - /** @return True if the node needs to be persisted. */ @Override public boolean isDirty() { @@ -75,8 +71,7 @@ public Optional>> getChildren() { @Override public BytesValue getRlp() { - // Getting the rlp representation is only needed when persisting a concrete node - throw new UnsupportedOperationException(); + return load().getRlp(); } @Override @@ -85,6 +80,12 @@ public BytesValue getRlpRef() { return RLP.encodeOne(hash); } + @Override + public boolean isReferencedByHash() { + // Stored nodes represent only nodes that are referenced by hash + return true; + } + @Override public Bytes32 getHash() { return hash; @@ -108,7 +109,10 @@ private Node load() { @Override public String print() { - final String value = load().print(); - return value; + if (loaded == null) { + return "StoredNode:" + "\n\tRef: " + getRlpRef(); + } else { + return load().print(); + } } } From aa0394b4618bf4dfbe073e36a093f33a0daae6b2 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 16:20:35 -0500 Subject: [PATCH 20/23] Create a TrieNodeDecoder helper --- .../sync/worldstate/TrieNodeDataRequest.java | 6 ++-- ...xception.java => MerkleTrieException.java} | 6 ++-- .../pantheon/ethereum/trie/StoredNode.java | 3 +- .../ethereum/trie/StoredNodeFactory.java | 25 +++++-------- .../ethereum/trie/TrieNodeDecoder.java | 36 +++++++++++++++++++ 5 files changed, 53 insertions(+), 23 deletions(-) rename ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/{MerkleStorageException.java => MerkleTrieException.java} (80%) create mode 100644 ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/TrieNodeDecoder.java diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java index ba3afb7989..abf9bbd512 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/worldstate/TrieNodeDataRequest.java @@ -14,7 +14,7 @@ import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.trie.Node; -import tech.pegasys.pantheon.ethereum.trie.StoredNodeFactory; +import tech.pegasys.pantheon.ethereum.trie.TrieNodeDecoder; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.List; @@ -22,7 +22,7 @@ abstract class TrieNodeDataRequest extends NodeDataRequest { - private static final StoredNodeFactory nodeFactory = StoredNodeFactory.create(); + private static final TrieNodeDecoder nodeDecoder = TrieNodeDecoder.create(); TrieNodeDataRequest(final Kind kind, final Hash hash) { super(kind, hash); @@ -35,7 +35,7 @@ public Stream getChildRequests() { return Stream.empty(); } - final Node node = nodeFactory.decode(getData()); + final Node node = nodeDecoder.decode(getData()); return getRequestsFromLoadedTrieNode(node); } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerkleStorageException.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerkleTrieException.java similarity index 80% rename from ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerkleStorageException.java rename to ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerkleTrieException.java index 3ed75052f4..b00a1f60b2 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerkleStorageException.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/MerkleTrieException.java @@ -16,13 +16,13 @@ * This exception is thrown when there is an issue retrieving or decoding values from {@link * MerkleStorage}. */ -public class MerkleStorageException extends RuntimeException { +public class MerkleTrieException extends RuntimeException { - public MerkleStorageException(final String message) { + public MerkleTrieException(final String message) { super(message); } - public MerkleStorageException(final String message, final Exception cause) { + public MerkleTrieException(final String message, final Exception cause) { super(message, cause); } } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java index 54137ea427..a9b0759829 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNode.java @@ -101,7 +101,8 @@ private Node load() { loaded = nodeFactory .retrieve(hash) - .orElseThrow(() -> new MerkleStorageException("Missing value for hash " + hash)); + .orElseThrow( + () -> new MerkleTrieException("Unable to load trie node value for hash " + hash)); } return loaded; diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java index 9bfc5cd9fe..c7131a26fe 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java @@ -43,11 +43,6 @@ public class StoredNodeFactory implements NodeFactory { this.valueDeserializer = valueDeserializer; } - public static StoredNodeFactory create() { - return new StoredNodeFactory<>( - (h) -> Optional.empty(), Function.identity(), Function.identity()); - } - @Override public Node createExtension(final BytesValue path, final Node child) { return handleNewNode(new ExtensionNode<>(path, child, this)); @@ -92,7 +87,7 @@ private Node handleNewNode(final Node node) { return node; } - public Optional> retrieve(final Bytes32 hash) throws MerkleStorageException { + public Optional> retrieve(final Bytes32 hash) throws MerkleTrieException { return nodeLoader .getNode(hash) .map( @@ -110,11 +105,11 @@ public Node decode(final BytesValue rlp) { } private Node decode(final BytesValue rlp, final Supplier errMessage) - throws MerkleStorageException { + throws MerkleTrieException { try { return decode(RLP.input(rlp), errMessage); } catch (final RLPException ex) { - throw new MerkleStorageException(errMessage.get(), ex); + throw new MerkleTrieException(errMessage.get(), ex); } } @@ -131,8 +126,7 @@ private Node decode(final RLPInput nodeRLPs, final Supplier errMessag try { path = CompactEncoding.decode(encodedPath); } catch (final IllegalArgumentException ex) { - throw new MerkleStorageException( - errMessage.get() + ": invalid path " + encodedPath, ex); + throw new MerkleTrieException(errMessage.get() + ": invalid path " + encodedPath, ex); } final int size = path.size(); @@ -146,7 +140,7 @@ private Node decode(final RLPInput nodeRLPs, final Supplier errMessag return decodeBranch(nodeRLPs, errMessage); default: - throw new MerkleStorageException( + throw new MerkleTrieException( errMessage.get() + format(": invalid list size %s", nodesCount)); } } finally { @@ -197,7 +191,7 @@ private BranchNode decodeBranch(final RLPInput nodeRLPs, final Supplier decodeLeaf( final BytesValue path, final RLPInput valueRlp, final Supplier errMessage) { if (valueRlp.nextIsNull()) { - throw new MerkleStorageException(errMessage.get() + ": leaf has null value"); + throw new MerkleTrieException(errMessage.get() + ": leaf has null value"); } final V value = decodeValue(valueRlp, errMessage); return new LeafNode<>(path, value, this, valueSerializer); @@ -206,7 +200,7 @@ private LeafNode decodeLeaf( @SuppressWarnings("unchecked") private NullNode decodeNull(final RLPInput nodeRLPs, final Supplier errMessage) { if (!nodeRLPs.nextIsNull()) { - throw new MerkleStorageException(errMessage.get() + ": list size 1 but not null"); + throw new MerkleTrieException(errMessage.get() + ": list size 1 but not null"); } nodeRLPs.skipNext(); return NULL_NODE; @@ -217,7 +211,7 @@ private V decodeValue(final RLPInput valueRlp, final Supplier errMessage try { bytes = valueRlp.readBytesValue(); } catch (final RLPException ex) { - throw new MerkleStorageException( + throw new MerkleTrieException( errMessage.get() + ": failed decoding value rlp " + valueRlp, ex); } return deserializeValue(errMessage, bytes); @@ -228,8 +222,7 @@ private V deserializeValue(final Supplier errMessage, final BytesValue b try { value = valueDeserializer.apply(bytes); } catch (final IllegalArgumentException ex) { - throw new MerkleStorageException( - errMessage.get() + ": failed deserializing value " + bytes, ex); + throw new MerkleTrieException(errMessage.get() + ": failed deserializing value " + bytes, ex); } return value; } diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/TrieNodeDecoder.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/TrieNodeDecoder.java new file mode 100644 index 0000000000..18ca328e50 --- /dev/null +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/TrieNodeDecoder.java @@ -0,0 +1,36 @@ +/* + * 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.trie; + +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.util.Optional; +import java.util.function.Function; + +public class TrieNodeDecoder { + + private final StoredNodeFactory nodeFactory; + + private TrieNodeDecoder() { + nodeFactory = + new StoredNodeFactory<>((h) -> Optional.empty(), Function.identity(), Function.identity()); + } + + public static TrieNodeDecoder create() { + return new TrieNodeDecoder(); + } + + public Node decode(final BytesValue rlp) { + return nodeFactory.decode(rlp); + } +} From 6ee99e7f1ebdb64894343784f60158801bf3d5db Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 16:51:16 -0500 Subject: [PATCH 21/23] Fix method names, remove unintended changes --- .../keyvalue/KeyValueStorageWorldStateStorageTest.java | 4 ++-- .../pantheon/ethereum/trie/StoredMerklePatriciaTrie.java | 6 ------ .../pegasys/pantheon/ethereum/trie/StoredNodeFactory.java | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java index 7132e5471b..ca95b5ce44 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorageTest.java @@ -24,13 +24,13 @@ public class KeyValueStorageWorldStateStorageTest { @Test - public void getsCode_returnsEmpty() { + public void getCode_returnsEmpty() { KeyValueStorageWorldStateStorage storage = emptyStorage(); assertThat(storage.getCode(Hash.EMPTY)).contains(BytesValue.EMPTY); } @Test - public void getAccountStateTrieNode_returnsNodeNode() { + public void getAccountStateTrieNode_returnsEmptyNode() { KeyValueStorageWorldStateStorage storage = emptyStorage(); assertThat(storage.getAccountStateTrieNode(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java index fbd565515c..63f6f3648c 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredMerklePatriciaTrie.java @@ -48,12 +48,6 @@ public StoredMerklePatriciaTrie( this(nodeLoader, MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, valueSerializer, valueDeserializer); } - public static StoredMerklePatriciaTrie create( - final NodeLoader nodeLoader, final Bytes32 rootHash) { - return new StoredMerklePatriciaTrie<>( - nodeLoader, rootHash, Function.identity(), Function.identity()); - } - /** * Create a trie. * diff --git a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java index c7131a26fe..41dca7855e 100644 --- a/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java +++ b/ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/StoredNodeFactory.java @@ -26,7 +26,7 @@ import java.util.function.Function; import java.util.function.Supplier; -public class StoredNodeFactory implements NodeFactory { +class StoredNodeFactory implements NodeFactory { @SuppressWarnings("rawtypes") private static final NullNode NULL_NODE = NullNode.instance(); From 3cc3b9cac5d8e1b3026cfad66ae1d93cbbd86c1b Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 20:31:48 -0500 Subject: [PATCH 22/23] Make KeyValueWorldStateStorage more readable, cut duplicate checks --- .../KeyValueStorageWorldStateStorage.java | 41 +++++++---------- .../worldstate/WorldStateArchive.java | 10 +---- .../worldstate/WorldStateArchiveTest.java | 44 ------------------- 3 files changed, 18 insertions(+), 77 deletions(-) delete mode 100644 ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java index e136fb80b9..51f7c2eb6a 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java @@ -13,7 +13,6 @@ package tech.pegasys.pantheon.ethereum.storage.keyvalue; import tech.pegasys.pantheon.ethereum.core.Hash; -import tech.pegasys.pantheon.ethereum.rlp.RLP; import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateStorage; import tech.pegasys.pantheon.services.kvstore.KeyValueStorage; @@ -21,7 +20,6 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.util.Optional; -import java.util.function.Function; public class KeyValueStorageWorldStateStorage implements WorldStateStorage { @@ -33,47 +31,42 @@ public KeyValueStorageWorldStateStorage(final KeyValueStorage keyValueStorage) { @Override public Optional getCode(final Bytes32 codeHash) { - return getCodeValue(codeHash, keyValueStorage::get); + if (codeHash.equals(Hash.EMPTY)) { + return Optional.of(BytesValue.EMPTY); + } else { + return keyValueStorage.get(codeHash); + } } @Override public Optional getAccountStateTrieNode(final Bytes32 nodeHash) { - return getTrieValue(nodeHash, keyValueStorage::get); + return getTrieNode(nodeHash); } @Override public Optional getAccountStorageTrieNode(final Bytes32 nodeHash) { - return getTrieValue(nodeHash, keyValueStorage::get); - } - - @Override - public Optional getNodeData(final Bytes32 hash) { - return getValue(hash, keyValueStorage::get); + return getTrieNode(nodeHash); } - private Optional getTrieValue( - final Bytes32 hash, final Function> getter) { - if (hash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { - return Optional.of(RLP.NULL); + private Optional getTrieNode(final Bytes32 nodeHash) { + if (nodeHash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { + return Optional.of(MerklePatriciaTrie.EMPTY_TRIE_NODE); } else { - return getter.apply(hash); + return keyValueStorage.get(nodeHash); } } - private Optional getCodeValue( - final Bytes32 hash, final Function> getter) { - if (hash.equals(Hash.EMPTY)) { + @Override + public Optional getNodeData(final Bytes32 hash) { + if (hash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { + return Optional.of(MerklePatriciaTrie.EMPTY_TRIE_NODE); + } else if (hash.equals(Hash.EMPTY)) { return Optional.of(BytesValue.EMPTY); } else { - return getter.apply(hash); + return keyValueStorage.get(hash); } } - private Optional getValue( - final Bytes32 hash, final Function> getter) { - return getTrieValue(hash, (h) -> getCodeValue(h, getter)); - } - @Override public Updater updater() { return new Updater(keyValueStorage.startTransaction()); diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java index dcaa93d6e7..f29742f2db 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchive.java @@ -45,15 +45,7 @@ public MutableWorldState getMutable() { } public Optional getNodeData(final Hash hash) { - if (hash.equals(Hash.EMPTY)) { - // No need to go to storage for an empty value - return Optional.of(BytesValue.EMPTY); - } else if (hash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) { - // No need to go to storage for an trie node - return Optional.of(MerklePatriciaTrie.EMPTY_TRIE_NODE); - } else { - return storage.getNodeData(hash); - } + return storage.getNodeData(hash); } public WorldStateStorage getStorage() { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java deleted file mode 100644 index 22e1db22aa..0000000000 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStateArchiveTest.java +++ /dev/null @@ -1,44 +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.worldstate; - -import static org.assertj.core.api.Assertions.assertThat; - -import tech.pegasys.pantheon.ethereum.core.Hash; -import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueStorageWorldStateStorage; -import tech.pegasys.pantheon.ethereum.trie.MerklePatriciaTrie; -import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; -import tech.pegasys.pantheon.util.bytes.BytesValue; - -import org.junit.Test; - -public class WorldStateArchiveTest { - - @Test - public void getNodeData_whenEmptyReturnsRequestForEmptyValue() { - WorldStateArchive archive = emptyArchive(); - assertThat(archive.getNodeData(Hash.EMPTY)).contains(BytesValue.EMPTY); - } - - @Test - public void getNodeData_whenEmptyReturnsRequestForEmptyTrieNode() { - WorldStateArchive archive = emptyArchive(); - assertThat(archive.getNodeData(Hash.wrap(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH))) - .contains(MerklePatriciaTrie.EMPTY_TRIE_NODE); - } - - private WorldStateArchive emptyArchive() { - WorldStateStorage storage = new KeyValueStorageWorldStateStorage(new InMemoryKeyValueStorage()); - return new WorldStateArchive(storage); - } -} From 30858a29d09bf9fd010860b8d1257a507f9dc789 Mon Sep 17 00:00:00 2001 From: Meredith Baxter Date: Fri, 25 Jan 2019 20:31:59 -0500 Subject: [PATCH 23/23] Fix javadoc --- .../java/tech/pegasys/pantheon/services/queue/BigQueue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java index 887feed38c..793669a8fb 100644 --- a/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java +++ b/services/queue/src/main/java/tech/pegasys/pantheon/services/queue/BigQueue.java @@ -17,7 +17,7 @@ /** * Represents a very large thread-safe queue that may exceed memory limits. * - * @param + * @param the type of data held in the queue */ public interface BigQueue extends Closeable {