Fix invalid result error access + move empty txs check (#3649)

* Fix invalid result error access + move empty txs check

Check for empty txs in getBlockTransactionData and
getExecutionPayloadBodyV1.

First step as more locations potentially require this on usage of
also getBlockTransactions and getBlockTransactionHashes.

Can also add additionally check for rootHash calc.

* Fix era import by adjusting getUncleHashes to not use getBlockBody

When era based import does not store block bodies (transactions here
specifically), then this retrieval will fail when using getBlockBody.

Rework instead to just get the uncles and not care about the other
parts of the block body.

* Fix txRoot check bug as zeroHash32 instead of emptyRoot was used

zeroHash32 check was used originally and copied in this PR.
This is incorrect as the root of an empty tree is not the same.

Author: kdeme

execution_chain/core/chain/forked_chain.nim

@@ -900,16 +900,15 @@ proc blockByHash*(c: ForkedChainRef, blockHash: Hash32): Result[Block, string] =
   c.hashToBlock.withValue(blockHash, loc):
     return ok(loc[].blk)
   var header = ?c.baseTxFrame.getBlockHeader(blockHash)
-  var blockBody = c.baseTxFrame.getBlockBody(header)
-  # Serves portal data if block not found in db
-  if blockBody.isErr or (blockBody.get.transactions.len == 0 and header.transactionsRoot != zeroHash32):
+  var blockBody = c.baseTxFrame.getBlockBody(header).valueOr:
+    # Serve portal data if block not found in db
     if c.isPortalActive:
       var blockBodyPortal = ?c.portal.getBlockBodyByHeader(header)
       return ok(EthBlock.init(move(header), move(blockBodyPortal)))
     else:
-      return err(blockBody.error)
+      return err(error)
 
-  ok(EthBlock.init(move(header), move(blockBody.get())))
+  ok(EthBlock.init(move(header), move(blockBody)))
 
 proc payloadBodyV1ByHash*(c: ForkedChainRef, blockHash: Hash32): Result[ExecutionPayloadBodyV1, string] =
   c.hashToBlock.withValue(blockHash, loc):
@@ -918,8 +917,8 @@ proc payloadBodyV1ByHash*(c: ForkedChainRef, blockHash: Hash32): Result[Executio
   var header = ?c.baseTxFrame.getBlockHeader(blockHash)
   var blk = c.baseTxFrame.getExecutionPayloadBodyV1(header)
 
-  # Serves portal data if block not found in db
-  if blk.isErr or (blk.get.transactions.len == 0 and header.transactionsRoot != zeroHash32):
+  if blk.isErr:
+    # Serve portal data if block not found in db
     if c.isPortalActive:
       var blockBodyPortal = ?c.portal.getBlockBodyByHeader(header)
       # Same as above
@@ -935,9 +934,8 @@ proc payloadBodyV1ByNumber*(c: ForkedChainRef, number: BlockNumber): Result[Exec
     var header = ?c.baseTxFrame.getBlockHeader(number)
     let blk = c.baseTxFrame.getExecutionPayloadBodyV1(header)
 
-    # Txs not there in db - Happens during era1/era import, when we don't store txs and receipts
-    if blk.isErr or (blk.get.transactions.len == 0 and header.transactionsRoot != emptyRoot):
-      # Serves portal data if block not found in database
+    if blk.isErr:
+      # Serve portal data if block not found in db
       if c.isPortalActive:
         var blockBodyPortal = ?c.portal.getBlockBodyByHeader(header)
         # same as above
@@ -957,17 +955,15 @@ proc blockByNumber*(c: ForkedChainRef, number: BlockNumber): Result[Block, strin
 
   if number <= c.base.number:
     var header = ?c.baseTxFrame.getBlockHeader(number)
-    var blockBody = c.baseTxFrame.getBlockBody(header)
-    # Txs not there in db - Happens during era1/era import, when we don't store txs and receipts
-    if blockBody.isErr or (blockBody.get.transactions.len == 0 and header.transactionsRoot != emptyRoot):
-      # Serves portal data if block not found in database
+    var blockBody = c.baseTxFrame.getBlockBody(header).valueOr:
+      # Serve portal data if block not found in db
       if c.isPortalActive:
         var blockBodyPortal = ?c.portal.getBlockBodyByHeader(header)
         return ok(EthBlock.init(move(header), move(blockBodyPortal)))
       else:
-        return err(blockBody.error)
-    else:
-      return ok(EthBlock.init(move(header), move(blockBody.get())))
+        return err(error)
+
+    return ok(EthBlock.init(move(header), move(blockBody)))
 
   loopIt(c.latest):
     if number >= it.number:

execution_chain/db/core_db/core_apps.nim

@@ -402,6 +402,11 @@ proc getTransactions*(
     var res: seq[Transaction]
     for encodedTx in db.getBlockTransactionData(txRoot):
       res.add(rlp.decode(encodedTx, Transaction))
+
+    # Txs not there in db - Happens during era1/era import, when we don't store txs and receipts
+    if (res.len == 0 and txRoot != emptyRoot):
+      return err("No transactions found in db for txRoot " & $txRoot)
+
     return ok(move(res))
 
 proc getBlockBody*(
@@ -450,8 +455,10 @@ proc getUncleHashes*(
       ): Result[seq[Hash32], string] =
   var res: seq[Hash32]
   for blockHash in blockHashes:
-    let body = ?db.getBlockBody(blockHash)
-    res &= body.uncles.mapIt(it.computeRlpHash)
+    let header = ?db.getBlockHeader(blockHash)
+    let uncles = ?db.getUncles(header.ommersHash)
+
+    res &= uncles.mapIt(it.computeRlpHash)
   ok(res)
 
 proc getUncleHashes*(

execution_chain/db/payload_body_db.nim

@@ -48,6 +48,10 @@ proc getExecutionPayloadBodyV1*(
   for encodedTx in db.getBlockTransactionData(header.txRoot):
     body.transactions.add TypedTransaction(encodedTx)
 
+  # Txs not there in db - Happens during era1/era import, when we don't store txs and receipts
+  if (body.transactions.len == 0 and header.txRoot != emptyRoot):
+    return err("No transactions found in db for txRoot " & $header.txRoot)
+
   if header.withdrawalsRoot.isSome:
     let withdrawalsRoot = header.withdrawalsRoot.value
     if withdrawalsRoot == emptyRoot: