diff --git a/lib/opFns.js b/lib/opFns.js index 6ad7d44e80..cdef721e20 100644 --- a/lib/opFns.js +++ b/lib/opFns.js @@ -219,11 +219,11 @@ module.exports = { } // otherwise load account then return balance - stateManager.getAccountBalance(address, function (err, value) { + stateManager.getAccount(address, function (err, account) { if (err) { return cb(err) } - cb(null, new BN(value)) + cb(null, new BN(account.balance)) }) }, ORIGIN: function (runState) { @@ -702,7 +702,6 @@ module.exports = { }) }, STATICCALL: function (gasLimit, toAddress, inOffset, inLength, outOffset, outLength, runState, done) { - var stateManager = runState.stateManager var value = new BN(0) toAddress = addressToBuffer(toAddress) @@ -723,22 +722,15 @@ module.exports = { outLength: outLength } - stateManager.accountIsEmpty(toAddress, function (err, empty) { - if (err) { - done(err) - return - } - - try { - checkCallMemCost(runState, options, localOpts) - checkOutOfGas(runState, options) - } catch (e) { - done(e.error) - return - } + try { + checkCallMemCost(runState, options, localOpts) + checkOutOfGas(runState, options) + } catch (e) { + done(e.error) + return + } - makeCall(runState, options, localOpts, done) - }) + makeCall(runState, options, localOpts, done) }, RETURN: function (offset, length, runState) { runState.returnValue = memLoad(runState, offset, length) @@ -790,9 +782,17 @@ module.exports = { runState.stopped = true var newBalance = new BN(contract.balance).add(new BN(toAccount.balance)) - async.series([ - stateManager.putAccountBalance.bind(stateManager, selfdestructToAddress, newBalance), - stateManager.putAccountBalance.bind(stateManager, contractAddress, new BN(0)) + async.waterfall([ + stateManager.getAccount.bind(stateManager, selfdestructToAddress), + function (account, cb) { + account.balance = newBalance + stateManager.putAccount(selfdestructToAddress, account, cb) + }, + stateManager.getAccount.bind(stateManager, contractAddress), + function (account, cb) { + account.balance = new BN(0) + stateManager.putAccount(contractAddress, account, cb) + } ], function (err) { // The reason for this is to avoid sending an array of results cb(err) @@ -970,6 +970,7 @@ function makeCall (runState, callOptions, localOpts, cb) { callOptions.block = runState.block callOptions.static = callOptions.static || false callOptions.selfdestruct = runState.selfdestruct + callOptions.storageReader = runState.storageReader // increment the runState.depth callOptions.depth = runState.depth + 1 @@ -1057,10 +1058,7 @@ function isCreateOpCode (opName) { function getContractStorage (runState, address, key, cb) { if (runState._common.gteHardfork('constantinople')) { - async.parallel({ - original: function (cb) { runState.originalState.getContractStorage(address, key, cb) }, - current: function (cb) { runState.stateManager.getContractStorage(address, key, cb) } - }, cb) + runState.storageReader.getContractStorage(address, key, cb) } else { runState.stateManager.getContractStorage(address, key, cb) } diff --git a/lib/runCall.js b/lib/runCall.js index dd5a871b5f..dcd7104575 100644 --- a/lib/runCall.js +++ b/lib/runCall.js @@ -3,6 +3,7 @@ const async = require('async') const ethUtil = require('ethereumjs-util') const BN = ethUtil.BN const exceptions = require('./exceptions.js') +const StorageReader = require('./storageReader') const ERROR = exceptions.ERROR @@ -48,6 +49,7 @@ module.exports = function (opts, cb) { var delegatecall = opts.delegatecall || false var isStatic = opts.static || false var salt = opts.salt || null + var storageReader = opts.storageReader || new StorageReader(stateManager) txValue = new BN(txValue) @@ -149,7 +151,7 @@ module.exports = function (opts, cb) { } var newBalance = new BN(account.balance).sub(txValue) account.balance = newBalance - stateManager.putAccountBalance(ethUtil.toBuffer(caller), newBalance, cb) + stateManager.putAccount(ethUtil.toBuffer(caller), account, cb) } function addTxValue (cb) { @@ -205,7 +207,8 @@ module.exports = function (opts, cb) { block: block, depth: depth, selfdestruct: selfdestruct, - static: isStatic + static: isStatic, + storageReader: storageReader } // run Code through vm diff --git a/lib/runCode.js b/lib/runCode.js index 01316bd9de..8769b13461 100644 --- a/lib/runCode.js +++ b/lib/runCode.js @@ -18,6 +18,7 @@ const Block = require('ethereumjs-block') const lookupOpInfo = require('./opcodes.js') const opFns = require('./opFns.js') const exceptions = require('./exceptions.js') +const StorageReader = require('./storageReader') const setImmediate = require('timers').setImmediate const BN = utils.BN @@ -64,7 +65,7 @@ module.exports = function (opts, cb) { var runState = { blockchain: self.blockchain, stateManager: stateManager, - originalState: stateManager.copy(), + storageReader: opts.storageReader || new StorageReader(stateManager), returnValue: false, stopped: false, vmError: false, diff --git a/lib/runTx.js b/lib/runTx.js index 46c8e7e223..e6e4fe1473 100644 --- a/lib/runTx.js +++ b/lib/runTx.js @@ -5,6 +5,7 @@ const BN = utils.BN const Bloom = require('./bloom.js') const Block = require('ethereumjs-block') const Account = require('ethereumjs-account') +const StorageReader = require('./storageReader') /** * Process a transaction. Run the vm. Transfers eth. Checks balances. @@ -39,6 +40,7 @@ module.exports = function (opts, cb) { var gasLimit var results var basefee + var storageReader = new StorageReader(self.stateManager) // tx is required if (!tx) { @@ -141,7 +143,8 @@ module.exports = function (opts, cb) { to: tx.to, value: tx.value, data: tx.data, - block: block + block: block, + storageReader: storageReader } if (tx.to.toString('hex') === '') { @@ -196,7 +199,7 @@ module.exports = function (opts, cb) { .add(new BN(fromAccount.balance)) fromAccount.balance = finalFromBalance - self.stateManager.putAccountBalance(utils.toBuffer(tx.from), finalFromBalance, next) + self.stateManager.putAccount(utils.toBuffer(tx.from), fromAccount, next) } var minerAccount diff --git a/lib/stateManager.js b/lib/stateManager.js index 833eb046e6..7137707e9d 100644 --- a/lib/stateManager.js +++ b/lib/stateManager.js @@ -52,29 +52,6 @@ proto.putAccount = function (address, account, cb) { cb() } -proto.getAccountBalance = function (address, cb) { - var self = this - self.getAccount(address, function (err, account) { - if (err) { - return cb(err) - } - cb(null, account.balance) - }) -} - -proto.putAccountBalance = function (address, balance, cb) { - var self = this - - self.getAccount(address, function (err, account) { - if (err) { - return cb(err) - } - - account.balance = balance - self.putAccount(address, account, cb) - }) -} - // sets the contract code on the account proto.putContractCode = function (address, value, cb) { var self = this @@ -307,6 +284,7 @@ proto.accountIsEmpty = function (address, cb) { return cb(err) } + // should be replaced by account.isEmpty() once updated cb(null, account.nonce.toString('hex') === '' && account.balance.toString('hex') === '' && account.codeHash.toString('hex') === utils.KECCAK256_NULL_S) }) } diff --git a/lib/storageReader.js b/lib/storageReader.js new file mode 100644 index 0000000000..36fa791e7d --- /dev/null +++ b/lib/storageReader.js @@ -0,0 +1,40 @@ +module.exports = StorageReader + +function StorageReader (stateManager) { + this._stateManager = stateManager + this._storageCache = new Map() +} + +const proto = StorageReader.prototype + +proto.getContractStorage = function getContractStorage (address, key, cb) { + const self = this + const addressHex = address.toString('hex') + const keyHex = key.toString('hex') + + self._stateManager.getContractStorage(address, key, function (err, current) { + if (err) return cb(err) + + let map = null + if (!self._storageCache.has(addressHex)) { + map = new Map() + self._storageCache.set(addressHex, map) + } else { + map = self._storageCache.get(addressHex) + } + + let original = null + + if (map.has(keyHex)) { + original = map.get(keyHex) + } else { + map.set(keyHex, current) + original = current + } + + cb(null, { + original, + current + }) + }) +} diff --git a/tests/api/storageReader.js b/tests/api/storageReader.js new file mode 100644 index 0000000000..b62ec7f8b0 --- /dev/null +++ b/tests/api/storageReader.js @@ -0,0 +1,82 @@ +const { promisify } = require('util') +const tape = require('tape') +const StorageReader = require('../../lib/storageReader') + +const mkStateManagerMock = () => { + let i = 0 + return { + getContractStorage: function (address, key, cb) { + cb(null, i++) + } + } +} + +tape('StateManager', (t) => { + t.test('should get value from stateManager', async (st) => { + const storageReader = new StorageReader(mkStateManagerMock()) + const getContractStorage = promisify((...args) => storageReader.getContractStorage(...args)) + const { + original, + current + } = await getContractStorage(Buffer.from([1]), Buffer.from([1, 1])) + + st.equal(original, 0) + st.equal(current, 0) + st.end() + }) + + t.test('should retrieve original from cache', async (st) => { + const storageReader = new StorageReader(mkStateManagerMock()) + const getContractStorage = promisify((...args) => storageReader.getContractStorage(...args)) + await getContractStorage(Buffer.from([1]), Buffer.from([1, 1])) + const { + original, + current + } = await getContractStorage(Buffer.from([1]), Buffer.from([1, 1])) + + st.equal(original, 0) + st.equal(current, 1) + st.end() + }) + + t.test('should cache keys separately', async (st) => { + const storageReader = new StorageReader(mkStateManagerMock()) + const getContractStorage = promisify((...args) => storageReader.getContractStorage(...args)) + await getContractStorage(Buffer.from([1]), Buffer.from([1, 1])) + const { + original, + current + } = await getContractStorage(Buffer.from([1]), Buffer.from([1, 2])) + + st.equal(original, 1) + st.equal(current, 1) + st.end() + }) + + t.test('should cache addresses separately', async (st) => { + const storageReader = new StorageReader(mkStateManagerMock()) + const getContractStorage = promisify((...args) => storageReader.getContractStorage(...args)) + await getContractStorage(Buffer.from([1]), Buffer.from([1, 1])) + const { + original, + current + } = await getContractStorage(Buffer.from([2]), Buffer.from([1, 1])) + + st.equal(original, 1) + st.equal(current, 1) + st.end() + }) + + t.test('should forward errors from stateManager.getContractStorage', async (st) => { + const storageReader = new StorageReader({ + getContractStorage: (address, key, cb) => cb(new Error('test')) + }) + const getContractStorage = promisify((...args) => storageReader.getContractStorage(...args)) + + await getContractStorage(Buffer.from([2]), Buffer.from([1, 1])) + .then(() => t.fail('should have returned error')) + .catch((e) => t.equal(e.message, 'test')) + + st.end() + }) +})