Merge branch 'main' into limit-trie-logs-false-for-full-sync

Signed-off-by: Simon Dudley <[email protected]>

CHANGELOG.md

@@ -25,10 +25,12 @@
 - Add trie log pruner metrics [#7352](https://github.com/hyperledger/besu/pull/7352)
 - Force bonsai-limit-trie-logs-enabled=false when sync-mode=FULL instead of startup error [#7357](https://github.com/hyperledger/besu/pull/7357)
 - `--Xbonsai-parallel-tx-processing-enabled` option enables executing transactions in parallel during block processing for Bonsai nodes
+- Add option `--poa-discovery-retry-bootnodes` for PoA networks to always use bootnodes during peer refresh, not just on first start [#7314](https://github.com/hyperledger/besu/pull/7314) 
 
 ### Bug fixes
 - Fix `eth_call` deserialization to correctly ignore unknown fields in the transaction object. [#7323](https://github.com/hyperledger/besu/pull/7323)
 - Prevent Besu from starting up with sync-mode=FULL and bonsai-limit-trie-logs-enabled=true for private networks [#7357](https://github.com/hyperledger/besu/pull/7357)
+- Avoid executing pruner preload during trie log subcommands [#7366](https://github.com/hyperledger/besu/pull/7366)
 
 ## 24.7.0
 

besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java

@@ -195,6 +195,7 @@ public class RunnerBuilder {
   private boolean legacyForkIdEnabled;
   private Optional<EnodeDnsConfiguration> enodeDnsConfiguration;
   private List<SubnetInfo> allowedSubnets = new ArrayList<>();
+  private boolean poaDiscoveryRetryBootnodes = true;
 
   /** Instantiates a new Runner builder. */
   public RunnerBuilder() {}
@@ -603,6 +604,17 @@ public RunnerBuilder allowedSubnets(final List<SubnetInfo> allowedSubnets) {
     return this;
   }
 
+  /**
+   * Flag to indicate if peer table refreshes should always query bootnodes
+   *
+   * @param poaDiscoveryRetryBootnodes whether to always query bootnodes
+   * @return the runner builder
+   */
+  public RunnerBuilder poaDiscoveryRetryBootnodes(final boolean poaDiscoveryRetryBootnodes) {
+    this.poaDiscoveryRetryBootnodes = poaDiscoveryRetryBootnodes;
+    return this;
+  }
+
   /**
    * Build Runner instance.
    *
@@ -625,6 +637,8 @@ public Runner build() {
         bootstrap = ethNetworkConfig.bootNodes();
       }
       discoveryConfiguration.setBootnodes(bootstrap);
+      discoveryConfiguration.setIncludeBootnodesOnPeerRefresh(
+          besuController.getGenesisConfigOptions().isPoa() && poaDiscoveryRetryBootnodes);
       LOG.info("Resolved {} bootnodes.", bootstrap.size());
       LOG.debug("Bootnodes = {}", bootstrap);
       discoveryConfiguration.setDnsDiscoveryURL(ethNetworkConfig.dnsDiscoveryUrl());
@@ -694,6 +708,7 @@ public Runner build() {
     final boolean fallbackEnabled = natMethod == NatMethod.AUTO || natMethodFallbackEnabled;
     final NatService natService = new NatService(buildNatManager(natMethod), fallbackEnabled);
     final NetworkBuilder inactiveNetwork = caps -> new NoopP2PNetwork();
+
     final NetworkBuilder activeNetwork =
         caps -> {
           return DefaultP2PNetwork.builder()

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -514,6 +514,19 @@ void setBannedNodeIds(final List<String> values) {
       }
     }
 
+    // Boolean option to set that in a PoA network the bootnodes should always be queried during
+    // peer table refresh. If this flag is disabled bootnodes are only sent FINDN requests on first
+    // startup, meaning that an offline bootnode or network outage at the client can prevent it
+    // discovering any peers without a restart.
+    @Option(
+        names = {"--poa-discovery-retry-bootnodes"},
+        description =
+            "Always use of bootnodes for discovery in PoA networks. Disabling this reverts "
+                + " to the same behaviour as non-PoA networks, where neighbours are only discovered from bootnodes on first startup."
+                + "(default: ${DEFAULT-VALUE})",
+        arity = "1")
+    private final Boolean poaDiscoveryRetryBootnodes = true;
+
     private Collection<Bytes> bannedNodeIds = new ArrayList<>();
 
     // Used to discover the default IP of the client.
@@ -2234,7 +2247,12 @@ private MiningParameters getMiningParameters() {
     return miningParameters;
   }
 
-  private DataStorageConfiguration getDataStorageConfiguration() {
+  /**
+   * Get the data storage configuration
+   *
+   * @return the data storage configuration
+   */
+  public DataStorageConfiguration getDataStorageConfiguration() {
     if (dataStorageConfiguration == null) {
       dataStorageConfiguration = dataStorageOptions.toDomainObject();
     }
@@ -2351,6 +2369,7 @@ private Runner synchronize(
             .rpcEndpointService(rpcEndpointServiceImpl)
             .enodeDnsConfiguration(getEnodeDnsConfiguration())
             .allowedSubnets(p2PDiscoveryOptionGroup.allowedSubnets)
+            .poaDiscoveryRetryBootnodes(p2PDiscoveryOptionGroup.poaDiscoveryRetryBootnodes)
             .build();
 
     addShutdownHook(runner);

besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java

@@ -28,6 +28,7 @@
 import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.storage.BonsaiWorldStateKeyValueStorage;
 import org.hyperledger.besu.ethereum.trie.diffbased.common.trielog.TrieLogPruner;
 import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration;
+import org.hyperledger.besu.ethereum.worldstate.ImmutableDataStorageConfiguration;
 import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
 
 import java.io.IOException;
@@ -82,7 +83,14 @@ public void run() {
   }
 
   private static BesuController createBesuController() {
-    return parentCommand.besuCommand.buildController();
+    final DataStorageConfiguration config = parentCommand.besuCommand.getDataStorageConfiguration();
+    // disable limit trie logs to avoid preloading during subcommand execution
+    return parentCommand
+        .besuCommand
+        .getControllerBuilder()
+        .dataStorageConfiguration(
+            ImmutableDataStorageConfiguration.copyOf(config).withBonsaiLimitTrieLogsEnabled(false))
+        .build();
   }
 
   @Command(

besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java

@@ -805,6 +805,28 @@ public void bootnodesUrlCliArgTakesPrecedenceOverGenesisFile() throws IOExceptio
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
 
+  @Test
+  public void poaDiscoveryRetryBootnodesValueTrueMustBeUsed() {
+    parseCommand("--poa-discovery-retry-bootnodes", "true");
+
+    verify(mockRunnerBuilder).poaDiscoveryRetryBootnodes(eq(true));
+    verify(mockRunnerBuilder).build();
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void poaDiscoveryRetryBootnodesValueFalseMustBeUsed() {
+    parseCommand("--poa-discovery-retry-bootnodes", "false");
+
+    verify(mockRunnerBuilder).poaDiscoveryRetryBootnodes(eq(false));
+    verify(mockRunnerBuilder).build();
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
   @Test
   public void callingWithBootnodesOptionButNoValueMustPassEmptyBootnodeList() {
     parseCommand("--bootnodes");

besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java

@@ -350,6 +350,7 @@ public void initMocks() throws Exception {
     when(mockRunnerBuilder.apiConfiguration(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.enodeDnsConfiguration(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.allowedSubnets(any())).thenReturn(mockRunnerBuilder);
+    when(mockRunnerBuilder.poaDiscoveryRetryBootnodes(anyBoolean())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.build()).thenReturn(mockRunner);
 
     final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();

besu/src/test/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommandTest.java

@@ -0,0 +1,74 @@
+/*
+ * 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.cli.subcommands.storage;
+
+import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.verify;
+
+import org.hyperledger.besu.cli.CommandTestAbstract;
+import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+class TrieLogSubCommandTest extends CommandTestAbstract {
+
+  @Test
+  void limitTrieLogsDefaultDisabledForAllSubcommands() {
+    assertTrieLogSubcommand("prune");
+    assertTrieLogSubcommand("count");
+    assertTrieLogSubcommand("import");
+    assertTrieLogSubcommand("export");
+  }
+
+  @Test
+  void limitTrieLogsDisabledForAllSubcommands() {
+    assertTrieLogSubcommandWithExplicitLimitEnabled("prune");
+    assertTrieLogSubcommandWithExplicitLimitEnabled("count");
+    assertTrieLogSubcommandWithExplicitLimitEnabled("import");
+    assertTrieLogSubcommandWithExplicitLimitEnabled("export");
+  }
+
+  private void assertTrieLogSubcommand(final String trieLogSubcommand) {
+    parseCommand("storage", "trie-log", trieLogSubcommand);
+    assertConfigurationIsDisabledBySubcommand();
+  }
+
+  private void assertTrieLogSubcommandWithExplicitLimitEnabled(final String trieLogSubcommand) {
+    parseCommand("--bonsai-limit-trie-logs-enabled=true", "storage", "trie-log", trieLogSubcommand);
+    assertConfigurationIsDisabledBySubcommand();
+  }
+
+  private void assertConfigurationIsDisabledBySubcommand() {
+    verify(mockControllerBuilder, atLeastOnce())
+        .dataStorageConfiguration(dataStorageConfigurationArgumentCaptor.capture());
+    final List<DataStorageConfiguration> configs =
+        dataStorageConfigurationArgumentCaptor.getAllValues();
+    assertThat(configs.get(0).getBonsaiLimitTrieLogsEnabled()).isTrue();
+    assertThat(configs.get(1).getBonsaiLimitTrieLogsEnabled()).isFalse();
+  }
+
+  @Test
+  void limitTrieLogsDefaultEnabledForBesuMainCommand() {
+    parseCommand();
+    verify(mockControllerBuilder, atLeastOnce())
+        .dataStorageConfiguration(dataStorageConfigurationArgumentCaptor.capture());
+    final List<DataStorageConfiguration> configs =
+        dataStorageConfigurationArgumentCaptor.getAllValues();
+    assertThat(configs).allMatch(DataStorageConfiguration::getBonsaiLimitTrieLogsEnabled);
+  }
+}

besu/src/test/resources/complete_config.toml

@@ -6,6 +6,7 @@ data-path="/opt/besu" # Path
 
 # network
 discovery-enabled=false
+poa-discovery-retry-bootnodes=true
 bootnodes=[
   "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",
   "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",

besu/src/test/resources/everything_config.toml

@@ -29,6 +29,7 @@ nat-method="NONE"
 Xnat-kube-service-name="besu"
 Xnat-method-fallback-enabled=true
 discovery-enabled=false
+poa-discovery-retry-bootnodes=true
 bootnodes=[
   "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",
   "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",

ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java

@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.blockcreation.txselection;
 
 import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.BLOCK_SELECTION_TIMEOUT;
+import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.INVALID_TX_EVALUATION_TOO_LONG;
 import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.SELECTED;
 import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.TX_EVALUATION_TOO_LONG;
 
@@ -419,11 +420,14 @@ private TransactionSelectionResult handleTransactionNotSelected(
 
     final var pendingTransaction = evaluationContext.getPendingTransaction();
 
-    // check if this tx took too much to evaluate, and in case remove it from the pool
+    // check if this tx took too much to evaluate, and in case it was invalid remove it from the
+    // pool, otherwise penalize it.
     final TransactionSelectionResult actualResult =
         isTimeout.get()
-            ? transactionTookTooLong(evaluationContext)
-                ? TX_EVALUATION_TOO_LONG
+            ? transactionTookTooLong(evaluationContext, selectionResult)
+                ? selectionResult.discard()
+                    ? INVALID_TX_EVALUATION_TOO_LONG
+                    : TX_EVALUATION_TOO_LONG
                 : BLOCK_SELECTION_TIMEOUT
             : selectionResult;
 
@@ -441,16 +445,21 @@ private TransactionSelectionResult handleTransactionNotSelected(
     return actualResult;
   }
 
-  private boolean transactionTookTooLong(final TransactionEvaluationContext evaluationContext) {
+  private boolean transactionTookTooLong(
+      final TransactionEvaluationContext evaluationContext,
+      final TransactionSelectionResult selectionResult) {
     final var evaluationTimer = evaluationContext.getEvaluationTimer();
     if (evaluationTimer.elapsed(TimeUnit.MILLISECONDS) > blockTxsSelectionMaxTime) {
       LOG.atWarn()
           .setMessage(
-              "Transaction {} is too late for inclusion, evaluated in {} that is over the max limit of {}ms"
-                  + ", removing it from the pool")
+              "Transaction {} is too late for inclusion, with result {}, evaluated in {} that is over the max limit of {}ms"
+                  + ", {}")
           .addArgument(evaluationContext.getPendingTransaction()::getHash)
+          .addArgument(selectionResult)
           .addArgument(evaluationTimer)
           .addArgument(blockTxsSelectionMaxTime)
+          .addArgument(
+              selectionResult.discard() ? "removing it from the pool" : "penalizing it in the pool")
           .log();
       return true;
     }

ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java

@@ -110,7 +110,8 @@ private TransactionSelectionResult transactionSelectionResultForInvalidResult(
    * @return True if the invalid reason is transient, false otherwise.
    */
   private boolean isTransientValidationError(final TransactionInvalidReason invalidReason) {
-    return invalidReason.equals(TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE)
+    return invalidReason.equals(TransactionInvalidReason.UPFRONT_COST_EXCEEDS_BALANCE)
+        || invalidReason.equals(TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE)
         || invalidReason.equals(TransactionInvalidReason.NONCE_TOO_HIGH);
   }
 }