go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

internal/ethapi: Set basefee for `AccessList` based on given block, not chain tip

Open jwasinger opened this issue 1 year ago • 5 comments

totally WIP, and I'm not sure this is correct. I need to further validate this tomorrow.

Should close https://github.com/ethereum/go-ethereum/issues/30145 . The problem is that for AccessList, we are filling transaction fields with TransactionArgs.setDefaults which afaict is meant to validate provided fields and give proper values to missing fields based on the state at the current chain tip.

eth_call-like methods seem to populate fields using TransactionArgs.CallDefaults, which also seems appropriate for eth_accessList as it is eth_call-like under the hood: it can be executed against historical blocks.

The referenced issue reflects this: if we calculate defaults for the transaction's dynamic fee fields based on the chain tip, but re-execute against a historical block, then we can encounter a situation where the calculated default for the gas tip is below the base fee of the historical block.

jwasinger avatar Oct 02 '24 15:10 jwasinger

Yeah I think it makes sense. We shouldn't need to estimate actual gas prices or gas limit. Sure those can influence EVM execution but the user can pass in those if they care/their contract depends on it.

Only one thing I noticed: not filling the nonce causes the To address computation possibly wrong for create txes.

s1na avatar Oct 03 '24 12:10 s1na

Only one thing I noticed: not filling the nonce causes the To address computation possibly wrong for create txes.

So this would mean that this is broken in all eth_call variants?

jwasinger avatar Oct 04 '24 11:10 jwasinger

It's not broken. The reason is that the nonce during creation is sourced from the state of the account, not what's in the transaction args.

jwasinger avatar Oct 04 '24 11:10 jwasinger

It's not broken. The reason is that the nonce during creation is sourced from the state of the account, not what's in the transaction args.

Yep for eth_call it's not broken but here it will be when using CallDefaults. It's because CreateAccessList uses the nonce from tx args and passes that to the tracer. We could either here use account state, or modify the tracer to use OnEnter && level == 0 to get the to address.

s1na avatar Oct 04 '24 15:10 s1na

~~If the provided nonce was not correct, the transaction will fail in TransactionArgs.TransitionDb. So I think the current code should be okay (?)~~

jwasinger avatar Oct 08 '24 08:10 jwasinger

@jwasinger please update the top-level PR description (I'm assuming the code has changed since that was written?), so that it's an accurate description. This is a typical such PR that we might revisit three years from now, when trying to figure out why something works in a particular way. So it helps if the desc is up to date with what the PR does.

Other than that, let's merge on green IMO

holiman avatar Nov 08 '24 12:11 holiman

Done. I tried to summarize it as succinctly as possible but it got a bit long :)

jwasinger avatar Nov 14 '24 10:11 jwasinger