Prepping 22.7.5 (#4482)

* Change min selection to an unsigned min (#4420) Change the "minimum" gas in the TangerineWhistleGasCalculator to use an unsigned comparison, to ensure a larger signed value is not selected. Signed-off-by: Danno Ferrin <[email protected]> * Update Security Policy contact info (#4447) * Update Security Policy contact info At the request of the EF, a besu-only security list was created, and is the first listed email. The out-of-date Jira location is also removed. Signed-off-by: Danno Ferrin <[email protected]> * Keep the best block built until now instead of the last one (#4455) Signed-off-by: Fabio Di Fabio <[email protected]> * Add Mainnet to merged networks (#4463) * Add Mainnet to the merged networks, to force fast-sync-min-peers to 1 Signed-off-by: Fabio Di Fabio <[email protected]> * Avoid to process transactions during initial sync (#4457) Signed-off-by: Fabio Di Fabio <[email protected]> * uprev version to 22.7.5 Signed-off-by: Justin Florentine <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Justin Florentine <[email protected]> Co-authored-by: Danno Ferrin <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]>
hyperledger · Oct 5, 2022 · f0115fd · f0115fd
1 parent 0d70113
commit f0115fd
Show file tree

Hide file tree

Showing 34 changed files with 456 additions and 171 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,14 +2,16 @@
 
 ## 22.7.5
 
-
 ### Additions and Improvements
+- Avoid sending added block events to transaction pool, and processing incoming transactions during initial sync [#4457](https://github.com/hyperledger/besu/pull/4457)
+- When building a new proposal, keep the best block built until now instead of the last one [#4455](https://github.com/hyperledger/besu/pull/4455)
+- Add Mainnet to merged networks [#4463](https://github.com/hyperledger/besu/pull/4463)
+
 ### Bug Fixes
 ### Download Links
 
 ## 22.7.4
 
-
 ### Bug Fixes
 - Remove records that track transactions by sender when they are empty to same memory in the transaction pool [#4415](https://github.com/hyperledger/besu/pull/4415)
 - Add Toml configuration file support for _--Xplugin-rocksdb-high-spec-enabled_ flag [#4438](https://github.com/hyperledger/besu/pull/4438)

diff --git a/SECURITY.md b/SECURITY.md
@@ -7,13 +7,15 @@ hear from you. We will take all security bugs seriously and if confirmed upon in
 patch it within a reasonable amount of time and release a public security bulletin discussing the
 impact and credit the discoverer.
 
-There are two ways to report a security bug. The easiest is to email a description of the flaw and
-any related information (e.g. reproduction steps, version) to
-[security at hyperledger dot org](mailto:[email protected]).
-
-The other way is to file a confidential security bug in our
-[JIRA bug tracking system](https://jira.hyperledger.org). Be sure to set the “Security Level” to
-“Security issue”.
+There are two email addresses where Hyperledger Besu accepts security bugs. The
+first, [security "dash" besu at lists dot hyperledger dot org](mailto:[email protected])
+is limited to a subset of Hyperledger Besu maintainers and Hyperledger staff. For highly sensitive
+bugs this is a preferred address. The second email
+address [security at hyperledger dot org](mailto:[email protected]) is limited to a subset of
+maintainers and staff of all Hyperledger projects, and may be viewed by maintainers outside of
+Hyperledger Besu. When sending information to either of these emails please be sure to include a
+description of the flaw and any related information (e.g. reproduction steps, version, known active
+use).
 
 The process by which the Hyperledger Security Team handles security bugs is documented further in
 our [Defect Response page](https://wiki.hyperledger.org/display/SEC/Defect+Response) on our

diff --git a/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/01_prepare_payload.json b/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/01_prepare_payload.json
@@ -25,7 +25,7 @@
         "latestValidHash": "0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a",
         "validationError": null
       },
-      "payloadId": "0x0000000021f32cc1"
+      "payloadId": "0x0065bd195a9b3bfb"
     }
   },
   "statusCode" : 200

diff --git a/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/02_get_payload.json b/acceptance-tests/tests/src/test/resources/jsonrpc/engine/test-cases/02_get_payload.json
@@ -3,7 +3,7 @@
     "jsonrpc": "2.0",
     "method": "engine_getPayloadV1",
     "params": [
-      "0x0000000021f32cc1"
+      "0x0065bd195a9b3bfb"
     ],
     "id": 67
   },

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java b/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java
@@ -89,6 +89,7 @@ public Optional<String> getDeprecationDate() {
 
   public static boolean isMergedNetwork(final NetworkName networkName) {
     switch (networkName) {
+      case MAINNET:
       case GOERLI:
       case ROPSTEN:
       case SEPOLIA:

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java
@@ -386,7 +386,7 @@ public BesuController build() {
             ethContext,
             clock,
             metricsSystem,
-            syncState::isInitialSyncPhaseDone,
+            syncState,
             miningParameters,
             transactionPoolConfiguration);
 

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -882,7 +882,7 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5);
+    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1);
 
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
 
@@ -1709,19 +1709,19 @@ public void helpShouldDisplayFastSyncOptions() {
   }
 
   @Test
-  public void checkValidDefaultFastSyncMinPeersOption() {
-    parseCommand("--sync-mode", "FAST");
+  public void checkValidDefaultFastSyncMinPeersPoS() {
+    parseCommand("--sync-mode", "FAST", "--network", "MAINNET");
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
 
     final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
     assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5);
+    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1);
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
 
   @Test
-  public void checkValidDefaultFastSyncMinPeersPreMergeOption() {
+  public void checkValidDefaultFastSyncMinPeersPoW() {
     parseCommand("--sync-mode", "FAST", "--network", "CLASSIC");
     verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
 
@@ -1732,18 +1732,6 @@ public void checkValidDefaultFastSyncMinPeersPreMergeOption() {
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
 
-  @Test
-  public void checkValidDefaultFastSyncMinPeersPostMergeOption() {
-    parseCommand("--sync-mode", "FAST", "--network", "GOERLI");
-    verify(mockControllerBuilder).synchronizerConfiguration(syncConfigurationCaptor.capture());
-
-    final SynchronizerConfiguration syncConfig = syncConfigurationCaptor.getValue();
-    assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST);
-    assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(1);
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
-    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
-  }
-
   @Test
   public void parsesValidFastSyncMinPeersOption() {
     parseCommand("--sync-mode", "FAST", "--fast-sync-min-peers", "11");

diff --git a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java
@@ -147,7 +147,7 @@ public void setUp() {
             mockEthContext,
             TestClock.system(ZoneId.systemDefault()),
             new NoOpMetricsSystem(),
-            syncState::isInitialSyncPhaseDone,
+            syncState,
             new MiningParameters.Builder().minTransactionGasPrice(Wei.ZERO).build(),
             txPoolConfig);
 

diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java
@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.consensus.merge;
 
+import static org.hyperledger.besu.util.Slf4jLambdaHelper.debugLambda;
+
 import org.hyperledger.besu.consensus.merge.blockcreation.PayloadIdentifier;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.ConsensusContext;
@@ -23,19 +25,26 @@
 import org.hyperledger.besu.ethereum.eth.sync.state.SyncState;
 import org.hyperledger.besu.util.Subscribers;
 
+import java.util.Comparator;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.EvictingQueue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class PostMergeContext implements MergeContext {
+  private static final Logger LOG = LoggerFactory.getLogger(PostMergeContext.class);
   static final int MAX_BLOCKS_IN_PROGRESS = 12;
 
   private static final AtomicReference<PostMergeContext> singleton = new AtomicReference<>();
 
+  private static final Comparator<Block> compareByGasUsedDesc =
+      Comparator.comparingLong((Block block) -> block.getHeader().getGasUsed()).reversed();
+
   private final AtomicReference<SyncState> syncState;
   private final AtomicReference<Difficulty> terminalTotalDifficulty;
   // initial postMerge state is indeterminate until it is set:
@@ -197,21 +206,57 @@ public boolean validateCandidateHead(final BlockHeader candidateHeader) {
   }
 
   @Override
-  public void putPayloadById(final PayloadIdentifier payloadId, final Block block) {
-    var priorsById = retrieveTuplesById(payloadId).collect(Collectors.toUnmodifiableList());
-    blocksInProgress.add(new PayloadTuple(payloadId, block));
-    priorsById.stream().forEach(blocksInProgress::remove);
+  public void putPayloadById(final PayloadIdentifier payloadId, final Block newBlock) {
+    synchronized (blocksInProgress) {
+      final Optional<Block> maybeCurrBestBlock = retrieveBlockById(payloadId);
+
+      maybeCurrBestBlock.ifPresentOrElse(
+          currBestBlock -> {
+            if (compareByGasUsedDesc.compare(newBlock, currBestBlock) < 0) {
+              debugLambda(
+                  LOG,
+                  "New proposal for payloadId {} {} is better than the previous one {}",
+                  payloadId::toHexString,
+                  () -> logBlockProposal(newBlock),
+                  () -> logBlockProposal(currBestBlock));
+              blocksInProgress.removeAll(
+                  retrieveTuplesById(payloadId).collect(Collectors.toUnmodifiableList()));
+              blocksInProgress.add(new PayloadTuple(payloadId, newBlock));
+            }
+          },
+          () -> blocksInProgress.add(new PayloadTuple(payloadId, newBlock)));
+
+      debugLambda(
+          LOG,
+          "Current best proposal for payloadId {} {}",
+          payloadId::toHexString,
+          () -> retrieveBlockById(payloadId).map(bb -> logBlockProposal(bb)).orElse("N/A"));
+    }
   }
 
   @Override
   public Optional<Block> retrieveBlockById(final PayloadIdentifier payloadId) {
-    return retrieveTuplesById(payloadId).map(tuple -> tuple.block).findFirst();
+    synchronized (blocksInProgress) {
+      return retrieveTuplesById(payloadId)
+          .map(tuple -> tuple.block)
+          .sorted(compareByGasUsedDesc)
+          .findFirst();
+    }
   }
 
   private Stream<PayloadTuple> retrieveTuplesById(final PayloadIdentifier payloadId) {
     return blocksInProgress.stream().filter(z -> z.payloadIdentifier.equals(payloadId));
   }
 
+  private String logBlockProposal(final Block block) {
+    return "block "
+        + block.toLogString()
+        + " gas used "
+        + block.getHeader().getGasUsed()
+        + " transactions "
+        + block.getBody().getTransactions().size();
+  }
+
   private static class PayloadTuple {
     final PayloadIdentifier payloadIdentifier;
     final Block block;

diff --git a/...ge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/...ge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java
@@ -175,17 +175,18 @@ public void changeTargetGasLimit(final Long newTargetGasLimit) {
   public PayloadIdentifier preparePayload(
       final BlockHeader parentHeader,
       final Long timestamp,
-      final Bytes32 random,
+      final Bytes32 prevRandao,
       final Address feeRecipient) {
 
     final PayloadIdentifier payloadIdentifier =
-        PayloadIdentifier.forPayloadParams(parentHeader.getBlockHash(), timestamp);
+        PayloadIdentifier.forPayloadParams(
+            parentHeader.getBlockHash(), timestamp, prevRandao, feeRecipient);
     final MergeBlockCreator mergeBlockCreator =
         this.mergeBlockCreator.forParams(parentHeader, Optional.ofNullable(feeRecipient));
 
     // put the empty block in first
     final Block emptyBlock =
-        mergeBlockCreator.createBlock(Optional.of(Collections.emptyList()), random, timestamp);
+        mergeBlockCreator.createBlock(Optional.of(Collections.emptyList()), prevRandao, timestamp);
 
     Result result = validateBlock(emptyBlock);
     if (result.blockProcessingOutputs.isPresent()) {
@@ -202,7 +203,7 @@ public PayloadIdentifier preparePayload(
           result.errorMessage);
     }
 
-    tryToBuildBetterBlock(timestamp, random, payloadIdentifier, mergeBlockCreator);
+    tryToBuildBetterBlock(timestamp, prevRandao, payloadIdentifier, mergeBlockCreator);
 
     return payloadIdentifier;
   }

diff --git a/.../main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java b/.../main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java
@@ -33,7 +33,7 @@ public interface MergeMiningCoordinator extends MiningCoordinator {
   PayloadIdentifier preparePayload(
       final BlockHeader parentHeader,
       final Long timestamp,
-      final Bytes32 random,
+      final Bytes32 prevRandao,
       final Address feeRecipient);
 
   @Override

diff --git a/...e/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java b/...e/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/PayloadIdentifier.java
@@ -14,13 +14,15 @@
  */
 package org.hyperledger.besu.consensus.merge.blockcreation;
 
+import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.plugin.data.Quantity;
 
 import java.math.BigInteger;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonValue;
+import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt64;
 
 public class PayloadIdentifier implements Quantity {
@@ -36,8 +38,16 @@ public PayloadIdentifier(final Long payloadId) {
     this.val = UInt64.valueOf(Math.abs(payloadId));
   }
 
-  public static PayloadIdentifier forPayloadParams(final Hash parentHash, final Long timestamp) {
-    return new PayloadIdentifier(((long) parentHash.toHexString().hashCode()) ^ timestamp);
+  public static PayloadIdentifier forPayloadParams(
+      final Hash parentHash,
+      final Long timestamp,
+      final Bytes32 prevRandao,
+      final Address feeRecipient) {
+    return new PayloadIdentifier(
+        timestamp
+            ^ ((long) parentHash.toHexString().hashCode()) << 8
+            ^ ((long) prevRandao.toHexString().hashCode()) << 16
+            ^ ((long) feeRecipient.toHexString().hashCode()) << 24);
   }
 
   @Override

diff --git a/...c/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java b/...c/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java
@@ -127,9 +127,9 @@ public void changeTargetGasLimit(final Long targetGasLimit) {
   public PayloadIdentifier preparePayload(
       final BlockHeader parentHeader,
       final Long timestamp,
-      final Bytes32 random,
+      final Bytes32 prevRandao,
       final Address feeRecipient) {
-    return mergeCoordinator.preparePayload(parentHeader, timestamp, random, feeRecipient);
+    return mergeCoordinator.preparePayload(parentHeader, timestamp, prevRandao, feeRecipient);
   }
 
   @Override

diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java
@@ -131,21 +131,55 @@ public void candidateHeadIsInvalidIfBeforeFinalizedBlock() {
   @Test
   public void putAndRetrieveFirstPayload() {
     Block mockBlock = mock(Block.class);
+
     PayloadIdentifier firstPayloadId = new PayloadIdentifier(1L);
     postMergeContext.putPayloadById(firstPayloadId, mockBlock);
 
     assertThat(postMergeContext.retrieveBlockById(firstPayloadId)).contains(mockBlock);
   }
 
   @Test
-  public void puttingTwoBlocksWithTheSamePayloadIdWeRetrieveTheLatter() {
-    Block mockBlock1 = mock(Block.class);
-    Block mockBlock2 = mock(Block.class);
+  public void puttingTwoBlocksWithTheSamePayloadIdWeRetrieveTheBest() {
+    BlockHeader zeroTxBlockHeader = mock(BlockHeader.class);
+    when(zeroTxBlockHeader.getGasUsed()).thenReturn(0L);
+    Block zeroTxBlock = mock(Block.class);
+    when(zeroTxBlock.getHeader()).thenReturn(zeroTxBlockHeader);
+
+    BlockHeader betterBlockHeader = mock(BlockHeader.class);
+    when(betterBlockHeader.getGasUsed()).thenReturn(11L);
+    Block betterBlock = mock(Block.class);
+    when(betterBlock.getHeader()).thenReturn(betterBlockHeader);
+
+    PayloadIdentifier payloadId = new PayloadIdentifier(1L);
+    postMergeContext.putPayloadById(payloadId, zeroTxBlock);
+    postMergeContext.putPayloadById(payloadId, betterBlock);
+
+    assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlock);
+  }
+
+  @Test
+  public void puttingABlockWithTheSamePayloadIdSmallerThanAnExistingOneWeRetrieveTheBest() {
+    BlockHeader zeroTxBlockHeader = mock(BlockHeader.class);
+    when(zeroTxBlockHeader.getGasUsed()).thenReturn(0L);
+    Block zeroTxBlock = mock(Block.class);
+    when(zeroTxBlock.getHeader()).thenReturn(zeroTxBlockHeader);
+
+    BlockHeader betterBlockHeader = mock(BlockHeader.class);
+    when(betterBlockHeader.getGasUsed()).thenReturn(11L);
+    Block betterBlock = mock(Block.class);
+    when(betterBlock.getHeader()).thenReturn(betterBlockHeader);
+
+    BlockHeader smallBlockHeader = mock(BlockHeader.class);
+    when(smallBlockHeader.getGasUsed()).thenReturn(5L);
+    Block smallBlock = mock(Block.class);
+    when(smallBlock.getHeader()).thenReturn(smallBlockHeader);
+
     PayloadIdentifier payloadId = new PayloadIdentifier(1L);
-    postMergeContext.putPayloadById(payloadId, mockBlock1);
-    postMergeContext.putPayloadById(payloadId, mockBlock2);
+    postMergeContext.putPayloadById(payloadId, zeroTxBlock);
+    postMergeContext.putPayloadById(payloadId, betterBlock);
+    postMergeContext.putPayloadById(payloadId, smallBlock);
 
-    assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(mockBlock2);
+    assertThat(postMergeContext.retrieveBlockById(payloadId)).contains(betterBlock);
   }
 
   @Test