Only cancels transactions if nonce has been incremented since the last estimateGas failure

ethers/contract.nim

@@ -8,6 +8,7 @@ import ./provider
 import ./signer
 import ./events
 import ./fields
+import ./exceptions
 
 export basics
 export provider
@@ -116,6 +117,38 @@ proc call(contract: Contract,
   let response = await contract.provider.call(transaction, overrides)
   return decodeResponse(ReturnType, returnMultiple, response)
 
+proc populateTransaction(
+  contract: Contract,
+  tx: Transaction
+): Future[Transaction] {.async.} =
+
+  if signer =? contract.signer:
+    try:
+      return await signer.populateTransaction(tx)
+    except EstimateGasError as e:
+      if nonce =? e.transaction.nonce:
+
+        if lastSeenNonce =? signer.lastSeenNonce and
+          lastSeenNonce > nonce:
+            discard await signer.cancelTransaction(e.transaction)
+            let revertReason = if not e.parent.isNil: e.parent.msg
+                               else: "unknown"
+            info "A cancellation transaction has been sent to prevent stuck " &
+              "transactions",
+              nonce = e.transaction.nonce,
+              revertReason
+
+        # nonce wasn't incremented by another transaction, so force update the
+        # lastSeenNonce
+        else:
+          signer.updateNonce(nonce - 1, force = true)
+          trace "nonce decremented -- estimateGas failed but no further " &
+            "nonces were generated. Prevents stuck txs.",
+            failedNonce = nonce,
+            newNonce = nonce - 1
+
+      raiseContractError e.msgStack
+
 proc send(contract: Contract,
           function: string,
           parameters: tuple,
@@ -123,7 +156,7 @@ proc send(contract: Contract,
          Future[?TransactionResponse] {.async.} =
   if signer =? contract.signer:
     let transaction = createTransaction(contract, function, parameters, overrides)
-    let populated = await signer.populateTransaction(transaction, cancelOnEstimateGasError = true)
+    let populated = await contract.populateTransaction(transaction)
     let txResp = await signer.sendTransaction(populated)
     return txResp.some
   else:

ethers/exceptions.nim

@@ -0,0 +1,7 @@
+import ./basics
+
+func msgStack*(error: ref EthersError): string =
+  var msg = error.msg
+  if not error.parent.isNil:
+    msg &= " -- Parent exception: " & error.parent.msg
+  return msg

ethers/providers/jsonrpc.nim

@@ -240,7 +240,8 @@ method signMessage*(signer: JsonRpcSigner,
 method sendTransaction*(signer: JsonRpcSigner,
                         transaction: Transaction): Future[TransactionResponse] {.async.} =
   convertError:
-    signer.updateNonce(transaction.nonce)
+    if nonce =? transaction.nonce:
+      signer.updateNonce(nonce)
     let
       client = await signer.provider.client
       hash = await client.eth_sendTransaction(transaction)

ethers/signer.nim

@@ -12,7 +12,10 @@ type
   Signer* = ref object of RootObj
     lastSeenNonce: ?UInt256
 
-type SignerError* = object of EthersError
+type
+  SignerError* = object of EthersError
+  EstimateGasError* = object of SignerError
+    transaction*: Transaction
 
 template raiseSignerError(message: string, parent: ref ProviderError = nil) =
   raise newException(SignerError, message, parent)
@@ -49,27 +52,35 @@ method estimateGas*(signer: Signer,
 method getChainId*(signer: Signer): Future[UInt256] {.base, gcsafe.} =
   signer.provider.getChainId()
 
-method getNonce(signer: Signer): Future[UInt256] {.base, gcsafe, async.} =
+func lastSeenNonce*(signer: Signer): ?UInt256 = signer.lastSeenNonce
+
+method getNonce*(signer: Signer): Future[UInt256] {.base, gcsafe, async.} =
   var nonce = await signer.getTransactionCount(BlockTag.pending)
-  
+
   if lastSeen =? signer.lastSeenNonce and lastSeen >= nonce:
     nonce = (lastSeen + 1.u256)
   signer.lastSeenNonce = some nonce
-  
+
   return nonce
 
-method updateNonce*(signer: Signer, nonce: ?UInt256) {.base, gcsafe.} =
-  without nonce =? nonce:
-    return
+method updateNonce*(
+  signer: Signer,
+  nonce: UInt256,
+  force = false
+) {.base, gcsafe.} =
+  ## When true, force updates nonce without first checking if it is higher than
+  ## the last seen nonce. NOTE: This should ONLY be used when absolutely needed,
+  ## eg when estimateGas fails, but no other nonces have been generated between
+  ## the estimateGas and updateNonce calls
 
   without lastSeen =? signer.lastSeenNonce:
     signer.lastSeenNonce = some nonce
     return
 
-  if nonce > lastSeen:
+  if force or nonce > lastSeen:
     signer.lastSeenNonce = some nonce
 
-method cancelTransaction(
+method cancelTransaction*(
   signer: Signer,
   tx: Transaction
 ): Future[TransactionResponse] {.async, base.} =
@@ -98,8 +109,7 @@ method cancelTransaction(
   return await signer.sendTransaction(cancelTx)
 
 method populateTransaction*(signer: Signer,
-                            transaction: Transaction,
-                            cancelOnEstimateGasError = false):
+                            transaction: Transaction):
                            Future[Transaction] {.base, async.} =
 
   if sender =? transaction.sender and sender != await signer.getAddress():
@@ -121,10 +131,10 @@ method populateTransaction*(signer: Signer,
     try:
       populated.gasLimit = some(await signer.estimateGas(populated))
     except ProviderError as e:
-      # send a 0-valued transaction with the errored nonce to prevent stuck txs
-      discard await signer.cancelTransaction(populated)
-      raiseSignerError "Estimate gas failed -- A cancellation transaction " &
-        "has been sent to prevent stuck transactions. See parent exception " &
-        "for revert reason.", e
+      let e = (ref EstimateGasError)(
+        msg: "Estimate gas failed",
+        transaction: populated,
+        parent: e)
+      raise e
 
   return populated

ethers/testing.nim

@@ -33,8 +33,9 @@ proc reverts*[T](call: Future[T], reason: string): Future[bool] {.async.} =
     else:
       discard await call
     return false
-  except EthersError as error:
-    var passed = reason == error.revertReason
+  except ProviderError, SignerError:
+    let error = getCurrentException()
+    var passed = reason == (ref EthersError)(error).revertReason
     if not passed and
        not error.parent.isNil and
        error.parent of (ref EthersError):

ethers/wallet.nim

@@ -70,5 +70,6 @@ proc signTransaction*(wallet: Wallet,
 
 method sendTransaction*(wallet: Wallet, transaction: Transaction): Future[TransactionResponse] {.async.} =
   let signed = await signTransaction(wallet, transaction)
-  wallet.updateNonce(transaction.nonce)
+  if nonce =? transaction.nonce:
+    wallet.updateNonce(nonce)
   return await provider(wallet).sendTransaction(signed)

testmodule/testContracts.nim

@@ -16,6 +16,7 @@ type
 
 method mint(token: TestToken, holder: Address, amount: UInt256): ?TransactionResponse {.base, contract.}
 method myBalance(token: TestToken): UInt256 {.base, contract, view.}
+method doRevert(token: TestToken, reason: string): ?TransactionResponse {.base, contract.}
 
 for url in ["ws://localhost:8545", "http://localhost:8545"]:
 
@@ -238,3 +239,33 @@ for url in ["ws://localhost:8545", "http://localhost:8545"]:
       check logs == @[
         Transfer(receiver: accounts[0], value: 100.u256)
       ]
+
+    test "transactions are cancelled once nonce has been incremented to prevent stuck transactions":
+      let signer = provider.getSigner()
+      token = TestToken.new(token.address, signer)
+
+      # emulate concurrent getNonce calls
+      let nonce0 = await signer.getNonce()
+      let nonce1 = await signer.getNonce()
+
+      expect ContractError:
+        discard await token.doRevert("some reason", TransactionOverrides(nonce: some nonce0))
+
+      let receipt = await token
+        .mint(accounts[0], 100.u256, TransactionOverrides(nonce: some nonce1))
+        .confirm(1)
+
+      check receipt.status == TransactionStatus.Success
+
+    test "transactions are not cancelled if nonce has not been incremented":
+      let signer = provider.getSigner()
+      token = TestToken.new(token.address, signer)
+
+      expect ContractError:
+        discard await token.doRevert("some reason")
+
+      let receipt = await token
+        .mint(accounts[0], 100.u256)
+        .confirm(1)
+
+      check receipt.status == TransactionStatus.Success

testnode/contracts/TestToken.sol

@@ -21,4 +21,9 @@ contract TestToken is ERC20 {
   function myBalance() public view returns (uint256)  {
     return balanceOf(msg.sender);
   }
+
+  function doRevert(string memory reason) public {
+    // Revert every tx with given reason
+    require(false, reason);
+  }
 }