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

[NC-1344] Create a simple WorldStateDownloader #657

Merged
merged 25 commits into from
Jan 26, 2019
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9593d1f
In progress - start writing WorldStateDownloader
mbaxter Jan 21, 2019
fad8ce1
Queue additonal requests as node data comes back from the network
mbaxter Jan 21, 2019
e1178b3
Fix bug
mbaxter Jan 23, 2019
1b2dff6
Fix some warnings, start adding tests
mbaxter Jan 23, 2019
17114a8
Finish first downloader test, fix some issues related codeless accounts
mbaxter Jan 23, 2019
ac572f3
Add final's
mbaxter Jan 23, 2019
2ef2c6a
Rename some classes and variable for clarity
mbaxter Jan 23, 2019
e205567
Rework downloader w thread-safety in mind
mbaxter Jan 24, 2019
91d7f6d
Clarify some method names
mbaxter Jan 24, 2019
4928b41
Fix comment
mbaxter Jan 24, 2019
3af28bd
Fix bug - add missing break statement
mbaxter Jan 24, 2019
98d808d
Add more tests
mbaxter Jan 24, 2019
cce5e7e
Minor cleanup
mbaxter Jan 24, 2019
365c32d
Rename AccountTuple and move it into the worldstate package
mbaxter Jan 25, 2019
6e1a67c
Remove TODO comments
mbaxter Jan 25, 2019
03018c5
Skip storage lookup for empty code and empty trie nodes
mbaxter Jan 25, 2019
ac98c31
Fix some issues related to thread-safety, fix error-handling bug
mbaxter Jan 25, 2019
af5ff05
Cut obsolete test
mbaxter Jan 25, 2019
c57737a
Clean up trie node handling
mbaxter Jan 25, 2019
aa0394b
Create a TrieNodeDecoder helper
mbaxter Jan 25, 2019
c996fdf
Merge branch 'master' into nc-1344/download-world-state
mbaxter Jan 25, 2019
6ee99e7
Fix method names, remove unintended changes
mbaxter Jan 25, 2019
e111aa7
Merge branch 'master' into nc-1344/download-world-state
mbaxter Jan 26, 2019
3cc3b9c
Make KeyValueWorldStateStorage more readable, cut duplicate checks
mbaxter Jan 26, 2019
30858a2
Fix javadoc
mbaxter Jan 26, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -29,23 +32,46 @@ public KeyValueStorageWorldStateStorage(final KeyValueStorage keyValueStorage) {
}

@Override
public Optional<BytesValue> getCode(final Hash codeHash) {
return keyValueStorage.get(codeHash);
public Optional<BytesValue> getCode(final Bytes32 codeHash) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of these methods use Hash and some use Bytes32 - just changed them all to Bytes32 since that is more generic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's also kind of annoying to have to wrap Bytes32 in Hash all over. Kind of wondering if we really need the Hash type ...

Copy link
Contributor

Choose a reason for hiding this comment

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

I like Hash providing a very clear type to say this is a hash, not just an arbitrary 32 bytes but I don't think it has to be a hard and fast "every hash must use the Hash type" kind of thing. Particularly in this low level kind of place where the meaning is unambiguous using Bytes32 makes sense to me.

return getCodeValue(codeHash, keyValueStorage::get);
}

@Override
public Optional<BytesValue> getAccountStateTrieNode(final Bytes32 nodeHash) {
return keyValueStorage.get(nodeHash);
return getTrieValue(nodeHash, keyValueStorage::get);
}

@Override
public Optional<BytesValue> getAccountStorageTrieNode(final Bytes32 nodeHash) {
return keyValueStorage.get(nodeHash);
return getTrieValue(nodeHash, keyValueStorage::get);
}

@Override
public Optional<BytesValue> getNodeData(final Hash hash) {
return keyValueStorage.get(hash);
public Optional<BytesValue> getNodeData(final Bytes32 hash) {
return getValue(hash, keyValueStorage::get);
}

private Optional<BytesValue> getTrieValue(
final Bytes32 hash, final Function<Bytes32, Optional<BytesValue>> getter) {
if (hash.equals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH)) {
return Optional.of(RLP.NULL);
} else {
return getter.apply(hash);
}
}

private Optional<BytesValue> getCodeValue(
final Bytes32 hash, final Function<Bytes32, Optional<BytesValue>> getter) {
if (hash.equals(Hash.EMPTY)) {
return Optional.of(BytesValue.EMPTY);
} else {
return getter.apply(hash);
}
}

private Optional<BytesValue> getValue(
final Bytes32 hash, final Function<Bytes32, Optional<BytesValue>> getter) {
return getTrieValue(hash, (h) -> getCodeValue(h, getter));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we've wound up applying the check for empty twice in the getNodeData path - once in WorldStateArchive and once here. This is probably the better place, but I actually prefer the more explicit code from WorldStateArchive - I know it essentially duplicates some code but explicitly checking for EMPTY and EMPTY_TRIE_NODE_HASH is just a bit more readable than delegating through optionals.

I'm not sure it matters, but it also avoids creating an extra object instances that this lambda version requires (to create a closure over the getter param).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough - updated

}

@Override
Expand All @@ -62,18 +88,33 @@ public Updater(final KeyValueStorage.Transaction transaction) {
}

@Override
public void putCode(final BytesValue code) {
transaction.put(Hash.hash(code), code);
public Updater putCode(final Bytes32 codeHash, final BytesValue code) {
if (code.size() == 0) {
// Don't save empty values
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's nothing special about a root node, so updated this constant name so that it can be used more generally.

}

public DefaultMutableWorldState(
Expand Down Expand Up @@ -103,31 +103,15 @@ 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);
StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(in);
return new AccountState(address, addressHash, accountValue);
}

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) {
StateTrieAccountValue accountValue =
new StateTrieAccountValue(nonce, balance, storageRoot, codeHash);
return RLP.encode(accountValue::writeTo);
}

@Override
Expand Down Expand Up @@ -187,28 +171,17 @@ 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<Bytes32, BytesValue> storageTrie;

private AccountState(
final Address address,
final Hash addressHash,
final long nonce,
final Wei balance,
final Hash storageRoot,
final Hash codeHash) {
final Address address, final Hash addressHash, final StateTrieAccountValue accountValue) {

this.address = address;
this.addressHash = addressHash;
this.nonce = nonce;
this.balance = balance;
this.storageRoot = storageRoot;
this.codeHash = codeHash;
this.accountValue = accountValue;
}

private MerklePatriciaTrie<Bytes32, BytesValue> storageTrie() {
Expand All @@ -217,7 +190,7 @@ private MerklePatriciaTrie<Bytes32, BytesValue> storageTrie() {
storageTrie = updatedTrie;
}
if (storageTrie == null) {
storageTrie = newAccountStorageTrie(storageRoot);
storageTrie = newAccountStorageTrie(getStorageRoot());
}
return storageTrie;
}
Expand All @@ -234,12 +207,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
Expand All @@ -249,6 +226,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;
}
Expand All @@ -262,7 +240,7 @@ public boolean hasCode() {

@Override
public Hash getCodeHash() {
return codeHash;
return accountValue.getCodeHash();
}

@Override
Expand Down Expand Up @@ -303,8 +281,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();
}
}
Expand Down Expand Up @@ -353,14 +331,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());
}
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* 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 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 StateTrieAccountValue {

private final long nonce;
private final Wei balance;
private final Hash storageRoot;
private final Hash codeHash;

public StateTrieAccountValue(
final long nonce, final Wei balance, final Hash storageRoot, final 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 StateTrieAccountValue 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 StateTrieAccountValue(nonce, balance, storageRoot, codeHash);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -45,6 +45,18 @@ public MutableWorldState getMutable() {
}

public Optional<BytesValue> 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() {
return storage;
}
}
Loading