TxPool: Bubble up error from packer to assembleBlock

nimbus/beacon/beacon_engine.nim

@@ -170,7 +170,9 @@ proc generatePayload*(ben: BeaconEngineRef,
 
     # someBaseFee = true: make sure blk.header
     # have the same blockHash with generated payload
-    let blk = xp.ethBlock(someBaseFee = true)
+    let blk = xp.assembleBlock(someBaseFee = true).valueOr:
+      return err(error)
+
     if blk.header.extraData.len > 32:
       return err "extraData length should not exceed 32 bytes"
 

nimbus/core/sealer.nim

@@ -61,7 +61,9 @@ proc validateSealer*(conf: NimbusConf, ctx: EthContext, chain: ChainRef): Result
 proc generateBlock(engine: SealingEngineRef,
                    outBlock: var EthBlock): Result[void, string] =
 
-  outBlock = engine.txPool.ethBlock()
+  outBlock = engine.txPool.assembleBlock().valueOr:
+    return err(error)
+
   if engine.chain.com.consensus == ConsensusType.POS:
     # Stop the block generator if we reach TTD
     engine.state = EnginePostMerge

nimbus/core/tx_pool.nim

@@ -241,7 +241,7 @@
 ##    xq.add(txs)                            # add transactions ..
 ##    ..                                     # .. into the buckets
 ##
-##    let newBlock = xq.ethBlock             # fetch current mining block
+##    let newBlock = xq.assembleBlock        # fetch current mining block
 ##
 ##    ..
 ##    mineThatBlock(newBlock) ...            # external mining & signing process
@@ -607,7 +607,7 @@ proc dirtyBuckets*(xp: TxPoolRef): bool =
   ## flag is also set.
   xp.pDirtyBuckets
 
-proc ethBlock*(xp: TxPoolRef, someBaseFee: bool = false): EthBlock
+proc assembleBlock*(xp: TxPoolRef, someBaseFee: bool = false): Result[EthBlock, string]
     {.gcsafe,raises: [CatchableError].} =
   ## Getter, retrieves a packed block ready for mining and signing depending
   ## on the internally cached block chain head, the txs in the pool and some
@@ -621,18 +621,25 @@ proc ethBlock*(xp: TxPoolRef, someBaseFee: bool = false): EthBlock
   ## Note that this getter runs *ad hoc* all the txs through the VM in
   ## order to build the block.
 
-  xp.packerVmExec                            # updates vmState
-  result.header = xp.chain.getHeader         # uses updated vmState
+  xp.packerVmExec().isOkOr:                  # updates vmState
+    return err(error)
+
+  var blk = EthBlock(
+    header: xp.chain.getHeader               # uses updated vmState
+  )
+
   for (_,nonceList) in xp.txDB.packingOrderAccounts(txItemPacked):
-    result.txs.add toSeq(nonceList.incNonce).mapIt(it.tx)
+    blk.txs.add toSeq(nonceList.incNonce).mapIt(it.tx)
 
   let com = xp.chain.com
   if com.forkGTE(Shanghai):
-    result.withdrawals = some(com.pos.withdrawals)
+    blk.withdrawals = some(com.pos.withdrawals)
 
   if someBaseFee:
     # make sure baseFee always has something
-    result.header.fee = some(result.header.fee.get(0.u256))
+    blk.header.fee = some(blk.header.fee.get(0.u256))
+
+  ok(blk)
 
 proc gasCumulative*(xp: TxPoolRef): GasInt =
   ## Getter, retrieves the gas that will be burned in the block after

nimbus/core/tx_pool/tx_tasks/tx_packer.nim

@@ -150,7 +150,7 @@ proc runTxCommit(pst: TxPackerStateRef; item: TxItemRef; gasBurned: GasInt)
 # Private functions: packer packerVmExec() helpers
 # ------------------------------------------------------------------------------
 
-proc vmExecInit(xp: TxPoolRef): TxPackerStateRef
+proc vmExecInit(xp: TxPoolRef): Result[TxPackerStateRef, string]
     {.gcsafe,raises: [CatchableError].} =
 
   # Flush `packed` bucket
@@ -169,14 +169,16 @@ proc vmExecInit(xp: TxPoolRef): TxPackerStateRef
   # EIP-4788
   if xp.chain.nextFork >= FkCancun:
     let beaconRoot = xp.chain.com.pos.parentBeaconBlockRoot
-    discard xp.chain.vmState.processBeaconBlockRoot(beaconRoot)
+    xp.chain.vmState.processBeaconBlockRoot(beaconRoot).isOkOr:
+      return err(error)
 
-  TxPackerStateRef( # return value
+  let packer = TxPackerStateRef( # return value
     xp: xp,
     tr: newCoreDbRef(LegacyDbMemory).mptPrune,
     balance: xp.chain.vmState.readOnlyStateDB.getBalance(xp.chain.feeRecipient),
     numBlobPerBlock: 0,
   )
+  ok(packer)
 
 proc vmExecGrabItem(pst: TxPackerStateRef; item: TxItemRef): Result[bool,void]
     {.gcsafe,raises: [CatchableError].}  =
@@ -288,14 +290,15 @@ proc vmExecCommit(pst: TxPackerStateRef)
 # Public functions
 # ------------------------------------------------------------------------------
 
-proc packerVmExec*(xp: TxPoolRef) {.gcsafe,raises: [CatchableError].} =
+proc packerVmExec*(xp: TxPoolRef): Result[void, string] {.gcsafe,raises: [CatchableError].} =
   ## Rebuild `packed` bucket by selection items from the `staged` bucket
   ## after executing them in the VM.
   let db = xp.chain.com.db
   let dbTx = db.beginTransaction
   defer: dbTx.dispose()
 
-  var pst = xp.vmExecInit
+  var pst = xp.vmExecInit.valueOr:
+    return err(error)
 
   block loop:
     for (_,nonceList) in pst.xp.txDB.packingOrderAccounts(txItemStaged):
@@ -309,6 +312,7 @@ proc packerVmExec*(xp: TxPoolRef) {.gcsafe,raises: [CatchableError].} =
             break account # continue with next account
 
   pst.vmExecCommit
+  ok()
   # Block chain will roll back automatically
 
 # ------------------------------------------------------------------------------

tests/test_txpool.nim

@@ -621,7 +621,7 @@ proc runTxPackerTests(noisy = true) =
 
           # employ packer
           # xq.jobCommit(forceMaintenance = true)
-          xq.packerVmExec
+          check xq.packerVmExec.isOk
           check xq.verify.isOK
 
           # verify that the test did not degenerate
@@ -647,7 +647,7 @@ proc runTxPackerTests(noisy = true) =
 
           # re-pack bucket
           #xq.jobCommit(forceMaintenance = true)
-          xq.packerVmExec
+          check xq.packerVmExec.isOk
           check xq.verify.isOK
 
           let
@@ -677,7 +677,7 @@ proc runTxPackerTests(noisy = true) =
           # re-pack bucket, packer needs extra trigger because there is
           # not necessarily a buckets re-org resulting in a change
           #xq.jobCommit(forceMaintenance = true)
-          xq.packerVmExec
+          check xq.packerVmExec.isOk
           check xq.verify.isOK
 
           let
@@ -736,7 +736,7 @@ proc runTxPackerTests(noisy = true) =
       test &"Run packer, profitability will not increase with block size":
 
         xq.flags = xq.flags - {packItemsMaxGasLimit}
-        xq.packerVmExec
+        check xq.packerVmExec.isOk
         let
           smallerBlockProfitability = xq.profitability
           smallerBlockSize = xq.gasCumulative
@@ -748,7 +748,7 @@ proc runTxPackerTests(noisy = true) =
           " slack=", xq.trgGasLimit - xq.gasCumulative
 
         xq.flags = xq.flags + {packItemsMaxGasLimit}
-        xq.packerVmExec
+        check xq.packerVmExec.isOk
 
         noisy.say "***", "max-packing",
           " profitability=", xq.profitability,
@@ -787,8 +787,13 @@ proc runTxPackerTests(noisy = true) =
         xq.flags = xq.flags #+ {packItemsMaxGasLimit}
 
         # Invoke packer
-        let blk = xq.ethBlock
+        let r = xq.assembleBlock()
+        if r.isErr:
+          debugEcho r.error
+          check false
+          return
 
+        let blk = r.get
         # Make sure that there are at least two txs on the packed block so
         # this test does not degenerate.
         check 1 < xq.chain.receipts.len

tests/test_txpool2.nim

@@ -156,7 +156,13 @@ proc runTxPoolCliqueTest*() =
       check xp.nItems.total == 1
 
     test "TxPool ethBlock":
-      blk = xp.ethBlock()
+      let res = xp.assembleBlock()
+      if res.isErr:
+        debugEcho res.error
+        check false
+        return
+
+      blk = res.get
       body = BlockBody(
         transactions: blk.txs,
         uncles: blk.uncles
@@ -192,7 +198,13 @@ proc runTxPoolCliqueTest*() =
         return
       check xp.nItems.total == 1
 
-      blk = xp.ethBlock()
+      let r = xp.assembleBlock()
+      if r.isErr:
+        debugEcho r.error
+        check false
+        return
+
+      blk = r.get
       body = BlockBody(
         transactions: blk.txs,
         uncles: blk.uncles
@@ -241,8 +253,13 @@ proc runTxPoolPosTest*() =
       com.pos.feeRecipient = feeRecipient
       com.pos.timestamp = EthTime.now()
 
-      blk = xp.ethBlock()
+      let r = xp.assembleBlock()
+      if r.isErr:
+        debugEcho r.error
+        check false
+        return
 
+      blk = r.get
       check com.isBlockAfterTtd(blk.header)
 
       body = BlockBody(
@@ -299,8 +316,13 @@ proc runTxPoolBlobhashTest*() =
       com.pos.feeRecipient = feeRecipient
       com.pos.timestamp = EthTime.now()
 
-      blk = xp.ethBlock()
+      let r = xp.assembleBlock()
+      if r.isErr:
+        debugEcho r.error
+        check false
+        return
 
+      blk = r.get
       check com.isBlockAfterTtd(blk.header)
 
       body = BlockBody(
@@ -380,7 +402,13 @@ proc runTxHeadDelta*(noisy = true) =
           com.pos.timestamp  = timestamp
           com.pos.feeRecipient = feeRecipient
 
-          var blk = xp.ethBlock()
+          let r = xp.assembleBlock()
+          if r.isErr:
+            debugEcho r.error
+            check false
+            return
+
+          let blk = r.get
           check com.isBlockAfterTtd(blk.header)
 
           let body = BlockBody(