From 68f462e3e4004d8ff1ab80ccab91a9fb636f53c2 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Fri, 14 Jun 2024 15:56:56 +0200 Subject: [PATCH] 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 | 35 +++++++++++++++++++++------- nimbus/db/core_db/core_apps.nim | 14 ++++++++--- nimbus/evm/state.nim | 19 +++++++++------ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/nimbus/core/chain/persist_blocks.nim b/nimbus/core/chain/persist_blocks.nim index e3e2d94d9..12982cea6 100644 --- a/nimbus/core/chain/persist_blocks.nim +++ b/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): - debug "Cannot update VmState", blockNumber = header.number - return err("Cannot update VmState to block " & $header.number) + 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 diff --git a/nimbus/db/core_db/core_apps.nim b/nimbus/db/core_db/core_apps.nim index 4c069c9e4..75b29fcc6 100644 --- a/nimbus/db/core_db/core_apps.nim +++ b/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. diff --git a/nimbus/evm/state.nim b/nimbus/evm/state.nim index 537b19faa..48a725dbd 100644 --- a/nimbus/evm/state.nim +++ b/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): - return self.reinit( + self.com.db.getBlockHeader(header.parentHash, parent) and + self.reinit( parent = parent, - header = header) + header = header, + linear = false) proc init*( self: BaseVMState; ## Object descriptor