Conversation
|
There's some new failing spec tests introduced in the 7928 changes. I am looking into them. |
|
There's currently a lot of tests failing. Likely, some the failures are related to 8037, but I have yet to deep dive into them. |
|
eth70 is ready to be merged :) |
|
I rebased this because 7778 was merged. I also merged the eth 70 changes in here. |
|
@MariusVanDerWijden I stripped the outdated 8037 changes from this branch. |
core/vm/gas_table.go
Outdated
| @@ -391,97 +421,155 @@ func gasCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize | |||
| } else if !evm.StateDB.Exist(address) { | |||
| gas += params.CallNewAccountGas | |||
There was a problem hiding this comment.
If we are post-amsterdam, we need to exclude this from the regular gas. I am attempting to integrate #33648 in to this branch, which should make this change.
a247ae0 to
e1521be
Compare
| prev := st.gasRemaining.RegularGas | ||
| // When the calldata floor exceeds actual gas used, any | ||
| // remaining state gas must also be consumed | ||
| targetRemaining := (st.initialGas.RegularGas + st.initialGas.StateGas) - floorDataGas |
There was a problem hiding this comment.
can we get into a situation where targetRemaining > params.MaxTxGasLimit? If the tx gas limit is high, and the gas used low, the state gas might be vastly larger than floorDataGas.
e5d453c to
b3113ba
Compare
CopyHeader copies all pointer-typed header fields (WithdrawalsHash, RequestsHash, SlotNumber, etc.) but was missing the deep copy for BlockAccessListHash added by EIP-7928. This caused the BAL hash to be silently shared between the original and the copy, leading to potential data races and incorrect nil-checks on copied headers.
Adds support for returning `blockAccessListHash` in the RPC block header response. This change includes `blockAccessListHash` in `RPCMarshalHeader` when `head.BlockAccessListHash` is not nil, aligning the RPC output with the Header struct which already contains this field.
* add method on StateReaderTracker to clear the accumulated reads * don't factor the BAL size into the payload size during construction in the miner * simplify miner code for constructing payloads-with-BALs via the use of aformentioned StateReaderTracker clear method * clean up the configuration of the BAL execution mode based on the preset flag specified
## Summary - The BAL refactoring in 5808d21 removed the line `env.header.GasUsed = env.gasPool.Used()` from `applyTransaction()`, causing all geth-proposed blocks to have `GasUsed=0` in the header - Every other client rejects these blocks during validation, so geth can never successfully propose on the network - Restores the single line that was present in upstream master ## Test plan - [x] Verified line exists in upstream/master at miner/worker.go:409 - [x] Confirmed all 34 geth-proposed blocks on a 4-client devnet had `GasUsed=0` via `debug_getBadBlocks` on besu - [ ] Build local image and verify geth proposals are accepted in a multi-client Kurtosis network
…ly read doesn't appear in both the read/write set in the BAL Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
…ss list hash on block header when calling 'Block.WithAccessList'
… made some changes to the --bal.executionmode flag
Rename `balHash` to `blockAccessListHash` in json encoding of block header. Fix miner panic when attempting to create pre-amsterdam blocks.
|
@MariusVanDerWijden I pushed a fix to an issue found by @qu0b . PTAL at 566992d and if it looks okay to you, we can add it to the 8037 branch. |
|
Oops. Those fixes broke some tests. Reverted them until I can get them working |
Co-authored-by: jwasinger <j-wasinger@hotmail.com>
…with access lists Co-authored-by: spencer <spencer.tb@ethereum.org>
To check whether a transaction can be applied, we validate that `blockGasLimit > txGasLimit + (cumulativeRegularGasUsed + cumulativeStateGasUsed)`. However, the check should only be applied to the bottleneck resource, i.e. `blockGasLimit > max(txRegularGasUsed+cumulativeRegularGasUsed, txStateGasUsed+ cumulativeStateGasUsed)`. The changes here break multiple tests. I am trying to determine why. --------- Co-authored-by: qu0b <stefan@starflinger.eu>
The BAL reader tracker captures access list reads at the reader level. When statedb has an account cached the BAL tracker is not informed of the access. This is ok during the lifetime of a transaction because you only need to record the access the first time. It is also ok during the lifetime of a block because BAL reads are block-level (same as statedb caches). Where I think the issue can rise is in the miner. Namely when building a block, if the miner picks up a tx which fails, it drops it and picks up another tx to include. There might be some edge case here where the failed tx which is not included poisons the cache and a future block which is included omits an account because it wasn't aware of the access.
Fixes the collision bugs and create gas ordering
Contains changes required for devnet 3. The BAL changes are in a branch here and build on several open PRs:
Other EIPs in devnet 3 with open PRs have been squashed into single commits and included on this branch:
EIPs missing from this branch: