Fixed nonce bug in EthGetTransactionCount (hyperledger#2633)

* Fixed bug where the EthGetTransactionCount would return a lower value for pending than latest when there are old transactions in the transaction pool. Wrote a failing test to show the expected behaviour and ensured that the max of latest and pending is used. Signed-off-by: Rob Dawson <rob.dawson@consensys.net> Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>
yudongpo · Aug 16, 2021 · dc8127f · dc8127f
1 parent 508dff4
commit dc8127f
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 
 ### Bug Fixes
 - Consider effective price and effective priority fee in transaction replacement rules [\#2529](https://github.com/hyperledger/besu/issues/2529)
+- GetTransactionCount should return the latest transaction count if it is greater than the transaction pool [\#2633](https://github.com/hyperledger/besu/pull/2633) 
 
 ### Early Access Features
 

diff --git a/...va/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionCount.java b/...va/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionCount.java
@@ -61,11 +61,11 @@ protected BlockParameterOrBlockHash blockParameterOrBlockHash(
   protected Object pendingResult(final JsonRpcRequestContext request) {
     final Address address = request.getRequiredParameter(0, Address.class);
     final OptionalLong pendingNonce = pendingTransactions.get().getNextNonceForSender(address);
-    if (pendingNonce.isPresent()) {
-      return Quantity.create(pendingNonce.getAsLong());
-    } else {
-      return latestResult(request);
-    }
+    final long latestNonce =
+        getBlockchainQueries()
+            .getTransactionCount(
+                address, getBlockchainQueries().getBlockchain().getChainHead().getHash());
+    return Quantity.create(Math.max(pendingNonce.orElse(0), latestNonce));
   }
 
   @Override

diff --git a/...rg/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionCountTest.java b/...rg/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionCountTest.java
@@ -46,8 +46,9 @@ public class EthGetTransactionCountTest {
 
   @Test
   public void shouldUsePendingTransactionsWhenToldTo() {
-    when(pendingTransactions.getNextNonceForSender(Address.fromHexString(pendingTransactionString)))
-        .thenReturn(OptionalLong.of(12));
+    final Address address = Address.fromHexString(pendingTransactionString);
+    when(pendingTransactions.getNextNonceForSender(address)).thenReturn(OptionalLong.of(12));
+    mockGetTransactionCount(address, 7L);
     final JsonRpcRequestContext request =
         new JsonRpcRequestContext(
             new JsonRpcRequest("1", "eth_getTransactionCount", pendingParams));
@@ -60,15 +61,33 @@ public void shouldUsePendingTransactionsWhenToldTo() {
   public void shouldUseLatestTransactionsWhenNoPendingTransactions() {
     final Address address = Address.fromHexString(pendingTransactionString);
     when(pendingTransactions.getNextNonceForSender(address)).thenReturn(OptionalLong.empty());
-    when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
-    when(blockchainQueries.getBlockchain().getChainHead()).thenReturn(chainHead);
-    when(blockchainQueries.getBlockchain().getChainHead().getHash()).thenReturn(Hash.ZERO);
-    when(blockchainQueries.getTransactionCount(address, Hash.ZERO)).thenReturn(7L);
+    mockGetTransactionCount(address, 7L);
     final JsonRpcRequestContext request =
         new JsonRpcRequestContext(
             new JsonRpcRequest("1", "eth_getTransactionCount", pendingParams));
     final JsonRpcSuccessResponse response =
         (JsonRpcSuccessResponse) ethGetTransactionCount.response(request);
     assertThat(response.getResult()).isEqualTo("0x7");
   }
+
+  @Test
+  public void shouldUseLatestWhenItIsBiggerThanPending() {
+    final Address address = Address.fromHexString(pendingTransactionString);
+    mockGetTransactionCount(address, 8);
+    when(pendingTransactions.getNextNonceForSender(Address.fromHexString(pendingTransactionString)))
+        .thenReturn(OptionalLong.of(4));
+    final JsonRpcRequestContext request =
+        new JsonRpcRequestContext(
+            new JsonRpcRequest("1", "eth_getTransactionCount", pendingParams));
+    final JsonRpcSuccessResponse response =
+        (JsonRpcSuccessResponse) ethGetTransactionCount.response(request);
+    assertThat(response.getResult()).isEqualTo("0x8");
+  }
+
+  private void mockGetTransactionCount(final Address address, final long transactionCount) {
+    when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
+    when(blockchainQueries.getBlockchain().getChainHead()).thenReturn(chainHead);
+    when(blockchainQueries.getBlockchain().getChainHead().getHash()).thenReturn(Hash.ZERO);
+    when(blockchainQueries.getTransactionCount(address, Hash.ZERO)).thenReturn(transactionCount);
+  }
 }