Bugfix/clique post merge fast sync (hyperledger#4276)

* if merge enabled, wrap two clique rules in composed Attached rule to enable fast-sync to proceed normally for post-merge networks * move BlockPropagationManager warning to debug until hyperledger#4274 Signed-off-by: garyschulte <garyschulte@gmail.com>
freemanzMrojo · Aug 19, 2022 · 5bb9a30 · 5bb9a30
1 parent 95d0825
commit 5bb9a30
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 - Fixes off-by-one error for mainnet TTD fallback [#4223](https://github.com/hyperledger/besu/pull/4223)
 - Fix off-by-one error in AbstractRetryingPeerTask [#4254](https://github.com/hyperledger/besu/pull/4254)
 - Fix encoding of key (short hex) in eth_getProof [#4261](https://github.com/hyperledger/besu/pull/4261)
+- Fix for post-merge networks fast-sync [#4224](https://github.com/hyperledger/besu/pull/4224), [#4276](https://github.com/hyperledger/besu/pull/4276)
 
 ## 22.7.0
 

diff --git a/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/controller/TransitionControllerBuilderTest.java
@@ -21,12 +21,15 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.consensus.clique.BlockHeaderValidationRulesetFactory;
 import org.hyperledger.besu.consensus.clique.CliqueContext;
+import org.hyperledger.besu.consensus.common.EpochManager;
 import org.hyperledger.besu.consensus.merge.MergeContext;
 import org.hyperledger.besu.consensus.merge.PostMergeContext;
 import org.hyperledger.besu.consensus.merge.TransitionProtocolSchedule;
 import org.hyperledger.besu.consensus.merge.blockcreation.TransitionCoordinator;
 import org.hyperledger.besu.crypto.NodeKeyUtils;
+import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
@@ -36,8 +39,12 @@
 import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager;
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
+import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
+import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
+import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
+import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
 import org.hyperledger.besu.ethereum.storage.StorageProvider;
 
 import java.util.Optional;
@@ -178,6 +185,56 @@ public void assertPostMergeScheduleForPostMergeExactlyAtTerminalDifficultyIfNotF
         .isEqualTo(postMergeProtocolSpec);
   }
 
+  @Test
+  public void assertCliqueDetachedHeaderValidationPreMerge() {
+    BlockHeaderValidator cliqueValidator =
+        BlockHeaderValidationRulesetFactory.cliqueBlockHeaderValidator(
+                5L, new EpochManager(5L), Optional.of(FeeMarket.london(1L)), true)
+            .build();
+    assertDetachedRulesForPostMergeBlocks(cliqueValidator);
+  }
+
+  @Test
+  public void assertPoWDetachedHeaderValidationPreMerge() {
+    BlockHeaderValidator powValidator =
+        MainnetBlockHeaderValidator.createBaseFeeMarketValidator(FeeMarket.london(1L), true)
+            .build();
+    assertDetachedRulesForPostMergeBlocks(powValidator);
+  }
+
+  void assertDetachedRulesForPostMergeBlocks(final BlockHeaderValidator validator) {
+    var preMergeProtocolSpec = mock(ProtocolSpec.class);
+    when(preMergeProtocolSpec.getBlockHeaderValidator()).thenReturn(validator);
+
+    when(preMergeProtocolSchedule.getByBlockNumber(anyLong())).thenReturn(preMergeProtocolSpec);
+
+    var mockParentBlock =
+        new BlockHeaderTestFixture()
+            .number(100L)
+            .baseFeePerGas(Wei.of(7L))
+            .gasLimit(5000L)
+            .buildHeader();
+
+    var mockBlock =
+        new BlockHeaderTestFixture()
+            .number(101L)
+            .timestamp(1000L)
+            .baseFeePerGas(Wei.of(7L))
+            .gasLimit(5000L)
+            .difficulty(Difficulty.of(0L))
+            .parentHash(mockParentBlock.getHash())
+            .buildHeader();
+
+    var mergeFriendlyValidation =
+        transitionProtocolSchedule
+            .getPreMergeSchedule()
+            .getByBlockNumber(1L)
+            .getBlockHeaderValidator()
+            .validateHeader(
+                mockBlock, mockParentBlock, protocolContext, HeaderValidationMode.DETACHED_ONLY);
+    assertThat(mergeFriendlyValidation).isTrue();
+  }
+
   TransitionCoordinator buildTransitionCoordinator(
       final BesuControllerBuilder preMerge, final MergeBesuControllerBuilder postMerge) {
     var builder = new TransitionBesuControllerBuilder(preMerge, postMerge);

diff --git a/.../main/java/org/hyperledger/besu/consensus/clique/BlockHeaderValidationRulesetFactory.java b/.../main/java/org/hyperledger/besu/consensus/clique/BlockHeaderValidationRulesetFactory.java
@@ -17,6 +17,7 @@
 import static org.hyperledger.besu.ethereum.mainnet.AbstractGasLimitSpecification.DEFAULT_MAX_GAS_LIMIT;
 import static org.hyperledger.besu.ethereum.mainnet.AbstractGasLimitSpecification.DEFAULT_MIN_GAS_LIMIT;
 
+import org.hyperledger.besu.config.MergeConfigOptions;
 import org.hyperledger.besu.consensus.clique.headervalidationrules.CliqueDifficultyValidationRule;
 import org.hyperledger.besu.consensus.clique.headervalidationrules.CliqueExtraDataValidationRule;
 import org.hyperledger.besu.consensus.clique.headervalidationrules.CoinbaseHeaderValidationRule;
@@ -28,6 +29,7 @@
 import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.AncestryValidationRule;
+import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.AttachedComposedFromDetachedRule;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.BaseFeeMarketBlockHeaderGasPriceValidationRule;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.ConstantFieldValidationRule;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.GasLimitRangeAndDeltaValidationRule;
@@ -37,6 +39,8 @@
 
 import java.util.Optional;
 
+import com.google.common.annotations.VisibleForTesting;
+
 public class BlockHeaderValidationRulesetFactory {
 
   /**
@@ -54,22 +58,28 @@ public static BlockHeaderValidator.Builder cliqueBlockHeaderValidator(
       final long secondsBetweenBlocks,
       final EpochManager epochManager,
       final Optional<BaseFeeMarket> baseFeeMarket) {
+    return cliqueBlockHeaderValidator(
+        secondsBetweenBlocks, epochManager, baseFeeMarket, MergeConfigOptions.isMergeEnabled());
+  }
+
+  @VisibleForTesting
+  public static BlockHeaderValidator.Builder cliqueBlockHeaderValidator(
+      final long secondsBetweenBlocks,
+      final EpochManager epochManager,
+      final Optional<BaseFeeMarket> baseFeeMarket,
+      final boolean isMergeEnabled) {
 
     final BlockHeaderValidator.Builder builder =
         new BlockHeaderValidator.Builder()
             .addRule(new AncestryValidationRule())
             .addRule(new TimestampBoundedByFutureParameter(10))
-            .addRule(new TimestampMoreRecentThanParent(secondsBetweenBlocks))
             .addRule(
                 new GasLimitRangeAndDeltaValidationRule(
                     DEFAULT_MIN_GAS_LIMIT, DEFAULT_MAX_GAS_LIMIT, baseFeeMarket))
-            .addRule(
-                new ConstantFieldValidationRule<>("MixHash", BlockHeader::getMixHash, Hash.ZERO))
             .addRule(
                 new ConstantFieldValidationRule<>(
                     "OmmersHash", BlockHeader::getOmmersHash, Hash.EMPTY_LIST_HASH))
             .addRule(new CliqueExtraDataValidationRule(epochManager))
-            .addRule(new VoteValidationRule())
             .addRule(new CliqueDifficultyValidationRule())
             .addRule(new SignerRateLimitValidationRule())
             .addRule(new CoinbaseHeaderValidationRule(epochManager))
@@ -79,6 +89,20 @@ public static BlockHeaderValidator.Builder cliqueBlockHeaderValidator(
       builder.addRule(new BaseFeeMarketBlockHeaderGasPriceValidationRule(baseFeeMarket.get()));
     }
 
+    var mixHashRule =
+        new ConstantFieldValidationRule<>("MixHash", BlockHeader::getMixHash, Hash.ZERO);
+    var voteValidationRule = new VoteValidationRule();
+    var cliqueTimestampRule = new TimestampMoreRecentThanParent(secondsBetweenBlocks);
+
+    if (isMergeEnabled) {
+      builder
+          .addRule(new AttachedComposedFromDetachedRule(mixHashRule))
+          .addRule(new AttachedComposedFromDetachedRule(voteValidationRule))
+          .addRule(new AttachedComposedFromDetachedRule(cliqueTimestampRule));
+    } else {
+      builder.addRule(mixHashRule).addRule(voteValidationRule).addRule(cliqueTimestampRule);
+    }
+
     return builder;
   }
 }
diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockHeaderValidator.java b/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockHeaderValidator.java
@@ -18,7 +18,7 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.AncestryValidationRule;
-import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.AttachedProofOfWorkValidationRule;
+import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.AttachedComposedFromDetachedRule;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.BaseFeeMarketBlockHeaderGasPriceValidationRule;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.CalculatedDifficultyValidationRule;
 import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.ConstantFieldValidationRule;
@@ -31,6 +31,7 @@
 
 import java.util.Optional;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.tuweni.bytes.Bytes;
 
 public final class MainnetBlockHeaderValidator {
@@ -124,6 +125,12 @@ public static BlockHeaderValidator.Builder createPgaBlockHeaderValidator(
 
   public static BlockHeaderValidator.Builder createBaseFeeMarketValidator(
       final BaseFeeMarket baseFeeMarket) {
+    return createBaseFeeMarketValidator(baseFeeMarket, MergeConfigOptions.isMergeEnabled());
+  }
+
+  @VisibleForTesting
+  public static BlockHeaderValidator.Builder createBaseFeeMarketValidator(
+      final BaseFeeMarket baseFeeMarket, final boolean isMergeEnabled) {
     var builder =
         new BlockHeaderValidator.Builder()
             .addRule(CalculatedDifficultyValidationRule::new)
@@ -138,19 +145,16 @@ public static BlockHeaderValidator.Builder createBaseFeeMarketValidator(
             .addRule((new BaseFeeMarketBlockHeaderGasPriceValidationRule(baseFeeMarket)));
 
     // if merge is enabled, use the attached version of the proof of work validation rule
-    if (MergeConfigOptions.isMergeEnabled()) {
-      builder.addRule(
-          new AttachedProofOfWorkValidationRule(
-              new EpochCalculator.DefaultEpochCalculator(),
-              PoWHasher.ETHASH_LIGHT,
-              Optional.of(baseFeeMarket)));
-
+    var powValidationRule =
+        new ProofOfWorkValidationRule(
+            new EpochCalculator.DefaultEpochCalculator(),
+            PoWHasher.ETHASH_LIGHT,
+            Optional.of(baseFeeMarket));
+
+    if (isMergeEnabled) {
+      builder.addRule(new AttachedComposedFromDetachedRule(powValidationRule));
     } else {
-      builder.addRule(
-          new ProofOfWorkValidationRule(
-              new EpochCalculator.DefaultEpochCalculator(),
-              PoWHasher.ETHASH_LIGHT,
-              Optional.of(baseFeeMarket)));
+      builder.addRule(powValidationRule);
     }
     return builder;
   }

diff --git a/...es/AttachedProofOfWorkValidationRule.java → ...les/AttachedComposedFromDetachedRule.java b/...es/AttachedProofOfWorkValidationRule.java → ...les/AttachedComposedFromDetachedRule.java
@@ -17,25 +17,18 @@
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.mainnet.AttachedBlockHeaderValidationRule;
-import org.hyperledger.besu.ethereum.mainnet.EpochCalculator;
-import org.hyperledger.besu.ethereum.mainnet.PoWHasher;
-import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
-
-import java.util.Optional;
+import org.hyperledger.besu.ethereum.mainnet.DetachedBlockHeaderValidationRule;
 
 /**
  * An attached proof of work validation rule that wraps the detached version of the same. Suitable
  * for use in block validator stacks supporting the merge.
  */
-public class AttachedProofOfWorkValidationRule implements AttachedBlockHeaderValidationRule {
+public class AttachedComposedFromDetachedRule implements AttachedBlockHeaderValidationRule {
 
-  private final ProofOfWorkValidationRule detachedRule;
+  private final DetachedBlockHeaderValidationRule detachedRule;
 
-  public AttachedProofOfWorkValidationRule(
-      final EpochCalculator epochCalculator,
-      final PoWHasher hasher,
-      final Optional<FeeMarket> feeMarket) {
-    this.detachedRule = new ProofOfWorkValidationRule(epochCalculator, hasher, feeMarket);
+  public AttachedComposedFromDetachedRule(final DetachedBlockHeaderValidationRule detachedRule) {
+    this.detachedRule = detachedRule;
   }
 
   @Override

diff --git a/...eum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java b/...eum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java
@@ -120,7 +120,7 @@ public void stop() {
       clearListeners();
       started.set(false);
     } else {
-      LOG.warn("Attempted to stop when we are not even running...");
+      LOG.debug("Attempted to stop when we are not even running...");
     }
   }