PR feedback: remove auto-cancellation of failed transactions

- remove auto-cancellation of failed transaction (failed during estimate gas) to prevent stuck txs
- replace it with an async lock during nonce sequencing + gas estimation
- simplified cancelTransaction (still exported) such that the new transaction is populated using populateTransaction, so that all gas and fees are reset
- moved reverting contract function into its own testing helpers module, and refactored any tests to use it
- updated the test helper reverts to check EstimateGasErrors

ethers/contract.nim

@@ -3,6 +3,7 @@ import std/macros
 import std/sequtils
 import pkg/chronos
 import pkg/contractabi
+import pkg/stew/byteutils
 import ./basics
 import ./provider
 import ./signer
@@ -117,38 +118,6 @@ 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,
@@ -156,7 +125,7 @@ proc send(contract: Contract,
          Future[?TransactionResponse] {.async.} =
   if signer =? contract.signer:
     let transaction = createTransaction(contract, function, parameters, overrides)
-    let populated = await contract.populateTransaction(transaction)
+    let populated = await signer.populateTransaction(transaction)
     let txResp = await signer.sendTransaction(populated)
     return txResp.some
   else:

ethers/providers/jsonrpc.nim

@@ -3,6 +3,7 @@ import std/tables
 import std/uri
 import pkg/json_rpc/rpcclient
 import pkg/json_rpc/errors
+import pkg/stew/byteutils
 import ../basics
 import ../provider
 import ../signer

ethers/signer.nim

@@ -68,7 +68,7 @@ method getChainId*(signer: Signer): Future[UInt256] {.base, gcsafe.} =
 
 func lastSeenNonce*(signer: Signer): ?UInt256 = signer.lastSeenNonce
 
-method getNonce*(signer: Signer): Future[UInt256] {.base, gcsafe, async.} =
+method getNonce(signer: Signer): Future[UInt256] {.base, gcsafe, async.} =
   var nonce = await signer.getTransactionCount(BlockTag.pending)
 
   if lastSeen =? signer.lastSeenNonce and lastSeen >= nonce:
@@ -86,7 +86,7 @@ method updateNonce*(
     signer.lastSeenNonce = some nonce
     return
 
-  if force or nonce > lastSeen:
+  if nonce > lastSeen:
     signer.lastSeenNonce = some nonce
 
 method decreaseNonce*(signer: Signer) {.base, gcsafe.} =
@@ -109,11 +109,11 @@ method populateTransaction*(signer: Signer,
 
   var populated = transaction
 
-  if transaction.sender.isNone:
+  if populated.sender.isNone:
     populated.sender = some(await signer.getAddress())
-  if transaction.chainId.isNone:
+  if populated.chainId.isNone:
     populated.chainId = some(await signer.getChainId())
-  if transaction.gasPrice.isNone and (populated.maxFee.isNone or populated.maxPriorityFee.isNone):
+  if transaction.gasPrice.isNone and (transaction.maxFee.isNone or transaction.maxPriorityFee.isNone):
     populated.gasPrice = some(await signer.getGasPrice())
 
   if transaction.nonce.isNone and transaction.gasLimit.isNone:

testmodule/testContracts.nim

@@ -9,14 +9,12 @@ import ./hardhat
 import ./helpers
 import ./miner
 import ./mocks
-import ./examples
 
 type
   TestToken = ref object of Erc20Token
 
 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"]:
 

testmodule/testTesting.nim

@@ -96,5 +96,14 @@ suite "Testing helpers - contracts":
     discard await provider.send("evm_revert", @[snapshot])
     await provider.close()
 
-  test "revert works with provider":
-    check await helpersContract.doRevert(revertReason).reverts(revertReason)
+  test "revert reason can be retrieved when transaction fails":
+    let txResp = helpersContract.doRevert(
+                  revertReason,
+                  # override gasLimit to skip estimating gas
+                  TransactionOverrides(gasLimit: some 10000000.u256)
+                )
+    check await txResp.confirm(1).reverts(revertReason)
+
+  test "revert reason can be retrieved when estimate gas fails":
+    let txResp = helpersContract.doRevert(revertReason)
+    check await txResp.reverts(revertReason)