From f9df024d799dcaba1944d19edcc66456768128dd Mon Sep 17 00:00:00 2001 From: Matthew Dean Date: Fri, 5 Oct 2018 16:01:05 +0100 Subject: [PATCH] Remove explicit direct cache interactions The method `warmCache` on `stateManager` and the `populateCache` option directly expose the existence of the state cache. This makes removing all references to the cache impossible. --- lib/index.js | 4 -- lib/opFns.js | 8 +-- lib/runBlock.js | 114 +++++++++++++++++++----------------------- lib/runCall.js | 19 ++++--- lib/runCode.js | 10 +--- lib/runTx.js | 111 +++++++++++++++++++--------------------- lib/stateManager.js | 16 +++--- tests/api/runBlock.js | 5 +- tests/api/runTx.js | 5 +- 9 files changed, 129 insertions(+), 163 deletions(-) diff --git a/lib/index.js b/lib/index.js index c502cda20d..01cfebe173 100644 --- a/lib/index.js +++ b/lib/index.js @@ -102,7 +102,3 @@ VM.prototype.copy = function () { VM.prototype.loadCompiled = function (address, src, cb) { this.stateManager.trie.db.put(address, src, cb) } - -VM.prototype.populateCache = function (addresses, cb) { - this.stateManager.warmCache(addresses, cb) -} diff --git a/lib/opFns.js b/lib/opFns.js index 05999b9e64..7c93bb9e92 100644 --- a/lib/opFns.js +++ b/lib/opFns.js @@ -385,8 +385,11 @@ module.exports = { stateManager.putContractStorage(address, key, value, function (err) { if (err) return cb(err) - runState.contract = stateManager.cache.get(address) - cb(null) + stateManager.getAccount(address, function (err, account) { + if (err) return cb(err) + runState.contract = account + cb(null) + }) }) }) }, @@ -934,7 +937,6 @@ function makeCall (runState, callOptions, localOpts, cb) { callOptions.origin = runState.origin callOptions.gasPrice = runState.gasPrice callOptions.block = runState.block - callOptions.populateCache = false callOptions.static = callOptions.static || false callOptions.selfdestruct = runState.selfdestruct diff --git a/lib/runBlock.js b/lib/runBlock.js index 0811fe7806..dbc7419d46 100644 --- a/lib/runBlock.js +++ b/lib/runBlock.js @@ -32,8 +32,6 @@ module.exports = function (opts, cb) { const receiptTrie = new Trie() // the total amount of gas used processing this block var gasUsed = new BN(0) - // miner account - var minerAccount var receipts = [] var txResults = [] var result @@ -47,8 +45,8 @@ module.exports = function (opts, cb) { // run everything async.series([ beforeBlock, - populateCache, - processTransactions + processTransactions, + payOmmersAndMiner ], parseBlockResults) function beforeBlock (cb) { @@ -59,22 +57,6 @@ module.exports = function (opts, cb) { self.emit('afterBlock', result, cb) } - // populates the cache with accounts that we know we will need - function populateCache (cb) { - var accounts = new Set() - accounts.add(block.header.coinbase.toString('hex')) - block.transactions.forEach(function (tx) { - accounts.add(tx.getSenderAddress().toString('hex')) - accounts.add(tx.to.toString('hex')) - }) - - block.uncleHeaders.forEach(function (uh) { - accounts.add(uh.coinbase.toString('hex')) - }) - - self.populateCache(accounts, cb) - } - /** * Processes all of the transaction in the block * @method processTransaction @@ -95,8 +77,7 @@ module.exports = function (opts, cb) { // run the tx through the VM self.runTx({ tx: tx, - block: block, - populateCache: false + block: block }, parseTxResult) function parseTxResult (err, result) { @@ -134,13 +115,58 @@ module.exports = function (opts, cb) { } receipts.push(txReceipt) - receiptTrie.put(rlp.encode(validReceiptCount), rlp.encode(rawTxReceipt)) - validReceiptCount++ - cb() + receiptTrie.put(rlp.encode(validReceiptCount), rlp.encode(rawTxReceipt), function () { + validReceiptCount++ + cb() + }) } } } + // credit all block rewards + function payOmmersAndMiner (cb) { + var ommers = block.uncleHeaders + + // pay each ommer + async.series([ + rewardOmmers, + rewardMiner + ], cb) + + function rewardOmmers (done) { + async.each(block.uncleHeaders, function (ommer, next) { + // calculate reward + var minerReward = new BN(self._common.param('pow', 'minerReward')) + var heightDiff = new BN(block.header.number).sub(new BN(ommer.number)) + var reward = ((new BN(8)).sub(heightDiff)).mul(minerReward.divn(8)) + + if (reward.ltn(0)) { + reward = new BN(0) + } + + rewardAccount(ommer.coinbase, reward, next) + }, done) + } + + function rewardMiner (done) { + // calculate nibling reward + var minerReward = new BN(self._common.param('pow', 'minerReward')) + var niblingReward = minerReward.divn(32) + var totalNiblingReward = niblingReward.muln(ommers.length) + var reward = minerReward.add(totalNiblingReward) + rewardAccount(block.header.coinbase, reward, done) + } + + function rewardAccount (address, reward, done) { + self.stateManager.getAccount(address, function (err, account) { + if (err) return done(err) + // give miner the block reward + account.balance = new BN(account.balance).add(reward) + self.stateManager.putAccount(address, account, done) + }) + } + } + // handle results or error from block run function parseBlockResults (err) { if (err) { @@ -149,9 +175,6 @@ module.exports = function (opts, cb) { return } - // credit all block rewards - payOmmersAndMiner() - // credit all block rewards if (generateStateRoot) { block.header.stateRoot = self.stateManager.trie.root @@ -186,39 +209,4 @@ module.exports = function (opts, cb) { }) }) } - - // credit all block rewards - function payOmmersAndMiner () { - var ommers = block.uncleHeaders - // pay each ommer - ommers.forEach(rewardOmmer) - - // calculate nibling reward - var minerReward = new BN(self._common.param('pow', 'minerReward')) - var niblingReward = minerReward.divn(32) - var totalNiblingReward = niblingReward.muln(ommers.length) - minerAccount = self.stateManager.cache.get(block.header.coinbase) - // give miner the block reward - minerAccount.balance = new BN(minerAccount.balance) - .add(minerReward) - .add(totalNiblingReward) - self.stateManager.cache.put(block.header.coinbase, minerAccount) - } - - // credit ommer - function rewardOmmer (ommer) { - // calculate reward - var minerReward = new BN(self._common.param('pow', 'minerReward')) - var heightDiff = new BN(block.header.number).sub(new BN(ommer.number)) - var reward = ((new BN(8)).sub(heightDiff)).mul(minerReward.divn(8)) - - if (reward.ltn(0)) { - reward = new BN(0) - } - - // credit miners account - var ommerAccount = self.stateManager.cache.get(ommer.coinbase) - ommerAccount.balance = reward.add(new BN(ommerAccount.balance)) - self.stateManager.cache.put(ommer.coinbase, ommerAccount) - } } diff --git a/lib/runCall.js b/lib/runCall.js index 7581e4d511..f5d798b617 100644 --- a/lib/runCall.js +++ b/lib/runCall.js @@ -31,7 +31,7 @@ module.exports = function (opts, cb) { var createdAddress var txValue = opts.value || Buffer.from([0]) var caller = opts.caller - var account = stateManager.cache.get(caller) + var account var block = opts.block var code = opts.code var txData = opts.data @@ -53,6 +53,7 @@ module.exports = function (opts, cb) { // run and parse async.series([ + loadFromAccount, subTxValue, loadToAccount, addTxValue, @@ -61,9 +62,15 @@ module.exports = function (opts, cb) { saveCode ], parseCallResult) + function loadFromAccount (done) { + stateManager.getAccount(caller, function (err, fromAccount) { + account = fromAccount + done(err) + }) + } + function loadToAccount (done) { // get receiver's account - // toAccount = stateManager.cache.get(toAddress) if (!toAddress) { // generate a new contract if no `to` code = txData @@ -98,8 +105,10 @@ module.exports = function (opts, cb) { }) } else { // else load the `to` account - toAccount = stateManager.cache.get(toAddress) - done() + stateManager.getAccount(toAddress, function (err, account) { + toAccount = account + done(err) + }) } } @@ -166,7 +175,6 @@ module.exports = function (opts, cb) { block: block, depth: depth, selfdestruct: selfdestruct, - populateCache: false, static: isStatic } @@ -175,7 +183,6 @@ module.exports = function (opts, cb) { codeRunner.call(self, runCodeOpts, parseRunResult) function parseRunResult (err, results) { - toAccount = self.stateManager.cache.get(toAddress) vmResults = results if (createdAddress) { diff --git a/lib/runCode.js b/lib/runCode.js index 2e1b36268c..8424facf94 100644 --- a/lib/runCode.js +++ b/lib/runCode.js @@ -76,7 +76,6 @@ module.exports = function (opts, cb) { origin: opts.origin || opts.caller || utils.zeros(32), callData: opts.data || Buffer.from([0]), code: opts.code, - populateCache: opts.populateCache === undefined ? true : opts.populateCache, static: opts.static || false } @@ -262,14 +261,7 @@ module.exports = function (opts, cb) { results.gasUsed = runState.gasLimit.sub(runState.gasLeft) } - if (runState.populateCache) { - self.stateManager.cache.flush(function () { - self.stateManager.cache.clear() - cb(err, results) - }) - } else { - cb(err, results) - } + cb(err, results) } } diff --git a/lib/runTx.js b/lib/runTx.js index beb0593d0c..c7273214c3 100644 --- a/lib/runTx.js +++ b/lib/runTx.js @@ -43,19 +43,17 @@ module.exports = function (opts, cb) { return } - if (opts.populateCache === undefined) { - opts.populateCache = true - } + self.stateManager.checkpoint() // run everything async.series([ - populateCache, runTxHook, + updateFromAccount, runCall, - flushCache, runAfterTxHook ], function (err) { - cb(err, results) + if (err) self.stateManager.revert(() => cb(err, results)) + else self.stateManager.commit((err) => cb(err, results)) }) // run the transaction hook @@ -68,54 +66,41 @@ module.exports = function (opts, cb) { self.emit('afterTx', results, cb) } - /** - * populates the cache with the 'to' and 'from' of the tx - */ - function populateCache (cb) { - if (opts.populateCache === false) { - return cb() - } + function updateFromAccount (cb) { + self.stateManager.getAccount(tx.from, function (err, fromAccount) { + if (err) { + cb(err) + return + } - var accounts = new Set() - accounts.add(tx.from.toString('hex')) - accounts.add(block.header.coinbase.toString('hex')) + var message + if (!opts.skipBalance && new BN(fromAccount.balance).lt(tx.getUpfrontCost())) { + message = "sender doesn't have enough funds to send tx. The upfront cost is: " + tx.getUpfrontCost().toString() + ' and the sender\'s account only has: ' + new BN(fromAccount.balance).toString() + cb(new Error(message)) + return + } else if (!opts.skipNonce && !(new BN(fromAccount.nonce).eq(new BN(tx.nonce)))) { + message = "the tx doesn't have the correct nonce. account has nonce of: " + new BN(fromAccount.nonce).toString() + ' tx has nonce of: ' + new BN(tx.nonce).toString() + cb(new Error(message)) + return + } - if (tx.to.toString('hex') !== '') { - accounts.add(tx.to.toString('hex')) - } + // increment the nonce + fromAccount.nonce = new BN(fromAccount.nonce).addn(1) + + basefee = tx.getBaseFee() + gasLimit = new BN(tx.gasLimit) + if (gasLimit.lt(basefee)) { + return cb(new Error('base fee exceeds gas limit')) + } + gasLimit.isub(basefee) - self.stateManager.warmCache(accounts, cb) + fromAccount.balance = new BN(fromAccount.balance).sub(new BN(tx.gasLimit).mul(new BN(tx.gasPrice))) + self.stateManager.putAccount(tx.from, fromAccount, cb) + }) } // sets up the environment and runs a `call` function runCall (cb) { - // check to the sender's account to make sure it has enough wei and the correct nonce - var fromAccount = self.stateManager.cache.get(tx.from) - var message - - if (!opts.skipBalance && new BN(fromAccount.balance).lt(tx.getUpfrontCost())) { - message = "sender doesn't have enough funds to send tx. The upfront cost is: " + tx.getUpfrontCost().toString() + ' and the sender\'s account only has: ' + new BN(fromAccount.balance).toString() - cb(new Error(message)) - return - } else if (!opts.skipNonce && !(new BN(fromAccount.nonce).eq(new BN(tx.nonce)))) { - message = "the tx doesn't have the correct nonce. account has nonce of: " + new BN(fromAccount.nonce).toString() + ' tx has nonce of: ' + new BN(tx.nonce).toString() - cb(new Error(message)) - return - } - - // increment the nonce - fromAccount.nonce = new BN(fromAccount.nonce).addn(1) - - basefee = tx.getBaseFee() - gasLimit = new BN(tx.gasLimit) - if (gasLimit.lt(basefee)) { - return cb(new Error('base fee exceeds gas limit')) - } - gasLimit.isub(basefee) - - fromAccount.balance = new BN(fromAccount.balance).sub(new BN(tx.gasLimit).mul(new BN(tx.gasPrice))) - self.stateManager.cache.put(tx.from, fromAccount) - var options = { caller: tx.from, gasLimit: gasLimit, @@ -123,8 +108,7 @@ module.exports = function (opts, cb) { to: tx.to, value: tx.value, data: tx.data, - block: block, - populateCache: false + block: block } if (tx.to.toString('hex') === '') { @@ -140,7 +124,6 @@ module.exports = function (opts, cb) { // generate the bloom for the tx results.bloom = txLogsBloom(results.vm.logs) - fromAccount = self.stateManager.cache.get(tx.from) // caculate the total gas used results.gasUsed = results.gasUsed.add(basefee) @@ -158,11 +141,21 @@ module.exports = function (opts, cb) { results.amountSpent = results.gasUsed.mul(new BN(tx.gasPrice)) async.series([ + loadFromAccount, updateFromAccount, + loadMinerAccount, updateMinerAccount, cleanupAccounts ], cb) + var fromAccount + function loadFromAccount (next) { + self.stateManager.getAccount(tx.from, function (err, account) { + fromAccount = account + next(err) + }) + } + function updateFromAccount (next) { // refund the leftover gas amount var finalFromBalance = new BN(tx.gasLimit).sub(results.gasUsed) @@ -173,8 +166,15 @@ module.exports = function (opts, cb) { self.stateManager.putAccountBalance(utils.toBuffer(tx.from), finalFromBalance, next) } + var minerAccount + function loadMinerAccount (next) { + self.stateManager.getAccount(block.header.coinbase, function (err, account) { + minerAccount = account + next(err) + }) + } + function updateMinerAccount (next) { - var minerAccount = self.stateManager.cache.get(block.header.coinbase) // add the amount spent on gas to the miner's account minerAccount.balance = new BN(minerAccount.balance) .add(results.amountSpent) @@ -202,15 +202,6 @@ module.exports = function (opts, cb) { } } } - - function flushCache (cb) { - self.stateManager.cache.flush(function () { - if (opts.populateCache) { - self.stateManager.cache.clear() - } - cb() - }) - } } /** diff --git a/lib/stateManager.js b/lib/stateManager.js index 48d77fc99b..95f0b81021 100644 --- a/lib/stateManager.js +++ b/lib/stateManager.js @@ -207,7 +207,9 @@ proto.commit = function (cb) { // setup cache checkpointing self.cache.commit() self._touchedStack.pop() - cb() + + if (self._touchedStack.length === 0) self.cache.flush(cb) + else cb() }) } @@ -219,7 +221,9 @@ proto.revert = function (cb) { self.cache.revert() self._storageTries = {} self._touched = self._touchedStack.pop() - cb() + + if (self._touchedStack.length === 0) self.cache.flush(cb) + else cb() } // @@ -236,14 +240,6 @@ proto.getStateRoot = function (cb) { }) } -/** - * @param {Set} addresses a set of addresses - * @param {Function} cb the callback function - */ -proto.warmCache = function (addresses, cb) { - this.cache.warm(addresses, cb) -} - proto.dumpStorage = function (address, cb) { var self = this self._getStorageTrie(address, function (err, trie) { diff --git a/tests/api/runBlock.js b/tests/api/runBlock.js index f08a123bf2..f55baab1e5 100644 --- a/tests/api/runBlock.js +++ b/tests/api/runBlock.js @@ -20,7 +20,6 @@ function setup (vm = null) { vm = { stateManager, emit: (e, val, cb) => cb(), - populateCache: stateManager.warmCache.bind(stateManager), runTx: (opts, cb) => cb(new Error('test')), runCall: (opts, cb) => cb(new Error('test')), _common: new Common('mainnet', 'byzantium') @@ -32,8 +31,7 @@ function setup (vm = null) { data: testData, p: { runBlock: promisify(runBlock.bind(vm)), - putAccount: promisify(vm.stateManager.putAccount.bind(vm.stateManager)), - cacheFlush: promisify(vm.stateManager.cache.flush.bind(vm.stateManager.cache)) + putAccount: promisify(vm.stateManager.putAccount.bind(vm.stateManager)) } } } @@ -94,7 +92,6 @@ tape('should fail when runCall fails', async (t) => { const acc = createAccount() await suite.p.putAccount(tx.from.toString('hex'), acc) } - await suite.p.cacheFlush() // The mocked VM uses a mocked runCall // which always returns an error. diff --git a/tests/api/runTx.js b/tests/api/runTx.js index b769783f05..a9b83de535 100644 --- a/tests/api/runTx.js +++ b/tests/api/runTx.js @@ -18,8 +18,7 @@ function setup (vm = null) { return { vm, runTx: promisify(runTx.bind(vm)), - putAccount: promisify(vm.stateManager.putAccount.bind(vm.stateManager)), - cacheFlush: promisify(vm.stateManager.cache.flush.bind(vm.stateManager.cache)) + putAccount: promisify(vm.stateManager.putAccount.bind(vm.stateManager)) } } @@ -63,7 +62,6 @@ tape('should fail when runCall fails', async (t) => { const tx = getTransaction(true, true) const acc = createAccount() await suite.putAccount(tx.from.toString('hex'), acc) - await suite.cacheFlush() shouldFail(t, suite.runTx({ tx, populateCache: true }), @@ -80,7 +78,6 @@ tape('should run simple tx without errors', async (t) => { const tx = getTransaction(true, true) const acc = createAccount() await suite.putAccount(tx.from.toString('hex'), acc) - await suite.cacheFlush() let res = await suite.runTx({ tx, populateCache: true }) t.true(res.gasUsed.gt(0), 'should have used some gas')