Skip to content

Commit

Permalink
StateManager interface simplification
Browse files Browse the repository at this point in the history
Simplifies the stateManager interface used by the VM by removing dependencies on the following methods:
* getAccountBalance
* putAccountBalance
* copy
  • Loading branch information
mattdean-digicatapult authored and holgerd77 committed Nov 15, 2018
1 parent 7de7a5b commit c346a04
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 53 deletions.
48 changes: 23 additions & 25 deletions lib/opFns.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 5 additions & 2 deletions lib/runCall.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/runCode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions lib/runTx.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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') === '') {
Expand Down Expand Up @@ -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
Expand Down
24 changes: 1 addition & 23 deletions lib/stateManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
}
Expand Down
40 changes: 40 additions & 0 deletions lib/storageReader.js
Original file line number Diff line number Diff line change
@@ -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
})
})
}
82 changes: 82 additions & 0 deletions tests/api/storageReader.js
Original file line number Diff line number Diff line change
@@ -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()
})
})

0 comments on commit c346a04

Please sign in to comment.