fix: nonce too high (#81)

* 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.

* remove lastSeenNonce

Internal nonce tracking is no longer needed since populate/sendTransaction is now locked. Even if cancelled midway, the nonce will get a refreshed value from the number of transactions from chain.

* chronos v4 exception tracking

* Add tests
This commit is contained in:
Eric 2024-10-25 15:08:00 +11:00 committed by GitHub
parent b68bea9909
commit 765379a662
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 91 additions and 89 deletions

View File

@ -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

View File

@ -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)

View File

@ -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)

View File

@ -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)

View File

@ -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

View File

@ -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