Report node local address as the coinbase in Clique and IBFT. (#1758)

Matches behaviour expected of eth_coinbase call by ethstats.net reporter. Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
2026-01-09 21:17:54 -05:00 · 2019-07-25 16:41:16 +10:00
parent ec5dc85403
commit df500925cc
11 changed files with 36 additions and 33 deletions
--- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutor.java
+++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutor.java
@@ -32,6 +32,7 @@ import tech.pegasys.pantheon.util.Subscribers;
 import tech.pegasys.pantheon.util.bytes.BytesValue;

 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.ExecutorService;

 import com.google.common.annotations.VisibleForTesting;
@@ -93,6 +94,11 @@ public class CliqueMinerExecutor extends AbstractMinerExecutor<CliqueContext, Cl
    return currentRunningMiner;
  }

+  @Override
+  public Optional<Address> getCoinbase() {
+    return Optional.of(localAddress);
+  }
+
  @VisibleForTesting
  BytesValue calculateExtraData(final BlockHeader parentHeader) {
    final List<Address> validators = Lists.newArrayList();
--- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinator.java
+++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinator.java
@@ -35,7 +35,7 @@ public class CliqueMiningCoordinator

  @Override
  protected boolean newChainHeadInvalidatesMiningOperation(final BlockHeader newChainHeadHeader) {
-    if (!currentRunningMiner.isPresent()) {
+    if (currentRunningMiner.isEmpty()) {
      return true;
    }

@@ -56,9 +56,6 @@ public class CliqueMiningCoordinator
    final boolean nodeIsMining = miningTracker.canMakeBlockNextRound(parentHeader);
    final boolean nodeIsInTurn = miningTracker.isProposerAfter(parentHeader);

-    if (nodeIsMining && nodeIsInTurn) {
-      return false;
-    }
-    return true;
+    return !nodeIsMining || !nodeIsInTurn;
  }
 }
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorFactory.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorFactory.java
@@ -107,6 +107,10 @@ public class IbftBlockCreatorFactory {
    return extraData.encode();
  }

+  public Address getLocalAddress() {
+    return localAddress;
+  }
+
  private static Optional<Vote> toVote(final Optional<ValidatorVote> input) {
    return input
        .map(v -> Optional.of(new Vote(v.getRecipient(), v.getVotePolarity())))
--- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftMiningCoordinator.java
+++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftMiningCoordinator.java
@@ -21,9 +21,12 @@ import tech.pegasys.pantheon.ethereum.blockcreation.MiningCoordinator;
 import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent;
 import tech.pegasys.pantheon.ethereum.chain.BlockAddedObserver;
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
+import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.Wei;
 import tech.pegasys.pantheon.util.bytes.BytesValue;

+import java.util.Optional;
+
 import org.apache.logging.log4j.Logger;

 public class IbftMiningCoordinator implements MiningCoordinator, BlockAddedObserver {
@@ -70,6 +73,11 @@ public class IbftMiningCoordinator implements MiningCoordinator, BlockAddedObser
    blockCreatorFactory.setExtraData(extraData);
  }

+  @Override
+  public Optional<Address> getCoinbase() {
+    return Optional.of(blockCreatorFactory.getLocalAddress());
+  }
+
  @Override
  public void onBlockAdded(final BlockAddedEvent event, final Blockchain blockchain) {
    if (event.isNewCanonicalHead()) {
--- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractMinerExecutor.java
+++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractMinerExecutor.java
@@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.blockcreation;

 import tech.pegasys.pantheon.ethereum.ProtocolContext;
 import tech.pegasys.pantheon.ethereum.chain.MinedBlockObserver;
+import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.MiningParameters;
 import tech.pegasys.pantheon.ethereum.core.Wei;
@@ -22,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.util.Subscribers;
 import tech.pegasys.pantheon.util.bytes.BytesValue;

+import java.util.Optional;
 import java.util.concurrent.ExecutorService;

 public abstract class AbstractMinerExecutor<
@@ -66,4 +68,6 @@ public abstract class AbstractMinerExecutor<
  public Wei getMinTransactionGasPrice() {
    return minTransactionGasPrice;
  }
+
+  public abstract Optional<Address> getCoinbase();
 }
--- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractMiningCoordinator.java
+++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/AbstractMiningCoordinator.java
@@ -18,6 +18,7 @@ import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent;
 import tech.pegasys.pantheon.ethereum.chain.BlockAddedObserver;
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
 import tech.pegasys.pantheon.ethereum.chain.MinedBlockObserver;
+import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.Wei;
 import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
@@ -135,6 +136,11 @@ public abstract class AbstractMiningCoordinator<
    executor.setExtraData(extraData);
  }

+  @Override
+  public Optional<Address> getCoinbase() {
+    return executor.getCoinbase();
+  }
+
  protected abstract boolean newChainHeadInvalidatesMiningOperation(
      final BlockHeader newChainHeadHeader);
 }
--- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutor.java
+++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutor.java
@@ -88,6 +88,7 @@ public class EthHashMinerExecutor extends AbstractMinerExecutor<Void, EthHashBlo
    }
  }

+  @Override
  public Optional<Address> getCoinbase() {
    return coinbase;
  }
--- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMiningCoordinator.java
+++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMiningCoordinator.java
@@ -43,11 +43,6 @@ public class EthHashMiningCoordinator extends AbstractMiningCoordinator<Void, Et
    executor.setCoinbase(coinbase);
  }

-  @Override
-  public Optional<Address> getCoinbase() {
-    return executor.getCoinbase();
-  }
-
  @Override
  public Optional<Long> hashesPerSecond() {
    final Optional<Long> currentHashesPerSecond =
--- a/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/MiningCoordinator.java
+++ b/ethereum/blockcreation/src/main/java/tech/pegasys/pantheon/ethereum/blockcreation/MiningCoordinator.java
@@ -37,10 +37,7 @@ public interface MiningCoordinator {
        "Current consensus mechanism prevents setting coinbase.");
  }

-  default Optional<Address> getCoinbase() {
-    throw new UnsupportedOperationException(
-        "Current consensus mechanism prevents querying of coinbase.");
-  }
+  Optional<Address> getCoinbase();

  default Optional<Long> hashesPerSecond() {
    return Optional.empty();
--- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthCoinbase.java
+++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthCoinbase.java
@@ -38,14 +38,10 @@ public class EthCoinbase implements JsonRpcMethod {

  @Override
  public JsonRpcResponse response(final JsonRpcRequest req) {
-    try {
-      final Optional<Address> coinbase = miningCoordinator.getCoinbase();
-      if (coinbase.isPresent()) {
-        return new JsonRpcSuccessResponse(req.getId(), coinbase.get().toString());
-      }
-      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.COINBASE_NOT_SPECIFIED);
-    } catch (final UnsupportedOperationException ex) {
-      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.INVALID_REQUEST);
+    final Optional<Address> coinbase = miningCoordinator.getCoinbase();
+    if (coinbase.isPresent()) {
+      return new JsonRpcSuccessResponse(req.getId(), coinbase.get().toString());
    }
+    return new JsonRpcErrorResponse(req.getId(), JsonRpcError.COINBASE_NOT_SPECIFIED);
  }
 }
--- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthCoinbaseTest.java
+++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthCoinbaseTest.java
@@ -77,17 +77,6 @@ public class EthCoinbaseTest {
    assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse);
  }

-  @Test
-  public void shouldReturnAnInvalidRequestIfUnderlyingOperationThrowsUnsupportedOperation() {
-    final JsonRpcRequest request = requestWithParams();
-    final JsonRpcResponse expectedResponse =
-        new JsonRpcErrorResponse(request.getId(), JsonRpcError.INVALID_REQUEST);
-    when(miningCoordinator.getCoinbase()).thenThrow(UnsupportedOperationException.class);
-
-    final JsonRpcResponse actualResponse = method.response(request);
-    assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse);
-  }
-
  private JsonRpcRequest requestWithParams(final Object... params) {
    return new JsonRpcRequest(JSON_RPC_VERSION, ETH_METHOD, params);
  }