From a7d9bfdc0a11fdd5d7681e95bdf7ed71375db708 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Mon, 29 Jul 2019 18:00:29 -0400 Subject: [PATCH] [PAN-2953] Track world state account key preimages (#1780) Signed-off-by: Adrian Sutton --- .../pantheon/ethereum/core/WorldState.java | 6 +- .../WorldStatePreimageKeyValueStorage.java | 22 ++- .../DebuggableMutableWorldState.java | 176 ------------------ .../worldstate/DefaultMutableWorldState.java | 23 ++- .../worldstate/WorldStatePreimageStorage.java | 5 + .../vm/GeneralStateReferenceTestTools.java | 4 +- .../pantheon/ethereum/vm/WorldStateMock.java | 11 +- .../DefaultMutableWorldStateTest.java | 76 ++++++++ 8 files changed, 136 insertions(+), 187 deletions(-) delete mode 100644 ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/WorldState.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/WorldState.java index 4bc4808ff7..8e5af56e48 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/WorldState.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/WorldState.java @@ -12,6 +12,8 @@ */ package tech.pegasys.pantheon.ethereum.core; +import tech.pegasys.pantheon.util.bytes.Bytes32; + import java.util.stream.Stream; /** @@ -34,8 +36,10 @@ public interface WorldState extends WorldView { /** * A stream of all the accounts in this world state. * + * @param startKeyHash The trie key at which to start iterating + * @param limit The maximum number of results to return * @return a stream of all the accounts (in no particular order) contained in the world state * represented by the root hash of this object at the time of the call. */ - Stream streamAccounts(); + Stream streamAccounts(Bytes32 startKeyHash, int limit); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/WorldStatePreimageKeyValueStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/WorldStatePreimageKeyValueStorage.java index 60addc0e2d..c577fa018c 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/WorldStatePreimageKeyValueStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/storage/keyvalue/WorldStatePreimageKeyValueStorage.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.storage.keyvalue; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.worldstate.WorldStatePreimageStorage; import tech.pegasys.pantheon.services.kvstore.KeyValueStorage; import tech.pegasys.pantheon.services.kvstore.KeyValueStorage.Transaction; @@ -29,7 +30,19 @@ public class WorldStatePreimageKeyValueStorage implements WorldStatePreimageStor @Override public Optional getStorageTrieKeyPreimage(final Bytes32 trieKey) { - return keyValueStorage.get(trieKey).map(Bytes32::wrap).map(UInt256::wrap); + return keyValueStorage + .get(trieKey) + .filter(val -> val.size() == UInt256.SIZE) + .map(Bytes32::wrap) + .map(UInt256::wrap); + } + + @Override + public Optional
getAccountTrieKeyPreimage(final Bytes32 trieKey) { + return keyValueStorage + .get(trieKey) + .filter(val -> val.size() == Address.SIZE) + .map(Address::wrap); } @Override @@ -51,6 +64,13 @@ public class WorldStatePreimageKeyValueStorage implements WorldStatePreimageStor return this; } + @Override + public WorldStatePreimageStorage.Updater putAccountTrieKeyPreimage( + final Bytes32 trieKey, final Address preimage) { + transaction.put(trieKey, preimage); + return this; + } + @Override public void commit() { transaction.commit(); diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java deleted file mode 100644 index 4af8368425..0000000000 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java +++ /dev/null @@ -1,176 +0,0 @@ -/* - * 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.ethereum.worldstate; - -import tech.pegasys.pantheon.ethereum.core.Account; -import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.MutableAccount; -import tech.pegasys.pantheon.ethereum.core.Wei; -import tech.pegasys.pantheon.ethereum.core.WorldState; -import tech.pegasys.pantheon.ethereum.core.WorldUpdater; -import tech.pegasys.pantheon.ethereum.storage.keyvalue.WorldStateKeyValueStorage; -import tech.pegasys.pantheon.ethereum.storage.keyvalue.WorldStatePreimageKeyValueStorage; -import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; - -import java.util.Collection; -import java.util.HashSet; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Stream; - -/** - * A simple extension of {@link DefaultMutableWorldState} that tracks in memory the mapping of hash - * to address for its accounts for debugging purposes. It also provides a full toString() method - * that display the content of the world state. It is obviously only mean for testing or debugging. - */ -public class DebuggableMutableWorldState extends DefaultMutableWorldState { - - // TODO: This is more complex than it should due to DefaultMutableWorldState.accounts() not being - // implmemented (pending NC-746). Once that is fixed, we won't need to keep the set of account - // hashes at all, just the hashtoAddress map (this is also why things are separated this way, - // it will make it easier to update later). - - private static class DebugInfo { - private final Set
accounts = new HashSet<>(); - - private void addAll(final DebugInfo other) { - this.accounts.addAll(other.accounts); - } - } - - private final DebugInfo info = new DebugInfo(); - - public DebuggableMutableWorldState() { - super( - new WorldStateKeyValueStorage(new InMemoryKeyValueStorage()), - new WorldStatePreimageKeyValueStorage(new InMemoryKeyValueStorage())); - } - - public DebuggableMutableWorldState(final WorldState worldState) { - super(worldState); - - if (worldState instanceof DebuggableMutableWorldState) { - final DebuggableMutableWorldState dws = ((DebuggableMutableWorldState) worldState); - info.addAll(dws.info); - } else { - // TODO: on NC-746 gets in, we can remove this. That is, post NC-746, we won't be relying - // on info.accounts to know that accounts exists, so the only thing we will not have in - // this branch is info.addressToHash, but that's not a huge deal. - throw new RuntimeException(worldState + " is not a debuggable word state"); - } - } - - @Override - public WorldUpdater updater() { - return new InfoCollectingUpdater(super.updater(), info); - } - - @Override - public Stream streamAccounts() { - return info.accounts.stream().map(this::get).filter(Objects::nonNull); - } - - @Override - public String toString() { - final StringBuilder builder = new StringBuilder(); - builder.append(rootHash()).append(":\n"); - streamAccounts() - .forEach( - account -> { - final Address address = account.getAddress(); - builder - .append(" ") - .append(address == null ? "" : address) - .append(" [") - .append(account.getAddressHash()) - .append("]:\n"); - builder.append(" nonce: ").append(account.getNonce()).append('\n'); - builder.append(" balance: ").append(account.getBalance()).append('\n'); - builder.append(" code: ").append(account.getCode()).append('\n'); - }); - return builder.toString(); - } - - private class InfoCollectingUpdater implements WorldUpdater { - private final WorldUpdater wrapped; - private final DebugInfo commitInfo; - private DebugInfo ownInfo = new DebugInfo(); - - InfoCollectingUpdater(final WorldUpdater wrapped, final DebugInfo info) { - this.wrapped = wrapped; - this.commitInfo = info; - } - - private void record(final Address address) { - ownInfo.accounts.add(address); - } - - @Override - public MutableAccount createAccount( - final Address address, final long nonce, final Wei balance) { - record(address); - return wrapped.createAccount(address, nonce, balance); - } - - @Override - public MutableAccount createAccount(final Address address) { - record(address); - return wrapped.createAccount(address); - } - - @Override - public MutableAccount getOrCreate(final Address address) { - record(address); - return wrapped.getOrCreate(address); - } - - @Override - public MutableAccount getMutable(final Address address) { - record(address); - return wrapped.getMutable(address); - } - - @Override - public void deleteAccount(final Address address) { - wrapped.deleteAccount(address); - } - - @Override - public Collection getTouchedAccounts() { - return wrapped.getTouchedAccounts(); - } - - @Override - public void revert() { - ownInfo = new DebugInfo(); - wrapped.revert(); - } - - @Override - public void commit() { - commitInfo.addAll(ownInfo); - wrapped.commit(); - } - - @Override - public WorldUpdater updater() { - return new InfoCollectingUpdater(wrapped.updater(), ownInfo); - } - - @Override - public Account get(final Address address) { - record(address); - return wrapped.get(address); - } - } -} 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 bf616084a4..50d54b17fb 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 @@ -51,6 +51,7 @@ public class DefaultMutableWorldState implements MutableWorldState { new HashMap<>(); private final Map updatedAccountCode = new HashMap<>(); private final Map newStorageKeyPreimages = new HashMap<>(); + private final Map newAccountKeyPreimages = new HashMap<>(); public DefaultMutableWorldState( final WorldStateStorage storage, final WorldStatePreimageStorage preimageStorage) { @@ -133,10 +134,16 @@ public class DefaultMutableWorldState implements MutableWorldState { } @Override - public Stream streamAccounts() { - // TODO: the current trie implementation doesn't have walking capability yet (pending NC-746) - // so this can't be implemented. - throw new UnsupportedOperationException("TODO"); + public Stream streamAccounts(final Bytes32 startKeyHash, final int limit) { + return accountStateTrie.entriesFrom(startKeyHash, limit).entrySet().stream() + .map( + entry -> + deserializeAccount( + // TODO: Account address should be an {@code Optional} rather than defaulting to + // ZERO + getAccountTrieKeyPreimage(entry.getKey()).orElse(Address.ZERO), + Hash.wrap(entry.getKey()), + entry.getValue())); } @Override @@ -172,6 +179,7 @@ public class DefaultMutableWorldState implements MutableWorldState { // Persist preimages final WorldStatePreimageStorage.Updater preimageUpdater = preimageStorage.updater(); newStorageKeyPreimages.forEach(preimageUpdater::putStorageTrieKeyPreimage); + newAccountKeyPreimages.forEach(preimageUpdater::putAccountTrieKeyPreimage); // Clear pending changes that we just flushed updatedStorageTries.clear(); @@ -188,6 +196,11 @@ public class DefaultMutableWorldState implements MutableWorldState { .or(() -> preimageStorage.getStorageTrieKeyPreimage(trieKey)); } + private Optional
getAccountTrieKeyPreimage(final Bytes32 trieKey) { + return Optional.ofNullable(newAccountKeyPreimages.get(trieKey)) + .or(() -> preimageStorage.getAccountTrieKeyPreimage(trieKey)); + } + // An immutable class that represents an individual account as stored in // in the world state's underlying merkle patricia trie. protected class AccountState implements Account { @@ -399,6 +412,8 @@ public class DefaultMutableWorldState implements MutableWorldState { storageRoot = Hash.wrap(storageTrie.getRootHash()); } + // Save address preimage + wrapped.newAccountKeyPreimages.put(updated.getAddressHash(), updated.getAddress()); // Lastly, save the new account. final BytesValue account = serializeAccount( diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStatePreimageStorage.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStatePreimageStorage.java index f0360f1721..b0303ad215 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStatePreimageStorage.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/WorldStatePreimageStorage.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.worldstate; +import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.util.bytes.Bytes32; import tech.pegasys.pantheon.util.uint.UInt256; @@ -21,12 +22,16 @@ public interface WorldStatePreimageStorage { Optional getStorageTrieKeyPreimage(Bytes32 trieKey); + Optional
getAccountTrieKeyPreimage(Bytes32 trieKey); + Updater updater(); interface Updater { Updater putStorageTrieKeyPreimage(Bytes32 trieKey, UInt256 preimage); + Updater putAccountTrieKeyPreimage(Bytes32 trieKey, Address preimage); + void commit(); void rollback(); diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java index 684d33a3fc..ba7fc250f9 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java @@ -25,7 +25,7 @@ import tech.pegasys.pantheon.ethereum.core.WorldUpdater; import tech.pegasys.pantheon.ethereum.mainnet.TransactionProcessor; import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidationParams; import tech.pegasys.pantheon.ethereum.rlp.RLP; -import tech.pegasys.pantheon.ethereum.worldstate.DebuggableMutableWorldState; +import tech.pegasys.pantheon.ethereum.worldstate.DefaultMutableWorldState; import tech.pegasys.pantheon.testutil.JsonTestParameters; import java.util.Arrays; @@ -98,7 +98,7 @@ public class GeneralStateReferenceTestTools { final WorldState initialWorldState = spec.initialWorldState(); final Transaction transaction = spec.transaction(); - final MutableWorldState worldState = new DebuggableMutableWorldState(initialWorldState); + final MutableWorldState worldState = new DefaultMutableWorldState(initialWorldState); // Several of the GeneralStateTests check if the transaction could potentially // consume more gas than is left for the block it's attempted to be included in. // This check is performed within the `BlockImporter` rather than inside the diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/WorldStateMock.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/WorldStateMock.java index 0ecbcc2299..6a0d5c2c69 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/WorldStateMock.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/WorldStateMock.java @@ -16,7 +16,10 @@ import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.MutableAccount; import tech.pegasys.pantheon.ethereum.core.Wei; import tech.pegasys.pantheon.ethereum.core.WorldUpdater; -import tech.pegasys.pantheon.ethereum.worldstate.DebuggableMutableWorldState; +import tech.pegasys.pantheon.ethereum.storage.keyvalue.WorldStateKeyValueStorage; +import tech.pegasys.pantheon.ethereum.storage.keyvalue.WorldStatePreimageKeyValueStorage; +import tech.pegasys.pantheon.ethereum.worldstate.DefaultMutableWorldState; +import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.uint.UInt256; @@ -27,7 +30,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; /** Represent a mock worldState for testing. */ -public class WorldStateMock extends DebuggableMutableWorldState { +public class WorldStateMock extends DefaultMutableWorldState { public static class AccountMock { private final long nonce; @@ -108,6 +111,8 @@ public class WorldStateMock extends DebuggableMutableWorldState { } private WorldStateMock() { - super(); + super( + new WorldStateKeyValueStorage(new InMemoryKeyValueStorage()), + new WorldStatePreimageKeyValueStorage(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 ac8f6f82ea..2916e3ec8c 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 @@ -41,6 +41,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.Test; @@ -162,6 +164,80 @@ public class DefaultMutableWorldStateTest { assertEquals(MerklePatriciaTrie.EMPTY_TRIE_NODE_HASH, worldState.rootHash()); } + @Test + public void streamAccounts_empty() { + final MutableWorldState worldState = createEmpty(); + final Stream accounts = worldState.streamAccounts(Bytes32.ZERO, 10); + assertThat(accounts.count()).isEqualTo(0L); + } + + @Test + public void streamAccounts_singleAccount() { + final MutableWorldState worldState = createEmpty(); + final WorldUpdater updater = worldState.updater(); + updater.createAccount(ADDRESS).setBalance(Wei.of(100000)); + updater.commit(); + + List accounts = + worldState.streamAccounts(Bytes32.ZERO, 10).collect(Collectors.toList()); + assertThat(accounts.size()).isEqualTo(1L); + assertThat(accounts.get(0).getAddress()).isEqualTo(ADDRESS); + assertThat(accounts.get(0).getBalance()).isEqualTo(Wei.of(100000)); + + // Check again after persisting + worldState.persist(); + accounts = worldState.streamAccounts(Bytes32.ZERO, 10).collect(Collectors.toList()); + assertThat(accounts.size()).isEqualTo(1L); + assertThat(accounts.get(0).getAddress()).isEqualTo(ADDRESS); + assertThat(accounts.get(0).getBalance()).isEqualTo(Wei.of(100000)); + } + + @Test + public void streamAccounts_multipleAccounts() { + final Address addr1 = Address.fromHexString("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b"); + final Address addr2 = Address.fromHexString("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0c"); + + final MutableWorldState worldState = createEmpty(); + final WorldUpdater updater = worldState.updater(); + + // Create an account + final MutableAccount accountA = updater.createAccount(addr1); + accountA.setBalance(Wei.of(100000)); + // Create another + final MutableAccount accountB = updater.createAccount(addr2); + accountB.setNonce(1); + // Commit changes + updater.commit(); + + final boolean accountAIsFirst = + accountA.getAddressHash().compareTo(accountB.getAddressHash()) < 0; + final Hash startHash = accountAIsFirst ? accountA.getAddressHash() : accountB.getAddressHash(); + + // Get first account + final List firstAccount = + worldState.streamAccounts(startHash, 1).collect(Collectors.toList()); + assertThat(firstAccount.size()).isEqualTo(1L); + assertThat(firstAccount.get(0).getAddress()) + .isEqualTo(accountAIsFirst ? accountA.getAddress() : accountB.getAddress()); + + // Get both accounts + final List allAccounts = + worldState.streamAccounts(Bytes32.ZERO, 2).collect(Collectors.toList()); + assertThat(allAccounts.size()).isEqualTo(2L); + assertThat(allAccounts.get(0).getAddress()) + .isEqualTo(accountAIsFirst ? accountA.getAddress() : accountB.getAddress()); + assertThat(allAccounts.get(1).getAddress()) + .isEqualTo(accountAIsFirst ? accountB.getAddress() : accountA.getAddress()); + + // Get second account + final Bytes32 startHashForSecondAccount = startHash.asUInt256().plus(1L).getBytes(); + final List secondAccount = + worldState.streamAccounts(startHashForSecondAccount, 100).collect(Collectors.toList()); + assertThat(secondAccount.size()).isEqualTo(1L); + assertThat(secondAccount.get(0).getAddress()) + .isEqualTo(accountAIsFirst ? accountB.getAddress() : accountA.getAddress()); + } + @Test public void commitAndPersist() { final KeyValueStorage storage = new InMemoryKeyValueStorage();