Refactor Body Validation logic (#7896)

Signed-off-by: Gabriel-Trintinalia <[email protected]>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java

@@ -171,7 +171,12 @@ public BlockProcessingResult validateAndProcessBlock(
         Optional<List<Request>> maybeRequests =
             result.getYield().flatMap(BlockProcessingOutputs::getRequests);
         if (!blockBodyValidator.validateBody(
-            context, block, receipts, worldState.rootHash(), ommerValidationMode)) {
+            context,
+            block,
+            receipts,
+            worldState.rootHash(),
+            ommerValidationMode,
+            BodyValidationMode.FULL)) {
           result = new BlockProcessingResult("failed to validate output of imported block");
           handleFailedBlockProcessing(block, result, shouldRecordBadBlock);
           return result;
@@ -246,15 +251,24 @@ public boolean validateBlockForSyncing(
       final HeaderValidationMode headerValidationMode,
       final HeaderValidationMode ommerValidationMode,
       final BodyValidationMode bodyValidationMode) {
+
+    if (bodyValidationMode == BodyValidationMode.FULL) {
+      throw new UnsupportedOperationException(
+          "Full body validation is not supported for syncing blocks");
+    }
+
     final BlockHeader header = block.getHeader();
     if (!blockHeaderValidator.validateHeader(header, context, headerValidationMode)) {
       String description = String.format("Failed header validation (%s)", headerValidationMode);
       badBlockManager.addBadBlock(block, BadBlockCause.fromValidationFailure(description));
       return false;
     }
 
-    if (!blockBodyValidator.validateBodyLight(
-        context, block, receipts, ommerValidationMode, bodyValidationMode)) {
+    if (bodyValidationMode == BodyValidationMode.NONE) {
+      return true;
+    }
+
+    if (!blockBodyValidator.validateBodyLight(context, block, receipts, ommerValidationMode)) {
       badBlockManager.addBadBlock(
           block, BadBlockCause.fromValidationFailure("Failed body validation (light)"));
       return false;

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BaseFeeBlockBodyValidator.java

@@ -41,11 +41,9 @@ public boolean validateBodyLight(
       final ProtocolContext context,
       final Block block,
       final List<TransactionReceipt> receipts,
-      final HeaderValidationMode ommerValidationMode,
-      final BodyValidationMode bodyValidationMode) {
+      final HeaderValidationMode ommerValidationMode) {
 
-    return super.validateBodyLight(
-            context, block, receipts, ommerValidationMode, bodyValidationMode)
+    return super.validateBodyLight(context, block, receipts, ommerValidationMode)
         && validateTransactionGasPrice(block);
   }
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockBodyValidator.java

@@ -33,14 +33,16 @@ public interface BlockBodyValidator {
    * @param worldStateRootHash The rootHash defining the world state after processing this block and
    *     all of its transactions.
    * @param ommerValidationMode The validation mode to use for ommer headers
+   * @param bodyValidationMode The validation mode to use for the body
    * @return {@code true} if valid; otherwise {@code false}
    */
   boolean validateBody(
-      ProtocolContext context,
-      Block block,
-      List<TransactionReceipt> receipts,
-      Hash worldStateRootHash,
-      final HeaderValidationMode ommerValidationMode);
+      final ProtocolContext context,
+      final Block block,
+      final List<TransactionReceipt> receipts,
+      final Hash worldStateRootHash,
+      final HeaderValidationMode ommerValidationMode,
+      final BodyValidationMode bodyValidationMode);
 
   /**
    * Validates that the block body is valid, but skips state root validation.
@@ -55,6 +57,5 @@ boolean validateBodyLight(
       ProtocolContext context,
       Block block,
       List<TransactionReceipt> receipts,
-      final HeaderValidationMode ommerValidationMode,
-      final BodyValidationMode bodyValidationMode);
+      final HeaderValidationMode ommerValidationMode);
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BodyValidationMode.java

@@ -21,6 +21,9 @@ public enum BodyValidationMode {
   /** Skip receipts and transactions root validation */
   LIGHT,
 
+  /** Validate transactions, state, and receipts root */
+  ROOT_ONLY,
+
   /** Fully validate the body */
   FULL;
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java

@@ -50,10 +50,39 @@ public boolean validateBody(
       final Block block,
       final List<TransactionReceipt> receipts,
       final Hash worldStateRootHash,
-      final HeaderValidationMode ommerValidationMode) {
+      final HeaderValidationMode ommerValidationMode,
+      final BodyValidationMode bodyValidationMode) {
+    if (bodyValidationMode == BodyValidationMode.NONE) {
+      return true;
+    }
+
+    if (bodyValidationMode == BodyValidationMode.LIGHT
+        || bodyValidationMode == BodyValidationMode.FULL) {
+      if (!validateBodyLight(context, block, receipts, ommerValidationMode)) {
+        return false;
+      }
+    }
+
+    if (bodyValidationMode == BodyValidationMode.ROOT_ONLY
+        || bodyValidationMode == BodyValidationMode.FULL) {
+      return validateBodyRoots(block, receipts, worldStateRootHash);
+    }
+    return true;
+  }
+
+  @VisibleForTesting
+  protected boolean validateBodyRoots(
+      final Block block, final List<TransactionReceipt> receipts, final Hash worldStateRootHash) {
+    final BlockHeader header = block.getHeader();
+    final BlockBody body = block.getBody();
+
+    final Bytes32 transactionsRoot = BodyValidation.transactionsRoot(body.getTransactions());
+    if (!validateTransactionsRoot(header, header.getTransactionsRoot(), transactionsRoot)) {
+      return false;
+    }
 
-    if (!validateBodyLight(
-        context, block, receipts, ommerValidationMode, BodyValidationMode.FULL)) {
+    final Bytes32 receiptsRoot = BodyValidation.receiptsRoot(receipts);
+    if (!validateReceiptsRoot(header, header.getReceiptsRoot(), receiptsRoot)) {
       return false;
     }
 
@@ -65,7 +94,6 @@ public boolean validateBody(
               LOG.warn("Transaction receipt found in the invalid block {}", receipt.toString()));
       return false;
     }
-
     return true;
   }
 
@@ -74,27 +102,9 @@ public boolean validateBodyLight(
       final ProtocolContext context,
       final Block block,
       final List<TransactionReceipt> receipts,
-      final HeaderValidationMode ommerValidationMode,
-      final BodyValidationMode bodyValidationMode) {
-    if (bodyValidationMode == BodyValidationMode.NONE) {
-      return true;
-    }
+      final HeaderValidationMode ommerValidationMode) {
 
     final BlockHeader header = block.getHeader();
-    final BlockBody body = block.getBody();
-
-    // these checks are only needed for full validation and can be skipped for light validation
-    if (bodyValidationMode == BodyValidationMode.FULL) {
-      final Bytes32 transactionsRoot = BodyValidation.transactionsRoot(body.getTransactions());
-      if (!validateTransactionsRoot(header, header.getTransactionsRoot(), transactionsRoot)) {
-        return false;
-      }
-
-      final Bytes32 receiptsRoot = BodyValidation.receiptsRoot(receipts);
-      if (!validateReceiptsRoot(header, header.getReceiptsRoot(), receiptsRoot)) {
-        return false;
-      }
-    }
 
     final long gasUsed =
         receipts.isEmpty() ? 0 : receipts.get(receipts.size() - 1).getCumulativeGasUsed();
@@ -113,15 +123,13 @@ public boolean validateBodyLight(
     if (!validateWithdrawals(block)) {
       return false;
     }
-
     return true;
   }
 
-  @VisibleForTesting
-  protected boolean validateTransactionsRoot(
+  private boolean validateTransactionsRoot(
       final BlockHeader header, final Bytes32 expected, final Bytes32 actual) {
     if (!expected.equals(actual)) {
-      LOG.info(
+      LOG.warn(
           "Invalid block {}: transaction root mismatch (expected={}, actual={})",
           header.toLogString(),
           expected,
@@ -160,8 +168,7 @@ private static boolean validateGasUsed(
     return true;
   }
 
-  @VisibleForTesting
-  protected boolean validateReceiptsRoot(
+  private boolean validateReceiptsRoot(
       final BlockHeader header, final Bytes32 expected, final Bytes32 actual) {
     if (!expected.equals(actual)) {
       LOG.warn(
@@ -175,7 +182,7 @@ protected boolean validateReceiptsRoot(
     return true;
   }
 
-  private static boolean validateStateRoot(
+  private boolean validateStateRoot(
       final BlockHeader header, final Bytes32 expected, final Bytes32 actual) {
     if (!expected.equals(actual)) {
       LOG.warn(

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/BlockImportExceptionHandlingTest.java

@@ -153,7 +153,8 @@ void shouldNotBadBlockWhenInternalErrorDuringPersisting() {
             eq(goodBlock),
             any(),
             any(),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
+            eq(HeaderValidationMode.DETACHED_ONLY),
+            any()))
         .thenReturn(true);
     assertThat(badBlockManager.getBadBlocks()).isEmpty();
     mainnetBlockValidator.validateAndProcessBlock(
@@ -189,7 +190,8 @@ void shouldNotBadBlockWhenInternalErrorOnBlockLookup() {
             eq(goodBlock),
             any(),
             any(),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
+            eq(HeaderValidationMode.DETACHED_ONLY),
+            any()))
         .thenReturn(true);
     assertThat(badBlockManager.getBadBlocks()).isEmpty();
     mainnetBlockValidator.validateAndProcessBlock(
@@ -257,7 +259,8 @@ void shouldNotBadBlockWhenInternalErrorDuringValidateBody() {
             eq(goodBlock),
             any(),
             any(),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
+            eq(HeaderValidationMode.DETACHED_ONLY),
+            any()))
         .thenThrow(new StorageException("database problem"));
     assertThat(badBlockManager.getBadBlocks()).isEmpty();
     mainnetBlockValidator.validateAndProcessBlock(

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java

@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
@@ -97,8 +98,9 @@ public void setup() {
     when(worldStateArchive.getMutable()).thenReturn(worldState);
     when(blockHeaderValidator.validateHeader(any(), any(), any())).thenReturn(true);
     when(blockHeaderValidator.validateHeader(any(), any(), any(), any())).thenReturn(true);
-    when(blockBodyValidator.validateBody(any(), any(), any(), any(), any())).thenReturn(true);
-    when(blockBodyValidator.validateBodyLight(any(), any(), any(), any(), any())).thenReturn(true);
+    when(blockBodyValidator.validateBody(any(), any(), any(), any(), any(), any()))
+        .thenReturn(true);
+    when(blockBodyValidator.validateBodyLight(any(), any(), any(), any())).thenReturn(true);
     when(blockProcessor.processBlock(any(), any(), any())).thenReturn(successfulProcessingResult);
     when(blockProcessor.processBlock(any(), any(), any(), any()))
         .thenReturn(successfulProcessingResult);
@@ -163,7 +165,8 @@ public void validateAndProcessBlock_whenHeaderInvalid() {
 
   @Test
   public void validateAndProcessBlock_whenBlockBodyInvalid() {
-    when(blockBodyValidator.validateBody(any(), eq(block), any(), any(), any())).thenReturn(false);
+    when(blockBodyValidator.validateBody(any(), eq(block), any(), any(), any(), any()))
+        .thenReturn(false);
 
     BlockProcessingResult result =
         mainnetBlockValidator.validateAndProcessBlock(
@@ -350,7 +353,7 @@ public void validateBlockForSyncing_onSuccess() {
             Collections.emptyList(),
             HeaderValidationMode.FULL,
             HeaderValidationMode.FULL,
-            BodyValidationMode.FULL);
+            BodyValidationMode.LIGHT);
 
     assertThat(isValid).isTrue();
     assertNoBadBlocks();
@@ -362,7 +365,6 @@ public void validateBlockValidation_onFailedHeaderForSyncing() {
     when(blockHeaderValidator.validateHeader(
             any(BlockHeader.class), eq(protocolContext), eq(headerValidationMode)))
         .thenReturn(false);
-    final BodyValidationMode bodyValidationMode = BodyValidationMode.FULL;
 
     final boolean isValid =
         mainnetBlockValidator.validateBlockForSyncing(
@@ -371,7 +373,7 @@ public void validateBlockValidation_onFailedHeaderForSyncing() {
             Collections.emptyList(),
             headerValidationMode,
             headerValidationMode,
-            bodyValidationMode);
+            BodyValidationMode.LIGHT);
 
     assertThat(isValid).isFalse();
     assertBadBlockIsTracked(block);
@@ -380,13 +382,8 @@ public void validateBlockValidation_onFailedHeaderForSyncing() {
   @Test
   public void validateBlockValidation_onFailedBodyForSyncing() {
     final HeaderValidationMode headerValidationMode = HeaderValidationMode.FULL;
-    final BodyValidationMode bodyValidationMode = BodyValidationMode.FULL;
     when(blockBodyValidator.validateBodyLight(
-            eq(protocolContext),
-            eq(block),
-            any(),
-            eq(headerValidationMode),
-            eq(bodyValidationMode)))
+            eq(protocolContext), eq(block), any(), eq(headerValidationMode)))
         .thenReturn(false);
 
     final boolean isValid =
@@ -396,12 +393,27 @@ public void validateBlockValidation_onFailedBodyForSyncing() {
             Collections.emptyList(),
             headerValidationMode,
             headerValidationMode,
-            bodyValidationMode);
+            BodyValidationMode.LIGHT);
 
     assertThat(isValid).isFalse();
     assertBadBlockIsTracked(block);
   }
 
+  @Test
+  public void shouldThrowIfValidateForSyncingWithFullBodyValidation() {
+    final HeaderValidationMode headerValidationMode = HeaderValidationMode.FULL;
+    assertThrows(
+        UnsupportedOperationException.class,
+        () ->
+            mainnetBlockValidator.validateBlockForSyncing(
+                protocolContext,
+                block,
+                Collections.emptyList(),
+                headerValidationMode,
+                headerValidationMode,
+                BodyValidationMode.FULL));
+  }
+
   private void assertNoBadBlocks() {
     assertThat(badBlockManager.getBadBlocks()).isEmpty();
   }