Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfc: Lazy-load fee beneficiary in mainnet handler #1774

Closed
wants to merge 3 commits into from

Conversation

clabby
Copy link
Contributor

@clabby clabby commented Sep 16, 2024

Overview

Lazy-loads the fee beneficiaries like geth's EVM does. In the case where no value is transferred to the fee beneficiary, the account is not actually loaded from state in geth. This is a small semantic difference between the two implementations that does not actually affect the outcome of the STF, but does make execution witnesses generated by geth incompatible with reth due to the lack of a MPT proof for the fee beneficiary account.

Because load_account hits the DB via Database::basic, a proof to the beneficiary account in the trie is required to complete execution.

This edge case shows up in empty L1/L2 blocks, specifically for the 4788 transaction, where gas_price == 0, no value is transferred, and no other transactions load the fee recipient.

Note that the fee recipients on Optimism are always loaded, so that logic remains untouched: https://github.com/ethereum-optimism/op-geth/blob/21010bb6b78b8478ff0121778b781411263e160d/core/state_transition.go#L585-L597

Refs

https://github.com/ethereum/go-ethereum/blob/ae707445f54972f4fddc7c6cf8ba2de083636e50/core/state_transition.go#L468-L471

@clabby clabby changed the title rfc: Lazy-load fee beneficiaries rfc: Lazy-load fee beneficiary in mainnet handler Sep 16, 2024
let basefee = *context.evm.env.block.basefee();

// Skip the transfer if basefee and max priority fee per gas is zero, like geth.
if basefee.is_zero()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we skip if basefee is zero? Shouldn't we skip only if coinbase_gas_price == 0

Copy link
Contributor Author

@clabby clabby Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -49,11 +66,7 @@ pub fn reward_beneficiary<EvmWiringT: EvmWiring, SPEC: Spec>(
.map_err(EVMError::Database)?;

coinbase_account.data.mark_touch();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are skipping touch on coinbase account.

Geth code looks like a edge case:
https://github.com/ethereum/go-ethereum/blob/ae707445f54972f4fddc7c6cf8ba2de083636e50/core/state/state_object.go#L452-L459

This can potentially be a bug for EIP-161 State clear. The trigger is if coinbase is empty_account (Created before EIP-161) and there is no fee for this tx.

@clabby
Copy link
Contributor Author

clabby commented Sep 17, 2024

Going to go ahead and close this for the time being - will re-visit as we start to work on standardizing the execution witnesses generated by op-reth and op-geth.

@clabby clabby closed this Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants