More work on withdrawals (#1503)

* 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.)

nimbus/core/executor/process_block.nim

@@ -32,7 +32,7 @@ import
 # ------------------------------------------------------------------------------
 
 func gwei(n: uint64): UInt256 =
-  (n * (10'u64 ^ 9'u64)).u256
+  n.u256 * (10 ^ 9).u256
 
 proc procBlkPreamble(vmState: BaseVMState;
                      header: BlockHeader; body: BlockBody): bool
@@ -78,6 +78,7 @@ proc procBlkPreamble(vmState: BaseVMState;
 
     for withdrawal in body.withdrawals.get:
       vmState.stateDB.addBalance(withdrawal.address, withdrawal.amount.gwei)
+      vmState.stateDB.deleteAccountIfEmpty(withdrawal.address)
 
   if vmState.cumulativeGasUsed != header.gasUsed:
     debug "gasUsed neq cumulativeGasUsed",

nimbus/core/executor/process_transaction.nim

@@ -88,17 +88,7 @@ proc processTransactionImpl(
       vmState.cumulativeGasUsed += gasBurned
       result = ok(gasBurned)
 
-  vmState.mutateStateDB:
-    for deletedAccount in vmState.selfDestructs:
-      db.deleteAccount deletedAccount
-
-    if fork >= FkSpurious:
-      vmState.touchedAccounts.incl(miner)
-      # EIP158/161 state clearing
-      for account in vmState.touchedAccounts:
-        if db.accountExists(account) and db.isEmptyAccount(account):
-          debug "state clearing", account
-          db.deleteAccount(account)
+  vmState.clearSelfDestructsAndEmptyAccounts(fork, miner)
 
   if vmState.generateWitness:
     vmState.stateDB.collectWitnessData()

nimbus/core/tx_pool/tx_tasks/tx_packer.nim

@@ -112,17 +112,7 @@ proc runTxCommit(pst: TxPackerStateRef; item: TxItemRef; gasBurned: GasInt)
   vmState.stateDB.addBalance(xp.chain.feeRecipient, reward)
 
   # Update account database
-  vmState.mutateStateDB:
-    for deletedAccount in vmState.selfDestructs:
-      db.deleteAccount deletedAccount
-
-    if FkSpurious <= xp.chain.nextFork:
-      vmState.touchedAccounts.incl(xp.chain.feeRecipient)
-      # EIP158/161 state clearing
-      for account in vmState.touchedAccounts:
-        if db.accountExists(account) and db.isEmptyAccount(account):
-          debug "state clearing", account
-          db.deleteAccount account
+  vmState.clearSelfDestructsAndEmptyAccounts(xp.chain.nextFork, xp.chain.feeRecipient)
 
   if vmState.generateWitness:
     vmState.stateDB.collectWitnessData()

nimbus/db/accounts_cache.nim

@@ -1,5 +1,6 @@
 import
   tables, hashes, sets,
+  chronicles,
   eth/[common, rlp], eth/trie/[hexary, db, trie_defs],
   ../constants, ../utils/utils, storage_types,
   ../../stateless/multi_keys,
@@ -443,6 +444,11 @@ proc deleteAccount*(ac: AccountsCache, address: EthAddress) =
   let acc = ac.getAccount(address)
   acc.kill()
 
+proc deleteAccountIfEmpty*(ac: AccountsCache, address: EthAddress): void =
+  if ac.accountExists(address) and ac.isEmptyAccount(address):
+    debug "state clearing", address
+    ac.deleteAccount(address)
+
 proc persist*(ac: AccountsCache, clearCache: bool = true) =
   # make sure all savepoint already committed
   doAssert(ac.savePoint.parentSavepoint.isNil)

nimbus/evm/state.nim

@@ -387,3 +387,14 @@ func forkDeterminationInfoForVMState*(vmState: BaseVMState): ForkDeterminationIn
   # should timestamp be adding 12 or something?
   # Also, can I get the TD? Do I need to?
   forkDeterminationInfo(vmState.blockNumber, vmState.timestamp)
+
+proc clearSelfDestructsAndEmptyAccounts*(vmState: BaseVMState, fork: EVMFork, miner: EthAddress): void =
+  vmState.mutateStateDB:
+    for deletedAccount in vmState.selfDestructs:
+      db.deleteAccount(deletedAccount)
+
+    if fork >= FkSpurious:
+      vmState.touchedAccounts.incl(miner)
+      # EIP158/161 state clearing
+      for account in vmState.touchedAccounts:
+        db.deleteAccountIfEmpty(account)

nimbus/vm_state.nim

@@ -18,6 +18,7 @@ export
   vms.`$`,
   vms.blockNumber,
   vms.buildWitness,
+  vms.clearSelfDestructsAndEmptyAccounts,
   vms.coinbase,
   vms.difficulty,
   vms.disableTracing,

tests/test_blockchain_json.nim

@@ -66,8 +66,12 @@ func normalizeNumber(n: JsonNode): JsonNode =
     result = n
   elif str == "0x00":
     result = newJString("0x0")
+  elif str == "0x0000000000000000000000000000000000000000":
+    # withdrawalsAddressBounds contains this; it's meant as an address, not a number,
+    # so it shouldn't be shortened to "0x0"
+    result = n
   elif str[2] == '0':
-    var i =  2
+    var i = 2
     while str[i] == '0':
       inc i
     result = newJString("0x" & str.substr(i))