From 506fb908b686318858f9e80e1a15f7e51384e2eb Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 28 Jan 2019 15:59:08 +0100 Subject: [PATCH 1/6] Add jsdoc comments for cache Signed-off-by: Sina Mahmoodi --- lib/cache.js | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/lib/cache.js b/lib/cache.js index a82ca726bc..90d2db03ab 100644 --- a/lib/cache.js +++ b/lib/cache.js @@ -9,12 +9,21 @@ var Cache = module.exports = function (trie) { this._trie = trie } +/** + * Puts account to cache under its address. + * @param {Buffer} key - Address of account + * @param {Account} val - Account + * @param {bool} fromTrie + */ Cache.prototype.put = function (key, val, fromTrie) { var modified = !fromTrie this._update(key, val, modified, false) } -// returns the queried account or an empty account +/** + * Returns the queried account or an empty account. + * @param {Buffer} key - Address of account + */ Cache.prototype.get = function (key) { var account = this.lookup(key) if (!account) { @@ -23,7 +32,10 @@ Cache.prototype.get = function (key) { return account } -// returns the queried account or undefined +/** + * Returns the queried account or undefined. + * @param {buffer} key - Address of account + */ Cache.prototype.lookup = function (key) { key = key.toString('hex') @@ -34,6 +46,11 @@ Cache.prototype.lookup = function (key) { } } +/** + * Looks up address in underlying trie. + * @param {Buffer} address - Address of account + * @param {Function} cb - Callback with params (err, account) + */ Cache.prototype._lookupAccount = function (address, cb) { var self = this self._trie.get(address, function (err, raw) { @@ -43,6 +60,12 @@ Cache.prototype._lookupAccount = function (address, cb) { }) } +/** + * Looks up address in cache, if not found, looks it up + * in the underlying trie. + * @param {Buffer} key - Address of account + * @param {Function} cb - Callback with params (err, account) + */ Cache.prototype.getOrLoad = function (key, cb) { var self = this var account = this.lookup(key) @@ -57,6 +80,12 @@ Cache.prototype.getOrLoad = function (key, cb) { } } +/** + * Warms cache by loading their respective account from trie + * and putting them in cache. + * @param {Array} addresses - Array of addresses + * @param {Function} cb - Callback + */ Cache.prototype.warm = function (addresses, cb) { var self = this // shim till async supports iterators @@ -75,6 +104,11 @@ Cache.prototype.warm = function (addresses, cb) { }, cb) } +/** + * Flushes cache by updating accounts that have been modified + * and removing accounts that have been deleted. + * @param {function} cb - Callback + */ Cache.prototype.flush = function (cb) { var it = this._cache.begin var self = this @@ -107,22 +141,39 @@ Cache.prototype.flush = function (cb) { }, cb) } +/** + * Marks current state of cache as checkpoint, which can + * later on be reverted or commited. + */ Cache.prototype.checkpoint = function () { this._checkpoints.push(this._cache) } +/** + * Revert changes to cache last checkpoint (no effect on trie). + */ Cache.prototype.revert = function () { this._cache = this._checkpoints.pop(this._cache) } +/** + * Commits to current state of cache (no effect on trie). + */ Cache.prototype.commit = function () { this._checkpoints.pop() } +/** + * Clears cache. + */ Cache.prototype.clear = function () { this._cache = Tree() } +/** + * Marks address as deleted in cache. + * @params {Buffer} key - Address + */ Cache.prototype.del = function (key) { this._update(key, new Account(), false, true) } From 8020a1a8dd97c8baf71317e97256d19423aafffd Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 28 Jan 2019 15:59:21 +0100 Subject: [PATCH 2/6] Add api tests for cache Signed-off-by: Sina Mahmoodi --- tests/api/cache.js | 108 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/api/cache.js diff --git a/tests/api/cache.js b/tests/api/cache.js new file mode 100644 index 0000000000..75e0b7c5a6 --- /dev/null +++ b/tests/api/cache.js @@ -0,0 +1,108 @@ +const { promisify } = require('util') +const tape = require('tape') +const Trie = require('merkle-patricia-tree/secure.js') +const Account = require('ethereumjs-account') +const Cache = require('../../lib/cache') +const utils = require('./utils') + +tape('cache initialization', (t) => { + t.test('should initialize', async (st) => { + const trie = new Trie() + const c = new Cache(trie) + st.ok(trie.root.equals(c._trie.root), 'initializes given trie') + st.end() + }) +}) + +tape('cache put and get account', (t) => { + const trie = new Trie() + const c = new Cache(trie) + const flushP = promisify(c.flush.bind(c)) + const trieGetP = promisify(trie.get.bind(trie)) + + const addr = Buffer.from('cd2a3d9f938e13cd947ec05abc7fe734df8dd826', 'hex') + const acc = utils.createAccount('0x00', '0xff11') + + t.test('should fail to get non-existent account', async (st) => { + const res = c.get(addr) + st.notOk(res.balance.equals(acc.balance)) + st.end() + }) + + t.test('should put account', async (st) => { + c.put(addr, acc) + const res = c.get(addr) + st.ok(res.balance.equals(acc.balance)) + st.end() + }) + + t.test('should not have flushed to trie', async (st) => { + const res = await trieGetP(addr) + st.notOk(res) + st.end() + }) + + t.test('should flush to trie', async (st) => { + await flushP() + st.end() + }) + + t.test('trie should contain flushed account', async (st) => { + const raw = await trieGetP(addr) + const res = new Account(raw) + st.ok(res.balance.equals(acc.balance)) + st.end() + }) + + t.test('should delete account from cache', async (st) => { + c.del(addr) + + const res = c.get(addr) + st.notOk(res.balance.equals(acc.balance)) + st.end() + }) + + t.test('should warm cache and load account from trie', async (st) => { + await promisify(c.warm.bind(c))([addr]) + + const res = c.get(addr) + st.ok(res.balance.equals(acc.balance)) + st.end() + }) + + t.test('should update loaded account and flush it', async (st) => { + const updatedAcc = utils.createAccount('0x00', '0xff00') + c.put(addr, updatedAcc) + await flushP() + + const raw = await trieGetP(addr) + const res = new Account(raw) + st.ok(res.balance.equals(updatedAcc.balance)) + st.end() + }) +}) + +tape('cache checkpointing', (t) => { + const trie = new Trie() + const c = new Cache(trie) + + const addr = Buffer.from('cd2a3d9f938e13cd947ec05abc7fe734df8dd826', 'hex') + const acc = utils.createAccount('0x00', '0xff11') + const updatedAcc = utils.createAccount('0x00', '0xff00') + + t.test('should revert to correct state', async (st) => { + c.put(addr, acc) + c.checkpoint() + c.put(addr, updatedAcc) + + let res = c.get(addr) + st.ok(res.balance.equals(updatedAcc.balance)) + + c.revert() + + res = c.get(addr) + st.ok(res.balance.equals(acc.balance)) + + st.end() + }) +}) From ed3a7dadca260b0aae82446aa4440b02d56864e7 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 28 Jan 2019 16:03:49 +0100 Subject: [PATCH 3/6] Convert cache to es6 class Signed-off-by: Sina Mahmoodi --- lib/cache.js | 334 ++++++++++++++++++++++++++------------------------- 1 file changed, 168 insertions(+), 166 deletions(-) diff --git a/lib/cache.js b/lib/cache.js index 90d2db03ab..e32b7503b1 100644 --- a/lib/cache.js +++ b/lib/cache.js @@ -3,195 +3,197 @@ const Tree = require('functional-red-black-tree') const Account = require('ethereumjs-account') const async = require('async') -var Cache = module.exports = function (trie) { - this._cache = Tree() - this._checkpoints = [] - this._trie = trie -} - -/** - * Puts account to cache under its address. - * @param {Buffer} key - Address of account - * @param {Account} val - Account - * @param {bool} fromTrie - */ -Cache.prototype.put = function (key, val, fromTrie) { - var modified = !fromTrie - this._update(key, val, modified, false) -} - -/** - * Returns the queried account or an empty account. - * @param {Buffer} key - Address of account - */ -Cache.prototype.get = function (key) { - var account = this.lookup(key) - if (!account) { - account = new Account() +module.exports = class Cache { + constructor (trie) { + this._cache = Tree() + this._checkpoints = [] + this._trie = trie } - return account -} -/** - * Returns the queried account or undefined. - * @param {buffer} key - Address of account - */ -Cache.prototype.lookup = function (key) { - key = key.toString('hex') + /** + * Puts account to cache under its address. + * @param {Buffer} key - Address of account + * @param {Account} val - Account + * @param {bool} fromTrie + */ + put (key, val, fromTrie) { + var modified = !fromTrie + this._update(key, val, modified, false) + } - var it = this._cache.find(key) - if (it.node) { - var account = new Account(it.value.val) + /** + * Returns the queried account or an empty account. + * @param {Buffer} key - Address of account + */ + get (key) { + var account = this.lookup(key) + if (!account) { + account = new Account() + } return account } -} -/** - * Looks up address in underlying trie. - * @param {Buffer} address - Address of account - * @param {Function} cb - Callback with params (err, account) - */ -Cache.prototype._lookupAccount = function (address, cb) { - var self = this - self._trie.get(address, function (err, raw) { - if (err) return cb(err) - var account = new Account(raw) - cb(null, account) - }) -} + /** + * Returns the queried account or undefined. + * @param {buffer} key - Address of account + */ + lookup (key) { + key = key.toString('hex') + + var it = this._cache.find(key) + if (it.node) { + var account = new Account(it.value.val) + return account + } + } -/** - * Looks up address in cache, if not found, looks it up - * in the underlying trie. - * @param {Buffer} key - Address of account - * @param {Function} cb - Callback with params (err, account) - */ -Cache.prototype.getOrLoad = function (key, cb) { - var self = this - var account = this.lookup(key) - if (account) { - async.nextTick(cb, null, account) - } else { - self._lookupAccount(key, function (err, account) { + /** + * Looks up address in underlying trie. + * @param {Buffer} address - Address of account + * @param {Function} cb - Callback with params (err, account) + */ + _lookupAccount (address, cb) { + var self = this + self._trie.get(address, function (err, raw) { if (err) return cb(err) - self._update(key, account, false, false) + var account = new Account(raw) cb(null, account) }) } -} -/** - * Warms cache by loading their respective account from trie - * and putting them in cache. - * @param {Array} addresses - Array of addresses - * @param {Function} cb - Callback - */ -Cache.prototype.warm = function (addresses, cb) { - var self = this - // shim till async supports iterators - var accountArr = [] - addresses.forEach(function (val) { - if (val) accountArr.push(val) - }) - - async.eachSeries(accountArr, function (addressHex, done) { - var address = Buffer.from(addressHex, 'hex') - self._lookupAccount(address, function (err, account) { - if (err) return done(err) - self._update(address, account, false, false) - done() + /** + * Looks up address in cache, if not found, looks it up + * in the underlying trie. + * @param {Buffer} key - Address of account + * @param {Function} cb - Callback with params (err, account) + */ + getOrLoad (key, cb) { + var self = this + var account = this.lookup(key) + if (account) { + async.nextTick(cb, null, account) + } else { + self._lookupAccount(key, function (err, account) { + if (err) return cb(err) + self._update(key, account, false, false) + cb(null, account) + }) + } + } + + /** + * Warms cache by loading their respective account from trie + * and putting them in cache. + * @param {Array} addresses - Array of addresses + * @param {Function} cb - Callback + */ + warm (addresses, cb) { + var self = this + // shim till async supports iterators + var accountArr = [] + addresses.forEach(function (val) { + if (val) accountArr.push(val) }) - }, cb) -} -/** - * Flushes cache by updating accounts that have been modified - * and removing accounts that have been deleted. - * @param {function} cb - Callback - */ -Cache.prototype.flush = function (cb) { - var it = this._cache.begin - var self = this - var next = true - async.whilst(function () { - return next - }, function (done) { - if (it.value && it.value.modified) { - it.value.modified = false - it.value.val = it.value.val.serialize() - self._trie.put(Buffer.from(it.key, 'hex'), it.value.val, function () { - next = it.hasNext - it.next() + async.eachSeries(accountArr, function (addressHex, done) { + var address = Buffer.from(addressHex, 'hex') + self._lookupAccount(address, function (err, account) { + if (err) return done(err) + self._update(address, account, false, false) done() }) - } else if (it.value && it.value.deleted) { - it.value.modified = false - it.value.deleted = false - it.value.val = (new Account()).serialize() - self._trie.del(Buffer.from(it.key, 'hex'), function () { + }, cb) + } + + /** + * Flushes cache by updating accounts that have been modified + * and removing accounts that have been deleted. + * @param {function} cb - Callback + */ + flush (cb) { + var it = this._cache.begin + var self = this + var next = true + async.whilst(function () { + return next + }, function (done) { + if (it.value && it.value.modified) { + it.value.modified = false + it.value.val = it.value.val.serialize() + self._trie.put(Buffer.from(it.key, 'hex'), it.value.val, function () { + next = it.hasNext + it.next() + done() + }) + } else if (it.value && it.value.deleted) { + it.value.modified = false + it.value.deleted = false + it.value.val = (new Account()).serialize() + self._trie.del(Buffer.from(it.key, 'hex'), function () { + next = it.hasNext + it.next() + done() + }) + } else { next = it.hasNext it.next() - done() - }) - } else { - next = it.hasNext - it.next() - async.nextTick(done) - } - }, cb) -} + async.nextTick(done) + } + }, cb) + } -/** - * Marks current state of cache as checkpoint, which can - * later on be reverted or commited. - */ -Cache.prototype.checkpoint = function () { - this._checkpoints.push(this._cache) -} + /** + * Marks current state of cache as checkpoint, which can + * later on be reverted or commited. + */ + checkpoint () { + this._checkpoints.push(this._cache) + } -/** - * Revert changes to cache last checkpoint (no effect on trie). - */ -Cache.prototype.revert = function () { - this._cache = this._checkpoints.pop(this._cache) -} + /** + * Revert changes to cache last checkpoint (no effect on trie). + */ + revert () { + this._cache = this._checkpoints.pop(this._cache) + } -/** - * Commits to current state of cache (no effect on trie). - */ -Cache.prototype.commit = function () { - this._checkpoints.pop() -} + /** + * Commits to current state of cache (no effect on trie). + */ + commit () { + this._checkpoints.pop() + } -/** - * Clears cache. - */ -Cache.prototype.clear = function () { - this._cache = Tree() -} + /** + * Clears cache. + */ + clear () { + this._cache = Tree() + } -/** - * Marks address as deleted in cache. - * @params {Buffer} key - Address - */ -Cache.prototype.del = function (key) { - this._update(key, new Account(), false, true) -} + /** + * Marks address as deleted in cache. + * @params {Buffer} key - Address + */ + del (key) { + this._update(key, new Account(), false, true) + } -Cache.prototype._update = function (key, val, modified, deleted) { - key = key.toString('hex') - var it = this._cache.find(key) - if (it.node) { - this._cache = it.update({ - val: val, - modified: modified, - deleted: deleted - }) - } else { - this._cache = this._cache.insert(key, { - val: val, - modified: modified, - deleted: deleted - }) + _update (key, val, modified, deleted) { + key = key.toString('hex') + var it = this._cache.find(key) + if (it.node) { + this._cache = it.update({ + val: val, + modified: modified, + deleted: deleted + }) + } else { + this._cache = this._cache.insert(key, { + val: val, + modified: modified, + deleted: deleted + }) + } } } From 00e5fb72731ec39dc3ec027e68b54bdec0c7f7df Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 28 Jan 2019 16:10:43 +0100 Subject: [PATCH 4/6] Minor syntactic changes Signed-off-by: Sina Mahmoodi --- lib/cache.js | 48 +++++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/lib/cache.js b/lib/cache.js index e32b7503b1..257d8e3b16 100644 --- a/lib/cache.js +++ b/lib/cache.js @@ -14,10 +14,10 @@ module.exports = class Cache { * Puts account to cache under its address. * @param {Buffer} key - Address of account * @param {Account} val - Account - * @param {bool} fromTrie + * @param {bool} [fromTrie] */ - put (key, val, fromTrie) { - var modified = !fromTrie + put (key, val, fromTrie = false) { + const modified = !fromTrie this._update(key, val, modified, false) } @@ -26,7 +26,7 @@ module.exports = class Cache { * @param {Buffer} key - Address of account */ get (key) { - var account = this.lookup(key) + let account = this.lookup(key) if (!account) { account = new Account() } @@ -40,9 +40,9 @@ module.exports = class Cache { lookup (key) { key = key.toString('hex') - var it = this._cache.find(key) + const it = this._cache.find(key) if (it.node) { - var account = new Account(it.value.val) + const account = new Account(it.value.val) return account } } @@ -53,8 +53,7 @@ module.exports = class Cache { * @param {Function} cb - Callback with params (err, account) */ _lookupAccount (address, cb) { - var self = this - self._trie.get(address, function (err, raw) { + this._trie.get(address, (err, raw) => { if (err) return cb(err) var account = new Account(raw) cb(null, account) @@ -68,14 +67,13 @@ module.exports = class Cache { * @param {Function} cb - Callback with params (err, account) */ getOrLoad (key, cb) { - var self = this - var account = this.lookup(key) + const account = this.lookup(key) if (account) { async.nextTick(cb, null, account) } else { - self._lookupAccount(key, function (err, account) { + this._lookupAccount(key, (err, account) => { if (err) return cb(err) - self._update(key, account, false, false) + this._update(key, account, false, false) cb(null, account) }) } @@ -88,18 +86,17 @@ module.exports = class Cache { * @param {Function} cb - Callback */ warm (addresses, cb) { - var self = this // shim till async supports iterators var accountArr = [] - addresses.forEach(function (val) { + addresses.forEach((val) => { if (val) accountArr.push(val) }) - async.eachSeries(accountArr, function (addressHex, done) { + async.eachSeries(accountArr, (addressHex, done) => { var address = Buffer.from(addressHex, 'hex') - self._lookupAccount(address, function (err, account) { + this._lookupAccount(address, (err, account) => { if (err) return done(err) - self._update(address, account, false, false) + this._update(address, account, false, false) done() }) }, cb) @@ -111,16 +108,13 @@ module.exports = class Cache { * @param {function} cb - Callback */ flush (cb) { - var it = this._cache.begin - var self = this - var next = true - async.whilst(function () { - return next - }, function (done) { + const it = this._cache.begin + let next = true + async.whilst(() => next, (done) => { if (it.value && it.value.modified) { it.value.modified = false it.value.val = it.value.val.serialize() - self._trie.put(Buffer.from(it.key, 'hex'), it.value.val, function () { + this._trie.put(Buffer.from(it.key, 'hex'), it.value.val, () => { next = it.hasNext it.next() done() @@ -129,7 +123,7 @@ module.exports = class Cache { it.value.modified = false it.value.deleted = false it.value.val = (new Account()).serialize() - self._trie.del(Buffer.from(it.key, 'hex'), function () { + this._trie.del(Buffer.from(it.key, 'hex'), () => { next = it.hasNext it.next() done() @@ -154,7 +148,7 @@ module.exports = class Cache { * Revert changes to cache last checkpoint (no effect on trie). */ revert () { - this._cache = this._checkpoints.pop(this._cache) + this._cache = this._checkpoints.pop() } /** @@ -181,7 +175,7 @@ module.exports = class Cache { _update (key, val, modified, deleted) { key = key.toString('hex') - var it = this._cache.find(key) + const it = this._cache.find(key) if (it.node) { this._cache = it.update({ val: val, From 51070a455574eb3ccdbd7ab2fce93f4803259d03 Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 28 Jan 2019 16:35:42 +0100 Subject: [PATCH 5/6] Rm cacheTest.js Signed-off-by: Sina Mahmoodi --- tests/cacheTest.js | 99 ---------------------------------------------- 1 file changed, 99 deletions(-) delete mode 100644 tests/cacheTest.js diff --git a/tests/cacheTest.js b/tests/cacheTest.js deleted file mode 100644 index df21e10f57..0000000000 --- a/tests/cacheTest.js +++ /dev/null @@ -1,99 +0,0 @@ -const tape = require('tape') -const VM = require('../') -var async = require('async') -var Account = require('ethereumjs-account') -var Transaction = require('ethereumjs-tx') -var Trie = require('merkle-patricia-tree') -var ethUtil = require('ethereumjs-util') - -tape('test the cache api', function (t) { - t.test('should have the correct value in the cache ', function (st) { - var account1 = { - address: Buffer.from('cd2a3d9f938e13cd947ec05abc7fe734df8dd826', 'hex'), - key: ethUtil.keccak256('cow') - } - - /* - contract Contract2 { - uint i - function Contract2() { - i = 1 - } - } - */ - var account2 = { - address: Buffer.from('985509582b2c38010bfaa3c8d2be60022d3d00da', 'hex'), - code: Buffer.from('60606040525b60016000600050819055505b600a80601e6000396000f30060606040526008565b00', 'hex') - } - - /* - contract Contract { - function test(uint i) returns (uint) { - return i - } - } - */ - var account3 = { - code: Buffer.from('6060604052606a8060116000396000f30060606040526000357c01000000000000000000000000000000000000000000000000000000009004806329e99f07146037576035565b005b6046600480359060200150605c565b6040518082815260200191505060405180910390f35b60008190506065565b91905056', 'hex') - } - - var vm = new VM(new Trie()) - - async.series([ - createAccount, - runCode, - runTx, - function (cb) { - vm.stateManager.trie.get(account2.address, function (err, val) { - t.assert(!err) - var a = new Account(val) - a.getCode(vm.stateManager.trie, function (err, v) { - t.assert(!err) - t.assert(v.toString('hex') === '60606040526008565b00') - cb() - }) - }) - } - ], function (err) { - t.assert(!err) - st.end() - }) - - function createAccount (cb) { - var account = new Account() - account.balance = '0xf00000000000000001' - vm.stateManager.trie.put(Buffer.from(account1.address, 'hex'), account.serialize(), cb) - } - - function runCode (cb) { - var account = new Account() - - vm.runCode({ - code: account2.code, - data: account2.code, - account: account, - gasLimit: 3141592, - address: account2.address, - caller: account1.address - }, function (err, result) { - if (err) return cb(err) - account.setCode(vm.stateManager.trie, result.return, function (err) { - if (err) cb(err) - else vm.stateManager.trie.put(account2.address, account.serialize(), cb) - }) - }) - } - - function runTx (cb) { - var tx = new Transaction({ - gasLimit: 3141592, - gasPrice: 1, - data: account3.code - }) - tx.sign(account1.key) - vm.runTx({ - tx: tx - }, cb) - } - }) -}) From 14829462b1a7f1f82d36db6f87f00e2d0984dbdc Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Mon, 28 Jan 2019 16:39:09 +0100 Subject: [PATCH 6/6] Rm cacheTest from tester Signed-off-by: Sina Mahmoodi --- tests/tester.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tester.js b/tests/tester.js index c11a5744d9..e474cb1304 100755 --- a/tests/tester.js +++ b/tests/tester.js @@ -242,7 +242,6 @@ function runTests (name, runnerArgs, cb) { function runAll () { require('./tester.js') - require('./cacheTest.js') require('./genesishashes.js') require('./constantinopleSstoreTest.js') async.series([