Merge branch 'master' into faster-hashes

Author: advaita-saha

.github/workflows/nimbus_verified_proxy.yml

@@ -34,6 +34,8 @@ on:
       - 'Makefile'
       - 'nimbus.nimble'
 
+  workflow_dispatch:
+
 concurrency: # Cancel stale PR builds (but not push builds)
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}
   cancel-in-progress: true
@@ -67,28 +69,9 @@ jobs:
           rocksdb-cache: true
           eest-cache: false
 
-      - name: Run verified proxy tests (Windows)
-        if: runner.os == 'Windows'
+      - name: Run verified proxy tests
         run: |
           gcc --version
-          DEFAULT_MAKE_FLAGS="-j${ncpu}"
-          mingw32-make ${DEFAULT_MAKE_FLAGS} nimbus_verified_proxy libverifproxy
-          build/nimbus_verified_proxy.exe --help
-          mingw32-make ${DEFAULT_MAKE_FLAGS} nimbus-verified-proxy-test
-
-      - name: Run verified proxy tests (Linux)
-        if: runner.os == 'Linux'
-        run: |
-          export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/usr/local/lib"
-          DEFAULT_MAKE_FLAGS="-j${ncpu}"
-          env CC=gcc make ${DEFAULT_MAKE_FLAGS} nimbus_verified_proxy libverifproxy
-          build/nimbus_verified_proxy --help
-          # CC is needed to select correct compiler 32/64 bit
-          env CC=gcc CXX=g++ make ${DEFAULT_MAKE_FLAGS} nimbus-verified-proxy-test
-
-      - name: Run verified proxy tests (Macos)
-        if: runner.os == 'Macos'
-        run: |
           DEFAULT_MAKE_FLAGS="-j${ncpu}"
           make ${DEFAULT_MAKE_FLAGS} nimbus_verified_proxy libverifproxy
           build/nimbus_verified_proxy --help

.gitmodules

@@ -229,3 +229,7 @@
 [submodule "vendor/constantine"]
 	path = vendor/constantine
 	url = https://github.com/mratsim/constantine
+[submodule "vendor/nim-mcl"]
+	path = vendor/nim-mcl
+	url = https://github.com/status-im/nim-mcl
+  branch = main

Makefile

@@ -290,17 +290,13 @@ portal-test-reproducibility:
 		[ "$$MD5SUM1" = "$$MD5SUM2" ] && echo -e "\e[92mSuccess: identical binaries.\e[39m" || \
 			{ echo -e "\e[91mFailure: the binary changed between builds.\e[39m"; exit 1; }
 
-# Portal tests
-all_eth_history_custom_chain_tests: | build deps
-	echo -e $(BUILD_MSG) "build/$@" && \
-	$(ENV_SCRIPT) nim c -r $(NIM_PARAMS) -d:chronicles_log_level=ERROR -d:mergeBlockNumber:38130 -o:build/$@ "portal/tests/eth_history_tests/$@.nim"
-
+# builds and runs the Portal test suite
 all_portal_tests: | build deps
 	echo -e $(BUILD_MSG) "build/$@" && \
 	$(ENV_SCRIPT) nim c -r $(NIM_PARAMS) -d:chronicles_log_level=ERROR -o:build/$@ "portal/tests/$@.nim"
 
-# builds and runs the Portal test suite
-portal-test: | all_portal_tests all_eth_history_custom_chain_tests
+# alias for all_portal_tests
+portal-test: | all_portal_tests
 
 # builds the Portal tools, wherever they are
 $(PORTAL_TOOLS): | build deps rocksdb

config.nims

@@ -94,6 +94,10 @@ elif defined(riscv64):
   # and seems to be the minimum extensions needed to build.
   switch("passC", "-march=rv64gc")
   switch("passL", "-march=rv64gc")
+elif defined(linux) and defined(arm64):
+  # clang can't handle "-march=native"
+  switch("passC", "-march=armv8-a")
+  switch("passL", "-march=armv8-a")
 else:
   switch("passC", "-march=native")
   switch("passL", "-march=native")

execution_chain/compile_info.nim

@@ -14,3 +14,7 @@
 const chronicles_line_numbers {.strdefine.} = "0"
 when chronicles_line_numbers notin ["0", "off"]:
   {.hint: "*** Compiling with logger line numbers enabled".}
+
+const enable_mcl_lib* {.booldefine.} = true
+when enable_mcl_lib:
+  {.hint: "*** Compiling with mcl library".}

execution_chain/config.nim

@@ -301,6 +301,11 @@ type
       defaultValue: 4'u64
       name: "debug-persist-batch-size" .}: uint64
 
+    dynamicBatchSize* {.
+      hidden
+      defaultValue: false
+      name: "debug-dynamic-batch-size" .}: bool
+
     rocksdbMaxOpenFiles {.
       hidden
       defaultValue: defaultMaxOpenFiles
@@ -358,6 +363,11 @@ type
       desc: "Eagerly check state roots when syncing finalized blocks"
       name: "debug-eager-state-root".}: bool
 
+    deserializeFcState* {.
+      hidden
+      defaultValue: true
+      name: "debug-deserialize-fc-state" .}: bool
+
     statelessProviderEnabled* {.
       separator: "\pSTATELESS PROVIDER OPTIONS:"
       hidden

execution_chain/constants.nim

@@ -8,15 +8,13 @@
 # at your option. This file may not be copied, modified, or distributed except
 # according to those terms.
 
+{.push raises: [], gcsafe.}
 {.used.}
 
 import
   stint,
   eth/common/[addresses, hashes, base]
 
-# proc default(t: typedesc): t = discard -- notused
-
-# constants
 const
   UINT_256_MAX*: UInt256 =                  high(UInt256)
   INT_256_MAX_AS_UINT256* =                 high(UInt256) shr 1
@@ -43,7 +41,7 @@ const
 
   GAS_LIMIT_MINIMUM* =                      5000
   GAS_LIMIT_MAXIMUM* =                      int64.high.GasInt # Maximum the gas limit (2^63-1).
-  DEFAULT_GAS_LIMIT* =                      45_000_000
+  DEFAULT_GAS_LIMIT* =                      60_000_000
 
   # https://eips.ethereum.org/EIPS/eip-7825
   TX_GAS_LIMIT* =                           1 shl 24 # 2^24

execution_chain/core/block_import.nim

@@ -83,7 +83,7 @@ proc importRlpBlocks*(importFile: string,
 proc importRlpBlocks*(conf: NimbusConf, com: CommonRef): Future[void] {.async: (raises: [CancelledError]).} =
   # Both baseDistance and persistBatchSize are 0,
   # we want changes persisted immediately
-  let chain = ForkedChainRef.init(com, baseDistance = 0, persistBatchSize = 0)
+  let chain = ForkedChainRef.init(com, baseDistance = 0, persistBatchSize = 1)
 
   # success or not, we quit after importing blocks
   for i, blocksFile in conf.blocksFile:

execution_chain/core/chain/forked_chain.nim

@@ -314,6 +314,8 @@ proc updateBase(c: ForkedChainRef, base: BlockRef): uint =
     # No update, return
     return
 
+  let startTime = Moment.now()
+
   # State root sanity check is performed to verify, before writing to disk,
   # that optimistically checked blocks indeed end up being stored with a
   # consistent state root.
@@ -338,8 +340,7 @@ with --debug-eager-state-root."""
 
   # Cleanup in-memory blocks starting from base backward
   # e.g. B2 backward.
-  var
-    count = 0'u
+  var count = 0'u
 
   for it in ancestors(base.parent):
     c.removeBlockFromCache(it)
@@ -352,6 +353,33 @@ with --debug-eager-state-root."""
   # Base block always have finalized marker
   c.base.finalize()
 
+  if c.dynamicBatchSize:
+    # Dynamicly adjust the persistBatchSize based on the recorded run time.
+    # The goal here is use the maximum batch size possible without blocking the
+    # event loop for too long which could negatively impact the p2p networking.
+    # Increasing the batch size can improve performance because the stateroot
+    # computation and persist calls are performed less frequently.
+    const
+      targetTime = 500.milliseconds
+      targetTimeDelta = 200.milliseconds
+      targetTimeLowerBound = (targetTime - targetTimeDelta).milliseconds
+      targetTimeUpperBound = (targetTime + targetTimeDelta).milliseconds
+      batchSizeLowerBound = 4
+      batchSizeUpperBound = 512
+
+    let
+      finishTime = Moment.now()
+      runTime = (finishTime - startTime).milliseconds
+
+    if runTime < targetTimeLowerBound and c.persistBatchSize <= batchSizeUpperBound:
+      c.persistBatchSize *= 2
+      info "Increased persistBatchSize", runTime, targetTime,
+        persistBatchSize = c.persistBatchSize
+    elif runTime > targetTimeUpperBound and c.persistBatchSize >= batchSizeLowerBound:
+      c.persistBatchSize = c.persistBatchSize div 2
+      info "Decreased persistBatchSize", runTime, targetTime,
+        persistBatchSize = c.persistBatchSize
+
   count
 
 proc processUpdateBase(c: ForkedChainRef): Future[Result[void, string]] {.async: (raises: [CancelledError]).} =
@@ -407,20 +435,20 @@ proc queueUpdateBase(c: ForkedChainRef, base: BlockRef)
                      else:
                        c.base
 
-  if prevQueuedBase.number == base.number:
+  if prevQueuedBase.number >= base.number:
     return
 
   var
-    number = base.number - min(base.number, PersistBatchSize)
-    steps  = newSeqOfCap[BlockRef]((base.number-prevQueuedBase.number) div PersistBatchSize + 1)
+    number = base.number - min(base.number, c.persistBatchSize)
+    steps  = newSeqOfCap[BlockRef]((base.number - prevQueuedBase.number) div c.persistBatchSize + 1)
     it = base
 
   steps.add base
 
   while it.number > prevQueuedBase.number:
     if it.number == number:
       steps.add it
-      number -= min(number, PersistBatchSize)
+      number -= min(number, c.persistBatchSize)
     it = it.parent
 
   for i in countdown(steps.len-1, 0):
@@ -501,8 +529,15 @@ proc validateBlock(c: ForkedChainRef,
   # Entering base auto forward mode while avoiding forkChoice
   # handled region(head - baseDistance)
   # e.g. live syncing with the tip very far from from our latest head
+  let
+    offset = c.baseDistance + c.persistBatchSize
+    number =
+      if offset >= c.latestFinalizedBlockNumber:
+        0.uint64
+      else:
+        c.latestFinalizedBlockNumber - offset
   if c.pendingFCU != zeroHash32 and
-     c.base.number < c.latestFinalizedBlockNumber - c.baseDistance - c.persistBatchSize:
+     c.base.number < number:
     let
       base = c.calculateNewBase(c.latestFinalizedBlockNumber, c.latest)
       prevBase = c.base.number
@@ -579,6 +614,7 @@ proc init*(
     com: CommonRef;
     baseDistance = BaseDistance;
     persistBatchSize = PersistBatchSize;
+    dynamicBatchSize = false;
     eagerStateRoot = false;
     enableQueue = false;
       ): T =
@@ -594,6 +630,8 @@ proc init*(
   ## This constructor also works well when resuming import after running
   ## `persistentBlocks()` used for `Era1` or `Era` import.
   ##
+  doAssert(persistBatchSize > 0)
+
   let
     baseTxFrame = com.db.baseTxFrame()
     base = baseTxFrame.getSavedStateBlockNumber
@@ -610,19 +648,20 @@ proc init*(
     fcuSafe = baseTxFrame.fcuSafe().valueOr:
       FcuHashAndNumber(hash: baseHash, number: base)
     fc = T(
-      com:             com,
-      base:            baseBlock,
-      latest:          baseBlock,
-      heads:           @[baseBlock],
-      hashToBlock:     {baseHash: baseBlock}.toTable,
-      baseTxFrame:     baseTxFrame,
-      baseDistance:    baseDistance,
-      persistBatchSize:persistBatchSize,
-      quarantine:      Quarantine.init(),
-      fcuHead:         fcuHead,
-      fcuSafe:         fcuSafe,
-      baseQueue:       initDeque[BlockRef](),
-      lastBaseLogTime: EthTime.now(),
+      com:              com,
+      base:             baseBlock,
+      latest:           baseBlock,
+      heads:            @[baseBlock],
+      hashToBlock:      {baseHash: baseBlock}.toTable,
+      baseTxFrame:      baseTxFrame,
+      baseDistance:     baseDistance,
+      persistBatchSize: persistBatchSize,
+      dynamicBatchSize: dynamicBatchSize,
+      quarantine:       Quarantine.init(),
+      fcuHead:          fcuHead,
+      fcuSafe:          fcuSafe,
+      baseQueue:        initDeque[BlockRef](),
+      lastBaseLogTime:  EthTime.now(),
     )
 
   # updateFinalized will stop ancestor lineage
@@ -712,10 +751,8 @@ proc forkChoice*(c: ForkedChainRef,
   c.updateHead(head)
   c.updateFinalized(finalized, head)
 
-  let
-    base = c.calculateNewBase(finalized.number, head)
-
-  if base.number == c.base.number:
+  let base = c.calculateNewBase(finalized.number, head)
+  if base.number <= c.base.number:
     # The base is not updated, return.
     return ok()
 

execution_chain/core/chain/forked_chain/chain_desc.nim

@@ -88,6 +88,10 @@ type
       # to move the base. And the bulk writing can works
       # efficiently.
 
+    dynamicBatchSize*: bool
+      # Enable adjusting the persistBatchSize dynamically based on the
+      # time it takes to update base.
+
     portal*: HistoryExpiryRef
       # History Expiry tracker and portal access entry point