diff --git a/ethers/contract.nim b/ethers/contract.nim index 41de35c..db6e68e 100644 --- a/ethers/contract.nim +++ b/ethers/contract.nim @@ -90,17 +90,17 @@ proc decodeResponse(T: type, bytes: seq[byte]): T = proc call(provider: Provider, transaction: Transaction, - overrides: TransactionOverrides): Future[seq[byte]] = + overrides: TransactionOverrides): Future[seq[byte]] {.async: (raises: [ProviderError]).} = if overrides of CallOverrides and blockTag =? CallOverrides(overrides).blockTag: - provider.call(transaction, blockTag) + await provider.call(transaction, blockTag) else: - provider.call(transaction) + await provider.call(transaction) proc call(contract: Contract, function: string, parameters: tuple, - overrides = TransactionOverrides()) {.async.} = + overrides = TransactionOverrides()) {.async: (raises: [ProviderError, SignerError]).} = var transaction = createTransaction(contract, function, parameters, overrides) if signer =? contract.signer and transaction.sender.isNone: @@ -112,7 +112,7 @@ proc call(contract: Contract, function: string, parameters: tuple, ReturnType: type, - overrides = TransactionOverrides()): Future[ReturnType] {.async.} = + overrides = TransactionOverrides()): Future[ReturnType] {.async: (raises: [ProviderError, SignerError, ContractError]).} = var transaction = createTransaction(contract, function, parameters, overrides) if signer =? contract.signer and transaction.sender.isNone: @@ -121,16 +121,20 @@ proc call(contract: Contract, let response = await contract.provider.call(transaction, overrides) return decodeResponse(ReturnType, response) -proc send(contract: Contract, - function: string, - parameters: tuple, - overrides = TransactionOverrides()): - Future[?TransactionResponse] {.async.} = +proc send( + contract: Contract, + function: string, + parameters: tuple, + overrides = TransactionOverrides() +): Future[?TransactionResponse] {.async: (raises: [AsyncLockError, CancelledError, CatchableError]).} = + if signer =? contract.signer: - let transaction = createTransaction(contract, function, parameters, overrides) - let populated = await signer.populateTransaction(transaction) - var txResp = await signer.sendTransaction(populated) - return txResp.some + withLock(signer): + let transaction = createTransaction(contract, function, parameters, overrides) + let populated = await signer.populateTransaction(transaction) + trace "sending contract transaction", function, params = $parameters + let txResp = await signer.sendTransaction(populated) + return txResp.some else: await call(contract, function, parameters, overrides) return TransactionResponse.none diff --git a/ethers/providers/jsonrpc.nim b/ethers/providers/jsonrpc.nim index 33442f8..b1e653e 100644 --- a/ethers/providers/jsonrpc.nim +++ b/ethers/providers/jsonrpc.nim @@ -327,8 +327,6 @@ method sendTransaction*( {.async: (raises:[SignerError, ProviderError]).} = convertError: - 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 b38b6ed..86e9093 100644 --- a/ethers/signer.nim +++ b/ethers/signer.nim @@ -8,7 +8,6 @@ export basics type Signer* = ref object of RootObj - lastSeenNonce: ?UInt256 populateLock: AsyncLock SignerError* = object of EthersError @@ -81,34 +80,26 @@ method getChainId*( method getNonce( signer: Signer): Future[UInt256] {.base, async: (raises: [SignerError, ProviderError]).} = - var nonce = await signer.getTransactionCount(BlockTag.pending) + return await signer.getTransactionCount(BlockTag.pending) - if lastSeen =? signer.lastSeenNonce and lastSeen >= nonce: - nonce = (lastSeen + 1.u256) - signer.lastSeenNonce = some nonce +template withLock*(signer: Signer, body: untyped) = + if signer.populateLock.isNil: + signer.populateLock = newAsyncLock() - return nonce - -method updateNonce*( - signer: Signer, - nonce: UInt256 -) {.base, gcsafe.} = - - without lastSeen =? signer.lastSeenNonce: - signer.lastSeenNonce = some nonce - return - - if nonce > lastSeen: - signer.lastSeenNonce = some nonce - -method decreaseNonce*(signer: Signer) {.base, gcsafe.} = - if lastSeen =? signer.lastSeenNonce and lastSeen > 0: - signer.lastSeenNonce = some lastSeen - 1 + await signer.populateLock.acquire() + try: + body + finally: + signer.populateLock.release() method populateTransaction*( signer: Signer, transaction: Transaction): Future[Transaction] - {.base, async: (raises: [CancelledError, AsyncLockError, ProviderError, SignerError]).} = + {.base, async: (raises: [CancelledError, ProviderError, SignerError]).} = + ## Populates a transaction with sender, chainId, gasPrice, nonce, and gasLimit. + ## NOTE: to avoid async concurrency issues, this routine should be called with + ## a lock if it is followed by sendTransaction. For reference, see the `send` + ## function in contract.nim. var address: Address convertError: @@ -119,20 +110,14 @@ method populateTransaction*( if chainId =? transaction.chainId and chainId != await signer.getChainId(): raiseSignerError("chain id mismatch") - if signer.populateLock.isNil: - signer.populateLock = newAsyncLock() - - await signer.populateLock.acquire() - var populated = transaction - try: - if transaction.sender.isNone: - populated.sender = some(address) - if transaction.chainId.isNone: - populated.chainId = some(await signer.getChainId()) - if transaction.gasPrice.isNone and (transaction.maxFee.isNone or transaction.maxPriorityFee.isNone): - populated.gasPrice = some(await signer.getGasPrice()) + if transaction.sender.isNone: + populated.sender = some(address) + if transaction.chainId.isNone: + populated.chainId = some(await signer.getChainId()) + 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: # when both nonce and gasLimit are not populated, we must ensure getNonce is @@ -143,27 +128,23 @@ method populateTransaction*( try: populated.gasLimit = some(await signer.estimateGas(populated, BlockTag.pending)) except EstimateGasError as e: - signer.decreaseNonce() raise e except ProviderError as e: - signer.decreaseNonce() raiseSignerError(e.msg) - else: - if transaction.nonce.isNone: - populated.nonce = some(await signer.getNonce()) - if transaction.gasLimit.isNone: - populated.gasLimit = some(await signer.estimateGas(populated, BlockTag.pending)) - - finally: - signer.populateLock.release() + else: + if transaction.nonce.isNone: + let nonce = await signer.getNonce() + populated.nonce = some nonce + if transaction.gasLimit.isNone: + populated.gasLimit = some(await signer.estimateGas(populated, BlockTag.pending)) return populated method cancelTransaction*( signer: Signer, tx: Transaction -): Future[TransactionResponse] {.base, async: (raises: [SignerError, ProviderError]).} = +): Future[TransactionResponse] {.base, async: (raises: [SignerError, CancelledError, AsyncLockError, ProviderError]).} = # cancels a transaction by sending with a 0-valued transaction to ourselves # with the failed tx's nonce @@ -172,7 +153,8 @@ method cancelTransaction*( without nonce =? tx.nonce: raiseSignerError "transaction must have nonce" - var cancelTx = Transaction(to: sender, value: 0.u256, nonce: some nonce) - convertError: - cancelTx = await signer.populateTransaction(cancelTx) - return await signer.sendTransaction(cancelTx) + withLock(signer): + convertError: + var cancelTx = Transaction(to: sender, value: 0.u256, nonce: some nonce) + cancelTx = await signer.populateTransaction(cancelTx) + return await signer.sendTransaction(cancelTx) diff --git a/ethers/signers/wallet.nim b/ethers/signers/wallet.nim index edc2254..752ae25 100644 --- a/ethers/signers/wallet.nim +++ b/ethers/signers/wallet.nim @@ -86,6 +86,4 @@ method sendTransaction*( {.async: (raises:[SignerError, ProviderError]).} = let signed = await signTransaction(wallet, transaction) - if nonce =? transaction.nonce: - wallet.updateNonce(nonce) return await provider(wallet).sendTransaction(signed) diff --git a/testmodule/providers/jsonrpc/testJsonRpcSigner.nim b/testmodule/providers/jsonrpc/testJsonRpcSigner.nim index e63a9a3..df2e824 100644 --- a/testmodule/providers/jsonrpc/testJsonRpcSigner.nim +++ b/testmodule/providers/jsonrpc/testJsonRpcSigner.nim @@ -84,24 +84,3 @@ suite "JsonRpcSigner": transaction.chainId = 0xdeadbeef.u256.some expect SignerError: discard await signer.populateTransaction(transaction) - - test "concurrent populate calls increment nonce": - let signer = provider.getSigner() - let count = await signer.getTransactionCount(BlockTag.pending) - var transaction1 = Transaction.example - var transaction2 = Transaction.example - var transaction3 = Transaction.example - - let populated = await allFinished( - signer.populateTransaction(transaction1), - signer.populateTransaction(transaction2), - signer.populateTransaction(transaction3) - ) - - transaction1 = await populated[0] - transaction2 = await populated[1] - transaction3 = await populated[2] - - check !transaction1.nonce == count - check !transaction2.nonce == count + 1.u256 - check !transaction3.nonce == count + 2.u256 diff --git a/testmodule/testContracts.nim b/testmodule/testContracts.nim index ed2158a..f7ef78e 100644 --- a/testmodule/testContracts.nim +++ b/testmodule/testContracts.nim @@ -272,3 +272,44 @@ for url in ["ws://" & providerUrl, "http://" & providerUrl]: .confirm(1) check receipt.status == TransactionStatus.Success + + test "can cancel procs that execute transactions": + let signer = provider.getSigner() + let token = TestToken.new(token.address, signer) + let countBefore = await signer.getTransactionCount(BlockTag.pending) + + proc executeTx {.async.} = + discard await token.mint(accounts[0], 100.u256) + + await executeTx().cancelAndWait() + let countAfter = await signer.getTransactionCount(BlockTag.pending) + check countBefore == countAfter + + test "concurrent transactions succeed even if one is cancelled": + let signer = provider.getSigner() + let token = TestToken.new(token.address, signer) + let balanceBefore = await token.myBalance() + + proc executeTx: Future[Confirmable] {.async.} = + return await token.mint(accounts[0], 100.u256) + + proc executeTxWithCancellation: Future[Confirmable] {.async.} = + let fut = token.mint(accounts[0], 100.u256) + fut.cancelSoon() + return await fut + + # emulate concurrent populateTransaction/sendTransaction calls, where the + # first one fails + let futs = await allFinished( + executeTxWithCancellation(), + executeTx(), + executeTx() + ) + let receipt1 = await futs[1].confirm(0) + let receipt2 = await futs[2].confirm(0) + + check receipt1.status == TransactionStatus.Success + check receipt2.status == TransactionStatus.Success + + let balanceAfter = await token.myBalance() + check balanceAfter == balanceBefore + 200.u256 \ No newline at end of file