From 73963e614647a893abc366cc99b6cdc751e28454 Mon Sep 17 00:00:00 2001 From: Eric <5089238+emizzle@users.noreply.github.com> Date: Mon, 2 Oct 2023 07:34:58 +0300 Subject: [PATCH] Only cancels transactions if nonce has been incremented since the last estimateGas failure --- ethers/contract.nim | 35 +++++++++++++++++++++++++- ethers/exceptions.nim | 7 ++++++ ethers/providers/jsonrpc.nim | 3 ++- ethers/signer.nim | 42 ++++++++++++++++++++------------ ethers/testing.nim | 5 ++-- ethers/wallet.nim | 3 ++- testmodule/testContracts.nim | 31 +++++++++++++++++++++++ testnode/contracts/TestToken.sol | 5 ++++ 8 files changed, 110 insertions(+), 21 deletions(-) create mode 100644 ethers/exceptions.nim diff --git a/ethers/contract.nim b/ethers/contract.nim index f362581..851af30 100644 --- a/ethers/contract.nim +++ b/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: diff --git a/ethers/exceptions.nim b/ethers/exceptions.nim new file mode 100644 index 0000000..dfecb2e --- /dev/null +++ b/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 \ No newline at end of file diff --git a/ethers/providers/jsonrpc.nim b/ethers/providers/jsonrpc.nim index 6bf01ee..ba30925 100644 --- a/ethers/providers/jsonrpc.nim +++ b/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) diff --git a/ethers/signer.nim b/ethers/signer.nim index b414f02..f0d7916 100644 --- a/ethers/signer.nim +++ b/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 diff --git a/ethers/testing.nim b/ethers/testing.nim index dcf48de..1304dd5 100644 --- a/ethers/testing.nim +++ b/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): diff --git a/ethers/wallet.nim b/ethers/wallet.nim index 22e13e5..72177be 100644 --- a/ethers/wallet.nim +++ b/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) diff --git a/testmodule/testContracts.nim b/testmodule/testContracts.nim index d9cbf19..fb49262 100644 --- a/testmodule/testContracts.nim +++ b/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 diff --git a/testnode/contracts/TestToken.sol b/testnode/contracts/TestToken.sol index 99e6e56..d971532 100644 --- a/testnode/contracts/TestToken.sol +++ b/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); + } }