Explain and improve price validation for London transactions during s…

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Fields `publicKey` and `raw` removed from RPC API `Transaction` result object [#4575](https://github.com/hyperledger/besu/pull/4575)
 
 ### Additions and Improvements
+- Explain and improve price validation for London and local transactions during block proposal selection [#4602](https://github.com/hyperledger/besu/pull/4602)
 - Support for ephemeral testnet Shandong.  EIPs are still in flux, besu does not fully sync yet, and the network is subject to restarts. [#//FIXME](https://github.com/hyperledger/besu/pull///FIXME)
 
 ### Bug Fixes

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/response/GraphQLError.java

@@ -31,7 +31,7 @@ public enum GraphQLError {
   INTRINSIC_GAS_EXCEEDS_LIMIT(-32003, "Intrinsic gas exceeds gas limit"),
   TRANSACTION_UPFRONT_COST_EXCEEDS_BALANCE(-32004, "Upfront cost exceeds account balance"),
   EXCEEDS_BLOCK_GAS_LIMIT(-32005, "Transaction gas limit exceeds block gas limit"),
-  INCORRECT_NONCE(-32006, "Incorrect nonce"),
+  INCORRECT_NONCE(-32006, "Nonce too high"),
   TX_SENDER_NOT_AUTHORIZED(-32007, "Sender account not authorized to send transactions"),
   CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE(-32008, "Initial sync is still in progress"),
   GAS_PRICE_TOO_LOW(-32009, "Gas price below configured minimum gas price"),
@@ -76,8 +76,8 @@ public static GraphQLError of(final TransactionInvalidReason transactionInvalidR
       case NONCE_TOO_LOW:
       case PRIVATE_NONCE_TOO_LOW:
         return NONCE_TOO_LOW;
-      case INCORRECT_NONCE:
-      case INCORRECT_PRIVATE_NONCE:
+      case NONCE_TOO_HIGH:
+      case PRIVATE_NONCE_TOO_HIGH:
         return INCORRECT_NONCE;
       case INTRINSIC_GAS_EXCEEDS_GAS_LIMIT:
         return INTRINSIC_GAS_EXCEEDS_LIMIT;

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java

@@ -25,9 +25,9 @@ public static JsonRpcError convertTransactionInvalidReason(
       case NONCE_TOO_LOW:
       case PRIVATE_NONCE_TOO_LOW:
         return JsonRpcError.NONCE_TOO_LOW;
-      case INCORRECT_NONCE:
-      case INCORRECT_PRIVATE_NONCE:
-        return JsonRpcError.INCORRECT_NONCE;
+      case NONCE_TOO_HIGH:
+      case PRIVATE_NONCE_TOO_HIGH:
+        return JsonRpcError.NONCE_TOO_HIGH;
       case INVALID_SIGNATURE:
         return JsonRpcError.INVALID_TRANSACTION_SIGNATURE;
       case INTRINSIC_GAS_EXCEEDS_GAS_LIMIT:

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java

@@ -58,7 +58,7 @@ public enum JsonRpcError {
   INTRINSIC_GAS_EXCEEDS_LIMIT(-32003, "Intrinsic gas exceeds gas limit"),
   TRANSACTION_UPFRONT_COST_EXCEEDS_BALANCE(-32004, "Upfront cost exceeds account balance"),
   EXCEEDS_BLOCK_GAS_LIMIT(-32005, "Transaction gas limit exceeds block gas limit"),
-  INCORRECT_NONCE(-32006, "Incorrect nonce"),
+  NONCE_TOO_HIGH(-32006, "Nonce too high"),
   TX_SENDER_NOT_AUTHORIZED(-32007, "Sender account not authorized to send transactions"),
   CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE(-32008, "Initial sync is still in progress"),
   GAS_PRICE_TOO_LOW(-32009, "Gas price below configured minimum gas price"),

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverterTest.java

@@ -37,8 +37,8 @@ public static Collection<Object[]> expectedErrorMapping() {
         new Object[][] {
           {TransactionInvalidReason.NONCE_TOO_LOW, JsonRpcError.NONCE_TOO_LOW},
           {TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW, JsonRpcError.NONCE_TOO_LOW},
-          {TransactionInvalidReason.INCORRECT_NONCE, JsonRpcError.INCORRECT_NONCE},
-          {TransactionInvalidReason.INCORRECT_PRIVATE_NONCE, JsonRpcError.INCORRECT_NONCE},
+          {TransactionInvalidReason.NONCE_TOO_HIGH, JsonRpcError.NONCE_TOO_HIGH},
+          {TransactionInvalidReason.PRIVATE_NONCE_TOO_HIGH, JsonRpcError.NONCE_TOO_HIGH},
           {TransactionInvalidReason.INVALID_SIGNATURE, JsonRpcError.INVALID_TRANSACTION_SIGNATURE},
           {
             TransactionInvalidReason.INTRINSIC_GAS_EXCEEDS_GAS_LIMIT,

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthSendRawTransactionTest.java

@@ -135,7 +135,7 @@ public void transactionWithNonceBelowAccountNonceIsRejected() {
   @Test
   public void transactionWithNonceAboveAccountNonceIsRejected() {
     verifyErrorForInvalidTransaction(
-        TransactionInvalidReason.INCORRECT_NONCE, JsonRpcError.INCORRECT_NONCE);
+        TransactionInvalidReason.NONCE_TOO_HIGH, JsonRpcError.NONCE_TOO_HIGH);
   }
 
   @Test

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/eea/EeaSendRawTransactionTest.java

@@ -179,7 +179,7 @@ public void transactionWithNonceBelowAccountNonceIsRejected() {
   @Test
   public void transactionWithNonceAboveAccountNonceIsRejected() {
     verifyErrorForInvalidTransaction(
-        TransactionInvalidReason.INCORRECT_NONCE, JsonRpcError.INCORRECT_NONCE);
+        TransactionInvalidReason.NONCE_TOO_HIGH, JsonRpcError.NONCE_TOO_HIGH);
   }
 
   @Test

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/goquorum/GoQuorumSendRawPrivateTransactionTest.java

@@ -171,7 +171,7 @@ public void transactionWithNonceBelowAccountNonceIsRejected() {
   @Test
   public void transactionWithNonceAboveAccountNonceIsRejected() {
     verifyErrorForInvalidTransaction(
-        TransactionInvalidReason.INCORRECT_NONCE, JsonRpcError.INCORRECT_NONCE);
+        TransactionInvalidReason.NONCE_TOO_HIGH, JsonRpcError.NONCE_TOO_HIGH);
   }
 
   @Test

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

@@ -57,7 +57,7 @@
  * <p>If a transaction is suitable for inclusion, the world state must be updated, and a receipt
  * generated.
  *
- * <p>The output from this class's exeuction will be:
+ * <p>The output from this class's execution will be:
  *
  * <ul>
  *   <li>A list of transactions to include in the block being constructed.
@@ -275,19 +275,8 @@ private TransactionSelectionResult evaluateTransaction(
       }
     }
 
-    // If the gas price specified by the transaction is less than this node is willing to accept,
-    // do not include it in the block.
-    // ToDo: why we accept this in the pool in the first place then?
-    final Wei actualMinTransactionGasPriceInBlock =
-        feeMarket
-            .getTransactionPriceCalculator()
-            .price(transaction, processableBlockHeader.getBaseFee());
-    if (minTransactionGasPrice.compareTo(actualMinTransactionGasPriceInBlock) > 0) {
-      LOG.warn(
-          "Gas fee of {} lower than configured minimum {}, deleting",
-          transaction,
-          minTransactionGasPrice);
-      return TransactionSelectionResult.DELETE_TRANSACTION_AND_CONTINUE;
+    if (transactionCurrentPriceBelowMin(transaction)) {
+      return TransactionSelectionResult.CONTINUE;
     }
 
     final WorldUpdater worldStateUpdater = worldState.updater();
@@ -342,14 +331,44 @@ private TransactionSelectionResult evaluateTransaction(
     return TransactionSelectionResult.CONTINUE;
   }
 
+  private boolean transactionCurrentPriceBelowMin(final Transaction transaction) {
+    // Here we only care about EIP1159 since for Frontier and local transactions the checks
+    // that we do when accepting them in the pool are enough
+    if (transaction.getType().supports1559FeeMarket()
+        && !pendingTransactions.isLocalSender(transaction.getSender())) {
+
+      // For EIP1559 transactions, the price is dynamic and depends on network conditions, so we can
+      // only calculate at this time the current minimum price the transaction is willing to pay
+      // and if it is above the minimum accepted by the node.
+      // If below we do not delete the transaction, since when we added the transaction to the pool,
+      // we assured sure that the maxFeePerGas is >= of the minimum price accepted by the node
+      // and so the price of the transaction could satisfy this rule in the future
+      final Wei currentMinTransactionGasPriceInBlock =
+          feeMarket
+              .getTransactionPriceCalculator()
+              .price(transaction, processableBlockHeader.getBaseFee());
+      if (minTransactionGasPrice.compareTo(currentMinTransactionGasPriceInBlock) > 0) {
+        LOG.trace(
+            "Current gas fee of {} is lower than configured minimum {}, skipping",
+            transaction,
+            minTransactionGasPrice);
+        return true;
+      }
+    }
+    return false;
+  }
+
   private TransactionSelectionResult transactionSelectionResultForInvalidResult(
       final Transaction transaction,
       final ValidationResult<TransactionInvalidReason> invalidReasonValidationResult) {
-    // If the transaction has an incorrect nonce, leave it in the pool and continue
-    if (isIncorrectNonce(invalidReasonValidationResult)) {
+
+    final var invalidReason = invalidReasonValidationResult.getInvalidReason();
+    // If the invalid reason is transient, then leave the transaction in the pool and continue
+    if (isTransientValidationError(invalidReason)) {
       traceLambda(
           LOG,
-          "Incorrect nonce for transaction {} keeping it in the pool",
+          "Transient validation error {} for transaction {} keeping it in the pool",
+          invalidReason::toString,
           transaction::toTraceLog);
       return TransactionSelectionResult.CONTINUE;
     }
@@ -358,10 +377,15 @@ private TransactionSelectionResult transactionSelectionResultForInvalidResult(
         LOG,
         "Delete invalid transaction {}, reason {}",
         transaction::toTraceLog,
-        invalidReasonValidationResult::getInvalidReason);
+        invalidReason::toString);
     return TransactionSelectionResult.DELETE_TRANSACTION_AND_CONTINUE;
   }
 
+  private boolean isTransientValidationError(final TransactionInvalidReason invalidReason) {
+    return invalidReason.equals(TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE)
+        || invalidReason.equals(TransactionInvalidReason.NONCE_TOO_HIGH);
+  }
+
   private ValidationResult<TransactionInvalidReason> validateTransaction(
       final ProcessableBlockHeader blockHeader,
       final Transaction transaction,
@@ -410,7 +434,7 @@ private void updateTransactionResultTracking(
   }
 
   private boolean isIncorrectNonce(final ValidationResult<TransactionInvalidReason> result) {
-    return result.getInvalidReason().equals(TransactionInvalidReason.INCORRECT_NONCE);
+    return result.getInvalidReason().equals(TransactionInvalidReason.NONCE_TOO_HIGH);
   }
 
   private TransactionProcessingResult publicResultForWhenWeHaveAPrivateTransaction(