From 6b3bf6e6f85d6dc5304f911cd69f8da74e327789 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Tue, 30 Oct 2018 23:07:56 -0230 Subject: [PATCH 1/2] Stop adding account to cache when checking if it is empty. --- lib/stateManager.js | 29 +++++++++++++-- tests/api/stateManager.js | 77 ++++++++++++++++++++++++++++++++++++++- tests/api/utils.js | 6 +-- 3 files changed, 104 insertions(+), 8 deletions(-) diff --git a/lib/stateManager.js b/lib/stateManager.js index 95f0b81021..a6e5df3ab7 100644 --- a/lib/stateManager.js +++ b/lib/stateManager.js @@ -40,6 +40,22 @@ proto.getAccount = function (address, cb) { this.cache.getOrLoad(address, cb) } +proto.getAccountPure = function (address, cb) { + const self = this + const cachedAccount = this.cache.get(address) + + if (cachedAccount) { + cb(null, cachedAccount) + } else { + self.trie.get(address, function (err, account) { + if (err) { + return cb(err) + } + cb(null, account) + }) + } +} + // saves the account proto.putAccount = function (address, account, cb) { var self = this @@ -285,9 +301,16 @@ proto.generateGenesis = function (initState, cb) { }, cb) } -proto.accountIsEmpty = function (address, cb) { +proto.accountIsEmpty = function (address, getAccountWithoutLoad, cb) { + if (cb === undefined) { + cb = getAccountWithoutLoad + getAccountWithoutLoad = null + } + var self = this - self.getAccount(address, function (err, account) { + var getAccountCall = getAccountWithoutLoad ? self.getAccountPure : self.getAccount + + getAccountCall.bind(this)(address, function (err, account) { if (err) { return cb(err) } @@ -301,7 +324,7 @@ proto.cleanupTouchedAccounts = function (cb) { var touchedArray = Array.from(self._touched) async.forEach(touchedArray, function (addressHex, next) { var address = Buffer.from(addressHex, 'hex') - self.accountIsEmpty(address, function (err, empty) { + self.accountIsEmpty(address, true, function (err, empty) { if (err) { next(err) return diff --git a/tests/api/stateManager.js b/tests/api/stateManager.js index 9d3ee1f25c..6890a5d84d 100644 --- a/tests/api/stateManager.js +++ b/tests/api/stateManager.js @@ -4,7 +4,7 @@ const util = require('ethereumjs-util') const StateManager = require('../../lib/stateManager') const { createAccount } = require('./utils') -tape('StateManager', (t) => { +tape.only('StateManager', (t) => { t.test('should instantiate', (st) => { const stateManager = new StateManager() @@ -18,7 +18,7 @@ tape('StateManager', (t) => { st.end() }) - t.test('should put and get account', async (st) => { + t.test('should put and get account, and add to the underlying cache if the account is not found', async (st) => { const stateManager = new StateManager() const account = createAccount() @@ -32,6 +32,79 @@ tape('StateManager', (t) => { ) st.equal(res.balance.toString('hex'), 'fff384') + + stateManager.cache.clear() + + res = await promisify(stateManager.getAccount.bind(stateManager))( + 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b' + ) + + st.equal(stateManager.cache._cache.keys[0], 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b') + + st.end() + }) + + t.test('should retrieve a cached account, without adding to the underlying cache if the account is not found', async (st) => { + const stateManager = new StateManager() + const account = createAccount() + + await promisify(stateManager.putAccount.bind(stateManager))( + 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b', + account + ) + + let res = await promisify(stateManager.getAccountPure.bind(stateManager))( + 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b' + ) + + st.equal(res.balance.toString('hex'), 'fff384') + + stateManager.cache.clear() + + res = await promisify(stateManager.getAccountPure.bind(stateManager))( + 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b' + ) + + st.notOk(stateManager.cache._cache.keys[0]) + + st.end() + }) + + t.test('should call the callback with a boolean representing emptiness, and not cache the account if passed correct params', async (st) => { + const stateManager = new StateManager() + + const promisifiedAccountIsEmpty = promisify(stateManager.accountIsEmpty.bind(stateManager), function (err, result) { + return err || result + }) + let res = await promisifiedAccountIsEmpty('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', true) + + st.ok(res) + st.notOk(stateManager.cache._cache.keys[0]) + + let res2 = await promisifiedAccountIsEmpty('0x095e7baea6a6c7c4c2dfeb977efac326af552d87') + + st.ok(res2) + st.equal(stateManager.cache._cache.keys[0], '0x095e7baea6a6c7c4c2dfeb977efac326af552d87') + + st.end() + }) + + t.test('should call the callback with a false boolean representing emptiness when the account is empty', async (st) => { + const stateManager = new StateManager() + const account = createAccount('0x1', '0x1') + + await promisify(stateManager.putAccount.bind(stateManager))( + 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b', + account + ) + + const promisifiedAccountIsEmpty = promisify(stateManager.accountIsEmpty.bind(stateManager), function (err, result) { + return err || result + }) + let res = await promisifiedAccountIsEmpty('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', true) + + st.notOk(res) + st.end() }) }) diff --git a/tests/api/utils.js b/tests/api/utils.js index 85a368d45b..c833960998 100644 --- a/tests/api/utils.js +++ b/tests/api/utils.js @@ -11,10 +11,10 @@ function createGenesis () { return genesis } -function createAccount () { +function createAccount (nonce, balance) { const raw = { - nonce: '0x00', - balance: '0xfff384' + nonce: nonce || '0x00', + balance: balance || '0xfff384' } const acc = new Account(raw) return acc From 614abdd92017b5b6e965c3c38ed1e09d05283858 Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Wed, 31 Oct 2018 13:04:21 -0230 Subject: [PATCH 2/2] Rename getAccountsPure to _getAccountsPure. --- lib/stateManager.js | 4 ++-- tests/api/stateManager.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/stateManager.js b/lib/stateManager.js index a6e5df3ab7..d4e6807188 100644 --- a/lib/stateManager.js +++ b/lib/stateManager.js @@ -40,7 +40,7 @@ proto.getAccount = function (address, cb) { this.cache.getOrLoad(address, cb) } -proto.getAccountPure = function (address, cb) { +proto._getAccountPure = function (address, cb) { const self = this const cachedAccount = this.cache.get(address) @@ -308,7 +308,7 @@ proto.accountIsEmpty = function (address, getAccountWithoutLoad, cb) { } var self = this - var getAccountCall = getAccountWithoutLoad ? self.getAccountPure : self.getAccount + var getAccountCall = getAccountWithoutLoad ? self._getAccountPure : self.getAccount getAccountCall.bind(this)(address, function (err, account) { if (err) { diff --git a/tests/api/stateManager.js b/tests/api/stateManager.js index 6890a5d84d..45e09b840a 100644 --- a/tests/api/stateManager.js +++ b/tests/api/stateManager.js @@ -4,7 +4,7 @@ const util = require('ethereumjs-util') const StateManager = require('../../lib/stateManager') const { createAccount } = require('./utils') -tape.only('StateManager', (t) => { +tape('StateManager', (t) => { t.test('should instantiate', (st) => { const stateManager = new StateManager() @@ -53,7 +53,7 @@ tape.only('StateManager', (t) => { account ) - let res = await promisify(stateManager.getAccountPure.bind(stateManager))( + let res = await promisify(stateManager._getAccountPure.bind(stateManager))( 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b' ) @@ -61,7 +61,7 @@ tape.only('StateManager', (t) => { stateManager.cache.clear() - res = await promisify(stateManager.getAccountPure.bind(stateManager))( + res = await promisify(stateManager._getAccountPure.bind(stateManager))( 'a94f5374fce5edbc8e2a8697c15331677e6ebf0b' )