logos-storage/nim-ethers
1
0
Fork 0
mirror of https://github.com/logos-storage/nim-ethers.git synced 2026-01-11 10:03:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

PR feedback: remove auto-cancellation of failed transactions

Browse Source 
- 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
This commit is contained in:
Eric 2023-10-20 17:14:12 +11:00
parent 73963e6146
commit f16ec78a61
No known key found for this signature in database
7 changed files with 158 additions and 136 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
ethers/contract.nim
View File

@ -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:
@ -279,7 +248,6 @@ proc confirm*(tx: Future[?TransactionResponse],
## `await token.connect(signer0)
## .mint(accounts[1], 100.u256)
## .confirm(3)`
without response =? (await tx):
raise newException(
EthersError,

9
ethers/providers/jsonrpc.nim
View File

@ -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
@ -28,7 +29,7 @@ type
subscriptions: JsonRpcSubscriptions
id: JsonNode
proc raiseProviderError(message: string) {.upraises: [JsonRpcProviderError].} =
proc raiseJsonRpcProviderError(message: string) {.upraises: [JsonRpcProviderError].} =
var message = message
try:
message = parseJson(message){"message"}.getStr
@ -40,11 +41,11 @@ template convertError(body) =
try:
body
except JsonRpcError as error:
raiseProviderError(error.msg)
raiseJsonRpcProviderError(error.msg)
# Catch all ValueErrors for now, at least until JsonRpcError is actually
# raised. PR created: https://github.com/status-im/nim-json-rpc/pull/151
except ValueError as error:
raiseProviderError(error.msg)
raiseJsonRpcProviderError(error.msg)
# Provider
@ -228,7 +229,7 @@ method getAddress*(signer: JsonRpcSigner): Future[Address] {.async.} =
if accounts.len > 0:
return accounts[0]
raiseProviderError "no address found"
raiseJsonRpcProviderError "no address found"
method signMessage*(signer: JsonRpcSigner,
message: seq[byte]): Future[seq[byte]] {.async.} =

111
ethers/signer.nim
View File

@ -11,6 +11,7 @@ logScope:
type
Signer* = ref object of RootObj
lastSeenNonce: ?UInt256
populateLock: AsyncLock
type
SignerError* = object of EthersError
@ -20,6 +21,16 @@ type
template raiseSignerError(message: string, parent: ref ProviderError = nil) =
raise newException(SignerError, message, parent)
proc raiseEstimateGasError(
transaction: Transaction,
parent: ref ProviderError = nil
) =
let e = (ref EstimateGasError)(
msg: "Estimate gas failed",
transaction: transaction,
parent: parent)
raise e
method provider*(signer: Signer): Provider {.base, gcsafe.} =
doAssert false, "not implemented"
@ -47,14 +58,17 @@ method estimateGas*(signer: Signer,
transaction: Transaction): Future[UInt256] {.base, async.} =
var transaction = transaction
transaction.sender = some(await signer.getAddress)
return await signer.provider.estimateGas(transaction)
try:
return await signer.provider.estimateGas(transaction)
except ProviderError as e:
raiseEstimateGasError transaction, e
method getChainId*(signer: Signer): Future[UInt256] {.base, gcsafe.} =
signer.provider.getChainId()
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:
@ -65,48 +79,48 @@ method getNonce*(signer: Signer): Future[UInt256] {.base, gcsafe, async.} =
method updateNonce*(
signer: Signer,
nonce: UInt256,
force = false
nonce: UInt256
) {.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 force or nonce > lastSeen:
if nonce > lastSeen:
signer.lastSeenNonce = some nonce
method cancelTransaction*(
method decreaseNonce*(signer: Signer) {.base, gcsafe.} =
if lastSeen =? signer.lastSeenNonce and lastSeen > 0:
signer.lastSeenNonce = some lastSeen - 1
proc ensureNonceSequence(
signer: Signer,
tx: Transaction
): Future[TransactionResponse] {.async, base.} =
# cancels a transaction by sending with a 0-valued transaction to ourselves
# with the failed tx's nonce
): Future[Transaction] {.async.} =
## Ensures that once the nonce is incremented, if estimate gas fails, the
## nonce is decremented. Disallows concurrent calls by use of AsyncLock.
## Immediately returns if tx already has a nonce or gasLimit.
without sender =? tx.sender:
raiseSignerError "transaction must have sender"
if sender != await signer.getAddress():
raiseSignerError "can only cancel a tx this signer has sent"
without nonce =? tx.nonce:
raiseSignerError "transaction must have nonce"
if not (tx.nonce.isNone and tx.gasLimit.isNone):
return tx
var populated = tx
if signer.populateLock.isNil:
signer.populateLock = newAsyncLock()
await signer.populateLock.acquire()
var cancelTx = tx
cancelTx.to = sender
cancelTx.value = 0.u256
cancelTx.nonce = some nonce
try:
cancelTx.gasLimit = some(await signer.estimateGas(cancelTx))
except ProviderError:
warn "failed to estimate gas for cancellation tx, sending anyway",
tx = $cancelTx
discard
populated.nonce = some(await signer.getNonce())
populated.gasLimit = some(await signer.estimateGas(populated))
except ProviderError, EstimateGasError:
let e = getCurrentException()
signer.decreaseNonce()
raise e
finally:
signer.populateLock.release()
trace "cancelling transaction to prevent stuck transactions", nonce
return await signer.sendTransaction(cancelTx)
return populated
method populateTransaction*(signer: Signer,
transaction: Transaction):
@ -117,24 +131,33 @@ method populateTransaction*(signer: Signer,
if chainId =? transaction.chainId and chainId != await signer.getChainId():
raiseSignerError("chain id mismatch")
var populated = transaction
var populated = await signer.ensureNonceSequence(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 (transaction.maxFee.isNone or transaction.maxPriorityFee.isNone):
if populated.gasPrice.isNone and (populated.maxFee.isNone or populated.maxPriorityFee.isNone):
populated.gasPrice = some(await signer.getGasPrice())
if transaction.nonce.isNone:
if populated.nonce.isNone:
populated.nonce = some(await signer.getNonce())
if transaction.gasLimit.isNone:
try:
populated.gasLimit = some(await signer.estimateGas(populated))
except ProviderError as e:
let e = (ref EstimateGasError)(
msg: "Estimate gas failed",
transaction: populated,
parent: e)
raise e
if populated.gasLimit.isNone:
populated.gasLimit = some(await signer.estimateGas(populated))
return populated
method cancelTransaction*(
signer: Signer,
tx: Transaction
): Future[TransactionResponse] {.async, base.} =
# cancels a transaction by sending with a 0-valued transaction to ourselves
# with the failed tx's nonce
without sender =? tx.sender:
raiseSignerError "transaction must have sender"
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)

22
ethers/testing.nim
View File

@ -2,8 +2,8 @@ import std/strutils
import ./provider
import ./signer
proc revertReason*(e: ref EthersError): string =
var msg = e.msg
proc revertReason*(emsg: string): string =
var msg = emsg
const revertPrefixes = @[
# hardhat
"Error: VM Exception while processing transaction: reverted with " &
@ -16,6 +16,10 @@ proc revertReason*(e: ref EthersError): string =
msg = msg.replace("\'")
return msg
proc revertReason*(e: ref EthersError): string =
var msg = e.msg
msg.revertReason
proc reverts*[T](call: Future[T]): Future[bool] {.async.} =
try:
when T is void:
@ -23,7 +27,7 @@ proc reverts*[T](call: Future[T]): Future[bool] {.async.} =
else:
discard await call
return false
except ProviderError, SignerError:
except ProviderError, EstimateGasError:
return true
proc reverts*[T](call: Future[T], reason: string): Future[bool] {.async.} =
@ -33,12 +37,12 @@ proc reverts*[T](call: Future[T], reason: string): Future[bool] {.async.} =
else:
discard await call
return false
except ProviderError, SignerError:
let error = getCurrentException()
var passed = reason == (ref EthersError)(error).revertReason
except ProviderError, EstimateGasError:
let e = getCurrentException()
var passed = reason == (ref EthersError)(e).revertReason
if not passed and
not error.parent.isNil and
error.parent of (ref EthersError):
let revertReason = (ref EthersError)(error.parent).revertReason
not e.parent.isNil and
e.parent of (ref EthersError):
let revertReason = (ref EthersError)(e.parent).revertReason
passed = reason == revertReason
return passed

14
testmodule/helpers.nim Normal file
View File

@ -0,0 +1,14 @@
import pkg/ethers
import ./hardhat
type
TestHelpers* = ref object of Contract
method doRevert*(
self: TestHelpers,
revertReason: string
): ?TransactionResponse {.base, contract.}
proc new*(_: type TestHelpers, signer: Signer): TestHelpers =
let deployment = readDeployment()
TestHelpers.new(!deployment.address(TestHelpers), signer)

35
testmodule/testContracts.nim
View File

@ -1,22 +1,20 @@
import std/json
import std/options
import std/strutils
import pkg/asynctest
import pkg/questionable
import pkg/stint
import pkg/ethers
import pkg/ethers/erc20
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"]:
@ -240,29 +238,28 @@ for url in ["ws://localhost:8545", "http://localhost:8545"]:
Transfer(receiver: accounts[0], value: 100.u256)
]
test "transactions are cancelled once nonce has been incremented to prevent stuck transactions":
test "concurrent transactions with first failing increment nonce correctly":
let signer = provider.getSigner()
token = TestToken.new(token.address, signer)
let token = TestToken.new(token.address, signer)
let helpersContract = TestHelpers.new(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)
# emulate concurrent populateTransaction calls, where the first one fails
let futs = await allFinished(
helpersContract.doRevert("some reason"),
token.mint(accounts[0], 100.u256)
)
check futs[0].error of EstimateGasError
let receipt = await futs[1].confirm(1)
check receipt.status == TransactionStatus.Success
test "transactions are not cancelled if nonce has not been incremented":
test "non-concurrent transactions with first failing increment nonce correctly":
let signer = provider.getSigner()
token = TestToken.new(token.address, signer)
let token = TestToken.new(token.address, signer)
let helpersContract = TestHelpers.new(signer)
expect ContractError:
discard await token.doRevert("some reason")
expect EstimateGasError:
discard await helpersContract.doRevert("some reason")
let receipt = await token
.mint(accounts[0], 100.u256)

67
testmodule/testTesting.nim
View File

@ -3,7 +3,7 @@ import pkg/asynctest
import pkg/chronos
import pkg/ethers
import pkg/ethers/testing
import ./hardhat
import ./helpers
suite "Testing helpers":
@ -13,13 +13,13 @@ suite "Testing helpers":
test "checks that call reverts":
proc call() {.async.} =
raise newException(ProviderError, $rpcResponse)
raise newException(EstimateGasError, $rpcResponse)
check await call().reverts()
test "checks reason for revert":
proc call() {.async.} =
raise newException(ProviderError, $rpcResponse)
raise newException(EstimateGasError, $rpcResponse)
check await call().reverts(revertReason)
@ -28,19 +28,31 @@ suite "Testing helpers":
check not await call().reverts()
test "reverts only checks ProviderErrors":
proc call() {.async.} =
raise newException(ContractError, "test")
test "reverts only checks ProviderErrors, EstimateGasErrors":
proc callProviderError() {.async.} =
raise newException(ProviderError, "test")
proc callEstimateGasError() {.async.} =
raise newException(EstimateGasError, "test")
proc callEthersError() {.async.} =
raise newException(EthersError, "test")
expect ContractError:
check await call().reverts()
check await callProviderError().reverts()
check await callEstimateGasError().reverts()
expect EthersError:
check await callEthersError().reverts()
test "reverts with reason only checks ProviderErrors":
proc call() {.async.} =
raise newException(ContractError, "test")
test "reverts with reason only checks ProviderErrors, EstimateGasErrors":
proc callProviderError() {.async.} =
raise newException(ProviderError, revertReason)
proc callEstimateGasError() {.async.} =
raise newException(EstimateGasError, revertReason)
proc callEthersError() {.async.} =
raise newException(EthersError, revertReason)
expect ContractError:
check await call().reverts(revertReason)
check await callProviderError().reverts(revertReason)
check await callEstimateGasError().reverts(revertReason)
expect EthersError:
check await callEthersError().reverts(revertReason)
test "reverts with reason is false when there is no revert":
proc call() {.async.} = discard
@ -49,29 +61,24 @@ suite "Testing helpers":
test "reverts is false when the revert reason doesn't match":
proc call() {.async.} =
raise newException(ProviderError, "other reason")
raise newException(EstimateGasError, "other reason")
check not await call().reverts(revertReason)
test "revert handles non-standard revert prefix":
let nonStdMsg = fmt"Provider VM Exception: reverted with {revertReason}"
proc call() {.async.} =
raise newException(ProviderError, nonStdMsg)
raise newException(EstimateGasError, nonStdMsg)
check await call().reverts(nonStdMsg)
test "works with functions that return a value":
proc call(): Future[int] {.async.} = return 42
check not await call().reverts()
check not await call().reverts("some reason")
check not await call().reverts(revertReason)
type
TestHelpers* = ref object of Contract
method revertsWith*(self: TestHelpers,
revertReason: string) {.base, contract, view.}
suite "Testing helpers - provider":
suite "Testing helpers - contracts":
var helpersContract: TestHelpers
var provider: JsonRpcProvider
@ -83,12 +90,20 @@ suite "Testing helpers - provider":
provider = JsonRpcProvider.new("ws://127.0.0.1:8545")
snapshot = await provider.send("evm_snapshot")
accounts = await provider.listAccounts()
let deployment = readDeployment()
helpersContract = TestHelpers.new(!deployment.address(TestHelpers), provider)
helpersContract = TestHelpers.new(provider.getSigner())
teardown:
discard await provider.send("evm_revert", @[snapshot])
await provider.close()
test "revert works with provider":
check await helpersContract.revertsWith(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)