Consistent nonce handling (#6941)

Make handling of absurdly large nonces more consistent across txpool and EVM. Signed-off-by: Danno Ferrin <danno@numisight.com>
hyperledger · Apr 12, 2024 · 27ade87 · 27ade87
1 parent bc04751
commit 27ade87
Show file tree

Hide file tree

Showing 10 changed files with 15 additions and 10 deletions.
diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java b/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java
@@ -233,7 +233,7 @@ public ValidationResult<TransactionInvalidReason> validateForSender(
               upfrontCost.toQuantityHexString(), senderBalance.toQuantityHexString()));
     }
 
-    if (transaction.getNonce() < senderNonce) {
+    if (Long.compareUnsigned(transaction.getNonce(), senderNonce) < 0) {
       return ValidationResult.invalid(
           TransactionInvalidReason.NONCE_TOO_LOW,
           String.format(

diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java b/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java
@@ -61,7 +61,7 @@ public ValidationResult<TransactionInvalidReason> validate(
 
     LOG.debug("Validating actual nonce {}, with expected nonce {}", transactionNonce, accountNonce);
 
-    if (accountNonce > transactionNonce) {
+    if (Long.compareUnsigned(accountNonce, transactionNonce) > 0) {
       final String errorMessage =
           String.format(
               "Private Transaction nonce %s, is lower than sender account nonce %s.",

diff --git a/.../src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java b/.../src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java
@@ -81,6 +81,7 @@ public class MainnetTransactionValidatorTest {
 
   private final Transaction basicTransaction =
       new TransactionTestFixture()
+          .nonce(30)
           .chainId(Optional.of(BigInteger.ONE))
           .createTransaction(senderKeys);
 

diff --git a/...perledger/besu/ethereum/eth/transactions/layered/AbstractSequentialTransactionsLayer.java b/...perledger/besu/ethereum/eth/transactions/layered/AbstractSequentialTransactionsLayer.java
@@ -47,7 +47,7 @@ public void remove(final PendingTransaction invalidatedTx, final RemovalReason r
 
     final var senderTxs = txsBySender.get(invalidatedTx.getSender());
     final long invalidNonce = invalidatedTx.getNonce();
-    if (senderTxs != null && invalidNonce <= senderTxs.lastKey()) {
+    if (senderTxs != null && Long.compareUnsigned(invalidNonce, senderTxs.lastKey()) <= 0) {
       // on sequential layers we need to push to next layer all the txs following the invalid one,
       // even if it belongs to a previous layer
 

diff --git a/...va/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java b/...va/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java
@@ -339,7 +339,9 @@ public synchronized void selectTransactions(
                     .filter(
                         candidatePendingTx ->
                             !alreadyChecked.contains(candidatePendingTx.getHash())
-                                && candidatePendingTx.getNonce() <= highPrioPendingTx.getNonce())
+                                && Long.compareUnsigned(
+                                        candidatePendingTx.getNonce(), highPrioPendingTx.getNonce())
+                                    <= 0)
                     .forEach(
                         candidatePendingTx -> {
                           alreadyChecked.add(candidatePendingTx.getHash());

diff --git a/.../main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java b/.../main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java
@@ -101,7 +101,9 @@ protected TransactionAddedResult canAdd(
             orderByGap.get(gap).add(pendingTransaction);
             return gap;
           }
-          if (pendingTransaction.getNonce() < txsBySender.get(sender).firstKey()) {
+          if (Long.compareUnsigned(
+                  pendingTransaction.getNonce(), txsBySender.get(sender).firstKey())
+              < 0) {
             orderByGap.get(currGap).remove(sender);
             orderByGap.get(gap).add(pendingTransaction);
             return gap;
@@ -431,7 +433,7 @@ protected void internalConsistencyCheck(
 
               while (itNonce.hasNext()) {
                 final long currNonce = itNonce.next().getKey();
-                assert prevNonce < currNonce : "non incremental nonce";
+                assert Long.compareUnsigned(prevNonce, currNonce) < 0 : "non incremental nonce";
                 prevNonce = currNonce;
               }
             });

diff --git a/...a/org/hyperledger/besu/ethereum/eth/transactions/sorter/PendingTransactionsForSender.java b/...a/org/hyperledger/besu/ethereum/eth/transactions/sorter/PendingTransactionsForSender.java
@@ -42,7 +42,7 @@ public void trackPendingTransaction(final PendingTransaction pendingTransaction)
     synchronized (pendingTransactions) {
       if (!pendingTransactions.isEmpty()) {
         final long expectedNext = pendingTransactions.lastKey() + 1;
-        if (nonce > (expectedNext) && nextGap.isEmpty()) {
+        if (Long.compareUnsigned(nonce, expectedNext) > 0 && nextGap.isEmpty()) {
           nextGap = OptionalLong.of(expectedNext);
         }
       }

diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java
@@ -519,7 +519,7 @@ public static void dumpWorldState(final WorldState worldState, final PrintWriter
                 out.println("  },");
               }
               out.print("  \"balance\": \"" + account.getBalance().toShortHexString() + "\"");
-              if (account.getNonce() > 0) {
+              if (account.getNonce() != 0) {
                 out.println(",");
                 out.println(
                     "  \"nonce\": \""

diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java
@@ -448,7 +448,7 @@ static T8nResult runTest(
                                 accountStorageEntry.getValue().toHexString()));
               }
               accountObject.put("balance", account.getBalance().toShortHexString());
-              if (account.getNonce() > 0) {
+              if (account.getNonce() != 0) {
                 accountObject.put(
                     "nonce", Bytes.ofUnsignedLong(account.getNonce()).toShortHexString());
               }

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java b/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java
@@ -98,7 +98,7 @@ public ContractCreationProcessor(
   private static boolean accountExists(final Account account) {
     // The account exists if it has sent a transaction
     // or already has its code initialized.
-    return account.getNonce() > 0 || !account.getCode().isEmpty();
+    return account.getNonce() != 0 || !account.getCode().isEmpty();
   }
 
   @Override