From 2becf31709738701362d13f7858c3dac1f2bfcfd Mon Sep 17 00:00:00 2001 From: adjisb <85941014+adjisb@users.noreply.github.com> Date: Fri, 24 Feb 2023 12:44:44 -0300 Subject: [PATCH 1/2] fix: eth get block transaction count by hash to work with forks (#3739) Co-authored-by: Andres Adjimann Co-authored-by: Micaiah Reid Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com> --- src/chains/ethereum/ethereum/src/api.ts | 14 ++++++-------- .../ethereum/ethereum/tests/forking/block.test.ts | 7 +++++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/chains/ethereum/ethereum/src/api.ts b/src/chains/ethereum/ethereum/src/api.ts index 357e619fc5..d1a44f82a6 100644 --- a/src/chains/ethereum/ethereum/src/api.ts +++ b/src/chains/ethereum/ethereum/src/api.ts @@ -1274,14 +1274,12 @@ export default class EthereumApi implements Api { @assertArgLength(1) async eth_getBlockTransactionCountByHash(hash: DATA) { const { blocks } = this.#blockchain; - const blockNum = await blocks.getNumberFromHash(hash); - if (!blockNum) return null; - - const rawBlock = await blocks.getRawByBlockNumber(Quantity.from(blockNum)); - if (!rawBlock) return null; - - const [, rawTransactions] = decode(rawBlock); - return Quantity.from(rawTransactions.length); + const block = await blocks + .getByHash(hash) + .catch(_ => null); + if (!block) return null; + const transactions = block.getTransactions(); + return Quantity.from(transactions.length); } /** diff --git a/src/chains/ethereum/ethereum/tests/forking/block.test.ts b/src/chains/ethereum/ethereum/tests/forking/block.test.ts index 296d535683..7289f5cc95 100644 --- a/src/chains/ethereum/ethereum/tests/forking/block.test.ts +++ b/src/chains/ethereum/ethereum/tests/forking/block.test.ts @@ -84,5 +84,12 @@ describe("forking", function () { const block = await provider.send("eth_getBlockByNumber", ["0x0", true]); assert.deepStrictEqual(block, block0); }); + + it("should get transaction count by hash from the original chain", async () => { + const block = await provider.send("eth_getBlockByNumber", ["0xB443", true]); + const blockTransactionCountByHash = await provider.send("eth_getBlockTransactionCountByHash", [block.hash]); + assert.deepStrictEqual(block.transactions.length, parseInt(blockTransactionCountByHash)); + }); + }); }); From ff0a29528991ba7e5cdf1739d4e7f49ada6cfcce Mon Sep 17 00:00:00 2001 From: Micaiah Reid Date: Wed, 1 Mar 2023 15:00:58 -0500 Subject: [PATCH 2/2] fix: log warning regarding transactions with future-nonces when in eager mode (#4166) Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com> --- .../ethereum/ethereum/src/blockchain.ts | 19 ++- .../tests/api/eth/greedyInstamining.test.ts | 100 ------------ .../ethereum/tests/api/eth/instamine.test.ts | 146 ++++++++++++++++++ 3 files changed, 161 insertions(+), 104 deletions(-) delete mode 100644 src/chains/ethereum/ethereum/tests/api/eth/greedyInstamining.test.ts create mode 100644 src/chains/ethereum/ethereum/tests/api/eth/instamine.test.ts diff --git a/src/chains/ethereum/ethereum/src/blockchain.ts b/src/chains/ethereum/ethereum/src/blockchain.ts index 36ae71f28c..191fd2c0e3 100644 --- a/src/chains/ethereum/ethereum/src/blockchain.ts +++ b/src/chains/ethereum/ethereum/src/blockchain.ts @@ -1054,11 +1054,22 @@ export default class Blockchain extends Emittery { process.nextTick(this.emit.bind(this), "pendingTransaction", transaction); } - const hash = transaction.hash; - if (this.#isPaused() || !this.#instamine) { + const { hash } = transaction; + const instamine = this.#instamine; + if (!instamine || this.#isPaused()) { return hash; } else { - if (this.#instamine && this.#options.miner.instamine === "eager") { + const options = this.#options; + // if the transaction is not executable, we just have to return the hash + if (instamine && options.miner.instamine === "eager") { + if (!isExecutable) { + // users have been confused about why ganache "hangs" when sending a + // transaction with a "too-high" nonce. This message should help them + // understand what's going on. + options.logging.logger.log( + `Transaction "${hash}" has a too-high nonce; this transaction has been added to the pool, and will be processed when its nonce is reached. See https://github.com/trufflesuite/ganache/issues/4165 for more information.` + ); + } // in eager instamine mode we must wait for the transaction to be saved // before we can return the hash const { status, error } = await transaction.once("finalized"); @@ -1067,7 +1078,7 @@ export default class Blockchain extends Emittery { // vmErrorsOnRPCResponse is enabled. if ( error && - (status === "rejected" || this.#options.chain.vmErrorsOnRPCResponse) + (status === "rejected" || options.chain.vmErrorsOnRPCResponse) ) throw error; } diff --git a/src/chains/ethereum/ethereum/tests/api/eth/greedyInstamining.test.ts b/src/chains/ethereum/ethereum/tests/api/eth/greedyInstamining.test.ts deleted file mode 100644 index d681ce7d93..0000000000 --- a/src/chains/ethereum/ethereum/tests/api/eth/greedyInstamining.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import getProvider from "../../helpers/getProvider"; -import assert from "assert"; - -describe("api", () => { - describe("eth", () => { - describe("eager", () => { - it("when in strict instamine mode, does not mine before returning the tx hash", async () => { - const provider = await getProvider({ - miner: { instamine: "strict" } - }); - const accounts = await provider.send("eth_accounts"); - - const hash = await provider.send("eth_sendTransaction", [ - { - from: accounts[0], - to: accounts[1], - value: "0x1" - } - ]); - const receipt = await provider.send("eth_getTransactionReceipt", [ - hash - ]); - assert.strictEqual(receipt, null); - }); - - it("when in eager instamine mode, mines before returns in the tx hash", async () => { - const provider = await getProvider({ - miner: { instamine: "eager" } - }); - const accounts = await provider.send("eth_accounts"); - - const hash = await provider.send("eth_sendTransaction", [ - { - from: accounts[0], - to: accounts[1], - value: "0x1" - } - ]); - const receipt = await provider.send("eth_getTransactionReceipt", [ - hash - ]); - assert.notStrictEqual(receipt, null); - }); - - it("handles transaction balance errors, callback style", done => { - getProvider({ - miner: { instamine: "eager" }, - chain: { vmErrorsOnRPCResponse: true } - }).then(async provider => { - const [from, to] = await provider.send("eth_accounts"); - const balance = parseInt( - await provider.send("eth_getBalance", [from]), - 16 - ); - const gasCost = 99967968750001; - // send a transaction that will spend some of the balance - provider.request({ - method: "eth_sendTransaction", - params: [ - { - from, - to - } - ] - }); - - // send another transaction while the previous transaction is still - // pending. this transaction appears to have enough balance to run, - // so the transaction pool will accept it, but when it runs in the VM - // it won't have enough balance to run. - provider.send( - { - jsonrpc: "2.0", - id: "1", - method: "eth_sendTransaction", - params: [ - { - from, - to, - value: `0x${(balance - gasCost).toString(16)}` - } - ] - }, - (e, r) => { - assert( - e.message.includes( - "sender doesn't have enough funds to send tx" - ) - ); - assert.strictEqual(e.message, (r as any).error.message); - assert.strictEqual((r as any).error.code, -32000); - assert.strictEqual(typeof (r as any).error.data.result, "string"); - done(); - } - ); - }); - }); - }); - }); -}); diff --git a/src/chains/ethereum/ethereum/tests/api/eth/instamine.test.ts b/src/chains/ethereum/ethereum/tests/api/eth/instamine.test.ts new file mode 100644 index 0000000000..bc3bf3a710 --- /dev/null +++ b/src/chains/ethereum/ethereum/tests/api/eth/instamine.test.ts @@ -0,0 +1,146 @@ +import getProvider from "../../helpers/getProvider"; +import assert from "assert"; +import { Logger } from "@ganache/ethereum-options/typings/src/logging-options"; + +describe("api", () => { + describe("eth", () => { + describe("instamine modes (eager/strict)", () => { + describe("strict", () => { + it("when in strict instamine mode, does not mine before returning the tx hash", async () => { + const provider = await getProvider({ + miner: { instamine: "strict" } + }); + const accounts = await provider.send("eth_accounts"); + + const hash = await provider.send("eth_sendTransaction", [ + { + from: accounts[0], + to: accounts[1], + value: "0x1" + } + ]); + const receipt = await provider.send("eth_getTransactionReceipt", [ + hash + ]); + assert.strictEqual(receipt, null); + }); + }); + + describe("eager", () => { + it("mines before returning the tx hash", async () => { + const provider = await getProvider({ + miner: { instamine: "eager" } + }); + const accounts = await provider.send("eth_accounts"); + + const hash = await provider.send("eth_sendTransaction", [ + { + from: accounts[0], + to: accounts[1], + value: "0x1" + } + ]); + const receipt = await provider.send("eth_getTransactionReceipt", [ + hash + ]); + assert.notStrictEqual(receipt, null); + }); + + it("log a message about future-nonce transactions in eager mode", async () => { + let logger: Logger; + const logPromise = new Promise(resolve => { + logger = { + log: (msg: string) => { + const regex = + /Transaction "0x[a-zA-z0-9]{64}" has a too-high nonce; this transaction has been added to the pool, and will be processed when its nonce is reached\. See https:\/\/github.com\/trufflesuite\/ganache\/issues\/4165 for more information\./; + if (regex.test(msg)) resolve(true); + } + }; + }); + + const provider = await getProvider({ + logging: { logger }, + miner: { instamine: "eager" }, + chain: { vmErrorsOnRPCResponse: true } + }); + const [from, to] = await provider.send("eth_accounts"); + const futureNonceTx = { from, to, nonce: "0x1" }; + const futureNonceProm = provider.send("eth_sendTransaction", [ + futureNonceTx + ]); + + // send a transaction to fill the nonce gap + provider.send("eth_sendTransaction", [{ from, to }]); // we don't await this on purpose. + + const result = await Promise.race([futureNonceProm, logPromise]); + // `logPromise` should resolve before the the hash gets returned + // (logPromise returns true) + assert.strictEqual(result, true); + + // now our nonce gap is filled so the original tx is mined + const receipt = await provider.send("eth_getTransactionReceipt", [ + await futureNonceProm + ]); + assert.notStrictEqual(receipt, null); + }); + + it("handles transaction balance errors, callback style", done => { + getProvider({ + miner: { instamine: "eager" }, + chain: { vmErrorsOnRPCResponse: true } + }).then(async provider => { + const [from, to] = await provider.send("eth_accounts"); + const balance = parseInt( + await provider.send("eth_getBalance", [from]), + 16 + ); + const gasCost = 99967968750001; + // send a transaction that will spend some of the balance + provider.request({ + method: "eth_sendTransaction", + params: [ + { + from, + to + } + ] + }); + + // send another transaction while the previous transaction is still + // pending. this transaction appears to have enough balance to run, + // so the transaction pool will accept it, but when it runs in the VM + // it won't have enough balance to run. + provider.send( + { + jsonrpc: "2.0", + id: "1", + method: "eth_sendTransaction", + params: [ + { + from, + to, + value: `0x${(balance - gasCost).toString(16)}` + } + ] + }, + (e, r) => { + assert( + e.message.includes( + "sender doesn't have enough funds to send tx" + ) + ); + assert.strictEqual(e.message, (r as any).error.message); + assert.strictEqual((r as any).error.code, -32000); + assert.strictEqual( + typeof (r as any).error.data.result, + "string" + ); + done(); + } + ); + }); + }); + }); + }); + }); +});