More on withdrawals (#1508)

* Gwei conversion should use u256 because u64 can overflow.

* Make withdrawals follow the EIP-158 state-clearing rules.

(i.e. Empty accounts should be deleted.)

* Allow the zero address in normalizeNumber.

(Necessary for one of the new withdrawals-related tests.)

* Another fix with a withdrawals-related test.

nimbus/core/executor/process_block.nim

@@ -70,15 +70,25 @@ proc procBlkPreamble(vmState: BaseVMState;
           return false
         vmState.receipts[txIndex] = vmState.makeReceipt(tx.txType)
 
-  if header.withdrawalsRoot.isSome:
+  if vmState.determineFork >= FkShanghai:
-    if body.withdrawals.get.calcWithdrawalsRoot != header.withdrawalsRoot.get:
+    if header.withdrawalsRoot.isNone:
-      debug "Mismatched withdrawalsRoot",
+      raise ValidationError.newException("Post-Shanghai block header must have withdrawalsRoot")
-        blockNumber = header.blockNumber
+    elif body.withdrawals.isNone:
-      return false
+      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:
+      for withdrawal in body.withdrawals.get:
-      vmState.stateDB.addBalance(withdrawal.address, withdrawal.amount.gwei)
+        vmState.stateDB.addBalance(withdrawal.address, withdrawal.amount.gwei)
-      vmState.stateDB.deleteAccountIfEmpty(withdrawal.address)
+        vmState.stateDB.deleteAccountIfEmpty(withdrawal.address)
+  else:
+    if header.withdrawalsRoot.isSome:
+      raise ValidationError.newException("Pre-Shanghai block header must not have withdrawalsRoot")
+    elif body.withdrawals.isSome:
+      raise ValidationError.newException("Pre-Shanghai block body must not have withdrawals")
 
   if vmState.cumulativeGasUsed != header.gasUsed:
     debug "gasUsed neq cumulativeGasUsed",

nimbus/evm/state.nim

@@ -388,6 +388,9 @@ func forkDeterminationInfoForVMState*(vmState: BaseVMState): ForkDeterminationIn
   # Also, can I get the TD? Do I need to?
   forkDeterminationInfo(vmState.blockNumber, vmState.timestamp)
 
+func determineFork*(vmState: BaseVMState): EVMFork =
+  vmState.com.toEVMFork(vmState.forkDeterminationInfoForVMState)
+
 proc clearSelfDestructsAndEmptyAccounts*(vmState: BaseVMState, fork: EVMFork, miner: EthAddress): void =
   vmState.mutateStateDB:
     for deletedAccount in vmState.selfDestructs:

nimbus/evm/state_transactions.nim

@@ -33,7 +33,7 @@ proc setupTxContext*(vmState: BaseVMState, origin: EthAddress, gasPrice: GasInt,
     if forkOverride.isSome:
       forkOverride.get
     else:
-      vmState.com.toEVMFork(vmState.forkDeterminationInfoForVMState)
+      vmState.determineFork
   vmState.gasCosts = vmState.fork.forkToSchedule
 
 

nimbus/transaction/call_evm.nim

@@ -101,7 +101,7 @@ proc rpcEstimateGas*(cd: RpcCallData, header: BlockHeader, com: CommonRef, gasCa
     gasLimit:   0.GasInt,          ## ???
     fee:        UInt256.none())    ## ???
   let vmState = BaseVMState.new(topHeader, com)
-  let fork    = com.toEVMFork(vmState.forkDeterminationInfoForVMState)
+  let fork    = vmState.determineFork
   let txGas   = gasFees[fork][GasTransaction] # txGas always 21000, use constants?
   var params  = toCallParams(vmState, cd, gasCap, header.fee)
 

nimbus/vm_state.nim

@@ -20,6 +20,7 @@ export
   vms.buildWitness,
   vms.clearSelfDestructsAndEmptyAccounts,
   vms.coinbase,
+  vms.determineFork,
   vms.difficulty,
   vms.disableTracing,
   vms.enableTracing,

tests/test_blockchain_json.nim

@@ -11,7 +11,7 @@
 import
   std/[json, os, tables, strutils, options],
   unittest2,
-  eth/rlp, eth/trie/trie_defs,
+  eth/rlp, eth/trie/trie_defs, eth/common/eth_types_rlp,
   stew/byteutils,
   ./test_helpers, ./test_allowed_to_fail,
   ../premix/parser, test_config,
@@ -127,6 +127,7 @@ proc parseWithdrawals(withdrawals: JsonNode): Option[seq[Withdrawal]] =
 proc parseBlocks(blocks: JsonNode): seq[TestBlock] =
   for fixture in blocks:
     var t: TestBlock
+    t.withdrawals = none[seq[Withdrawal]]()
     for key, value in fixture:
       case key
       of "blockHeader":
@@ -231,7 +232,6 @@ proc applyFixtureBlockToChain(tester: var Tester, tb: var TestBlock,
   var rlp = rlpFromBytes(tb.blockRLP)
   tb.header = rlp.read(EthHeader).header
   tb.body = rlp.readRecordType(BlockBody, false)
-  tb.body.withdrawals = tb.withdrawals
   tester.importBlock(com, tb, checkSeal, validation)
 
 func shouldCheckSeal(tester: Tester): bool =

vendor/nim-eth

@@ -1 +1 @@
-Subproject commit d2ba7537924e18c02b22bb1df3fc5c8a9380ff54
+Subproject commit 9e89f0dccc54e4c8a670d073175de720af3423dc