From a513d66f9b4c8c64603b01be3999e7f481e7f117 Mon Sep 17 00:00:00 2001 From: Eric <5089238+emizzle@users.noreply.github.com> Date: Wed, 16 Oct 2024 20:06:07 +1100 Subject: [PATCH] fix nonce issues by locking populate and send transaction Concurrent asynchronous population of transactions cause issues with nonces not being in sync with the transaction count for an account on chain. This was being mitigated by tracking a "last seen" nonce and locking inside of `populateTransaction` so that the nonce could be populated in a concurrent fashion. However, if there was an async cancellation before the transaction was sent, then the nonce would become out of sync. One solution was to decrease the nonce if a cancellation occurred. The other solution, in this commit, is simply to lock the populate and sendTransaction calls together, so that there will not be concurrent nonce discrepancies. This removes the need for "lastSeenNonce" and is overall more simple. --- ethers/contract.nim | 15 +++++--- ethers/signer.nim | 84 ++++++++++++++++++++++++--------------------- 2 files changed, 55 insertions(+), 44 deletions(-) diff --git a/ethers/contract.nim b/ethers/contract.nim index eda4480..418dada 100644 --- a/ethers/contract.nim +++ b/ethers/contract.nim @@ -126,10 +126,17 @@ proc send(contract: Contract, overrides = TransactionOverrides()): Future[?TransactionResponse] {.async.} = if signer =? contract.signer: - let transaction = createTransaction(contract, function, parameters, overrides) - let populated = await signer.populateTransaction(transaction) - let txResp = await signer.sendTransaction(populated) - return txResp.some + + var params: seq[string] = @[] + for param in parameters.fields: + params.add $param + + withLock(signer): + var transaction = createTransaction(contract, function, parameters, overrides) + transaction = await signer.populateTransaction(transaction) + trace "sending transaction", function, params + let txResp = await signer.sendTransaction(transaction) + return txResp.some else: await call(contract, function, parameters, overrides) return TransactionResponse.none diff --git a/ethers/signer.nim b/ethers/signer.nim index 2ba3656..6a22481 100644 --- a/ethers/signer.nim +++ b/ethers/signer.nim @@ -63,13 +63,19 @@ method getChainId*(signer: Signer): Future[UInt256] {.base, gcsafe.} = signer.provider.getChainId() method getNonce(signer: Signer): Future[UInt256] {.base, gcsafe, async.} = - var nonce = await signer.getTransactionCount(BlockTag.pending) + return await signer.getTransactionCount(BlockTag.pending) + +template withLock*(signer: Signer, body: untyped) = + if signer.populateLock.isNil: + signer.populateLock = newAsyncLock() + + await signer.populateLock.acquire() + try: + body + finally: + signer.populateLock.release() - if lastSeen =? signer.lastSeenNonce and lastSeen >= nonce: - nonce = (lastSeen + 1.u256) - signer.lastSeenNonce = some nonce - return nonce method updateNonce*( signer: Signer, @@ -90,48 +96,45 @@ method decreaseNonce*(signer: Signer) {.base, gcsafe.} = method populateTransaction*(signer: Signer, transaction: Transaction): Future[Transaction] {.base, async.} = + ## 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. if sender =? transaction.sender and sender != await signer.getAddress(): raiseSignerError("from address mismatch") 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(await signer.getAddress()) - 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(await signer.getAddress()) + 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 - # followed by an estimateGas so we can determine if there was an error. If - # there is an error, the nonce must be decreased to prevent nonce gaps and - # stuck transactions - populated.nonce = some(await signer.getNonce()) - try: - populated.gasLimit = some(await signer.estimateGas(populated, BlockTag.pending)) - except ProviderError, EstimateGasError: - let e = getCurrentException() - signer.decreaseNonce() - raise e + if transaction.nonce.isNone and transaction.gasLimit.isNone: + # when both nonce and gasLimit are not populated, we must ensure getNonce is + # followed by an estimateGas so we can determine if there was an error. If + # there is an error, the nonce must be decreased to prevent nonce gaps and + # stuck transactions + let nonce = await signer.getNonce() + populated.nonce = some nonce + try: + populated.gasLimit = some(await signer.estimateGas(populated, BlockTag.pending)) + except ProviderError, EstimateGasError: + let e = getCurrentException() + signer.decreaseNonce() + raise e - 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 @@ -147,6 +150,7 @@ method cancelTransaction*( without nonce =? tx.nonce: raiseSignerError "transaction must have nonce" - var cancelTx = Transaction(to: sender, value: 0.u256, nonce: some nonce) - cancelTx = await signer.populateTransaction(cancelTx) - return await signer.sendTransaction(cancelTx) + withLock(signer): + var cancelTx = Transaction(to: sender, value: 0.u256, nonce: some nonce) + cancelTx = await signer.populateTransaction(cancelTx) + return await signer.sendTransaction(cancelTx)