Clique createemptyblocks transition (#6608)

Support clique transitions for createemptyblocks boolean --------- Signed-off-by: Jason Frame <jason.frame@consensys.net> Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Co-authored-by: Jason Frame <jason.frame@consensys.net>
hyperledger · Mar 4, 2024 · dbc128f · dbc128f
1 parent 8b3b57d
commit dbc128f
Show file tree

Hide file tree

Showing 10 changed files with 112 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 ### Additions and Improvements
 - Extend `Blockchain` service [#6592](https://github.com/hyperledger/besu/pull/6592)
 - Add bft-style blockperiodseconds transitions to Clique [#6596](https://github.com/hyperledger/besu/pull/6596)
+- Add createemptyblocks transitions to Clique [#6608](https://github.com/hyperledger/besu/pull/6608)
 - RocksDB database metadata refactoring [#6555](https://github.com/hyperledger/besu/pull/6555)
 - Make layered txpool aware of minGasPrice and minPriorityFeePerGas dynamic options [#6611](https://github.com/hyperledger/besu/pull/6611)
 - Update commons-compress to 1.26.0 [#6648](https://github.com/hyperledger/besu/pull/6648)

diff --git a/...rc/test/java/org/hyperledger/besu/tests/acceptance/clique/CliqueMiningAcceptanceTest.java b/...rc/test/java/org/hyperledger/besu/tests/acceptance/clique/CliqueMiningAcceptanceTest.java
@@ -64,7 +64,7 @@ public void shouldNotMineBlocksIfNoTransactionsWhenCreateEmptyBlockIsFalse() thr
   }
 
   @Test
-  public void shouldMineBlocksOnlyWhenTransactionsArePresentWhenCreateEmptyBlockIsFalse()
+  public void shouldMineBlocksOnlyWhenTransactionsArePresentWhenCreateEmptyBlocksIsFalse()
       throws IOException {
     final var cliqueOptionsNoEmptyBlocks =
         new CliqueOptions(
@@ -134,7 +134,7 @@ public void shouldStillMineWhenANodeFailsAndHasSufficientValidators() throws IOE
   }
 
   @Test
-  public void shouldMineBlocksWithBlockPeriodAccordingToTransitions() throws IOException {
+  public void shouldMineBlocksAccordingToBlockPeriodTransitions() throws IOException {
 
     final var cliqueOptions = new CliqueOptions(3, CliqueOptions.DEFAULT.epochLength(), true);
     final BesuNode minerNode = besu.createCliqueNode("miner1", cliqueOptions);
@@ -144,6 +144,8 @@ public void shouldMineBlocksWithBlockPeriodAccordingToTransitions() throws IOExc
         Map.of("block", 3, "blockperiodseconds", 2);
     final Map<String, Object> decreasePeriodTo1_Transition =
         Map.of("block", 4, "blockperiodseconds", 1);
+    // ensure previous blockperiodseconds transition is carried over
+    final Map<String, Object> dummy_Transition = Map.of("block", 5, "createemptyblocks", true);
     final Map<String, Object> increasePeriodTo2_Transition =
         Map.of("block", 6, "blockperiodseconds", 2);
 
@@ -155,6 +157,7 @@ public void shouldMineBlocksWithBlockPeriodAccordingToTransitions() throws IOExc
             List.of(
                 decreasePeriodTo2_Transition,
                 decreasePeriodTo1_Transition,
+                dummy_Transition,
                 increasePeriodTo2_Transition));
     minerNode.setGenesisConfig(genesisWithTransitions);
 
@@ -176,6 +179,59 @@ public void shouldMineBlocksWithBlockPeriodAccordingToTransitions() throws IOExc
     assertThat(block6Timestamp - block5Timestamp).isCloseTo(2, withPercentage(20));
   }
 
+  @Test
+  public void shouldMineBlocksAccordingToCreateEmptyBlocksTransitions() throws IOException {
+
+    final var cliqueOptionsEmptyBlocks =
+        new CliqueOptions(2, CliqueOptions.DEFAULT.epochLength(), true);
+    final BesuNode minerNode = besu.createCliqueNode("miner1", cliqueOptionsEmptyBlocks);
+
+    // setup transitions
+    final Map<String, Object> noEmptyBlocks_Transition =
+        Map.of("block", 3, "createemptyblocks", false);
+    final Map<String, Object> emptyBlocks_Transition =
+        Map.of("block", 4, "createemptyblocks", true);
+    final Map<String, Object> secondNoEmptyBlocks_Transition =
+        Map.of("block", 6, "createemptyblocks", false);
+    // ensure previous createemptyblocks transition is carried over
+    final Map<String, Object> dummy_Transition = Map.of("block", 7, "blockperiodseconds", 1);
+
+    final Optional<String> initialGenesis =
+        minerNode.getGenesisConfigProvider().create(List.of(minerNode));
+    final String genesisWithTransitions =
+        prependTransitionsToCliqueOptions(
+            initialGenesis.orElseThrow(),
+            List.of(
+                noEmptyBlocks_Transition,
+                emptyBlocks_Transition,
+                secondNoEmptyBlocks_Transition,
+                dummy_Transition));
+    minerNode.setGenesisConfig(genesisWithTransitions);
+
+    final Account sender = accounts.createAccount("account1");
+
+    // Mine 2 blocks
+    cluster.start(minerNode);
+    minerNode.verify(blockchain.reachesHeight(minerNode, 1));
+
+    // tx required to mine block
+    cluster.verify(clique.noNewBlockCreated(minerNode));
+    minerNode.execute(accountTransactions.createTransfer(sender, 50));
+    minerNode.verify(clique.blockIsCreatedByProposer(minerNode));
+
+    // Mine 2 more blocks so chain head is 5
+    minerNode.verify(blockchain.reachesHeight(minerNode, 2));
+
+    // tx required to mine block 6
+    cluster.verify(clique.noNewBlockCreated(minerNode));
+    minerNode.execute(accountTransactions.createTransfer(sender, 50));
+    minerNode.verify(clique.blockIsCreatedByProposer(minerNode));
+
+    // check createemptyblocks transition carried over when other transition activated...
+    // tx required to mine block 7
+    cluster.verify(clique.noNewBlockCreated(minerNode));
+  }
+
   private long getTimestampForBlock(final BesuNode minerNode, final int blockNumber) {
     return minerNode
         .execute(

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java
@@ -54,7 +54,6 @@ public class CliqueBesuControllerBuilder extends BesuControllerBuilder {
 
   private Address localAddress;
   private EpochManager epochManager;
-  private boolean createEmptyBlocks = true;
   private final BlockInterface blockInterface = new CliqueBlockInterface();
   private ForksSchedule<CliqueConfigOptions> forksSchedule;
 
@@ -63,7 +62,6 @@ protected void prepForBuild() {
     localAddress = Util.publicKeyToAddress(nodeKey.getPublicKey());
     final CliqueConfigOptions cliqueConfig = configOptionsSupplier.get().getCliqueConfigOptions();
     final long blocksPerEpoch = cliqueConfig.getEpochLength();
-    createEmptyBlocks = cliqueConfig.getCreateEmptyBlocks();
 
     epochManager = new EpochManager(blocksPerEpoch);
     forksSchedule = CliqueForksSchedulesFactory.create(configOptionsSupplier.get());
@@ -96,7 +94,7 @@ protected MiningCoordinator createMiningCoordinator(
                 localAddress,
                 forksSchedule),
             epochManager,
-            createEmptyBlocks,
+            forksSchedule,
             ethProtocolManager.ethContext().getScheduler());
     final CliqueMiningCoordinator miningCoordinator =
         new CliqueMiningCoordinator(

diff --git a/config/src/main/java/org/hyperledger/besu/config/CliqueFork.java b/config/src/main/java/org/hyperledger/besu/config/CliqueFork.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.config;
 
+import java.util.Optional;
 import java.util.OptionalInt;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
@@ -28,6 +29,9 @@ public class CliqueFork implements Fork {
   /** The constant BLOCK_PERIOD_SECONDS_KEY. */
   public static final String BLOCK_PERIOD_SECONDS_KEY = "blockperiodseconds";
 
+  /** The constant CREATE_EMPTY_BLOCKS_KEY. */
+  public static final String CREATE_EMPTY_BLOCKS_KEY = "createemptyblocks";
+
   /** The Fork config root. */
   protected final ObjectNode forkConfigRoot;
 
@@ -63,4 +67,13 @@ public long getForkBlock() {
   public OptionalInt getBlockPeriodSeconds() {
     return JsonUtil.getPositiveInt(forkConfigRoot, BLOCK_PERIOD_SECONDS_KEY);
   }
+
+  /**
+   * Gets create empty blocks.
+   *
+   * @return the create empty blocks
+   */
+  public Optional<Boolean> getCreateEmptyBlocks() {
+    return JsonUtil.getBoolean(forkConfigRoot, CREATE_EMPTY_BLOCKS_KEY);
+  }
 }
diff --git a/...ique/src/main/java/org/hyperledger/besu/consensus/clique/CliqueForksSchedulesFactory.java b/...ique/src/main/java/org/hyperledger/besu/consensus/clique/CliqueForksSchedulesFactory.java
@@ -44,6 +44,7 @@ private static CliqueConfigOptions createCliqueConfigOptions(
 
     var options = ImmutableCliqueConfigOptions.builder().from(lastSpec.getValue());
     fork.getBlockPeriodSeconds().ifPresent(options::blockPeriodSeconds);
+    fork.getCreateEmptyBlocks().ifPresent(options::createEmptyBlocks);
     return options.build();
   }
 }
diff --git a/...us/clique/src/main/java/org/hyperledger/besu/consensus/clique/CliqueProtocolSchedule.java b/...us/clique/src/main/java/org/hyperledger/besu/consensus/clique/CliqueProtocolSchedule.java
@@ -42,6 +42,8 @@
 import java.util.Optional;
 import java.util.function.Function;
 
+import com.google.common.annotations.VisibleForTesting;
+
 /** Defines the protocol behaviours for a blockchain using Clique. */
 public class CliqueProtocolSchedule {
 
@@ -89,7 +91,7 @@ public static ProtocolSchedule create(
                         applyCliqueSpecificModifications(
                             epochManager,
                             forkSpec.getValue().getBlockPeriodSeconds(),
-                            cliqueConfig.getCreateEmptyBlocks(),
+                            forkSpec.getValue().getCreateEmptyBlocks(),
                             localNodeAddress,
                             builder)));
     final ProtocolSpecAdapters specAdapters = new ProtocolSpecAdapters(specMap);
@@ -116,6 +118,7 @@ public static ProtocolSchedule create(
    * @param badBlockManager the cache to use to keep invalid blocks
    * @return the protocol schedule
    */
+  @VisibleForTesting
   public static ProtocolSchedule create(
       final GenesisConfigOptions config,
       final ForksSchedule<CliqueConfigOptions> forksSchedule,

diff --git a/...e/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMiner.java b/...e/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMiner.java
@@ -14,7 +14,9 @@
  */
 package org.hyperledger.besu.consensus.clique.blockcreation;
 
+import org.hyperledger.besu.config.CliqueConfigOptions;
 import org.hyperledger.besu.consensus.clique.CliqueHelpers;
+import org.hyperledger.besu.consensus.common.ForksSchedule;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.blockcreation.AbstractBlockScheduler;
@@ -36,7 +38,7 @@ public class CliqueBlockMiner extends BlockMiner<CliqueBlockCreator> {
   private static final int WAIT_IN_MS_BETWEEN_EMPTY_BUILD_ATTEMPTS = 1_000;
 
   private final Address localAddress;
-  private final boolean createEmptyBlocks;
+  private final ForksSchedule<CliqueConfigOptions> forksSchedule;
 
   /**
    * Instantiates a new Clique block miner.
@@ -48,7 +50,7 @@ public class CliqueBlockMiner extends BlockMiner<CliqueBlockCreator> {
    * @param scheduler the scheduler
    * @param parentHeader the parent header
    * @param localAddress the local address
-   * @param createEmptyBlocks whether clique should allow the creation of empty blocks.
+   * @param forksSchedule the transitions
    */
   public CliqueBlockMiner(
       final Function<BlockHeader, CliqueBlockCreator> blockCreator,
@@ -58,10 +60,10 @@ public CliqueBlockMiner(
       final AbstractBlockScheduler scheduler,
       final BlockHeader parentHeader,
       final Address localAddress,
-      final boolean createEmptyBlocks) {
+      final ForksSchedule<CliqueConfigOptions> forksSchedule) {
     super(blockCreator, protocolSchedule, protocolContext, observers, scheduler, parentHeader);
     this.localAddress = localAddress;
-    this.createEmptyBlocks = createEmptyBlocks;
+    this.forksSchedule = forksSchedule;
   }
 
   @Override
@@ -76,7 +78,7 @@ protected boolean mineBlock() throws InterruptedException {
 
   @Override
   protected boolean shouldImportBlock(final Block block) throws InterruptedException {
-    if (createEmptyBlocks) {
+    if (forksSchedule.getFork(block.getHeader().getNumber()).getValue().getCreateEmptyBlocks()) {
       return true;
     }
 

diff --git a/...rc/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java b/...rc/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java
@@ -14,10 +14,12 @@
  */
 package org.hyperledger.besu.consensus.clique.blockcreation;
 
+import org.hyperledger.besu.config.CliqueConfigOptions;
 import org.hyperledger.besu.consensus.clique.CliqueContext;
 import org.hyperledger.besu.consensus.clique.CliqueExtraData;
 import org.hyperledger.besu.consensus.common.ConsensusHelpers;
 import org.hyperledger.besu.consensus.common.EpochManager;
+import org.hyperledger.besu.consensus.common.ForksSchedule;
 import org.hyperledger.besu.cryptoservices.NodeKey;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.ethereum.ProtocolContext;
@@ -48,7 +50,7 @@ public class CliqueMinerExecutor extends AbstractMinerExecutor<CliqueBlockMiner>
   private final Address localAddress;
   private final NodeKey nodeKey;
   private final EpochManager epochManager;
-  private final boolean createEmptyBlocks;
+  private final ForksSchedule<CliqueConfigOptions> forksSchedule;
 
   /**
    * Instantiates a new Clique miner executor.
@@ -60,7 +62,7 @@ public class CliqueMinerExecutor extends AbstractMinerExecutor<CliqueBlockMiner>
    * @param miningParams the mining params
    * @param blockScheduler the block scheduler
    * @param epochManager the epoch manager
-   * @param createEmptyBlocks whether clique should allow the creation of empty blocks.
+   * @param forksSchedule the clique transitions
    * @param ethScheduler the scheduler for asynchronous block creation tasks
    */
   public CliqueMinerExecutor(
@@ -71,7 +73,7 @@ public CliqueMinerExecutor(
       final MiningParameters miningParams,
       final AbstractBlockScheduler blockScheduler,
       final EpochManager epochManager,
-      final boolean createEmptyBlocks,
+      final ForksSchedule<CliqueConfigOptions> forksSchedule,
       final EthScheduler ethScheduler) {
     super(
         protocolContext,
@@ -83,7 +85,7 @@ public CliqueMinerExecutor(
     this.nodeKey = nodeKey;
     this.localAddress = Util.publicKeyToAddress(nodeKey.getPublicKey());
     this.epochManager = epochManager;
-    this.createEmptyBlocks = createEmptyBlocks;
+    this.forksSchedule = forksSchedule;
     miningParams.setCoinbase(localAddress);
   }
 
@@ -113,7 +115,7 @@ public CliqueBlockMiner createMiner(
         blockScheduler,
         parentHeader,
         localAddress,
-        createEmptyBlocks);
+        forksSchedule);
   }
 
   @Override

diff --git a/...c/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMinerTest.java b/...c/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockMinerTest.java
@@ -23,7 +23,12 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.config.CliqueConfigOptions;
+import org.hyperledger.besu.config.ImmutableCliqueConfigOptions;
+import org.hyperledger.besu.config.JsonCliqueConfigOptions;
 import org.hyperledger.besu.consensus.clique.CliqueContext;
+import org.hyperledger.besu.consensus.common.ForkSpec;
+import org.hyperledger.besu.consensus.common.ForksSchedule;
 import org.hyperledger.besu.consensus.common.validator.ValidatorProvider;
 import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
@@ -54,10 +59,20 @@
 import java.util.function.Function;
 
 import com.google.common.collect.Lists;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 class CliqueBlockMinerTest {
 
+  private ForksSchedule<CliqueConfigOptions> forksSchedule;
+
+  @BeforeEach
+  public void setup() {
+    var options = ImmutableCliqueConfigOptions.builder().from(JsonCliqueConfigOptions.DEFAULT);
+    options.createEmptyBlocks(false);
+    forksSchedule = new ForksSchedule<>(List.of(new ForkSpec<>(0, options.build())));
+  }
+
   @Test
   void doesNotMineBlockIfNoTransactionsWhenEmptyBlocksNotAllowed() throws InterruptedException {
     final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture();
@@ -100,7 +115,7 @@ void doesNotMineBlockIfNoTransactionsWhenEmptyBlocksNotAllowed() throws Interrup
             scheduler,
             headerBuilder.buildHeader(),
             Address.ZERO,
-            false); // parent header is arbitrary for the test.
+            forksSchedule); // parent header is arbitrary for the test.
 
     final boolean result = miner.mineBlock();
     assertThat(result).isFalse();
@@ -155,7 +170,7 @@ void minesBlockIfHasTransactionsWhenEmptyBlocksNotAllowed() throws InterruptedEx
             scheduler,
             headerBuilder.buildHeader(),
             Address.ZERO,
-            false); // parent header is arbitrary for the test.
+            forksSchedule); // parent header is arbitrary for the test.
 
     final boolean result = miner.mineBlock();
     assertThat(result).isTrue();

diff --git a/...est/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/...est/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java
@@ -124,7 +124,7 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() {
             miningParameters,
             mock(CliqueBlockScheduler.class),
             new EpochManager(EPOCH_LENGTH),
-            true,
+            null,
             ethScheduler);
 
     // NOTE: Passing in the *parent* block, so must be 1 less than EPOCH
@@ -160,7 +160,7 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() {
             miningParameters,
             mock(CliqueBlockScheduler.class),
             new EpochManager(EPOCH_LENGTH),
-            true,
+            null,
             ethScheduler);
 
     // Parent block was epoch, so the next block should contain no validators.
@@ -196,7 +196,7 @@ public void shouldUseLatestVanityData() {
             miningParameters,
             mock(CliqueBlockScheduler.class),
             new EpochManager(EPOCH_LENGTH),
-            true,
+            null,
             ethScheduler);
 
     executor.setExtraData(modifiedVanityData);