Added CliqueMiningTracker to make some things easier

Tests are still written with a full blockchain - which is semi-useful.
PegaSysEng · Dec 5, 2018 · 2aa45b8 · 2aa45b8
1 parent 29d08bc
commit 2aa45b8
Show file tree

Hide file tree

Showing 4 changed files with 151 additions and 58 deletions.
diff --git a/...nsus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueMiningTracker.java b/...nsus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueMiningTracker.java
@@ -0,0 +1,40 @@
+/*
+ * 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.consensus.clique;
+
+import tech.pegasys.pantheon.ethereum.ProtocolContext;
+import tech.pegasys.pantheon.ethereum.core.Address;
+import tech.pegasys.pantheon.ethereum.core.BlockHeader;
+
+public class CliqueMiningTracker {
+
+  private final Address localAddress;
+  private final ProtocolContext<CliqueContext> protocolContext;
+
+  public CliqueMiningTracker(
+      final Address localAddress, final ProtocolContext<CliqueContext> protocolContext) {
+    this.localAddress = localAddress;
+    this.protocolContext = protocolContext;
+  }
+
+  public boolean isProposerAfter(final BlockHeader header) {
+    final Address nextProposer =
+        CliqueHelpers.getProposerForBlockAfter(
+            header, protocolContext.getConsensusState().getVoteTallyCache());
+    return localAddress.equals(nextProposer);
+  }
+
+  public boolean canMakeBlockNextRound(final BlockHeader header) {
+    return CliqueHelpers.addressIsAllowedToProduceNextBlock(localAddress, protocolContext, header);
+  }
+}
diff --git a/...in/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinator.java b/...in/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinator.java
@@ -13,30 +13,25 @@
 package tech.pegasys.pantheon.consensus.clique.blockcreation;
 
 import tech.pegasys.pantheon.consensus.clique.CliqueContext;
-import tech.pegasys.pantheon.consensus.clique.CliqueHelpers;
-import tech.pegasys.pantheon.ethereum.ProtocolContext;
+import tech.pegasys.pantheon.consensus.clique.CliqueMiningTracker;
 import tech.pegasys.pantheon.ethereum.blockcreation.AbstractMiningCoordinator;
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
-import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.Block;
 import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
 
 public class CliqueMiningCoordinator
     extends AbstractMiningCoordinator<CliqueContext, CliqueBlockMiner> {
 
-  private final Address localAddress;
-  private final ProtocolContext<CliqueContext> protocolContext;
+  private final CliqueMiningTracker miningTracker;
 
   public CliqueMiningCoordinator(
       final Blockchain blockchain,
       final CliqueMinerExecutor executor,
       final SyncState syncState,
-      final ProtocolContext<CliqueContext> protocolContext,
-      final Address localAddress) {
+      final CliqueMiningTracker miningTracker) {
     super(blockchain, executor, syncState);
-    this.localAddress = localAddress;
-    this.protocolContext = protocolContext;
+    this.miningTracker = miningTracker;
   }
 
   @Override
@@ -52,13 +47,8 @@ protected boolean importedBlockInvalidatesMiningOperation(final Block newBlock)
       return true;
     }
 
-    final boolean nodeIsMining = CliqueHelpers.addressIsAllowedToProduceNextBlock(
-        localAddress, protocolContext, parentHeader);
-
-    final boolean nodeIsInTurn = CliqueHelpers.getProposerForBlockAfter(
-        parentHeader,
-        protocolContext.getConsensusState().getVoteTallyCache())
-        .equals(localAddress);
+    final boolean nodeIsMining = miningTracker.canMakeBlockNextRound(parentHeader);
+    final boolean nodeIsInTurn = miningTracker.isProposerAfter(parentHeader);
 
     if (nodeIsMining && nodeIsInTurn) {
       return false;

diff --git a/...ava/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinatorTest.java b/...ava/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMiningCoordinatorTest.java
@@ -1,3 +1,15 @@
+/*
+ * 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.consensus.clique.blockcreation;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -10,22 +22,13 @@
 import static org.mockito.Mockito.when;
 import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain;
 
-import java.util.List;
-import org.assertj.core.util.Lists;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
 import tech.pegasys.pantheon.consensus.clique.CliqueContext;
+import tech.pegasys.pantheon.consensus.clique.CliqueMiningTracker;
 import tech.pegasys.pantheon.consensus.clique.TestHelpers;
 import tech.pegasys.pantheon.consensus.clique.VoteTallyCache;
 import tech.pegasys.pantheon.consensus.common.VoteTally;
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
 import tech.pegasys.pantheon.ethereum.ProtocolContext;
-import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent;
-import tech.pegasys.pantheon.ethereum.chain.BlockAddedEvent.EventType;
 import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain;
 import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.Block;
@@ -36,6 +39,16 @@
 import tech.pegasys.pantheon.ethereum.core.Util;
 import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
 
+import java.util.List;
+
+import org.assertj.core.util.Lists;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
 @RunWith(MockitoJUnitRunner.class)
 public class CliqueMiningCoordinatorTest {
 
@@ -44,23 +57,18 @@ public class CliqueMiningCoordinatorTest {
   private final Address proposerAddress = Util.publicKeyToAddress(proposerKeys.getPublicKey());
   private final Address validatorAddress = Util.publicKeyToAddress(validatorKeys.getPublicKey());
 
-  private final List<Address> validators =
-      Lists.newArrayList(validatorAddress, proposerAddress);
+  private final List<Address> validators = Lists.newArrayList(validatorAddress, proposerAddress);
 
   private final BlockHeaderTestFixture headerTestFixture = new BlockHeaderTestFixture();
 
-  @Mock
-  private MutableBlockchain blockChain;
-  @Mock
-  private ProtocolContext<CliqueContext> protocolContext;
-  @Mock
-  private CliqueMinerExecutor minerExecutor;
-  @Mock
-  private CliqueBlockMiner blockMiner;
-  @Mock
-  private SyncState syncState;
-  @Mock
-  private VoteTallyCache voteTallyCache;
+  private CliqueMiningTracker miningTracker;
+
+  @Mock private MutableBlockchain blockChain;
+  @Mock private ProtocolContext<CliqueContext> protocolContext;
+  @Mock private CliqueMinerExecutor minerExecutor;
+  @Mock private CliqueBlockMiner blockMiner;
+  @Mock private SyncState syncState;
+  @Mock private VoteTallyCache voteTallyCache;
 
   @Before
   public void setup() {
@@ -75,8 +83,11 @@ public void setup() {
     final CliqueContext cliqueContext = new CliqueContext(voteTallyCache, null, null);
 
     when(protocolContext.getConsensusState()).thenReturn(cliqueContext);
+    when(protocolContext.getBlockchain()).thenReturn(blockChain);
     when(minerExecutor.startAsyncMining(any(), any())).thenReturn(blockMiner);
     when(syncState.isInSync()).thenReturn(true);
+
+    miningTracker = new CliqueMiningTracker(proposerAddress, protocolContext);
   }
 
   @Test
@@ -87,10 +98,9 @@ public void outOfTurnBlockImportedDoesNotInterruptInTurnMiningOperation() {
     when(blockMiner.getParentHeader()).thenReturn(blockChain.getChainHeadHeader());
 
     // Note also - validators is an hard-ordered LIST, thus in-turn will follow said list - block_1
-    //should be created by proposer.
+    // should be created by proposer.
     final CliqueMiningCoordinator coordinator =
-        new CliqueMiningCoordinator(blockChain, minerExecutor, syncState, protocolContext,
-            proposerAddress);
+        new CliqueMiningCoordinator(blockChain, minerExecutor, syncState, miningTracker);
 
     coordinator.enable();
 
@@ -101,7 +111,6 @@ public void outOfTurnBlockImportedDoesNotInterruptInTurnMiningOperation() {
     final Block importedBlock = createEmptyBlock(1, blockChain.getChainHeadHash(), validatorKeys);
 
     blockChain.appendBlock(importedBlock, Lists.emptyList());
-    final BlockAddedEvent event = BlockAddedEvent.createForHeadAdvancement(importedBlock);
 
     // The minerExecutor should not be invoked as the mining operation was conducted by an in-turn
     // validator, and the created block came from an out-turn validator.
@@ -110,16 +119,14 @@ public void outOfTurnBlockImportedDoesNotInterruptInTurnMiningOperation() {
 
   @Test
   public void outOfTurnBlockImportedAtHigherLevelInterruptsMiningOperation() {
-    // As the head of the blockChain is 0 (which effectively doesn't have a signer, all validators
+    // As the head of the blockChain is 1 (which effectively doesn't have a signer, all validators
     // are able to propose.
-
     when(blockMiner.getParentHeader()).thenReturn(blockChain.getChainHeadHeader());
 
     // Note also - validators is an hard-ordered LIST, thus in-turn will follow said list - block_1
-    //should be created by proposer.
+    // should be created by proposer.
     final CliqueMiningCoordinator coordinator =
-        new CliqueMiningCoordinator(blockChain, minerExecutor, syncState, protocolContext,
-            proposerAddress);
+        new CliqueMiningCoordinator(blockChain, minerExecutor, syncState, miningTracker);
 
     coordinator.enable();
 
@@ -131,7 +138,6 @@ public void outOfTurnBlockImportedAtHigherLevelInterruptsMiningOperation() {
     final Block importedBlock = createEmptyBlock(2, blockChain.getChainHeadHash(), validatorKeys);
 
     blockChain.appendBlock(importedBlock, Lists.emptyList());
-    final BlockAddedEvent event = BlockAddedEvent.createForHeadAdvancement(importedBlock);
 
     // The minerExecutor should not be invoked as the mining operation was conducted by an in-turn
     // validator, and the created block came from an out-turn validator.
@@ -141,17 +147,70 @@ public void outOfTurnBlockImportedAtHigherLevelInterruptsMiningOperation() {
   }
 
   @Test
-  public void inTurnBlockImportedInterruptsOutOfTurnMiningOperation() {
+  public void outOfTurnBlockImportedInterruptsOutOfTurnMiningOperation() {
+    blockChain.appendBlock(
+        createEmptyBlock(1, blockChain.getChainHeadHash(), validatorKeys), Lists.emptyList());
+
+    when(blockMiner.getParentHeader()).thenReturn(blockChain.getChainHeadHeader());
+
+    // Note also - validators is an hard-ordered LIST, thus in-turn will follow said list - block_2
+    // should be created by 'validator', thus Proposer is out-of-turn.
+    final CliqueMiningCoordinator coordinator =
+        new CliqueMiningCoordinator(blockChain, minerExecutor, syncState, miningTracker);
 
+    coordinator.enable();
+
+    verify(minerExecutor, times(1)).startAsyncMining(any(), any());
+
+    reset(minerExecutor);
+    when(minerExecutor.startAsyncMining(any(), any())).thenReturn(blockMiner);
+
+    final Block importedBlock = createEmptyBlock(2, blockChain.getChainHeadHash(), validatorKeys);
+
+    blockChain.appendBlock(importedBlock, Lists.emptyList());
+
+    // The minerExecutor should not be invoked as the mining operation was conducted by an in-turn
+    // validator, and the created block came from an out-turn validator.
+    ArgumentCaptor<BlockHeader> varArgs = ArgumentCaptor.forClass(BlockHeader.class);
+    verify(minerExecutor, times(1)).startAsyncMining(any(), varArgs.capture());
+    assertThat(varArgs.getValue()).isEqualTo(blockChain.getChainHeadHeader());
   }
 
-  private Block createEmptyBlock(final long blockNumber, final Hash parentHash,
-      final KeyPair signer) {
+  @Test
+  public void outOfTurnBlockImportedInterruptsNonRunningMiner() {
+    blockChain.appendBlock(
+        createEmptyBlock(1, blockChain.getChainHeadHash(), proposerKeys), Lists.emptyList());
+
+    when(blockMiner.getParentHeader()).thenReturn(blockChain.getChainHeadHeader());
+
+    // Note also - validators is an hard-ordered LIST, thus in-turn will follow said list - block_2
+    // should be created by 'validator', thus Proposer is out-of-turn.
+    final CliqueMiningCoordinator coordinator =
+        new CliqueMiningCoordinator(blockChain, minerExecutor, syncState, miningTracker);
+
+    coordinator.enable();
+
+    verify(minerExecutor, times(1)).startAsyncMining(any(), any());
+
+    reset(minerExecutor);
+    when(minerExecutor.startAsyncMining(any(), any())).thenReturn(blockMiner);
+
+    final Block importedBlock = createEmptyBlock(2, blockChain.getChainHeadHash(), validatorKeys);
+
+    blockChain.appendBlock(importedBlock, Lists.emptyList());
+
+    // The minerExecutor should not be invoked as the mining operation was conducted by an in-turn
+    // validator, and the created block came from an out-turn validator.
+    ArgumentCaptor<BlockHeader> varArgs = ArgumentCaptor.forClass(BlockHeader.class);
+    verify(minerExecutor, times(1)).startAsyncMining(any(), varArgs.capture());
+    assertThat(varArgs.getValue()).isEqualTo(blockChain.getChainHeadHeader());
+  }
+
+  private Block createEmptyBlock(
+      final long blockNumber, final Hash parentHash, final KeyPair signer) {
     headerTestFixture.number(blockNumber).parentHash(parentHash);
     final BlockHeader header =
         TestHelpers.createCliqueSignedBlockHeader(headerTestFixture, signer, validators);
-    return new Block(
-        header, new BlockBody(Lists.emptyList(), Lists.emptyList()));
+    return new Block(header, new BlockBody(Lists.emptyList(), Lists.emptyList()));
   }
-
-}
+}
diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/CliquePantheonController.java
@@ -18,6 +18,7 @@
 import tech.pegasys.pantheon.config.GenesisConfigFile;
 import tech.pegasys.pantheon.consensus.clique.CliqueBlockInterface;
 import tech.pegasys.pantheon.consensus.clique.CliqueContext;
+import tech.pegasys.pantheon.consensus.clique.CliqueMiningTracker;
 import tech.pegasys.pantheon.consensus.clique.CliqueProtocolSchedule;
 import tech.pegasys.pantheon.consensus.clique.VoteTallyCache;
 import tech.pegasys.pantheon.consensus.clique.blockcreation.CliqueBlockScheduler;
@@ -181,7 +182,10 @@ public static PantheonController<CliqueContext> init(
             epochManger);
     final CliqueMiningCoordinator miningCoordinator =
         new CliqueMiningCoordinator(
-            blockchain, miningExecutor, syncState, protocolContext, localAddress);
+            blockchain,
+            miningExecutor,
+            syncState,
+            new CliqueMiningTracker(localAddress, protocolContext));
     miningCoordinator.addMinedBlockObserver(ethProtocolManager);
 
     // Clique mining is implicitly enabled.