Skip to content

Commit

Permalink
Update fork choice import for resuming after stop (#2746)
Browse files Browse the repository at this point in the history
* Update `ForkedChainRef` constructor

why:
  Initialisation is based on the canonical head which is always zero
  after resuming a stopped `ForkedChainRef` based import.

* Update new-base calculator

why:
  There is some ambiguous code which might not do what the comment
  implies. In short, an unsigned condition like `2u - 3u < 1u => false`
  is coded where the comment suggests that `2 - 3 < 1 => true` is meant.

  This patch fixes notorious crashes when resuming import after a stop.
  • Loading branch information
mjfh authored Oct 17, 2024
1 parent 4733759 commit 7d41a99
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 5 deletions.
48 changes: 46 additions & 2 deletions nimbus/core/chain/forked_chain.nim
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func calculateNewBase(c: ForkedChainRef,
max(headHeader.number, c.baseDistance) - c.baseDistance)

# The distance is less than `baseDistance`, don't move the base
if targetNumber - c.baseHeader.number <= c.baseDistance:
if targetNumber <= c.baseHeader.number + c.baseDistance:
return BaseDesc(hash: c.baseHash, header: c.baseHeader)

shouldNotKeyError:
Expand Down Expand Up @@ -394,11 +394,55 @@ proc updateHeadIfNecessary(c: ForkedChainRef,
# Public functions
# ------------------------------------------------------------------------------

proc init*(
T: type ForkedChainRef;
com: CommonRef;
baseDistance = BaseDistance.uint64;
extraValidation = true;
): T =
## Constructor that uses the current database ledger state for initialising.
## This state coincides with the canonical head that would be used for
## setting up the descriptor.
##
## With `ForkedChainRef` based import, the canonical state lives only inside
## a level one database transaction. Thus it will readily be available on the
## running system with tools such as `getCanonicalHead()`. But it will never
## be saved on the database.
##
## This constructor also works well when resuming import after running
## `persistentBlocks()` used for `Era1` or `Era` import.
##
let
base = com.db.getSavedStateBlockNumber
var
baseHash: Hash32
baseHeader: Header
try:
baseHash = com.db.getBlockHash(base)
baseHeader = com.db.getBlockHeader(baseHash)
except BlockNotFound:
raiseAssert "Base header missing for #" & $base

# update global syncStart
com.syncStart = baseHeader.number

T(com: com,
db: com.db,
baseHeader: baseHeader,
cursorHash: baseHash,
baseHash: baseHash,
cursorHeader: baseHeader,
extraValidation: extraValidation,
baseDistance: baseDistance,
txRecords: initTable[Hash256, (Hash256, uint64)]())

proc newForkedChain*(com: CommonRef,
baseHeader: Header,
baseDistance: uint64 = BaseDistance,
extraValidation: bool = true): ForkedChainRef =

## This constructor allows to set up the base state which might be needed
## for some particular test or other applications. Otherwise consider
## `init()`.
let baseHash = baseHeader.blockHash

var chain = ForkedChainRef(
Expand Down
5 changes: 2 additions & 3 deletions nimbus/nimbus_execution_client.nim
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ proc basicServices(nimbus: NimbusNode,
# txPool must be informed of active head
# so it can know the latest account state
# e.g. sender nonce, etc
let head = com.db.getCanonicalHead()
nimbus.chainRef = newForkedChain(com, head)
doAssert nimbus.txPool.smartHead(head, nimbus.chainRef)
nimbus.chainRef = ForkedChainRef.init(com)
doAssert nimbus.txPool.smartHead(nimbus.chainRef.latestHeader,nimbus.chainRef)

nimbus.beaconEngine = BeaconEngineRef.new(nimbus.txPool, nimbus.chainRef)

Expand Down

0 comments on commit 7d41a99

Please sign in to comment.