From 4994fc0641a5f6406aec6b85467b8926ab8f617b Mon Sep 17 00:00:00 2001 From: David Murdoch Date: Fri, 12 Jul 2019 13:59:22 -0400 Subject: [PATCH 1/3] Allow a reaaallyyy high gasLimit for eth_call and eth_estimateGas --- lib/blockchain_double.js | 92 +++++++++++------------------ lib/statemanager.js | 11 ++-- lib/subproviders/geth_api_double.js | 36 ++++++++--- test/call.js | 32 +++++++--- 4 files changed, 95 insertions(+), 76 deletions(-) diff --git a/lib/blockchain_double.js b/lib/blockchain_double.js index 7e6d502690..86e2e7a740 100644 --- a/lib/blockchain_double.js +++ b/lib/blockchain_double.js @@ -481,93 +481,71 @@ BlockchainDouble.prototype.sortByPriceAndNonce = function() { self.pending_transactions = sortedTransactions; }; -BlockchainDouble.prototype.processCall = function(tx, blockNumber, callback) { - var self = this; - - var runCall = function(tx, err, parentBlock) { +BlockchainDouble.prototype.readyCall = function(tx, blockNumber, callback) { + const readyCall = (tx, err, parentBlock) => { if (err) { return callback(err); } - // create a fake block with this fake transaction - self.createBlock(parentBlock, function(err, newBlock) { + this.createBlock(parentBlock, (err, newBlock) => { if (err) { return callback(err); } + newBlock.transactions.push(tx); - var runArgs = { + // gas estimates and eth_calls shouldn't be subject to block gas limits + newBlock.header.gasLimit = tx.gasLimit; + + const runArgs = { tx: tx, block: newBlock, skipBalance: true, skipNonce: true }; - - var stateTrie = self.createStateTrie(self.data.trie_db, parentBlock.header.stateRoot); - var vm = self.createVMFromStateTrie(stateTrie); - - vm.runTx(runArgs, function(vmerr, result) { - // This is a check that has been in there for awhile. I'm unsure if it's required, but it can't hurt. - if (vmerr && vmerr instanceof Error === false) { - vmerr = new Error("VM error: " + vmerr); - } - - // If we're given an error back directly, it's worse than a runtime error. Expose it and get out. - if (vmerr) { - return callback(vmerr, err); - } - - // If no error, check for a runtime error. This can return null if no runtime error. - vmerr = RuntimeError.fromResults([tx], { results: [result] }); - - callback(vmerr, result); - }); + const stateTrie = this.createStateTrie(this.data.trie_db, parentBlock.header.stateRoot); + const vm = this.createVMFromStateTrie(stateTrie); + callback(null, vm, runArgs); }); }; - // Delegate block selection if (blockNumber === "latest") { - self.latestBlock(runCall.bind(null, tx)); + this.latestBlock(readyCall.bind(null, tx)); } else { - self.getBlock(blockNumber, runCall.bind(null, tx)); + this.getBlock(blockNumber, readyCall.bind(null, tx)); } }; -BlockchainDouble.prototype.estimateGas = function(tx, blockNumber, callback) { - var self = this; - - var runCall = function(tx, err, parentBlock) { +BlockchainDouble.prototype.processCall = function(tx, blockNumber, callback) { + this.readyCall(tx, blockNumber, (err, vm, runArgs) => { if (err) { - return callback(err); + callback(err); + return; } - // create a fake block with this fake transaction - self.createBlock(parentBlock, function(err, newBlock) { + vm.runTx(runArgs, (err, result) => { + // If we're given an error back directly, it's worse than a runtime error. Expose it and get out. if (err) { - return callback(err); + callback(err); + return; } - newBlock.transactions.push(tx); - - var runArgs = { - tx: tx, - block: newBlock, - skipBalance: true, - skipNonce: true - }; - var stateTrie = self.createStateTrie(self.data.trie_db, parentBlock.header.stateRoot); - var vm = self.createVMFromStateTrie(stateTrie); - - estimateGas(vm, runArgs, err, callback); + // If no vm error, check for a runtime error. This can return null if no runtime error. + const runtimeErr = RuntimeError.fromResults([tx], { results: [result] }); + callback(runtimeErr, result); }); - }; + }); +}; - // Delegate block selection - if (blockNumber === "latest") { - self.latestBlock(runCall.bind(null, tx)); - } else { - self.getBlock(blockNumber, runCall.bind(null, tx)); - } +BlockchainDouble.prototype.estimateGas = function(tx, blockNumber, callback) { + this.readyCall(tx, blockNumber, (err, vm, runArgs) => { + if (err) { + callback(err); + return; + } + + estimateGas(vm, runArgs, err, callback); + }); }; /** diff --git a/lib/statemanager.js b/lib/statemanager.js index 6d6b7ad76b..94ead38128 100644 --- a/lib/statemanager.js +++ b/lib/statemanager.js @@ -324,7 +324,7 @@ StateManager.prototype.queueTransaction = function(method, txJsonRpc, blockNumbe let tx; try { tx = Transaction.fromJSON(txJsonRpc, type); - this._setTransactionDefaults(tx); + this._setTransactionDefaults(tx, method === "eth_sendTransaction"); } catch (e) { callback(e); return; @@ -332,8 +332,8 @@ StateManager.prototype.queueTransaction = function(method, txJsonRpc, blockNumbe this._queueTransaction(method, tx, from, blockNumber, callback); }; -StateManager.prototype._setTransactionDefaults = function(tx) { - if (tx.gasLimit.length === 0) { +StateManager.prototype._setTransactionDefaults = function(tx, isTransaction) { + if (isTransaction && tx.gasLimit.length === 0) { tx.gasLimit = utils.toBuffer(this.blockchain.defaultTransactionGasLimit); } @@ -356,7 +356,10 @@ StateManager.prototype._queueTransaction = function(method, tx, from, blockNumbe } // If the transaction has a higher gas limit than the block gas limit, error. - if (to.number(tx.gasLimit) > to.number(this.blockchain.blockGasLimit)) { + if ( + (method === "eth_sendRawTransaction" || method === "eth_sendTransaction") && + to.number(tx.gasLimit) > to.number(this.blockchain.blockGasLimit) + ) { return callback(new TXRejectedError("Exceeds block gas limit")); } diff --git a/lib/subproviders/geth_api_double.js b/lib/subproviders/geth_api_double.js index d23b923250..c237fc19b1 100644 --- a/lib/subproviders/geth_api_double.js +++ b/lib/subproviders/geth_api_double.js @@ -1,4 +1,5 @@ var utils = require("ethereumjs-util"); +var BN = utils.BN; var inherits = require("util").inherits; var StateManager = require("../statemanager.js"); var to = require("../utils/to"); @@ -10,6 +11,13 @@ const { BlockOutOfRangeError } = require("../utils/errorhelper"); var Subprovider = require("web3-provider-engine/subproviders/subprovider.js"); +const maxUInt64 = + "0x" + + new BN(2) + .pow(new BN(64)) + .isubn(1) + .toString("hex"); + inherits(GethApiDouble, Subprovider); function GethApiDouble(options, provider) { @@ -333,18 +341,30 @@ GethApiDouble.prototype.eth_sendRawTransaction = function(rawTx, callback) { this.state.queueRawTransaction(data, callback); }; -GethApiDouble.prototype.eth_call = function(txData, blockNumber, callback) { - if (!txData.gas) { - txData.gas = this.state.blockchain.blockGasLimit; +GethApiDouble.prototype._setCallGasLimit = function(txData) { + // if the caller didn't specify a gas limit make sure we set one + if (!txData.gas && !txData.gasLimit) { + // if the user configured a global `callGasLimit` + // use it + const globalCallGasLimit = this.options.callGasLimit; + if (globalCallGasLimit != null) { + txData.gas = globalCallGasLimit; + } else { + // otherwise, set a very high gas limit. We'd use Infinity, or some VM flag to ignore gasLimit checks like + // geth does, but the VM doesn't currently support that for `runTx`. + // https://github.com/ethereumjs/ethereumjs-vm/blob/4bbb6e394a344717890d618a6be1cf67b8e5b74d/lib/runTx.ts#L71 + txData.gas = maxUInt64; + // txData.gas = this.state.blockchain.blockGasLimit; + } } - - this.state.queueTransaction("eth_call", txData, blockNumber, callback); // :( +}; +GethApiDouble.prototype.eth_call = function(txData, blockNumber, callback) { + this._setCallGasLimit(txData); + this.state.queueTransaction("eth_call", txData, blockNumber, callback); }; GethApiDouble.prototype.eth_estimateGas = function(txData, blockNumber, callback) { - if (!txData.gas) { - txData.gas = this.state.blockchain.blockGasLimit; - } + this._setCallGasLimit(txData); this.state.queueTransaction("eth_estimateGas", txData, blockNumber, callback); }; diff --git a/test/call.js b/test/call.js index 3157e3781e..ee7a4b3760 100644 --- a/test/call.js +++ b/test/call.js @@ -2,19 +2,37 @@ const assert = require("assert"); const bootstrap = require("./helpers/contract/bootstrap"); describe("eth_call", function() { - let context; - - before("Setting up web3 and contract", async function() { - this.timeout(10000); + it("should use the call gas limit if no call gas limit is specified in the call", async function() { const contractRef = { contractFiles: ["EstimateGas"], contractSubdirectory: "gas" }; + let context; - context = await bootstrap(contractRef); + context = await bootstrap(contractRef, { + callGasLimit: "0x6691b7" + }); + const { accounts, instance } = context; + + const name = "0x54696d"; // Byte code for "Tim" + const description = "0x4120677265617420677579"; // Byte code for "A great guy" + const value = 5; + + // this call uses more than the default transaction gas limit and will + // therefore fail if the block gas limit isn't used for calls + const status = await instance.methods.add(name, description, value).call({ from: accounts[0] }); + + assert.strictEqual(status, true); }); - it("should use the block gas limit if no gas limit is specified", async function() { + it("should use maxUInt64 call gas limit if no gas limit is specified in the provider or the call", async function() { + const contractRef = { + contractFiles: ["EstimateGas"], + contractSubdirectory: "gas" + }; + let context; + + context = await bootstrap(contractRef); const { accounts, instance } = context; const name = "0x54696d"; // Byte code for "Tim" @@ -22,7 +40,7 @@ describe("eth_call", function() { const value = 5; // this call uses more than the default transaction gas limit and will - // therefore fail if the block gas limit isn't used for calls + // therefore fail if the maxUInt64 limit isn't used for calls const status = await instance.methods.add(name, description, value).call({ from: accounts[0] }); assert.strictEqual(status, true); From bffbe01268ffa460eb6d1c1ce0120dfe8b8c7b0a Mon Sep 17 00:00:00 2001 From: David Murdoch Date: Fri, 12 Jul 2019 14:13:21 -0400 Subject: [PATCH 2/3] Use MAX_SAFE_INTEGER instead of something bigger --- lib/subproviders/geth_api_double.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/subproviders/geth_api_double.js b/lib/subproviders/geth_api_double.js index c237fc19b1..74db9c64fb 100644 --- a/lib/subproviders/geth_api_double.js +++ b/lib/subproviders/geth_api_double.js @@ -1,5 +1,4 @@ var utils = require("ethereumjs-util"); -var BN = utils.BN; var inherits = require("util").inherits; var StateManager = require("../statemanager.js"); var to = require("../utils/to"); @@ -11,12 +10,7 @@ const { BlockOutOfRangeError } = require("../utils/errorhelper"); var Subprovider = require("web3-provider-engine/subproviders/subprovider.js"); -const maxUInt64 = - "0x" + - new BN(2) - .pow(new BN(64)) - .isubn(1) - .toString("hex"); +const maxUInt64 = "0x" + Number.MAX_SAFE_INTEGER.toString(16); inherits(GethApiDouble, Subprovider); From a7f17c58c75ac27a680202ab43e7cd00d0a8e8b3 Mon Sep 17 00:00:00 2001 From: David Murdoch Date: Mon, 15 Jul 2019 11:42:51 -0400 Subject: [PATCH 3/3] remove dead code, move and refactor comments --- lib/blockchain_double.js | 2 +- lib/subproviders/geth_api_double.js | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/blockchain_double.js b/lib/blockchain_double.js index 86e2e7a740..39e43f0bfe 100644 --- a/lib/blockchain_double.js +++ b/lib/blockchain_double.js @@ -494,7 +494,7 @@ BlockchainDouble.prototype.readyCall = function(tx, blockNumber, callback) { newBlock.transactions.push(tx); - // gas estimates and eth_calls shouldn't be subject to block gas limits + // gas estimates and eth_calls aren't subject to block gas limits newBlock.header.gasLimit = tx.gasLimit; const runArgs = { diff --git a/lib/subproviders/geth_api_double.js b/lib/subproviders/geth_api_double.js index 74db9c64fb..8ec058fcaa 100644 --- a/lib/subproviders/geth_api_double.js +++ b/lib/subproviders/geth_api_double.js @@ -338,17 +338,15 @@ GethApiDouble.prototype.eth_sendRawTransaction = function(rawTx, callback) { GethApiDouble.prototype._setCallGasLimit = function(txData) { // if the caller didn't specify a gas limit make sure we set one if (!txData.gas && !txData.gasLimit) { - // if the user configured a global `callGasLimit` - // use it const globalCallGasLimit = this.options.callGasLimit; if (globalCallGasLimit != null) { + // if the user configured a global `callGasLimit` use it txData.gas = globalCallGasLimit; } else { // otherwise, set a very high gas limit. We'd use Infinity, or some VM flag to ignore gasLimit checks like // geth does, but the VM doesn't currently support that for `runTx`. // https://github.com/ethereumjs/ethereumjs-vm/blob/4bbb6e394a344717890d618a6be1cf67b8e5b74d/lib/runTx.ts#L71 txData.gas = maxUInt64; - // txData.gas = this.state.blockchain.blockGasLimit; } } };