Make explicit that streamed accounts may be missing their address (#1828)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AbstractWorldUpdater.java

@@ -234,6 +234,11 @@ public abstract class AbstractWorldUpdater<W extends WorldView, A extends Accoun
       return address;
     }
 
+    @Override
+    public Hash getAddressHash() {
+      return Hash.hash(getAddress());
+    }
+
     @Override
     public long getNonce() {
       return nonce;

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Account.java

@@ -12,146 +12,23 @@
  */
 package tech.pegasys.pantheon.ethereum.core;
 
-import tech.pegasys.pantheon.util.bytes.Bytes32;
-import tech.pegasys.pantheon.util.bytes.BytesValue;
-import tech.pegasys.pantheon.util.uint.UInt256;
-
-import java.util.NavigableMap;
-
 /**
  * A world state account.
  *
- * <p>An account has four properties associated with it:
- *
- * <ul>
- *   <li><b>Nonce:</b> the number of transactions sent (and committed) from the account.
- *   <li><b>Balance:</b> the amount of Wei (10^18 Ether) owned by the account.
- *   <li><b>Storage:</b> a key-value mapping between 256-bit integer values. Note that formally, a
- *       key always has an associated value in that mapping, with 0 being the default (and thus, in
- *       practice, only non-zero mappings are stored and setting a key to the value 0 is akin to
- *       "removing" that key).
- *   <li><b>Code:</b> arbitrary-length sequence of bytes that corresponds to EVM bytecode.
- *   <li><b>Version:</b> the version of the EVM bytecode.
- * </ul>
+ * <p>In addition to holding the account state, a full account provides access to the account
+ * address, which is not stored directly in the world state trie (account's are indexed by the hash
+ * of their address).
  */
-public interface Account {
+public interface Account extends AccountState {
 
   long DEFAULT_NONCE = 0L;
   Wei DEFAULT_BALANCE = Wei.ZERO;
   int DEFAULT_VERSION = 0;
 
-  /**
-   * The Keccak-256 hash of the account address.
-   *
-   * <p>Note that the world state does not store account addresses, only their hashes, and so this
-   * is how account are truly identified. So while accounts can be queried by their address (through
-   * first computing their hash), one cannot infer the address from an account simply from the world
-   * state.
-   *
-   * @return the Keccak-256 hash of the account address.
-   */
-  default Hash getAddressHash() {
-    return Hash.hash(getAddress());
-  }
-
   /**
    * The account address.
    *
    * @return the account address
    */
   Address getAddress();
-
-  /**
-   * The account nonce, that is the number of transactions sent from that account.
-   *
-   * @return the account nonce.
-   */
-  long getNonce();
-
-  /**
-   * The available balance of that account.
-   *
-   * @return the balance, in Wei, of the account.
-   */
-  Wei getBalance();
-
-  /**
-   * The EVM bytecode associated with this account.
-   *
-   * @return the account code (which can be empty).
-   */
-  BytesValue getCode();
-
-  /**
-   * The hash of the EVM bytecode associated with this account.
-   *
-   * @return the hash of the account code (which may be {@link Hash#EMPTY}).
-   */
-  Hash getCodeHash();
-
-  /**
-   * Whether the account has (non empty) EVM bytecode associated to it.
-   *
-   * <p>This is functionally equivalent to {@code !code().isEmpty()}, though could be implemented
-   * more efficiently.
-   *
-   * @return Whether the account has EVM bytecode associated to it.
-   */
-  default boolean hasCode() {
-    return !getCode().isEmpty();
-  }
-
-  /**
-   * The version of the EVM bytecode associated with this account.
-   *
-   * @return the version of the account code. Default is zero.
-   */
-  int getVersion();
-
-  /**
-   * Retrieves a value in the account storage given its key.
-   *
-   * @param key the key to retrieve in the account storage.
-   * @return the value associated to {@code key} in the account storage. Note that this is never
-   *     {@code null}, but 0 acts as a default value.
-   */
-  UInt256 getStorageValue(UInt256 key);
-
-  /**
-   * Retrieves the original value from before the current transaction in the account storage given
-   * its key.
-   *
-   * @param key the key to retrieve in the account storage.
-   * @return the original value associated to {@code key} in the account storage. Note that this is
-   *     never {@code null}, but 0 acts as a default value.
-   */
-  UInt256 getOriginalStorageValue(UInt256 key);
-
-  /**
-   * Whether the account is "empty".
-   *
-   * <p>An account is defined as empty if:
-   *
-   * <ul>
-   *   <li>its nonce is 0;
-   *   <li>its balance is 0;
-   *   <li>its associated code is empty (the zero-length byte sequence).
-   * </ul>
-   *
-   * @return {@code true} if the account is empty with regard to the definition above, {@code false}
-   *     otherwise.
-   */
-  default boolean isEmpty() {
-    return getNonce() == 0 && getBalance().isZero() && !hasCode();
-  }
-
-  /**
-   * Retrieve up to {@code limit} storage entries beginning from the first entry with hash equal to
-   * or greater than {@code startKeyHash}.
-   *
-   * @param startKeyHash the first key hash to return.
-   * @param limit the maximum number of entries to return.
-   * @return the requested storage entries as a map of key hash to entry.
-   */
-  NavigableMap<Bytes32, AccountStorageEntry> storageEntriesFrom(Bytes32 startKeyHash, int limit);
 }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/AccountState.java

@@ -0,0 +1,144 @@
+/*
+ * 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.core;
+
+import tech.pegasys.pantheon.util.bytes.Bytes32;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
+import tech.pegasys.pantheon.util.uint.UInt256;
+
+import java.util.NavigableMap;
+
+/**
+ * An account state.
+ *
+ * <p>An account has four properties associated with it:
+ *
+ * <ul>
+ *   <li><b>Nonce:</b> the number of transactions sent (and committed) from the account.
+ *   <li><b>Balance:</b> the amount of Wei (10^18 Ether) owned by the account.
+ *   <li><b>Storage:</b> a key-value mapping between 256-bit integer values. Note that formally, a
+ *       key always has an associated value in that mapping, with 0 being the default (and thus, in
+ *       practice, only non-zero mappings are stored and setting a key to the value 0 is akin to
+ *       "removing" that key).
+ *   <li><b>Code:</b> arbitrary-length sequence of bytes that corresponds to EVM bytecode.
+ *   <li><b>Version:</b> the version of the EVM bytecode.
+ * </ul>
+ */
+public interface AccountState {
+
+  /**
+   * The Keccak-256 hash of the account address.
+   *
+   * <p>Note that the world state does not store account addresses, only their hashes, and so this
+   * is how account are truly identified. So while accounts can be queried by their address (through
+   * first computing their hash), one cannot infer the address from an account simply from the world
+   * state.
+   *
+   * @return the Keccak-256 hash of the account address.
+   */
+  Hash getAddressHash();
+
+  /**
+   * The account nonce, that is the number of transactions sent from that account.
+   *
+   * @return the account nonce.
+   */
+  long getNonce();
+
+  /**
+   * The available balance of that account.
+   *
+   * @return the balance, in Wei, of the account.
+   */
+  Wei getBalance();
+
+  /**
+   * The EVM bytecode associated with this account.
+   *
+   * @return the account code (which can be empty).
+   */
+  BytesValue getCode();
+
+  /**
+   * The hash of the EVM bytecode associated with this account.
+   *
+   * @return the hash of the account code (which may be {@link Hash#EMPTY}).
+   */
+  Hash getCodeHash();
+
+  /**
+   * Whether the account has (non empty) EVM bytecode associated to it.
+   *
+   * <p>This is functionally equivalent to {@code !code().isEmpty()}, though could be implemented
+   * more efficiently.
+   *
+   * @return Whether the account has EVM bytecode associated to it.
+   */
+  default boolean hasCode() {
+    return !getCode().isEmpty();
+  }
+
+  /**
+   * The version of the EVM bytecode associated with this account.
+   *
+   * @return the version of the account code. Default is zero.
+   */
+  int getVersion();
+
+  /**
+   * Retrieves a value in the account storage given its key.
+   *
+   * @param key the key to retrieve in the account storage.
+   * @return the value associated to {@code key} in the account storage. Note that this is never
+   *     {@code null}, but 0 acts as a default value.
+   */
+  UInt256 getStorageValue(UInt256 key);
+
+  /**
+   * Retrieves the original value from before the current transaction in the account storage given
+   * its key.
+   *
+   * @param key the key to retrieve in the account storage.
+   * @return the original value associated to {@code key} in the account storage. Note that this is
+   *     never {@code null}, but 0 acts as a default value.
+   */
+  UInt256 getOriginalStorageValue(UInt256 key);
+
+  /**
+   * Whether the account is "empty".
+   *
+   * <p>An account is defined as empty if:
+   *
+   * <ul>
+   *   <li>its nonce is 0;
+   *   <li>its balance is 0;
+   *   <li>its associated code is empty (the zero-length byte sequence).
+   * </ul>
+   *
+   * @return {@code true} if the account is empty with regard to the definition above, {@code false}
+   *     otherwise.
+   */
+  default boolean isEmpty() {
+    return getNonce() == 0 && getBalance().isZero() && !hasCode();
+  }
+
+  /**
+   * Retrieve up to {@code limit} storage entries beginning from the first entry with hash equal to
+   * or greater than {@code startKeyHash}.
+   *
+   * @param startKeyHash the first key hash to return.
+   * @param limit the maximum number of entries to return.
+   * @return the requested storage entries as a map of key hash to entry.
+   */
+  NavigableMap<Bytes32, AccountStorageEntry> storageEntriesFrom(Bytes32 startKeyHash, int limit);
+}

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/WorldState.java

@@ -13,7 +13,11 @@
 package tech.pegasys.pantheon.ethereum.core;
 
 import tech.pegasys.pantheon.util.bytes.Bytes32;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
+import tech.pegasys.pantheon.util.uint.UInt256;
 
+import java.util.NavigableMap;
+import java.util.Optional;
 import java.util.stream.Stream;
 
 /**
@@ -41,5 +45,65 @@ public interface WorldState extends WorldView {
    * @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<Account> streamAccounts(Bytes32 startKeyHash, int limit);
+  Stream<StreamableAccount> streamAccounts(Bytes32 startKeyHash, int limit);
+
+  class StreamableAccount implements AccountState {
+    private final Optional<Address> address;
+    private final AccountState accountState;
+
+    public StreamableAccount(final Optional<Address> address, final AccountState accountState) {
+      this.address = address;
+      this.accountState = accountState;
+    }
+
+    public Optional<Address> getAddress() {
+      return address;
+    }
+
+    @Override
+    public Hash getAddressHash() {
+      return accountState.getAddressHash();
+    }
+
+    @Override
+    public long getNonce() {
+      return accountState.getNonce();
+    }
+
+    @Override
+    public Wei getBalance() {
+      return accountState.getBalance();
+    }
+
+    @Override
+    public BytesValue getCode() {
+      return accountState.getCode();
+    }
+
+    @Override
+    public Hash getCodeHash() {
+      return accountState.getCodeHash();
+    }
+
+    @Override
+    public int getVersion() {
+      return accountState.getVersion();
+    }
+
+    @Override
+    public UInt256 getStorageValue(final UInt256 key) {
+      return accountState.getStorageValue(key);
+    }
+
+    @Override
+    public UInt256 getOriginalStorageValue(final UInt256 key) {
+      return accountState.getOriginalStorageValue(key);
+    }
+
+    @Override
+    public NavigableMap<Bytes32, AccountStorageEntry> storageEntriesFrom(
+        final Bytes32 startKeyHash, final int limit) {
+      return accountState.storageEntriesFrom(startKeyHash, limit);
+    }
+  }
 }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldState.java

@@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.worldstate;
 
 import tech.pegasys.pantheon.ethereum.core.AbstractWorldUpdater;
 import tech.pegasys.pantheon.ethereum.core.Account;
+import tech.pegasys.pantheon.ethereum.core.AccountState;
 import tech.pegasys.pantheon.ethereum.core.AccountStorageEntry;
 import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.Hash;
@@ -110,11 +111,11 @@ public class DefaultMutableWorldState implements MutableWorldState {
         .orElse(null);
   }
 
-  private AccountState deserializeAccount(
+  private WorldStateAccount deserializeAccount(
       final Address address, final Hash addressHash, final BytesValue encoded) throws RLPException {
     final RLPInput in = RLP.input(encoded);
     final StateTrieAccountValue accountValue = StateTrieAccountValue.readFrom(in);
-    return new AccountState(address, addressHash, accountValue);
+    return new WorldStateAccount(address, addressHash, accountValue);
   }
 
   private static BytesValue serializeAccount(
@@ -134,16 +135,16 @@ public class DefaultMutableWorldState implements MutableWorldState {
   }
 
   @Override
-  public Stream<Account> streamAccounts(final Bytes32 startKeyHash, final int limit) {
+  public Stream<StreamableAccount> 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()));
+            entry -> {
+              final Optional<Address> address = getAccountTrieKeyPreimage(entry.getKey());
+              final AccountState account =
+                  deserializeAccount(
+                      address.orElse(Address.ZERO), Hash.wrap(entry.getKey()), entry.getValue());
+              return new StreamableAccount(address, account);
+            });
   }
 
   @Override
@@ -203,7 +204,7 @@ public class DefaultMutableWorldState implements MutableWorldState {
 
   // 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 {
+  protected class WorldStateAccount implements Account {
 
     private final Address address;
     private final Hash addressHash;
@@ -213,7 +214,7 @@ public class DefaultMutableWorldState implements MutableWorldState {
     // Lazily initialized since we don't always access storage.
     private volatile MerklePatriciaTrie<Bytes32, BytesValue> storageTrie;
 
-    private AccountState(
+    private WorldStateAccount(
         final Address address, final Hash addressHash, final StateTrieAccountValue accountValue) {
 
       this.address = address;
@@ -337,14 +338,14 @@ public class DefaultMutableWorldState implements MutableWorldState {
   }
 
   protected static class Updater
-      extends AbstractWorldUpdater<DefaultMutableWorldState, AccountState> {
+      extends AbstractWorldUpdater<DefaultMutableWorldState, WorldStateAccount> {
 
     protected Updater(final DefaultMutableWorldState world) {
       super(world);
     }
 
     @Override
-    protected AccountState getForMutation(final Address address) {
+    protected WorldStateAccount getForMutation(final Address address) {
       final DefaultMutableWorldState wrapped = wrappedWorldView();
       final Hash addressHash = Hash.hash(address);
       return wrapped
@@ -376,8 +377,8 @@ public class DefaultMutableWorldState implements MutableWorldState {
         wrapped.updatedAccountCode.remove(address);
       }
 
-      for (final UpdateTrackingAccount<AccountState> updated : updatedAccounts()) {
-        final AccountState origin = updated.getWrappedAccount();
+      for (final UpdateTrackingAccount<WorldStateAccount> updated : updatedAccounts()) {
+        final WorldStateAccount origin = updated.getWrappedAccount();
 
         // Save the code in key-value storage ...
         Hash codeHash = origin == null ? Hash.EMPTY : origin.getCodeHash();

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/worldstate/DefaultMutableWorldStateTest.java

@@ -27,6 +27,7 @@ import tech.pegasys.pantheon.ethereum.core.MutableAccount;
 import tech.pegasys.pantheon.ethereum.core.MutableWorldState;
 import tech.pegasys.pantheon.ethereum.core.Wei;
 import tech.pegasys.pantheon.ethereum.core.WorldState;
+import tech.pegasys.pantheon.ethereum.core.WorldState.StreamableAccount;
 import tech.pegasys.pantheon.ethereum.core.WorldUpdater;
 import tech.pegasys.pantheon.ethereum.storage.keyvalue.WorldStateKeyValueStorage;
 import tech.pegasys.pantheon.ethereum.storage.keyvalue.WorldStatePreimageKeyValueStorage;
@@ -167,7 +168,7 @@ public class DefaultMutableWorldStateTest {
   @Test
   public void streamAccounts_empty() {
     final MutableWorldState worldState = createEmpty();
-    final Stream<Account> accounts = worldState.streamAccounts(Bytes32.ZERO, 10);
+    final Stream<StreamableAccount> accounts = worldState.streamAccounts(Bytes32.ZERO, 10);
     assertThat(accounts.count()).isEqualTo(0L);
   }
 
@@ -178,17 +179,17 @@ public class DefaultMutableWorldStateTest {
     updater.createAccount(ADDRESS).setBalance(Wei.of(100000));
     updater.commit();
 
-    List<Account> accounts =
+    List<StreamableAccount> 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).getAddress()).hasValue(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).getAddress()).hasValue(ADDRESS);
     assertThat(accounts.get(0).getBalance()).isEqualTo(Wei.of(100000));
   }
 
@@ -214,28 +215,28 @@ public class DefaultMutableWorldStateTest {
     final Hash startHash = accountAIsFirst ? accountA.getAddressHash() : accountB.getAddressHash();
 
     // Get first account
-    final List<Account> firstAccount =
+    final List<StreamableAccount> firstAccount =
         worldState.streamAccounts(startHash, 1).collect(Collectors.toList());
     assertThat(firstAccount.size()).isEqualTo(1L);
     assertThat(firstAccount.get(0).getAddress())
-        .isEqualTo(accountAIsFirst ? accountA.getAddress() : accountB.getAddress());
+        .hasValue(accountAIsFirst ? accountA.getAddress() : accountB.getAddress());
 
     // Get both accounts
-    final List<Account> allAccounts =
+    final List<StreamableAccount> 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());
+        .hasValue(accountAIsFirst ? accountA.getAddress() : accountB.getAddress());
     assertThat(allAccounts.get(1).getAddress())
-        .isEqualTo(accountAIsFirst ? accountB.getAddress() : accountA.getAddress());
+        .hasValue(accountAIsFirst ? accountB.getAddress() : accountA.getAddress());
 
     // Get second account
     final Bytes32 startHashForSecondAccount = startHash.asUInt256().plus(1L).getBytes();
-    final List<Account> secondAccount =
+    final List<StreamableAccount> secondAccount =
         worldState.streamAccounts(startHashForSecondAccount, 100).collect(Collectors.toList());
     assertThat(secondAccount.size()).isEqualTo(1L);
     assertThat(secondAccount.get(0).getAddress())
-        .isEqualTo(accountAIsFirst ? accountB.getAddress() : accountA.getAddress());
+        .hasValue(accountAIsFirst ? accountB.getAddress() : accountA.getAddress());
   }
 
   @Test

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/DebugAccountRange.java

@@ -12,10 +12,11 @@
  */
 package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods;
 
-import tech.pegasys.pantheon.ethereum.core.Account;
+import tech.pegasys.pantheon.ethereum.core.Address;
 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.StreamableAccount;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.BlockParameterOrBlockHash;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.JsonRpcParameter;
@@ -84,7 +85,7 @@ public class DebugAccountRange implements JsonRpcMethod {
     if (state.isEmpty()) {
       return emptyResponse(request);
     } else {
-      final List<Account> accounts =
+      final List<StreamableAccount> accounts =
           state
               .get()
               .streamAccounts(Bytes32.fromHexStringLenient(addressHash), maxResults + 1)
@@ -102,7 +103,7 @@ public class DebugAccountRange implements JsonRpcMethod {
                   .collect(
                       Collectors.toMap(
                           account -> account.getAddressHash().toString(),
-                          account -> account.getAddress().toString())),
+                          account -> account.getAddress().orElse(Address.ZERO).toString())),
               nextKey.toString()));
     }
   }