From 8d9b2522eaa91c856ca9e23fae315d8f3398f8c2 Mon Sep 17 00:00:00 2001 From: Adam Spitz Date: Fri, 17 Mar 2023 14:16:24 -0400 Subject: [PATCH] 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 | 26 ++++++++++++++++++-------- nimbus/evm/state.nim | 3 +++ nimbus/evm/state_transactions.nim | 2 +- nimbus/transaction/call_evm.nim | 2 +- nimbus/vm_state.nim | 1 + tests/test_blockchain_json.nim | 4 ++-- vendor/nim-eth | 2 +- 7 files changed, 27 insertions(+), 13 deletions(-) diff --git a/nimbus/core/executor/process_block.nim b/nimbus/core/executor/process_block.nim index deee5c3d0..f44034805 100644 --- a/nimbus/core/executor/process_block.nim +++ b/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 body.withdrawals.get.calcWithdrawalsRoot != header.withdrawalsRoot.get: - debug "Mismatched withdrawalsRoot", - blockNumber = header.blockNumber - return false + if vmState.determineFork >= FkShanghai: + if header.withdrawalsRoot.isNone: + raise ValidationError.newException("Post-Shanghai block header must have withdrawalsRoot") + elif 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) - vmState.stateDB.deleteAccountIfEmpty(withdrawal.address) + for withdrawal in body.withdrawals.get: + vmState.stateDB.addBalance(withdrawal.address, withdrawal.amount.gwei) + 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", diff --git a/nimbus/evm/state.nim b/nimbus/evm/state.nim index d973dbd4c..ac54b3b5a 100644 --- a/nimbus/evm/state.nim +++ b/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: diff --git a/nimbus/evm/state_transactions.nim b/nimbus/evm/state_transactions.nim index 0d74dbe12..1aa5084e6 100644 --- a/nimbus/evm/state_transactions.nim +++ b/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 diff --git a/nimbus/transaction/call_evm.nim b/nimbus/transaction/call_evm.nim index da09b06af..0f34bc995 100644 --- a/nimbus/transaction/call_evm.nim +++ b/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) diff --git a/nimbus/vm_state.nim b/nimbus/vm_state.nim index 625b36c58..20fb5c6d2 100644 --- a/nimbus/vm_state.nim +++ b/nimbus/vm_state.nim @@ -20,6 +20,7 @@ export vms.buildWitness, vms.clearSelfDestructsAndEmptyAccounts, vms.coinbase, + vms.determineFork, vms.difficulty, vms.disableTracing, vms.enableTracing, diff --git a/tests/test_blockchain_json.nim b/tests/test_blockchain_json.nim index 8f8010be5..782c224fe 100644 --- a/tests/test_blockchain_json.nim +++ b/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 = diff --git a/vendor/nim-eth b/vendor/nim-eth index d2ba75379..9e89f0dcc 160000 --- a/vendor/nim-eth +++ b/vendor/nim-eth @@ -1 +1 @@ -Subproject commit d2ba7537924e18c02b22bb1df3fc5c8a9380ff54 +Subproject commit 9e89f0dccc54e4c8a670d073175de720af3423dc