fix shanghai withdrawal validation

previously, the withdrawal validation is in process_block only,
but the one in persist block, which is also used in synchronizer
is not validated properly.

.github/workflows/nightly_build.yml

@@ -226,7 +226,7 @@ jobs:
           echo '```' >> release_notes.md
 
       - name: Delete tag
-        uses: dev-drprasad/delete-tag-and-release@v0.2.0
+        uses: dev-drprasad/delete-tag-and-release@v1.0.1
         with:
           delete_release: true
           tag_name: nightly

.github/workflows/simulators.yml

@@ -162,7 +162,7 @@ jobs:
           cat windows_amd64_stat/* >> stat_notes.md
 
       - name: Delete tag
-        uses: dev-drprasad/delete-tag-and-release@v0.2.0
+        uses: dev-drprasad/delete-tag-and-release@v1.0.1
         with:
           delete_release: true
           tag_name: sim-stat

hive_integration/nodocker/consensus/extract_consensus_data.nim

@@ -9,7 +9,7 @@
 # according to those terms.
 
 import
-  std/[json, strutils, options],
+  std/[json],
   stew/byteutils,
   ../../../tools/common/helpers,
   ../../../nimbus/common/chain_config

nimbus/common/genesis.nim

@@ -94,6 +94,9 @@ proc toGenesisHeader*(
   if g.difficulty.isZero and fork <= London:
     result.difficulty = GENESIS_DIFFICULTY
 
+  if fork >= Shanghai:
+    result.withdrawalsRoot = some(EMPTY_ROOT_HASH)
+
 proc toGenesisHeader*(
     genesis: Genesis;
     fork: HardFork;

nimbus/core/chain/chain_desc.nim

@@ -14,8 +14,7 @@ import
   ../../common/common,
   ../../utils/utils,
   ../pow,
-  ../clique,
-  ../validate
+  ../clique
 
 export
   common

nimbus/core/chain/persist_blocks.nim

@@ -109,8 +109,7 @@ proc persistBlocksImpl(c: ChainRef; headers: openArray[BlockHeader];
         let res = c.com.validateHeaderAndKinship(
           header,
           body,
-          checkSealOK = false, # TODO: how to checkseal from here
-          pow = c.pow)
+          checkSealOK = false) # TODO: how to checkseal from here
         if res.isErr:
           debug "block validation error",
             msg = res.error

nimbus/core/executor/process_block.nim

@@ -85,20 +85,15 @@ proc procBlkPreamble(vmState: BaseVMState;
   if vmState.determineFork >= FkShanghai:
     if header.withdrawalsRoot.isNone:
       raise ValidationError.newException("Post-Shanghai block header must have withdrawalsRoot")
-    elif body.withdrawals.isNone:
+    if body.withdrawals.isNone:
       raise ValidationError.newException("Post-Shanghai block body must have withdrawals")
-    else:
-      if body.withdrawals.get.calcWithdrawalsRoot != header.withdrawalsRoot.get:
-        debug "Mismatched withdrawalsRoot",
-          blockNumber = header.blockNumber
-        return false
 
     for withdrawal in body.withdrawals.get:
       vmState.stateDB.addBalance(withdrawal.address, withdrawal.amount.gwei)
   else:
     if header.withdrawalsRoot.isSome:
       raise ValidationError.newException("Pre-Shanghai block header must not have withdrawalsRoot")
-    elif body.withdrawals.isSome:
+    if body.withdrawals.isSome:
       raise ValidationError.newException("Pre-Shanghai block body must not have withdrawals")
 
   if vmState.cumulativeGasUsed != header.gasUsed:

nimbus/core/validate.nim

@@ -10,7 +10,6 @@
 
 import
   std/[sequtils, sets, times, strutils],
-  ../common/common,
   ../db/accounts_cache,
   ".."/[transaction, common/common],
   ".."/[errors],
@@ -24,8 +23,6 @@ from stew/byteutils
   import nil
 
 export
-  pow.PowRef,
-  pow.new,
   results
 
 {.push raises: [].}
@@ -73,8 +70,7 @@ proc validateSeal(pow: PowRef; header: BlockHeader): Result[void,string] =
   ok()
 
 proc validateHeader(com: CommonRef; header, parentHeader: BlockHeader;
-                    txs: openArray[Transaction]; checkSealOK: bool;
-                    pow: PowRef): Result[void,string] =
+                    body: BlockBody; checkSealOK: bool): Result[void,string] =
 
   template inDAOExtraRange(blockNumber: BlockNumber): bool =
     # EIP-799
@@ -88,7 +84,7 @@ proc validateHeader(com: CommonRef; header, parentHeader: BlockHeader;
   if header.extraData.len > 32:
     return err("BlockHeader.extraData larger than 32 bytes")
 
-  if header.gasUsed == 0 and 0 < txs.len:
+  if header.gasUsed == 0 and 0 < body.transactions.len:
     return err("zero gasUsed but transactions present");
 
   if header.gasUsed < 0 or header.gasUsed > header.gasLimit:
@@ -123,10 +119,10 @@ proc validateHeader(com: CommonRef; header, parentHeader: BlockHeader;
       return err("provided header difficulty is too low")
 
     if checkSealOK:
-      return pow.validateSeal(header)
+      return com.pow.validateSeal(header)
 
-  ? com.validateWithdrawals(header)
-  ? com.validateEip4844Header(header, parentHeader, txs)
+  ? com.validateWithdrawals(header, body)
+  ? com.validateEip4844Header(header, parentHeader, body.transactions)
 
   ok()
 
@@ -148,8 +144,8 @@ func validateUncle(currBlock, uncle, uncleParent: BlockHeader):
 
 
 proc validateUncles(com: CommonRef; header: BlockHeader;
-                    uncles: openArray[BlockHeader]; checkSealOK: bool;
-                    pow: PowRef): Result[void,string] =
+                    uncles: openArray[BlockHeader];
+                    checkSealOK: bool): Result[void,string] =
   let hasUncles = uncles.len > 0
   let shouldHaveUncles = header.ommersHash != EMPTY_UNCLE_HASH
 
@@ -213,7 +209,7 @@ proc validateUncles(com: CommonRef; header: BlockHeader;
 
     # Now perform VM level validation of the uncle
     if checkSealOK:
-      result = pow.validateSeal(uncle)
+      result = com.pow.validateSeal(uncle)
       if result.isErr:
         return
 
@@ -380,10 +376,8 @@ proc validateTransaction*(
 proc validateHeaderAndKinship*(
     com: CommonRef;
     header: BlockHeader;
-    uncles: openArray[BlockHeader];
-    txs: openArray[Transaction];
-    checkSealOK: bool;
-    pow: PowRef): Result[void, string] =
+    body: BlockBody;
+    checkSealOK: bool): Result[void, string] =
   if header.isGenesis:
     if header.extraData.len > 32:
       return err("BlockHeader.extraData larger than 32 bytes")
@@ -396,41 +390,22 @@ proc validateHeaderAndKinship*(
     return err("Failed to load block header from DB")
 
   result = com.validateHeader(
-    header, parent, txs, checkSealOK, pow)
+    header, parent, body, checkSealOK)
   if result.isErr:
     return
 
-  if uncles.len > MAX_UNCLES:
+  if body.uncles.len > MAX_UNCLES:
     return err("Number of uncles exceed limit.")
 
   if not chainDB.exists(header.stateRoot):
     return err("`state_root` was not found in the db.")
 
   if com.consensus != ConsensusType.POS:
-    result = com.validateUncles(header, uncles, checkSealOK, pow)
+    result = com.validateUncles(header, body.uncles, checkSealOK)
 
   if result.isOk:
     result = com.validateGasLimitOrBaseFee(header, parent)
 
-proc validateHeaderAndKinship*(
-    com: CommonRef;
-    header: BlockHeader;
-    body: BlockBody;
-    checkSealOK: bool;
-    pow: PowRef): Result[void, string] =
-
-  com.validateHeaderAndKinship(
-    header, body.uncles, body.transactions, checkSealOK, pow)
-
-proc validateHeaderAndKinship*(
-    com: CommonRef;
-    ethBlock: EthBlock;
-    checkSealOK: bool;
-    pow: PowRef): Result[void,string] =
-  com.validateHeaderAndKinship(
-    ethBlock.header, ethBlock.uncles, ethBlock.txs,
-    checkSealOK, pow)
-
 # ------------------------------------------------------------------------------
 # End
 # ------------------------------------------------------------------------------

nimbus/core/withdrawals.nim

@@ -15,9 +15,25 @@ import
 {.push raises: [].}
 
 # https://eips.ethereum.org/EIPS/eip-4895
-func validateWithdrawals*(
-    com: CommonRef, header: BlockHeader
+proc validateWithdrawals*(
+    com: CommonRef, header: BlockHeader, body: BlockBody
 ): Result[void, string] =
+
+  if com.forkGTE(Shanghai):
+    if header.withdrawalsRoot.isNone:
+      return err("Post-Shanghai block header must have withdrawalsRoot")
+    elif body.withdrawals.isNone:
+      return err("Post-Shanghai block body must have withdrawals")
+    else:
+      try:
+        if body.withdrawals.get.calcWithdrawalsRoot != header.withdrawalsRoot.get:
+          return err("Mismatched withdrawalsRoot blockNumber =" & $header.blockNumber)
+      except RlpError as ex:
+        return err(ex.msg)
+  else:
     if header.withdrawalsRoot.isSome:
-    return err("Withdrawals not yet implemented")
+      return err("Pre-Shanghai block header must not have withdrawalsRoot")
+    elif body.withdrawals.isSome:
+      return err("Pre-Shanghai block body must not have withdrawals")
+
   return ok()

nimbus/evm/precompiles.nim

@@ -9,7 +9,7 @@
 # according to those terms.
 
 import
-  std/[math, macros],
+  std/[macros],
   stew/results,
   "."/[types, blake2b_f, blscurve],
   ./interpreter/[gas_meter, gas_costs, utils/utils_numeric],

nimbus/sync/legacy.nim

@@ -180,7 +180,7 @@ proc validateDifficulty(ctx: LegacySyncRef,
     return false
 
 proc validateHeader(ctx: LegacySyncRef, header: BlockHeader,
-                    txs: openArray[Transaction],
+                    body: BlockBody,
                     height = none(BlockNumber)): bool
                     {.raises: [CatchableError].} =
   if header.parentHash == GENESIS_PARENT_HASH:
@@ -237,13 +237,13 @@ proc validateHeader(ctx: LegacySyncRef, header: BlockHeader,
         parentNumber=parentHeader.blockNumber
       return false
 
-  res = com.validateWithdrawals(header)
+  res = com.validateWithdrawals(header, body)
   if res.isErr:
     trace "validate withdrawals error",
       msg=res.error
     return false
 
-  res = com.validateEip4844Header(header, parentHeader, txs)
+  res = com.validateEip4844Header(header, parentHeader, body.transactions)
   if res.isErr:
     trace "validate eip4844 error",
       msg=res.error
@@ -1053,7 +1053,13 @@ proc handleNewBlock(ctx: LegacySyncRef,
       number=blk.header.blockNumber
     return
 
-  if not ctx.validateHeader(blk.header, blk.txs):
+  let body = BlockBody(
+    transactions: blk.txs,
+    uncles: blk.uncles,
+    withdrawals: blk.withdrawals
+  )
+
+  if not ctx.validateHeader(blk.header, body):
     error "invalid header from peer",
       peer, hash=short(blk.header.blockHash)
     return

tests/test_blockchain_json.nim

@@ -49,8 +49,6 @@ type
     network      : string
     postStateHash: Hash256
 
-var pow = PowRef.new
-
 proc testFixture(node: JsonNode, testStatusIMPL: var TestStatus, debugMode = false, trace = false)
 
 func normalizeNumber(n: JsonNode): JsonNode =
@@ -206,7 +204,7 @@ proc importBlock(tester: var Tester, com: CommonRef,
 
   if validation:
     let rc = com.validateHeaderAndKinship(
-      tb.header, tb.body, checkSeal, pow)
+      tb.header, tb.body, checkSeal)
     if rc.isErr:
       raise newException(
         ValidationError, "validateHeaderAndKinship: " & rc.error)
@@ -259,7 +257,7 @@ proc runTester(tester: var Tester, com: CommonRef, testStatusIMPL: var TestStatu
 
         # manually validating
         let res = com.validateHeaderAndKinship(
-                    tb.header, tb.body, checkSeal, pow)
+                    tb.header, tb.body, checkSeal)
         check res.isOk
         when defined(noisy):
           if res.isErr: