Stop adding account to cache when checking if it is empty.

ethereumjs · Oct 31, 2018 · 23922ba · 23922ba
1 parent 906eb6c
commit 23922ba
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 8 deletions.
diff --git 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
@@ -289,9 +305,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)
     }
@@ -305,7 +328,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
@@ -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,82 @@ 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 stateManagerCache = stateManager.cache._cache
+
+    const promisifiedAccountIsEmpty = promisify(stateManager.accountIsEmpty.bind(stateManager), function (err, result) {
+      return err || result
+    })
+    let res = await promisifiedAccountIsEmpty('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', true)
+    let cachedAccount = stateManager.cache.lookup.bind(stateManager.cache)('a94f5374fce5edbc8e2a8697c15331677e6ebf0b')
+
+    st.ok(res)
+    st.notOk(stateManager.cache._cache.keys[0])
+
+    let res2 = await promisifiedAccountIsEmpty('0x095e7baea6a6c7c4c2dfeb977efac326af552d87')
+    cachedAccount = stateManager.cache.lookup.bind(stateManager.cache)('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
@@ -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