Fix GraphQL IBFT Coinbase Query (#1282)

When IBFT produces blocks the coinbase may correspond to an empty
account (as mining rewards are not paid out).  In this case allow
GraphQL to return an all zero account for the empty account only in
context of a miner.  Other tests exist that verify a plain accounts
query of a non-existent account continues to throws an exception.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

CHANGELOG.md

@@ -20,6 +20,8 @@ The intent is that the major Java VM version or Java VM type shipped with the de
 ### Bug Fixes
 - Offchain permissioning - fixed bug where sync status check prevented peering if static nodes configured. [\#1252](https://github.com/hyperledger/besu/issues/1252)
 
+- GraphQL queries of `miner` in IBFT networks will no longer return an error.  PR [\#1282](https://github.com/hyperledger/besu/pull/1282) issue [\#1272](https://github.com/hyperledger/besu/issues/1272).
+
 #### Previously identified known issues
  
 - [Logs queries missing results against chain head](KNOWN_ISSUES.md#Logs-queries-missing-results-against-chain-head)

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/BlockAdapterBase.java

@@ -83,7 +83,7 @@ public class BlockAdapterBase extends AdapterBase {
     return Optional.of(header.getReceiptsRoot());
   }
 
-  public Optional<AccountAdapter> getMiner(final DataFetchingEnvironment environment) {
+  public Optional<AdapterBase> getMiner(final DataFetchingEnvironment environment) {
 
     final BlockchainQueries query = getBlockchainQueries(environment);
     long blockNumber = header.getNumber();
@@ -91,8 +91,10 @@ public class BlockAdapterBase extends AdapterBase {
     if (bn != null) {
       blockNumber = bn;
     }
-    return Optional.of(
-        new AccountAdapter(query.getWorldState(blockNumber).get().get(header.getCoinbase())));
+
+    return Optional.ofNullable(query.getWorldState(blockNumber).get().get(header.getCoinbase()))
+        .map(account -> (AdapterBase) new AccountAdapter(account))
+        .or(() -> Optional.of(new EmptyAccountAdapter(header.getCoinbase())));
   }
 
   public Optional<Bytes> getExtraData() {

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/pojoadapter/EmptyAccountAdapter.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.api.graphql.internal.pojoadapter;
+
+import org.hyperledger.besu.ethereum.core.Address;
+import org.hyperledger.besu.ethereum.core.Wei;
+
+import java.util.Optional;
+
+import graphql.schema.DataFetchingEnvironment;
+import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.bytes.Bytes32;
+
+@SuppressWarnings("unused") // reflected by GraphQL
+public class EmptyAccountAdapter extends AdapterBase {
+  private final Address address;
+
+  public EmptyAccountAdapter(final Address address) {
+    this.address = address;
+  }
+
+  public Optional<Address> getAddress() {
+    return Optional.of(address);
+  }
+
+  public Optional<Wei> getBalance() {
+    return Optional.of(Wei.ZERO);
+  }
+
+  public Optional<Long> getTransactionCount() {
+    return Optional.of(0L);
+  }
+
+  public Optional<Bytes> getCode() {
+    return Optional.of(Bytes.EMPTY);
+  }
+
+  public Optional<Bytes32> getStorage(final DataFetchingEnvironment environment) {
+    return Optional.of(Bytes32.ZERO);
+  }
+}

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/AbstractDataFetcherTest.java

@@ -16,6 +16,8 @@ package org.hyperledger.besu.ethereum.api.graphql;
 
 import org.hyperledger.besu.ethereum.api.graphql.internal.pojoadapter.NormalBlockAdapter;
 import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability;
 
 import java.util.Optional;
@@ -24,15 +26,12 @@ import java.util.Set;
 import graphql.schema.DataFetcher;
 import graphql.schema.DataFetchingEnvironment;
 import org.junit.Before;
-import org.junit.Rule;
-import org.junit.rules.ExpectedException;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 
 public abstract class AbstractDataFetcherTest {
 
   DataFetcher<Optional<NormalBlockAdapter>> fetcher;
-  private GraphQLDataFetchers fetchers;
 
   @Mock protected Set<Capability> supportedCapabilities;
 
@@ -42,11 +41,13 @@ public abstract class AbstractDataFetcherTest {
 
   @Mock protected BlockchainQueries query;
 
-  @Rule public ExpectedException thrown = ExpectedException.none();
+  @Mock protected BlockHeader header;
+
+  @Mock protected MutableWorldState mutableWorldState;
 
   @Before
   public void before() {
-    fetchers = new GraphQLDataFetchers(supportedCapabilities);
+    final GraphQLDataFetchers fetchers = new GraphQLDataFetchers(supportedCapabilities);
     fetcher = fetchers.getBlockDataFetcher();
     Mockito.when(environment.getContext()).thenReturn(context);
     Mockito.when(context.getBlockchainQueries()).thenReturn(query);

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/BlockDataFetcherTest.java

@@ -14,7 +14,15 @@
  */
 package org.hyperledger.besu.ethereum.api.graphql;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.when;
+
+import org.hyperledger.besu.ethereum.api.graphql.internal.pojoadapter.EmptyAccountAdapter;
+import org.hyperledger.besu.ethereum.api.graphql.internal.pojoadapter.NormalBlockAdapter;
 import org.hyperledger.besu.ethereum.api.query.BlockWithMetadata;
+import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.Hash;
 
 import java.util.Optional;
@@ -23,7 +31,6 @@ import org.apache.tuweni.bytes.Bytes;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentMatchers;
-import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -32,24 +39,48 @@ public class BlockDataFetcherTest extends AbstractDataFetcherTest {
   @Test
   public void bothNumberAndHashThrows() throws Exception {
     final Hash fakedHash = Hash.hash(Bytes.of(1));
-    Mockito.when(environment.getArgument(ArgumentMatchers.eq("number"))).thenReturn(1L);
-    Mockito.when(environment.getArgument(ArgumentMatchers.eq("hash"))).thenReturn(fakedHash);
+    when(environment.getArgument(ArgumentMatchers.eq("number"))).thenReturn(1L);
+    when(environment.getArgument(ArgumentMatchers.eq("hash"))).thenReturn(fakedHash);
 
-    thrown.expect(GraphQLException.class);
-    fetcher.get(environment);
+    assertThatThrownBy(() -> fetcher.get(environment)).isInstanceOf(GraphQLException.class);
   }
 
   @Test
   public void onlyNumber() throws Exception {
 
-    Mockito.when(environment.getArgument(ArgumentMatchers.eq("number"))).thenReturn(1L);
-    Mockito.when(environment.getArgument(ArgumentMatchers.eq("hash"))).thenReturn(null);
+    when(environment.getArgument(ArgumentMatchers.eq("number"))).thenReturn(1L);
+    when(environment.getArgument(ArgumentMatchers.eq("hash"))).thenReturn(null);
 
-    Mockito.when(environment.getContext()).thenReturn(context);
-    Mockito.when(context.getBlockchainQueries()).thenReturn(query);
-    Mockito.when(query.blockByNumber(ArgumentMatchers.anyLong()))
+    when(environment.getContext()).thenReturn(context);
+    when(context.getBlockchainQueries()).thenReturn(query);
+    when(query.blockByNumber(ArgumentMatchers.anyLong()))
         .thenReturn(Optional.of(new BlockWithMetadata<>(null, null, null, null, 0)));
 
     fetcher.get(environment);
   }
+
+  @Test
+  public void ibftMiner() throws Exception {
+    // IBFT can mine blocks with a coinbase that is an empty account, hence not stored and returned
+    // as null. The compromise is to report zeros and empty on query from a block.
+    final Address testAddress = Address.fromHexString("0xdeadbeef");
+
+    when(environment.getArgument(ArgumentMatchers.eq("number"))).thenReturn(1L);
+    when(environment.getArgument(ArgumentMatchers.eq("hash"))).thenReturn(null);
+
+    when(environment.getContext()).thenReturn(context);
+    when(context.getBlockchainQueries()).thenReturn(query);
+    when(query.blockByNumber(ArgumentMatchers.anyLong()))
+        .thenReturn(Optional.of(new BlockWithMetadata<>(header, null, null, null, 0)));
+    when(header.getCoinbase()).thenReturn(testAddress);
+    when(query.getWorldState(anyLong())).thenReturn(Optional.of(mutableWorldState));
+
+    final Optional<NormalBlockAdapter> maybeBlock = fetcher.get(environment);
+    assertThat(maybeBlock).isPresent();
+    assertThat(maybeBlock.get().getMiner(environment)).isPresent();
+    assertThat(((EmptyAccountAdapter) maybeBlock.get().getMiner(environment).get()).getBalance())
+        .isPresent();
+    assertThat(((EmptyAccountAdapter) maybeBlock.get().getMiner(environment).get()).getAddress())
+        .contains(testAddress);
+  }
 }