Ensured that the blockchain queries class handles optionals better. (#486)

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

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

@@ -28,6 +28,7 @@ import tech.pegasys.pantheon.util.bytes.Bytes32;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
 import java.util.NavigableMap;
+import java.util.Optional;
 
 public class DebugStorageRangeAt implements JsonRpcMethod {
 
@@ -57,15 +58,18 @@ public class DebugStorageRangeAt implements JsonRpcMethod {
     final Hash startKey = parameters.required(request.getParams(), 3, Hash.class);
     final int limit = parameters.required(request.getParams(), 4, Integer.class);
 
-    final TransactionWithMetadata transactionWithMetadata =
+    final Optional<TransactionWithMetadata> optional =
         blockchainQueries.transactionByBlockHashAndIndex(blockHash, transactionIndex);
-
+    return optional
-    return blockReplay
+        .map(
-        .afterTransactionInBlock(
+            transactionWithMetadata ->
-            blockHash,
+                (blockReplay
-            transactionWithMetadata.getTransaction().hash(),
+                    .afterTransactionInBlock(
-            (transaction, blockHeader, blockchain, worldState, transactionProcessor) ->
+                        blockHash,
-                extractStorageAt(request, accountAddress, startKey, limit, worldState))
+                        transactionWithMetadata.getTransaction().hash(),
+                        (transaction, blockHeader, blockchain, worldState, transactionProcessor) ->
+                            extractStorageAt(request, accountAddress, startKey, limit, worldState))
+                    .orElseGet(() -> new JsonRpcSuccessResponse(request.getId(), null))))
         .orElseGet(() -> new JsonRpcSuccessResponse(request.getId(), null));
   }
 

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

@@ -23,6 +23,8 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessRe
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.results.TransactionCompleteResult;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.results.TransactionResult;
 
+import java.util.Optional;
+
 public class EthGetTransactionByBlockHashAndIndex implements JsonRpcMethod {
 
   private final BlockchainQueries blockchain;
@@ -44,13 +46,10 @@ public class EthGetTransactionByBlockHashAndIndex implements JsonRpcMethod {
     final Hash hash = parameters.required(request.getParams(), 0, Hash.class);
     final int index =
         parameters.required(request.getParams(), 1, UnsignedIntParameter.class).getValue();
-    final TransactionWithMetadata transactionWithMetadata =
+    final Optional<TransactionWithMetadata> transactionWithMetadata =
         blockchain.transactionByBlockHashAndIndex(hash, index);
     final TransactionResult result =
-        transactionWithMetadata == null
+        transactionWithMetadata.map(TransactionCompleteResult::new).orElse(null);
-            ? null
-            : new TransactionCompleteResult(transactionWithMetadata);
-
     return new JsonRpcSuccessResponse(request.getId(), result);
   }
 }

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

@@ -20,6 +20,8 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.queries.BlockchainQueries
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.queries.TransactionWithMetadata;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.results.TransactionCompleteResult;
 
+import java.util.Optional;
+
 public class EthGetTransactionByBlockNumberAndIndex extends AbstractBlockParameterMethod {
 
   public EthGetTransactionByBlockNumberAndIndex(
@@ -41,10 +43,8 @@ public class EthGetTransactionByBlockNumberAndIndex extends AbstractBlockParamet
   protected Object resultByBlockNumber(final JsonRpcRequest request, final long blockNumber) {
     final int index =
         parameters().required(request.getParams(), 1, UnsignedIntParameter.class).getValue();
-    final TransactionWithMetadata transactionWithMetadata =
+    final Optional<TransactionWithMetadata> transactionWithMetadata =
         blockchainQueries().transactionByBlockNumberAndIndex(blockNumber, index);
-    return transactionWithMetadata == null
+    return transactionWithMetadata.map(TransactionCompleteResult::new).orElse(null);
-        ? null
-        : new TransactionCompleteResult(transactionWithMetadata);
   }
 }

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/queries/BlockchainQueries.java

@@ -408,11 +408,13 @@ public class BlockchainQueries {
    * @param txIndex The index of the transaction to return.
    * @return The transaction at the specified location.
    */
-  public TransactionWithMetadata transactionByBlockNumberAndIndex(
+  public Optional<TransactionWithMetadata> transactionByBlockNumberAndIndex(
       final long blockNumber, final int txIndex) {
     checkArgument(txIndex >= 0);
-    final BlockHeader header = blockchain.getBlockHeader(blockNumber).get();
+    return blockchain
-    return transactionByHeaderAndIndex(header, txIndex);
+        .getBlockHeader(blockNumber)
+        .map(header -> Optional.ofNullable(transactionByHeaderAndIndex(header, txIndex)))
+        .orElse(Optional.empty());
   }
 
   /**
@@ -422,11 +424,13 @@ public class BlockchainQueries {
    * @param txIndex The index of the transaction to return.
    * @return The transaction at the specified location.
    */
-  public TransactionWithMetadata transactionByBlockHashAndIndex(
+  public Optional<TransactionWithMetadata> transactionByBlockHashAndIndex(
       final Hash blockHeaderHash, final int txIndex) {
     checkArgument(txIndex >= 0);
-    final BlockHeader header = blockchain.getBlockHeader(blockHeaderHash).get();
+    return blockchain
-    return transactionByHeaderAndIndex(header, txIndex);
+        .getBlockHeader(blockHeaderHash)
+        .map(header -> Optional.ofNullable(transactionByHeaderAndIndex(header, txIndex)))
+        .orElse(Optional.empty());
   }
 
   /**
@@ -528,10 +532,14 @@ public class BlockchainQueries {
 
   public List<LogWithMetadata> matchingLogs(final Hash blockhash, final LogsQuery query) {
     final List<LogWithMetadata> matchingLogs = Lists.newArrayList();
+    Optional<BlockHeader> blockHeader = blockchain.getBlockHeader(blockhash);
+    if (!blockHeader.isPresent()) {
+      return matchingLogs;
+    }
     final List<TransactionReceipt> receipts = blockchain.getTxReceipts(blockhash).get();
     final List<Transaction> transaction =
         blockchain.getBlockBody(blockhash).get().getTransactions();
-    final long number = blockchain.getBlockHeader(blockhash).get().getNumber();
+    final long number = blockHeader.get().getNumber();
     final boolean logHasBeenRemoved = !blockchain.blockIsOnCanonicalChain(blockhash);
     return generateLogWithMetadata(
         receipts, number, query, blockhash, matchingLogs, transaction, logHasBeenRemoved);

ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/EthJsonRpcHttpBySpecTest.java

@@ -94,6 +94,7 @@ public class EthJsonRpcHttpBySpecTest extends AbstractEthJsonRpcHttpServiceTest
 
     specs.put(EthGetLogs.class, "eth_getLogs_invalidInput");
     specs.put(EthGetLogs.class, "eth_getLogs_blockhash");
+    specs.put(EthGetLogs.class, "eth_getLogs_blockhash_missingBlockHash");
     specs.put(EthGetLogs.class, "eth_getLogs_toBlockOutOfRange");
     specs.put(EthGetLogs.class, "eth_getLogs_fromBlockExceedToBlock");
     specs.put(EthGetLogs.class, "eth_getLogs_nullParam");

ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/DebugStorageRangeAtTest.java

@@ -93,7 +93,7 @@ public class DebugStorageRangeAtTest {
             });
 
     when(blockchainQueries.transactionByBlockHashAndIndex(blockHash, TRANSACTION_INDEX))
-        .thenReturn(transactionWithMetadata);
+        .thenReturn(Optional.of(transactionWithMetadata));
     when(worldState.get(accountAddress)).thenReturn(account);
     when(blockReplay.afterTransactionInBlock(eq(blockHash), eq(transactionHash), any()))
         .thenAnswer(this::callAction);

ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthGetTransactionByBlockHashAndIndexTest.java

@@ -0,0 +1,51 @@
+/*
+ * 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.jsonrpc.internal.methods;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import tech.pegasys.pantheon.crypto.Hash;
+import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest;
+import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.JsonRpcParameter;
+import tech.pegasys.pantheon.ethereum.jsonrpc.internal.queries.BlockchainQueries;
+import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse;
+import tech.pegasys.pantheon.ethereum.jsonrpc.internal.results.Quantity;
+import tech.pegasys.pantheon.util.bytes.Bytes32;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class EthGetTransactionByBlockHashAndIndexTest {
+  private EthGetTransactionByBlockHashAndIndex method;
+  @Mock private BlockchainQueries blockchain;
+
+  @Test
+  public void shouldReturnNullWhenBlockHashDoesNotExist() {
+    method = new EthGetTransactionByBlockHashAndIndex(blockchain, new JsonRpcParameter());
+    Bytes32 hash = Hash.keccak256(BytesValue.wrap("horse".getBytes(UTF_8)));
+    JsonRpcSuccessResponse response = (JsonRpcSuccessResponse) method.response(request(hash, 1));
+    assertThat(response.getResult()).isEqualTo(null);
+  }
+
+  private JsonRpcRequest request(final Bytes32 hash, final long index) {
+    return new JsonRpcRequest(
+        "2.0",
+        "eth_getTransactionByBlockHashAndIndex",
+        new Object[] {String.valueOf(hash), Quantity.create(index)});
+  }
+}

ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/queries/BlockchainQueriesTest.java

@@ -66,6 +66,16 @@ public class BlockchainQueriesTest {
     assertBlockMatchesResult(targetBlock, result);
   }
 
+  @Test
+  public void transactionByBlockHashAndIndexForInvalidHash() {
+    final BlockchainWithData data = setupBlockchain(2);
+    final BlockchainQueries queries = data.blockchainQueries;
+
+    final Optional<TransactionWithMetadata> transactionWithMetadata =
+        queries.transactionByBlockHashAndIndex(gen.hash(), 1);
+    assertFalse(transactionWithMetadata.isPresent());
+  }
+
   @Test
   public void getBlockByHashForInvalidHash() {
     final BlockchainWithData data = setupBlockchain(2);
@@ -332,6 +342,14 @@ public class BlockchainQueriesTest {
     assertThat(logs).allMatch(LogWithMetadata::isRemoved);
   }
 
+  @Test
+  public void matchingLogsShouldReturnAnEmptyListWhenGivenAnInvalidBlockHash() {
+    final BlockchainWithData data = setupBlockchain(3);
+    final BlockchainQueries queries = data.blockchainQueries;
+    List<LogWithMetadata> logs = queries.matchingLogs(Hash.ZERO, new LogsQuery.Builder().build());
+    assertThat(logs).isEmpty();
+  }
+
   @Test
   public void getOmmerByBlockHashAndIndexShouldReturnEmptyWhenBlockDoesNotExist() {
     final BlockchainWithData data = setupBlockchain(3);

ethereum/jsonrpc/src/test/resources/tech/pegasys/pantheon/ethereum/jsonrpc/eth_getLogs_blockhash_missingBlockHash.json

@@ -0,0 +1,18 @@
+{
+  "request": {
+    "id": 406,
+    "jsonrpc": "2.0",
+    "method": "eth_getLogs",
+    "params": [{
+      "address": [],
+      "topics": [["0x000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b", "0x65c9ac8011e286e89d02a269890f41d67ca2cc597b2c76c7c69321ff492be580"]],
+      "blockhash": "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    }]
+  },
+  "response": {
+    "jsonrpc": "2.0",
+    "id": 406,
+    "result" : [ ]
+  },
+  "statusCode": 200
+}