From 27ade870ea47b63b5a3300e07289adf22052bd91 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Fri, 12 Apr 2024 12:32:20 -0600 Subject: [PATCH] Consistent nonce handling (#6941) Make handling of absurdly large nonces more consistent across txpool and EVM. Signed-off-by: Danno Ferrin --- .../besu/ethereum/mainnet/MainnetTransactionValidator.java | 2 +- .../besu/ethereum/privacy/PrivateTransactionValidator.java | 2 +- .../ethereum/mainnet/MainnetTransactionValidatorTest.java | 1 + .../layered/AbstractSequentialTransactionsLayer.java | 2 +- .../transactions/layered/LayeredPendingTransactions.java | 4 +++- .../eth/transactions/layered/SparseTransactions.java | 6 ++++-- .../transactions/sorter/PendingTransactionsForSender.java | 2 +- .../java/org/hyperledger/besu/evmtool/EvmToolCommand.java | 2 +- .../main/java/org/hyperledger/besu/evmtool/T8nExecutor.java | 2 +- .../besu/evm/processor/ContractCreationProcessor.java | 2 +- 10 files changed, 15 insertions(+), 10 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java index b23b056692d..8b1ae6ff6af 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java @@ -233,7 +233,7 @@ public ValidationResult 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/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java index 9bbfa6194be..11fe74d02e6 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java @@ -61,7 +61,7 @@ public ValidationResult 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/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java index 6ec75e0e645..92a353319bc 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java +++ b/ethereum/core/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/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractSequentialTransactionsLayer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractSequentialTransactionsLayer.java index 245f8916413..0afba82470f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractSequentialTransactionsLayer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/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/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java index 878c1b0793b..0192f1cd668 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java +++ b/ethereum/eth/src/main/java/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/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java index e036f2fd215..414f12b3d26 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java +++ b/ethereum/eth/src/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/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/PendingTransactionsForSender.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/PendingTransactionsForSender.java index f4cd95cba21..634727b9df2 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/PendingTransactionsForSender.java +++ b/ethereum/eth/src/main/java/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 index 014279bf4a6..6a8134e999e 100644 --- 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 index c3e831a25a1..683458bf7c0 100644 --- 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 index f18473b0bf2..987b72b3635 100644 --- 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