From ec2bd4a9c55b9fa61667818e0f5111d9e4dede20 Mon Sep 17 00:00:00 2001 From: Adam Spitz Date: Thu, 16 Mar 2023 16:34:47 -0400 Subject: [PATCH] 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 | 3 ++- nimbus/core/executor/process_transaction.nim | 12 +----------- nimbus/core/tx_pool/tx_tasks/tx_packer.nim | 12 +----------- nimbus/db/accounts_cache.nim | 6 ++++++ nimbus/evm/state.nim | 11 +++++++++++ nimbus/vm_state.nim | 1 + tests/test_blockchain_json.nim | 6 +++++- 7 files changed, 27 insertions(+), 24 deletions(-) diff --git a/nimbus/core/executor/process_block.nim b/nimbus/core/executor/process_block.nim index 0391abe4c..deee5c3d0 100644 --- a/nimbus/core/executor/process_block.nim +++ b/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", diff --git a/nimbus/core/executor/process_transaction.nim b/nimbus/core/executor/process_transaction.nim index 1ee23ccea..1185163d2 100644 --- a/nimbus/core/executor/process_transaction.nim +++ b/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() diff --git a/nimbus/core/tx_pool/tx_tasks/tx_packer.nim b/nimbus/core/tx_pool/tx_tasks/tx_packer.nim index 92ac5feee..3b75b7353 100644 --- a/nimbus/core/tx_pool/tx_tasks/tx_packer.nim +++ b/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() diff --git a/nimbus/db/accounts_cache.nim b/nimbus/db/accounts_cache.nim index e2176bb57..b2a36ce04 100644 --- a/nimbus/db/accounts_cache.nim +++ b/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) diff --git a/nimbus/evm/state.nim b/nimbus/evm/state.nim index 8a38f94e3..d973dbd4c 100644 --- a/nimbus/evm/state.nim +++ b/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) diff --git a/nimbus/vm_state.nim b/nimbus/vm_state.nim index edbfc8563..625b36c58 100644 --- a/nimbus/vm_state.nim +++ b/nimbus/vm_state.nim @@ -18,6 +18,7 @@ export vms.`$`, vms.blockNumber, vms.buildWitness, + vms.clearSelfDestructsAndEmptyAccounts, vms.coinbase, vms.difficulty, vms.disableTracing, diff --git a/tests/test_blockchain_json.nim b/tests/test_blockchain_json.nim index bf55fc2ab..8f8010be5 100644 --- a/tests/test_blockchain_json.nim +++ b/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))