avoid state root lookup when computing linear history (#2362)

State lookups potentially trigger expensive re-hashings - this is the
first of several steps to remove the unnecessary ones from the general
flow of block processing

* avoid re-reading parent block header from database when it's already
in memory

nimbus/core/chain/persist_blocks.nim

@@ -80,25 +80,40 @@ proc persistBlocksImpl(
   c.com.hardForkTransition(blocks[0].header)
 
   # Note that `0 < headers.len`, assured when called from `persistBlocks()`
-  let vmState = ?c.getVmState(blocks[0].header)
-
   let
+    vmState = ?c.getVmState(blocks[0].header)
     fromBlock = blocks[0].header.number
     toBlock = blocks[blocks.high()].header.number
   trace "Persisting blocks", fromBlock, toBlock
 
   var
+    blks = 0
     txs = 0
     gas = GasInt(0)
+    parentHash: Hash256 # only needed after the first block
   for blk in blocks:
     template header(): BlockHeader =
       blk.header
 
     c.com.hardForkTransition(header)
 
-    if not vmState.reinit(header):
+    if blks > 0:
+      template parent(): BlockHeader = blocks[blks - 1].header
+      let updated =
+        if header.number == parent.number + 1 and header.parentHash == parentHash:
+          vmState.reinit(parent = parent, header = header, linear = true)
+        else:
+          # TODO remove this code path and process only linear histories in this
+          #      function
+          vmState.reinit(header = header)
+
+      if not updated:
         debug "Cannot update VmState", blockNumber = header.number
         return err("Cannot update VmState to block " & $header.number)
+    else:
+      # TODO weirdly, some tests depend on this reinit being called, even though
+      #      in theory it's a fresh instance that should not need it (?)
+      doAssert vmState.reinit(header = header)
 
     if c.extraValidation and c.verifyFrom <= header.number:
       # TODO: how to checkseal from here
@@ -111,10 +126,11 @@ proc persistBlocksImpl(
     #      body.transactions.calcTxRoot == header.txRoot:
     #     vmState.dumpDebuggingMetaData(header, body)
     #     warn "Validation error. Debugging metadata dumped."
-
+    let blockHash = header.blockHash()
     if NoPersistHeader notin flags:
       if not c.db.persistHeader(
-        header, c.com.consensus == ConsensusType.POS, c.com.startOfHistory
+        blockHash, header, c.com.consensus == ConsensusType.POS,
+        c.com.startOfHistory
       ):
         return err("Could not persist header")
 
@@ -132,9 +148,10 @@ proc persistBlocksImpl(
     # between eth_blockNumber and eth_syncing
     c.com.syncCurrent = header.number
 
+    blks += 1
     txs += blk.transactions.len
     gas += blk.header.gasUsed
-
+    parentHash = blockHash
   dbTx.commit()
 
   # Save and record the block number before the last saved block state.
@@ -152,7 +169,7 @@ proc persistBlocksImpl(
       except CatchableError as exc:
         warn "Could not clean up old blocks from history", err = exc.msg
 
-  ok((blocks.len, txs, gas))
+  ok((blks, txs, gas))
 
 # ------------------------------------------------------------------------------
 # Public `ChainDB` methods

nimbus/db/core_db/core_apps.nim

@@ -922,13 +922,11 @@ proc persistHeader*(
 
 proc persistHeader*(
     db: CoreDbRef;
+    blockHash: Hash256;
     header: BlockHeader;
     forceCanonical: bool;
     startOfHistory = GENESIS_PARENT_HASH;
       ): bool =
-  let
-    blockHash = header.blockHash
-
   if not db.persistHeader(blockHash, header, startOfHistory):
     return false
 
@@ -948,6 +946,16 @@ proc persistHeader*(
   db.setAsCanonicalChainHead(blockHash, header)
   true
 
+proc persistHeader*(
+    db: CoreDbRef;
+    header: BlockHeader;
+    forceCanonical: bool;
+    startOfHistory = GENESIS_PARENT_HASH;
+      ): bool =
+  let
+    blockHash = header.blockHash
+  db.persistHeader(blockHash, header, forceCanonical, startOfHistory)
+
 proc persistUncles*(db: CoreDbRef, uncles: openArray[BlockHeader]): Hash256 =
   ## Persists the list of uncles to the database.
   ## Returns the uncles hash.

nimbus/evm/state.nim

@@ -82,11 +82,13 @@ proc new*(
 
 proc reinit*(self:     BaseVMState;     ## Object descriptor
              parent:   BlockHeader;     ## parent header, account sync pos.
-             blockCtx: BlockContext
+             blockCtx: BlockContext;
+             linear: bool
              ): bool =
   ## Re-initialise state descriptor. The `LedgerRef` database is
   ## re-initilaise only if its `rootHash` doe not point to `parent.stateRoot`,
-  ## already. Accumulated state data are reset.
+  ## already. Accumulated state data are reset. When linear, we assume that
+  ## the state recently processed the parent block.
   ##
   ## This function returns `true` unless the `LedgerRef` database could be
   ## queries about its `rootHash`, i.e. `isTopLevelClean` evaluated `true`. If
@@ -97,7 +99,7 @@ proc reinit*(self:     BaseVMState;     ## Object descriptor
       tracer = self.tracer
       com    = self.com
       db     = com.db
-      ac     = if self.stateDB.rootHash == parent.stateRoot: self.stateDB
+      ac     = if linear or self.stateDB.rootHash == parent.stateRoot: self.stateDB
                else: LedgerRef.init(db, parent.stateRoot)
       flags  = self.flags
     self[].reset
@@ -114,6 +116,7 @@ proc reinit*(self:     BaseVMState;     ## Object descriptor
 proc reinit*(self:   BaseVMState; ## Object descriptor
              parent: BlockHeader; ## parent header, account sync pos.
              header: BlockHeader; ## header with tx environment data fields
+             linear: bool
              ): bool =
   ## Variant of `reinit()`. The `parent` argument is used to sync the accounts
   ## cache and the `header` is used as a container to pass the `timestamp`,
@@ -121,9 +124,10 @@ proc reinit*(self:   BaseVMState; ## Object descriptor
   ##
   ## It requires the `header` argument properly initalised so that for PoA
   ## networks, the miner address is retrievable via `ecRecover()`.
-  result = self.reinit(
+  self.reinit(
     parent   = parent,
     blockCtx = self.com.blockCtx(header),
+    linear = linear
     )
 
 proc reinit*(self:      BaseVMState; ## Object descriptor
@@ -133,10 +137,11 @@ proc reinit*(self:      BaseVMState; ## Object descriptor
   ## `header.parentHash`, is used to fetch the `parent` BlockHeader to be
   ## used in the `update()` variant, above.
   var parent: BlockHeader
-  if self.com.db.getBlockHeader(header.parentHash, parent):
+  self.com.db.getBlockHeader(header.parentHash, parent) and
-    return self.reinit(
+    self.reinit(
       parent    = parent,
-      header    = header)
+      header    = header,
+      linear    = false)
 
 proc init*(
       self:   BaseVMState;     ## Object descriptor