Replace boolean return with BlockImportResult object (hyperledger#4386)

* Replace boolean return with BlockImportResult object Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net> Co-authored-by: Gabriel Trintinalia <gabriel.trintinalia@consensys.net>
daniellehrner · Oct 4, 2022 · a80ed4e · a80ed4e
1 parent 016016d
commit a80ed4e
Show file tree

Hide file tree

Showing 25 changed files with 180 additions and 82 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/chainimport/JsonBlockImporter.java b/besu/src/main/java/org/hyperledger/besu/chainimport/JsonBlockImporter.java
@@ -26,6 +26,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
 import org.hyperledger.besu.ethereum.core.Transaction;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.evm.worldstate.WorldState;
 
@@ -146,9 +147,9 @@ private void importBlock(final Block block) {
             .getByBlockNumber(block.getHeader().getNumber())
             .getBlockImporter();
 
-    final boolean imported =
+    final BlockImportResult importResult =
         importer.importBlock(controller.getProtocolContext(), block, HeaderValidationMode.NONE);
-    if (imported) {
+    if (importResult.isImported()) {
       LOG.info(
           "Successfully created and imported block at height {} ({})",
           block.getHeader().getNumber(),
@@ -220,7 +221,7 @@ private BlockHeader getParentHeader(final BlockData blockData, final List<Block>
 
     if (importedBlocks.size() > 0 && blockData.getNumber().isPresent()) {
       final long targetParentBlockNumber = blockData.getNumber().get() - 1L;
-      Optional<BlockHeader> maybeHeader =
+      final Optional<BlockHeader> maybeHeader =
           importedBlocks.stream()
               .map(Block::getHeader)
               .filter(h -> h.getNumber() == targetParentBlockNumber)
@@ -230,7 +231,7 @@ private BlockHeader getParentHeader(final BlockData blockData, final List<Block>
       }
     }
 
-    long blockNumber;
+    final long blockNumber;
     if (blockData.getNumber().isPresent()) {
       blockNumber = blockData.getNumber().get() - 1L;
     } else if (importedBlocks.size() > 0) {

diff --git a/besu/src/main/java/org/hyperledger/besu/chainimport/RlpBlockImporter.java b/besu/src/main/java/org/hyperledger/besu/chainimport/RlpBlockImporter.java
@@ -25,6 +25,7 @@
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
@@ -208,15 +209,15 @@ private void evaluateBlock(
       cumulativeTimer.start();
       segmentTimer.start();
       final BlockImporter blockImporter = protocolSpec.getBlockImporter();
-      final boolean blockImported =
+      final BlockImportResult blockImported =
           blockImporter.importBlock(
               context,
               block,
               skipPowValidation
                   ? HeaderValidationMode.LIGHT_SKIP_DETACHED
                   : HeaderValidationMode.SKIP_DETACHED,
               skipPowValidation ? HeaderValidationMode.LIGHT : HeaderValidationMode.FULL);
-      if (!blockImported) {
+      if (!blockImported.isImported()) {
         throw new IllegalStateException(
             "Invalid block at block number " + header.getNumber() + ".");
       }

diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java
@@ -48,6 +48,7 @@
 import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
@@ -465,9 +466,9 @@ private static void setupState(
       final ProtocolSpec protocolSpec =
           protocolSchedule.getByBlockNumber(block.getHeader().getNumber());
       final BlockImporter blockImporter = protocolSpec.getBlockImporter();
-      final boolean result =
+      final BlockImportResult result =
           blockImporter.importBlock(protocolContext, block, HeaderValidationMode.FULL);
-      if (!result) {
+      if (!result.isImported()) {
         throw new IllegalStateException("Unable to import block " + block.getHeader().getNumber());
       }
     }

diff --git a/...n-test/java/org/hyperledger/besu/consensus/ibft/tests/round/IbftRoundIntegrationTest.java b/...n-test/java/org/hyperledger/besu/consensus/ibft/tests/round/IbftRoundIntegrationTest.java
@@ -47,6 +47,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
@@ -103,15 +104,15 @@ public void setup() {
     throwingMessageFactory = new MessageFactory(nodeKey);
     transmitter = new IbftMessageTransmitter(throwingMessageFactory, multicaster);
 
-    BftExtraData proposedExtraData =
+    final BftExtraData proposedExtraData =
         new BftExtraData(Bytes.wrap(new byte[32]), emptyList(), empty(), 0, emptyList());
     final BlockHeaderTestFixture headerTestFixture = new BlockHeaderTestFixture();
     headerTestFixture.extraData(bftExtraDataEncoder.encode(proposedExtraData));
     headerTestFixture.number(1);
     final BlockHeader header = headerTestFixture.buildHeader();
     proposedBlock = new Block(header, new BlockBody(emptyList(), emptyList()));
 
-    when(blockImporter.importBlock(any(), any(), any())).thenReturn(true);
+    when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(true));
 
     protocolContext =
         new ProtocolContext(

diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java
@@ -38,6 +38,7 @@
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
@@ -103,7 +104,7 @@ public void startRoundWith(
 
     final RoundChangeCertificate roundChangeCertificate =
         roundChangeArtifacts.getRoundChangeCertificate();
-    Block blockToPublish;
+    final Block blockToPublish;
     if (!bestBlockFromRoundChange.isPresent()) {
       LOG.debug("Sending proposal with new block. round={}", roundState.getRoundIdentifier());
       blockToPublish = blockCreator.createBlock(headerTimestamp);
@@ -260,9 +261,9 @@ private void importBlockToChain() {
           blockToImport.getHash());
     }
     LOG.trace("Importing block with extraData={}", extraData);
-    final boolean result =
+    final BlockImportResult result =
         blockImporter.importBlock(protocolContext, blockToImport, HeaderValidationMode.FULL);
-    if (!result) {
+    if (!result.isImported()) {
       LOG.error(
           "Failed to import block to chain. block={} extraData={} blockHeader={}",
           blockNumber,

diff --git a/...us/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRoundTest.java b/...us/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRoundTest.java
@@ -52,6 +52,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
@@ -120,7 +121,7 @@ public void setup() {
 
     when(blockCreator.createBlock(anyLong())).thenReturn(proposedBlock);
 
-    when(blockImporter.importBlock(any(), any(), any())).thenReturn(true);
+    when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(true));
 
     subscribers.subscribe(minedBlockObserver);
   }

diff --git a/...on-test/java/org/hyperledger/besu/consensus/qbft/test/round/QbftRoundIntegrationTest.java b/...on-test/java/org/hyperledger/besu/consensus/qbft/test/round/QbftRoundIntegrationTest.java
@@ -49,6 +49,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
@@ -113,7 +114,7 @@ public void setup() {
     final BlockHeader header = headerTestFixture.buildHeader();
     proposedBlock = new Block(header, new BlockBody(emptyList(), emptyList()));
 
-    when(blockImporter.importBlock(any(), any(), any())).thenReturn(true);
+    when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(true));
 
     protocolContext =
         new ProtocolContext(

diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java
@@ -42,6 +42,7 @@
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
@@ -107,7 +108,7 @@ public void startRoundWith(
     final Optional<PreparedCertificate> bestPreparedCertificate =
         roundChangeArtifacts.getBestPreparedPeer();
 
-    Block blockToPublish;
+    final Block blockToPublish;
     if (bestPreparedCertificate.isEmpty()) {
       LOG.debug("Sending proposal with new block. round={}", roundState.getRoundIdentifier());
       blockToPublish = blockCreator.createBlock(headerTimestamp);
@@ -278,9 +279,9 @@ private void importBlockToChain() {
           blockToImport.getHash());
     }
     LOG.trace("Importing proposed block with extraData={}", extraData);
-    final boolean result =
+    final BlockImportResult result =
         blockImporter.importBlock(protocolContext, blockToImport, HeaderValidationMode.FULL);
-    if (!result) {
+    if (!result.isImported()) {
       LOG.error(
           "Failed to import proposed block to chain. block={} extraData={} blockHeader={}",
           blockNumber,

diff --git a/...est/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java b/...est/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java
@@ -64,6 +64,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
 import org.hyperledger.besu.ethereum.core.Util;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
 import org.hyperledger.besu.util.Subscribers;
 
@@ -146,6 +147,7 @@ public void setup() {
     when(messageValidatorFactory.createFutureRoundProposalMessageValidator(anyLong(), any()))
         .thenReturn(futureRoundProposalMessageValidator);
     when(messageValidatorFactory.createMessageValidator(any(), any())).thenReturn(messageValidator);
+    when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(false));
 
     protocolContext =
         new ProtocolContext(

diff --git a/...us/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java b/...us/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java
@@ -56,6 +56,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
 import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;
 import org.hyperledger.besu.util.Subscribers;
@@ -128,7 +129,7 @@ public void setup() {
 
     when(blockCreator.createBlock(anyLong())).thenReturn(proposedBlock);
 
-    when(blockImporter.importBlock(any(), any(), any())).thenReturn(true);
+    when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(true));
 
     subscribers.subscribe(minedBlockObserver);
   }

diff --git a/...m/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockMiner.java b/...m/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockMiner.java
@@ -20,6 +20,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
 import org.hyperledger.besu.ethereum.core.Transaction;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.util.Subscribers;
@@ -140,9 +141,9 @@ protected boolean mineBlock() throws InterruptedException {
 
     final BlockImporter importer =
         protocolSchedule.getByBlockNumber(block.getHeader().getNumber()).getBlockImporter();
-    final boolean blockImported =
+    final BlockImportResult blockImportResult =
         importer.importBlock(protocolContext, block, HeaderValidationMode.FULL);
-    if (blockImported) {
+    if (blockImportResult.isImported()) {
       notifyNewBlockListeners(block);
       final double taskTimeInSec = stopwatch.elapsed(TimeUnit.MILLISECONDS) / 1000.0;
       LOG.info(
@@ -159,7 +160,7 @@ protected boolean mineBlock() throws InterruptedException {
       LOG.error("Illegal block mined, could not be imported to local chain.");
     }
 
-    return blockImported;
+    return blockImportResult.isImported();
   }
 
   public void cancel() {

diff --git a/...ockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/BlockMinerTest.java b/...ockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/BlockMinerTest.java
@@ -28,6 +28,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.BlockImporter;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.MutableProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
@@ -64,7 +65,7 @@ public void blockCreatedIsAddedToBlockChain() throws InterruptedException {
     final ProtocolSchedule protocolSchedule = singleSpecSchedule(protocolSpec);
 
     when(protocolSpec.getBlockImporter()).thenReturn(blockImporter);
-    when(blockImporter.importBlock(any(), any(), any())).thenReturn(true);
+    when(blockImporter.importBlock(any(), any(), any())).thenReturn(new BlockImportResult(true));
 
     final MinedBlockObserver observer = mock(MinedBlockObserver.class);
     final DefaultBlockScheduler scheduler = mock(DefaultBlockScheduler.class);
@@ -103,7 +104,11 @@ public void failureToImportDoesNotTriggerObservers() throws InterruptedException
     final ProtocolSchedule protocolSchedule = singleSpecSchedule(protocolSpec);
 
     when(protocolSpec.getBlockImporter()).thenReturn(blockImporter);
-    when(blockImporter.importBlock(any(), any(), any())).thenReturn(false, false, true);
+    when(blockImporter.importBlock(any(), any(), any()))
+        .thenReturn(
+            new BlockImportResult(false),
+            new BlockImportResult(false),
+            new BlockImportResult(true));
 
     final MinedBlockObserver observer = mock(MinedBlockObserver.class);
     final DefaultBlockScheduler scheduler = mock(DefaultBlockScheduler.class);

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockImporter.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockImporter.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.core;
 
 import org.hyperledger.besu.ethereum.ProtocolContext;
+import org.hyperledger.besu.ethereum.mainnet.BlockImportResult;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 
 import java.util.List;
@@ -34,10 +35,10 @@ public interface BlockImporter {
    * @param context The context to attempt to update
    * @param block The block
    * @param headerValidationMode Determines the validation to perform on this header.
-   * @return {@code true} if the block was added somewhere in the blockchain; otherwise {@code
-   *     false}
+   * @return {@code BlockImportResult}
+   * @see BlockImportResult
    */
-  default boolean importBlock(
+  default BlockImportResult importBlock(
       final ProtocolContext context,
       final Block block,
       final HeaderValidationMode headerValidationMode) {
@@ -51,10 +52,10 @@ default boolean importBlock(
    * @param block The block
    * @param headerValidationMode Determines the validation to perform on this header.
    * @param ommerValidationMode Determines the validation to perform on ommer headers.
-   * @return {@code true} if the block was added somewhere in the blockchain; otherwise {@code
-   *     false}
+   * @return {@code BlockImportResult}
+   * @see BlockImportResult
    */
-  boolean importBlock(
+  BlockImportResult importBlock(
       ProtocolContext context,
       Block block,
       HeaderValidationMode headerValidationMode,
@@ -69,10 +70,10 @@ boolean importBlock(
    * @param receipts The receipts associated with this block.
    * @param headerValidationMode Determines the validation to perform on this header.
    * @param ommerValidationMode Determines the validation to perform on ommer headers.
-   * @return {@code true} if the block was added somewhere in the blockchain; otherwise {@code
-   *     false}
+   * @return {@code BlockImportResult}
+   * @see BlockImportResult
    */
-  boolean fastImportBlock(
+  BlockImportResult fastImportBlock(
       ProtocolContext context,
       Block block,
       List<TransactionReceipt> receipts,

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockImportResult.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockImportResult.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * 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.mainnet;
+
+/** The result of a block import. */
+public class BlockImportResult {
+  private final BlockImportStatus status;
+
+  public enum BlockImportStatus {
+    IMPORTED,
+    NOT_IMPORTED,
+    ALREADY_IMPORTED
+  }
+
+  public BlockImportResult(final boolean status) {
+    this.status = status ? BlockImportStatus.IMPORTED : BlockImportStatus.NOT_IMPORTED;
+  }
+
+  public BlockImportResult(final BlockImportStatus status) {
+    this.status = status;
+  }
+
+  /**
+   * The result of the block import call
+   *
+   * @return {@code true} if the block was added somewhere in the blockchain; otherwise {@code
+   *     false}
+   */
+  public boolean isImported() {
+    return status == BlockImportStatus.IMPORTED || status == BlockImportStatus.ALREADY_IMPORTED;
+  }
+
+  public BlockImportStatus getStatus() {
+    return status;
+  }
+}