From cb95cbc15a4e405e01ad24d24dedc660454be473 Mon Sep 17 00:00:00 2001 From: Mark Spanbroek Date: Thu, 29 Jun 2023 09:59:48 +0200 Subject: [PATCH] Make BlockHandler callback synchronous (breaking change) Refactored the confirm() implementation to work with a synchronous callback --- ethers/provider.nim | 111 ++++++----------- ethers/providers/jsonrpc/subscriptions.nim | 4 +- .../providers/jsonrpc/testJsonRpcProvider.nim | 113 +++--------------- .../jsonrpc/testJsonRpcSubscriptions.nim | 6 +- 4 files changed, 62 insertions(+), 172 deletions(-) diff --git a/ethers/provider.nim b/ethers/provider.nim index ce49ef9..eb09474 100644 --- a/ethers/provider.nim +++ b/ethers/provider.nim @@ -41,7 +41,7 @@ type cumulativeGasUsed*: UInt256 status*: TransactionStatus LogHandler* = proc(log: Log) {.gcsafe, upraises:[].} - BlockHandler* = proc(blck: Block): Future[void] {.gcsafe, upraises:[].} + BlockHandler* = proc(blck: Block) {.gcsafe, upraises:[].} Topic* = array[32, byte] Block* = object number*: ?UInt256 @@ -102,98 +102,65 @@ method subscribe*(provider: Provider, method unsubscribe*(subscription: Subscription) {.base, async.} = doAssert false, "not implemented" -# Removed from `confirm` closure and exported so it can be tested. -# Likely there is a better way -func confirmations*(receiptBlk, atBlk: UInt256): UInt256 = - ## Calculates the number of confirmations between two blocks - if atBlk < receiptBlk: - return 0.u256 - else: - return (atBlk - receiptBlk) + 1 # add 1 for current block - -# Removed from `confirm` closure and exported so it can be tested. -# Likely there is a better way -func hasBeenMined*(receipt: TransactionReceipt, - atBlock: UInt256, - wantedConfirms: int): bool = - ## Returns true if the transaction receipt has been returned from the node - ## with a valid block number and block hash and the specified number of - ## blocks have passed since the tx was mined (confirmations) - - if number =? receipt.blockNumber and - number > 0 and - # from ethers.js: "geth-etc" returns receipts before they are ready - receipt.blockHash.isSome: - - return number.confirmations(atBlock) >= wantedConfirms.u256 - - return false - proc confirm*(tx: TransactionResponse, - wantedConfirms: Positive = EthersDefaultConfirmations, - timeoutInBlocks: Natural = EthersReceiptTimeoutBlks): - Future[TransactionReceipt] - {.async, upraises: [EthersError].} = # raises for clarity + confirmations = EthersDefaultConfirmations, + timeout = EthersReceiptTimeoutBlks): + Future[TransactionReceipt] + {.async, upraises: [EthersError].} = ## Waits for a transaction to be mined and for the specified number of blocks ## to pass since it was mined (confirmations). ## A timeout, in blocks, can be specified that will raise an error if too many ## blocks have passed without the tx having been mined. - var subscription: Subscription - let - provider = tx.provider - retFut = newFuture[TransactionReceipt]("wait") + var blockNumber: UInt256 + let blockEvent = newAsyncEvent() - # used to check for block timeouts - let startBlock = await provider.getBlockNumber() + proc onBlockNumber(number: UInt256) = + blockNumber = number + blockEvent.fire() - proc newBlock(blk: Block) {.async.} = - ## subscription callback, called every time a new block event is sent from - ## the node + proc onBlock(blck: Block) = + if number =? blck.number: + onBlockNumber(number) - # if ethereum node doesn't include blockNumber in the event - without blkNum =? blk.number: - return + onBlockNumber(await tx.provider.getBlockNumber()) + let subscription = await tx.provider.subscribe(onBlock) - if receipt =? (await provider.getTransactionReceipt(tx.hash)) and - receipt.hasBeenMined(blkNum, wantedConfirms): - # fire and forget - discard subscription.unsubscribe() - if not retFut.finished: - retFut.complete(receipt) + let finish = blockNumber + timeout.u256 + var receipt: ?TransactionReceipt - elif timeoutInBlocks > 0: - let blocksPassed = (blkNum - startBlock) + 1 - if blocksPassed >= timeoutInBlocks.u256: - discard subscription.unsubscribe() - if not retFut.finished: - let message = - "Transaction was not mined in " & $timeoutInBlocks & " blocks" - retFut.fail(newException(EthersError, message)) + while true: + await blockEvent.wait() + blockEvent.clear() - # If our tx is already mined, return the receipt. Otherwise, check each - # new block to see if the tx has been mined - if receipt =? (await provider.getTransactionReceipt(tx.hash)) and - receipt.hasBeenMined(startBlock, wantedConfirms): - return receipt - else: - subscription = await provider.subscribe(newBlock) - return (await retFut) + if blockNumber >= finish: + await subscription.unsubscribe() + raise newException(EthersError, "tx not mined before timeout") + + if receipt.?blockNumber.isNone: + receipt = await tx.provider.getTransactionReceipt(tx.hash) + + without receipt =? receipt and txBlockNumber =? receipt.blockNumber: + continue + + if txBlockNumber + confirmations.u256 <= blockNumber + 1: + await subscription.unsubscribe() + return receipt proc confirm*(tx: Future[TransactionResponse], - wantedConfirms: Positive = EthersDefaultConfirmations, - timeoutInBlocks: Natural = EthersReceiptTimeoutBlks): + confirmations: int = EthersDefaultConfirmations, + timeout: int = EthersReceiptTimeoutBlks): Future[TransactionReceipt] {.async.} = ## Convenience method that allows wait to be chained to a sendTransaction ## call, eg: ## `await signer.sendTransaction(populated).confirm(3)` let txResp = await tx - return await txResp.confirm(wantedConfirms, timeoutInBlocks) + return await txResp.confirm(confirmations, timeout) proc confirm*(tx: Future[?TransactionResponse], - wantedConfirms: Positive = EthersDefaultConfirmations, - timeoutInBlocks: Natural = EthersReceiptTimeoutBlks): + confirmations: int = EthersDefaultConfirmations, + timeout: int = EthersReceiptTimeoutBlks): Future[TransactionReceipt] {.async.} = ## Convenience method that allows wait to be chained to a contract ## transaction, eg: @@ -207,7 +174,7 @@ proc confirm*(tx: Future[?TransactionResponse], "Transaction hash required. Possibly was a call instead of a send?" ) - return await txResp.confirm(wantedConfirms, timeoutInBlocks) + return await txResp.confirm(confirmations, timeout) method close*(provider: Provider) {.async, base.} = discard diff --git a/ethers/providers/jsonrpc/subscriptions.nim b/ethers/providers/jsonrpc/subscriptions.nim index 8d0d6d6..a5598a4 100644 --- a/ethers/providers/jsonrpc/subscriptions.nim +++ b/ethers/providers/jsonrpc/subscriptions.nim @@ -68,7 +68,7 @@ method subscribeBlocks(subscriptions: WebSocketSubscriptions, {.async.} = proc callback(id, arguments: JsonNode) = if blck =? Block.fromJson(arguments["result"]).catch: - asyncSpawn onBlock(blck) + onBlock(blck) let id = await subscriptions.client.eth_subscribe("newHeads") subscriptions.callbacks[id] = callback return id @@ -135,7 +135,7 @@ method subscribeBlocks(subscriptions: PollingSubscriptions, proc getBlock(hash: BlockHash) {.async.} = try: if blck =? (await subscriptions.client.eth_getBlockByHash(hash, false)): - await onBlock(blck) + onBlock(blck) except CatchableError: discard diff --git a/testmodule/providers/jsonrpc/testJsonRpcProvider.nim b/testmodule/providers/jsonrpc/testJsonRpcProvider.nim index 106a43f..e85c500 100644 --- a/testmodule/providers/jsonrpc/testJsonRpcProvider.nim +++ b/testmodule/providers/jsonrpc/testJsonRpcProvider.nim @@ -48,7 +48,7 @@ for url in ["ws://localhost:8545", "http://localhost:8545"]: let oldBlock = !await provider.getBlock(BlockTag.latest) discard await provider.send("evm_mine") var newBlock: Block - let blockHandler = proc(blck: Block) {.async.} = newBlock = blck + let blockHandler = proc(blck: Block) = newBlock = blck let subscription = await provider.subscribe(blockHandler) discard await provider.send("evm_mine") check eventually newBlock.number.isSome @@ -67,48 +67,23 @@ for url in ["ws://localhost:8545", "http://localhost:8545"]: check UInt256.fromHex("0x" & txResp.hash.toHex) > 0 test "can wait for a transaction to be confirmed": - let signer = provider.getSigner() - let transaction = Transaction.example - let populated = await signer.populateTransaction(transaction) + for confirmations in 0..3: + let signer = provider.getSigner() + let transaction = Transaction.example + let populated = await signer.populateTransaction(transaction) + let confirming = signer.sendTransaction(populated).confirm(confirmations) + await sleepAsync(100.millis) # wait for tx to be mined + await provider.mineBlocks(confirmations - 1) + let receipt = await confirming + check receipt.blockNumber.isSome - # must not be awaited so we can get newHeads inside of .wait - let futMined = provider.mineBlocks(5) - - let receipt = await signer.sendTransaction(populated).confirm(3) - let endBlock = await provider.getBlockNumber() - - check receipt.blockNumber.isSome # was eventually mined - - # >= 3 because more blocks may have been mined by the time the - # check in `.wait` was done. - # +1 for the block the tx was mined in - check (endBlock - !receipt.blockNumber) + 1 >= 3 - - await futMined - - test "waiting for block to be mined times out": - - # must not be awaited so we can get newHeads inside of .wait - let futMined = provider.mineBlocks(7) - - let startBlock = await provider.getBlockNumber() - let response = TransactionResponse(hash: TransactionHash.example, - provider: provider) - try: - discard await response.confirm(wantedConfirms = 2, - timeoutInBlocks = 5) - - await futMined - except EthersError as e: - check e.msg == "Transaction was not mined in 5 blocks" - - let endBlock = await provider.getBlockNumber() - - # >= 5 because more blocks may have been mined by the time the - # check in `.wait` was done. - # +1 for including the start block - check (endBlock - startBlock) + 1 >= 5 # +1 including start block - if not futMined.completed and not futMined.finished: await futMined + test "confirmation times out": + let hash = TransactionHash.example + let tx = TransactionResponse(provider: provider, hash: hash) + let confirming = tx.confirm(confirmations = 2, timeout = 5) + await provider.mineBlocks(5) + expect EthersError: + discard await confirming test "Conversion: missing block number in Block isNone": @@ -207,58 +182,6 @@ for url in ["ws://localhost:8545", "http://localhost:8545"]: var txReceipt = TransactionReceipt.fromJson(txReceiptJson) check txReceipt.blockHash.isNone - test "confirmations calculated correctly": - # when receipt block number is higher than current block number, - # should return 0 - check confirmations(2.u256, 1.u256) == 0.u256 - - # Same receipt and current block counts as one confirmation - check confirmations(1.u256, 1.u256) == 1.u256 - - check confirmations(1.u256, 2.u256) == 2.u256 - - test "checks if transation has been mined correctly": - - var receipt: TransactionReceipt - var currentBlock = 1.u256 - var wantedConfirms = 1 - let blockHash = hexToByteArray[32]( - "0x7b00154e06fe4f27a87208eba220efb4dbc52f7429549a39a17bba2e0d98b960" - ).some - - # missing blockHash - receipt = TransactionReceipt( - blockNumber: 1.u256.some - ) - check not receipt.hasBeenMined(currentBlock, wantedConfirms) - - # missing block number - receipt = TransactionReceipt( - blockHash: blockHash - ) - check not receipt.hasBeenMined(currentBlock, wantedConfirms) - - # block number is 0 - receipt = TransactionReceipt( - blockNumber: 0.u256.some - ) - check not receipt.hasBeenMined(currentBlock, wantedConfirms) - - # not enough confirms - receipt = TransactionReceipt( - blockNumber: 1.u256.some - ) - check not receipt.hasBeenMined(currentBlock, wantedConfirms) - - # success - receipt = TransactionReceipt( - blockNumber: 1.u256.some, - blockHash: blockHash - ) - currentBlock = int.high.u256 - wantedConfirms = int.high - check receipt.hasBeenMined(currentBlock, wantedConfirms) - test "raises JsonRpcProviderError when something goes wrong": let provider = JsonRpcProvider.new("http://invalid.") expect JsonRpcProviderError: @@ -270,6 +193,6 @@ for url in ["ws://localhost:8545", "http://localhost:8545"]: expect JsonRpcProviderError: discard await provider.getBlock(BlockTag.latest) expect JsonRpcProviderError: - discard await provider.subscribe(proc(_: Block) {.async.} = discard) + discard await provider.subscribe(proc(_: Block) = discard) expect JsonRpcProviderError: discard await provider.getSigner().sendTransaction(Transaction.example) diff --git a/testmodule/providers/jsonrpc/testJsonRpcSubscriptions.nim b/testmodule/providers/jsonrpc/testJsonRpcSubscriptions.nim index 2540556..568a7c5 100644 --- a/testmodule/providers/jsonrpc/testJsonRpcSubscriptions.nim +++ b/testmodule/providers/jsonrpc/testJsonRpcSubscriptions.nim @@ -20,7 +20,7 @@ template subscriptionTests(subscriptions, client) = test "subscribes to new blocks": var latestBlock: Block - proc callback(blck: Block) {.async.} = + proc callback(blck: Block) = latestBlock = blck let subscription = await subscriptions.subscribeBlocks(callback) discard await client.call("evm_mine", newJArray()) @@ -31,7 +31,7 @@ template subscriptionTests(subscriptions, client) = test "stops listening to new blocks when unsubscribed": var count = 0 - proc callback(blck: Block) {.async.} = + proc callback(blck: Block) = inc count let subscription = await subscriptions.subscribeBlocks(callback) discard await client.call("evm_mine", newJArray()) @@ -44,7 +44,7 @@ template subscriptionTests(subscriptions, client) = test "stops listening to new blocks when provider is closed": var count = 0 - proc callback(blck: Block) {.async.} = + proc callback(blck: Block) = inc count let subscription = await subscriptions.subscribeBlocks(callback) discard await client.call("evm_mine", newJArray())