From 079881d305e27ffc25c9234a79b45e67a00188a0 Mon Sep 17 00:00:00 2001 From: Felipe Selmo Date: Fri, 30 Sep 2022 09:19:41 -0600 Subject: [PATCH] Better clarify some EIP-161 nuances. --- eth/_utils/db.py | 16 +++------ eth/vm/base.py | 36 +++++++++---------- eth/vm/forks/frontier/state.py | 6 ++++ eth/vm/forks/spurious_dragon/_utils.py | 9 +++-- tests/core/consensus/test_clique_consensus.py | 2 +- 5 files changed, 33 insertions(+), 36 deletions(-) diff --git a/eth/_utils/db.py b/eth/_utils/db.py index 0bb3e28ab2..06bd13c7f0 100644 --- a/eth/_utils/db.py +++ b/eth/_utils/db.py @@ -28,15 +28,9 @@ def get_block_header_by_hash(block_hash: Hash32, db: ChainDatabaseAPI) -> BlockH def apply_state_dict(state: StateAPI, state_dict: AccountState) -> None: for account, account_data in state_dict.items(): - balance, nonce, code, storage = ( - account_data["balance"], - account_data["nonce"], - account_data["code"], - account_data["storage"], - ) - state.set_balance(account, balance) - state.set_nonce(account, nonce) - state.set_code(account, code) - - for slot, value in storage.items(): + state.set_balance(account, account_data["balance"]) + state.set_nonce(account, account_data["nonce"]) + state.set_code(account, account_data["code"]) + + for slot, value in account_data["storage"].items(): state.set_storage(account, slot, value) diff --git a/eth/vm/base.py b/eth/vm/base.py index 2062e2a6a3..8971d9ffdc 100644 --- a/eth/vm/base.py +++ b/eth/vm/base.py @@ -372,28 +372,26 @@ def _assign_block_rewards(self, block: BlockAPI) -> None: len(block.uncles) * self.get_nephew_reward() ) - if block_reward != 0: - self.state.delta_balance(block.header.coinbase, block_reward) - self.logger.debug( - "BLOCK REWARD: %s -> %s", - block_reward, - encode_hex(block.header.coinbase), - ) - else: - self.logger.debug("No block reward given to %s", block.header.coinbase) + # EIP-161: + # Even if block reward is zero, the coinbase is still touched here. This was + # not likely to ever happen in PoW, except maybe in some very niche cases, but + # happens now in PoS. In these cases, the coinbase may end up zeroed after the + # computation and thus should be marked for deletion since it was touched. + self.state.delta_balance(block.header.coinbase, block_reward) + self.logger.debug( + "BLOCK REWARD: %s -> %s", + block_reward, + encode_hex(block.header.coinbase), + ) for uncle in block.uncles: uncle_reward = self.get_uncle_reward(block.number, uncle) - - if uncle_reward != 0: - self.state.delta_balance(uncle.coinbase, uncle_reward) - self.logger.debug( - "UNCLE REWARD REWARD: %s -> %s", - uncle_reward, - encode_hex(uncle.coinbase), - ) - else: - self.logger.debug("No uncle reward given to %s", uncle.coinbase) + self.logger.debug( + "UNCLE REWARD REWARD: %s -> %s", + uncle_reward, + encode_hex(uncle.coinbase), + ) + self.state.delta_balance(uncle.coinbase, uncle_reward) def finalize_block(self, block: BlockAPI) -> BlockAndMetaWitness: if block.number > 0: diff --git a/eth/vm/forks/frontier/state.py b/eth/vm/forks/frontier/state.py index 8f16ba77e4..d5ae8878d1 100644 --- a/eth/vm/forks/frontier/state.py +++ b/eth/vm/forks/frontier/state.py @@ -182,6 +182,12 @@ def finalize_computation( # Beneficiary Fees gas_used = transaction.gas - gas_remaining - gas_refund transaction_fee = gas_used * self.vm_state.get_tip(transaction) + + # EIP-161: + # Even if the txn fee is zero, the coinbase is still touched here. Post-merge, + # with no block reward, in the cases where the txn fee is also zero, the + # coinbase may end up zeroed after the computation and thus should be marked + # for deletion since it was touched. self.vm_state.logger.debug2( 'TRANSACTION FEE: %s -> %s', transaction_fee, diff --git a/eth/vm/forks/spurious_dragon/_utils.py b/eth/vm/forks/spurious_dragon/_utils.py index b1b1da180c..35fd999ffa 100644 --- a/eth/vm/forks/spurious_dragon/_utils.py +++ b/eth/vm/forks/spurious_dragon/_utils.py @@ -29,11 +29,10 @@ def collect_touched_accounts( See also: https://github.com/ethereum/EIPs/issues/716 """ - # collect the coinbase account if it is in a zero-state. The coinbase is always - # touched via block transaction fee. - coinbase = computation.state.coinbase - if computation.state.account_is_empty(coinbase): - yield coinbase + # EIP-161: + # The coinbase is always touched via block transaction fee and block rewards + # (pre-merge). + yield computation.state.coinbase # collect those explicitly marked for deletion ("beneficiary" is of SELFDESTRUCT) for beneficiary in sorted(set(computation.accounts_to_delete.values())): diff --git a/tests/core/consensus/test_clique_consensus.py b/tests/core/consensus/test_clique_consensus.py index 036b228cf9..e3f75105b6 100644 --- a/tests/core/consensus/test_clique_consensus.py +++ b/tests/core/consensus/test_clique_consensus.py @@ -247,7 +247,7 @@ def test_import_block(paragon_chain): signed_header = sign_block_header(vm.get_block().header.copy( extra_data=VANITY_LENGTH * b'0' + SIGNATURE_LENGTH * b'0', - state_root=b'\x99\xaa\xf5CF^\x95_\xce~\xe4)\x00\xb1zr\x1dr\xd6\x00N^\xa6\xdc\xc41\x90~\xb7te\x00', # noqa: E501 + state_root=b'\x02g\xd5{\xf9\x9f\x9e\xab)\x06\x1eY\x9a\xb7W\x95\xfd\xae\x9a:\x83m%\xbb\xcc\xe1\xca\xe3\x85d\xa7\x05', # noqa: E501 transaction_root=b'\xd1\t\xc4\x150\x9f\xb0\xb4H{\xfd$?Q\x16\x90\xac\xb2L[f\x98\xdd\xc6*\xf7\n\x84f\xafg\xb3', # noqa: E501 nonce=NONCE_DROP, gas_used=21000,