Provide nicer error messages when genesis config is invalid. (#1029)

PegaSysEng · Mar 4, 2019 · 18de723 · 18de723
1 parent 73f1f29
commit 18de723
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 28 deletions.
diff --git a/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigFile.java b/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigFile.java
@@ -63,7 +63,7 @@ public GenesisConfigOptions getConfigOptions() {
   }
 
   public Stream<GenesisAllocation> getAllocations() {
-    final JsonObject allocations = configRoot.getJsonObject("alloc");
+    final JsonObject allocations = configRoot.getJsonObject("alloc", new JsonObject());
     return allocations.fieldNames().stream()
         .map(key -> new GenesisAllocation(key, allocations.getJsonObject(key)));
   }
@@ -80,34 +80,47 @@ public String getExtraData() {
     return configRoot.getString("extradata", "");
   }
 
-  public Long getGasLimit() {
-    return Long.decode(getRequiredString("gaslimit"));
+  public long getGasLimit() {
+    return parseLong("gasLimit", getRequiredString("gaslimit"));
   }
 
   public String getMixHash() {
     return configRoot.getString("mixhash", "");
   }
 
   public String getNonce() {
-    return configRoot.getString("nonce", "");
+    return configRoot.getString("nonce", "0x0");
   }
 
   public Optional<String> getCoinbase() {
     return Optional.ofNullable(configRoot.getString("coinbase"));
   }
 
   public long getTimestamp() {
-    return Long.parseLong(configRoot.getString("timestamp", "0x0").substring(2), 16);
+    return parseLong("timestamp", configRoot.getString("timestamp", "0x0"));
   }
 
   private String getRequiredString(final String key) {
     if (!configRoot.containsKey(key)) {
       throw new IllegalArgumentException(
-          String.format("Invalid Genesis block configuration, missing value for '%s'", key));
+          String.format("Invalid genesis block configuration, missing value for '%s'", key));
     }
     return configRoot.getString(key);
   }
 
+  private long parseLong(final String name, final String value) {
+    try {
+      return Long.decode(value);
+    } catch (final NumberFormatException e) {
+      throw new IllegalArgumentException(
+          "Invalid genesis block configuration, "
+              + name
+              + " must be a number but was '"
+              + value
+              + "'");
+    }
+  }
+
   /* Converts the {@link JsonObject} describing the Genesis Block to a {@link Map}. This method
    * converts all nested {@link JsonObject} to {@link Map} as well. Also, note that all keys are
    * converted to lowercase for easier lookup since the keys in a 'genesis.json' file are assumed

diff --git a/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigFileTest.java b/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigFileTest.java
@@ -106,12 +106,12 @@ public void shouldDefaultMixHashToEmptyString() {
 
   @Test
   public void shouldGetNonce() {
-    assertThat(configWithProperty("nonce", "ABCD").getNonce()).isEqualTo("ABCD");
+    assertThat(configWithProperty("nonce", "0x10").getNonce()).isEqualTo("0x10");
   }
 
   @Test
-  public void shouldDefaultNonceToEmptyString() {
-    assertThat(EMPTY_CONFIG.getNonce()).isEmpty();
+  public void shouldDefaultNonceToZero() {
+    assertThat(EMPTY_CONFIG.getNonce()).isEqualTo("0x0");
   }
 
   @Test
@@ -164,13 +164,19 @@ public void shouldGetAllocations() {
             entry("f17f52151ebef6c7334fad080c5704d77216b732", "90000000000000000000000"));
   }
 
+  @Test
+  public void shouldGetEmptyAllocationsWhenAllocNotPresent() {
+    final GenesisConfigFile config = GenesisConfigFile.fromConfig("{}");
+    assertThat(config.getAllocations()).isEmpty();
+  }
+
   private GenesisConfigFile configWithProperty(final String key, final String value) {
     return GenesisConfigFile.fromConfig("{\"" + key + "\":\"" + value + "\"}");
   }
 
   private void assertInvalidConfiguration(final ThrowingCallable getter) {
     assertThatThrownBy(getter)
         .isInstanceOf(IllegalArgumentException.class)
-        .hasMessageContaining("Invalid Genesis block configuration");
+        .hasMessageContaining("Invalid genesis block configuration");
   }
 }
diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/GenesisState.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/chain/GenesisState.java
@@ -30,7 +30,6 @@
 import tech.pegasys.pantheon.ethereum.storage.keyvalue.KeyValueStorageWorldStateStorage;
 import tech.pegasys.pantheon.ethereum.worldstate.DefaultMutableWorldState;
 import tech.pegasys.pantheon.services.kvstore.InMemoryKeyValueStorage;
-import tech.pegasys.pantheon.util.bytes.Bytes32;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
@@ -40,6 +39,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -153,38 +153,58 @@ private static <C> BlockHeader buildHeader(
   private static Address parseCoinbase(final GenesisConfigFile genesis) {
     return genesis
         .getCoinbase()
-        .map(Address::fromHexString)
+        .map(str -> withNiceErrorMessage("coinbase", str, Address::fromHexString))
         .orElseGet(() -> Address.wrap(BytesValue.wrap(new byte[Address.SIZE])));
   }
 
+  private static <T> T withNiceErrorMessage(
+      final String name, final String value, final Function<String, T> parser) {
+    try {
+      return parser.apply(value);
+    } catch (final IllegalArgumentException e) {
+      throw createInvalidBlockConfigException(name, value, e);
+    }
+  }
+
+  private static IllegalArgumentException createInvalidBlockConfigException(
+      final String name, final String value, final IllegalArgumentException e) {
+    return new IllegalArgumentException(
+        "Invalid " + name + " in genesis block configuration: " + value, e);
+  }
+
   private static Hash parseParentHash(final GenesisConfigFile genesis) {
-    return Hash.wrap(Bytes32.fromHexString(genesis.getParentHash()));
+    return withNiceErrorMessage("parentHash", genesis.getParentHash(), Hash::fromHexStringLenient);
   }
 
   private static BytesValue parseExtraData(final GenesisConfigFile genesis) {
-    return BytesValue.fromHexString(genesis.getExtraData());
+    return withNiceErrorMessage("extraData", genesis.getExtraData(), BytesValue::fromHexString);
   }
 
   private static UInt256 parseDifficulty(final GenesisConfigFile genesis) {
-    return UInt256.fromHexString(genesis.getDifficulty());
+    return withNiceErrorMessage("difficulty", genesis.getDifficulty(), UInt256::fromHexString);
   }
 
   private static Hash parseMixHash(final GenesisConfigFile genesis) {
-    return Hash.wrap(Bytes32.fromHexString(genesis.getMixHash()));
-  }
-
-  private static long parseNonce(final GenesisConfigFile genesis) {
-    String nonce = genesis.getNonce().toLowerCase(Locale.US);
-    if (nonce.startsWith("0x")) {
-      nonce = nonce.substring(2);
-    }
-    return Long.parseUnsignedLong(nonce, 16);
+    return withNiceErrorMessage("mixHash", genesis.getMixHash(), Hash::fromHexStringLenient);
   }
 
   private static Stream<GenesisAccount> parseAllocations(final GenesisConfigFile genesis) {
     return genesis.getAllocations().map(GenesisAccount::fromAllocation);
   }
 
+  private static long parseNonce(final GenesisConfigFile genesis) {
+    return withNiceErrorMessage(
+        "nonce",
+        genesis.getNonce(),
+        value -> {
+          String nonce = value.toLowerCase(Locale.US);
+          if (nonce.startsWith("0x")) {
+            nonce = nonce.substring(2);
+          }
+          return Long.parseUnsignedLong(nonce, 16);
+        });
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
@@ -213,8 +233,8 @@ private GenesisAccount(
         final String balance,
         final String hexCode,
         final Map<String, Object> storage) {
-      this.address = Address.fromHexString(hexAddress);
-      this.balance = parseBalance(balance);
+      this.address = withNiceErrorMessage("address", hexAddress, Address::fromHexString);
+      this.balance = withNiceErrorMessage("balance", balance, this::parseBalance);
       this.code = hexCode != null ? BytesValue.fromHexString(hexCode) : null;
       this.storage = parseStorage(storage);
     }
@@ -234,7 +254,10 @@ private Map<UInt256, UInt256> parseStorage(final Map<String, Object> storage) {
       final Map<UInt256, UInt256> parsedStorage = new HashMap<>();
       storage.forEach(
           (key, value) ->
-              parsedStorage.put(UInt256.fromHexString(key), UInt256.fromHexString((String) value)));
+              parsedStorage.put(
+                  withNiceErrorMessage("storage key", key, UInt256::fromHexString),
+                  withNiceErrorMessage(
+                      "storage value", String.valueOf(value), UInt256::fromHexString)));
       return parsedStorage;
     }
 

diff --git a/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/chain/genesis2.json b/ethereum/core/src/test/resources/tech/pegasys/pantheon/ethereum/chain/genesis2.json
@@ -5,7 +5,6 @@
     "eip155Block": 0,
     "eip158Block": 0
   },
-  "alloc": {},
   "coinbase": "0x0000000000000000000000000000000000000000",
   "difficulty": "0x0000001",
   "extraData": "",