Add Dynamic fee transactions, EIP-1559 for London #2013

carver · 2021-08-09T23:58:58Z

What was wrong?

Continuing the work of #2005

How was it fixed?

Creating headers now requires the VM, add API and update usages
New transaction type
Old-style transactions added max_priority_fee_per_gas and max_fee_per_gas, with both set to gas_price
The VM has to check with the state to determine gas price for transaction validation
Sorry, this is a beast. Not sure how effectively it can be sliced up.

Some notes from in-progress work:

lots of places where we assume that the header format is stable (it now has 16 fields instead of 15). This affects everywhere we do rlp encode/decode, as well as generating the genesis block (if it's a London genesis)
tests tend to assume that low gas prices are okay, but now they always need to be more than 1gwei in order to be valid
make eth_call free again (spoof the header's base gas price to 0)
create a genesis header by passing in a None parent, which means not having to (currently: not being able to) specify some genesis fields that we've never really cared to specify, like bloom
cleanup work to replace all straggler Header.from_parent() usages, which all assumed (incorrectly) a stable definition of the header format
some sloppy tests became apparent, and required some fixes (like when we were building a header chain with a non-pow chain, and verifying it with a pow chain, we were actually verifying across all combinations of VMs, where we build with Frontier and test against London, which started breaking of course)
added some missing London opcode tests, and fixed a missing gas-limit validation in London
more cleanup of code/tests that directly invoked headers, or tried to set non-configurable genesis fields (benchmarks, test_opcodes, and test_blockchain)
add convenience VM.create_genesis_header, which is clearer/nicer than VM.create_header_from_parent(parent=None)
uncovered some hidden bugs in test_vm, now apparent because of distinct header types: an overly broad test was failing in the wrong way, and another test was incorrectly testing the cross-product of all the VMs in two chains, rather than just testing the same VM in both chains side-by-side
completely removed the unused class HeaderChain, rather than fix it to work with changing header types
fixed an infinite loop, where a test tried to fill up a block, but went forever on London (London was not yet validating that gas_used <= gas_limit)
choose better header timestamps (instead of parent + 1) -- uncovered a clique validation bug, which was testing the timestamp against the local timezone
fix bug in transaction fund validation (was not multiplying price by gas before balance check), and add a couple basic tests for it

To-Do

Clean up commit history

Add entry to the release notes

Cute Animal Picture

- add initial EIP-1559 transaction validation - add initial EIP-1559 transaction execution - rename gas_target to gas_limit - add london VM in existing tests - update block according to latest EIP updates

eth/chains/base.py

eth/db/chain.py

eth/vm/forks/london/headers.py

carver · 2021-08-12T17:40:27Z

eth/vm/forks/london/blocks.py

+    ]
+
+
+class LondonBlockHeader(rlp.Serializable, BlockHeaderAPI):


consider inheriting from BlockHeader

carver

To be continued tomorrow

eth/db/chain.py

eth/rlp/headers.py

eth/tools/factories/transaction.py

eth/vm/forks/berlin/transactions.py

eth/vm/forks/london/blocks.py

eth/vm/forks/london/headers.py

eth/vm/forks/london/receipts.py

tests/core/vm/test_vm.py

eth/validation.py

eth/vm/forks/london/transactions.py

carver · 2021-08-13T19:43:41Z

eth/vm/forks/london/transactions.py

+    def hash(self) -> Hash32:
+        raise NotImplementedError("Call hash() on the TypedTransaction instead")
+
+    def get_intrinsic_gas(self) -> int:


Move to shared utility methods with Berlin

eth/vm/forks/london/validation.py

eth/vm/state.py

carver · 2021-08-13T23:08:42Z

Oops, I accidentally squashed that last commit 3551c90 @kclowes and @fselmo . It used to be just the failing test, but now includes the fix for decoding uncles across fork boundaries. (Plus some other bonuses mentioned in the commit) See you Monday!

carver · 2021-08-18T22:54:13Z

Ignoring the last two nice-to-have comments, time to get this merged...

eth/vm/forks/london/transactions.py

tests/core/vm/test_london.py

carver · 2021-08-19T00:01:31Z

All that's left is squashing & release notes

- prefer exceptions for base gas fee to None, where feasible - update mainnet block number for London - increase default gas price in tests to 10 gwei to easily out-price the 1559 min gas fee of 1gwei - make old transactions understand max_priority_fee_per_gas and max_fee_per_gas (just return the gas_price, as the 1559 spec suggests) - validate the transaction nonce on new london transactions - enable creating a genesis London header (was hard-coded to pre-London-style) generally allow create_header_from_parent to accept a None parent, for genesis creation - use the correct VM's block header when creating a temp block for txn evaluation - update tests that were configured with a gas price of < 1 gwei - Drop support for Header.from_parent which is essentially always wrong. Should use vm_class.create_header_from_parent instead. - Make eth_call free again - when running a call, set the header's base gas fee to 0 - API update: state_in_temp_block -> in_costless_state to reference ^ - Correctly load uncles across fork boundary - Add test to validate uncles across chain VMs - Use a universal header sedes, since we can't load the VM before decoding the header - Decode block bodies with both London and pre-London uncles - Bonus bugfix: must double the header's gas limit at the VM boundary - Implement base fee adjustment, with tests - target must be half of limit - gas_used_delta bug when parent_gas_used > parent_gas_target: was subtracting base fee instead of target - Apply gas limit rules correctly Note that the 'diff' approach had a bug when 'diff' was negative. That approach needed a math.abs() to correctly validate the limit.

The alternative is to modify create_header_from_parent to accept and assign all the rest of the header fields. Since none of these values are important to customize, I'm going with this easier/smaller change.

Direct invocation of the header object is not allowed anymore, because different VMs now have different header types (as of EIP-1559 in London). So we must always invoke header creation through the VM. - Remove the BlockHeader() init in test_blockchain. - Remove HeaderChain, which seems to be completely unused in the code base. - Embrace that we need one global type that can decode all headers. - Add VM.create_genesis_header The genesis header now needs to be VM-aware, to handle EIP-1559. Rather than pass in a parent of None, it looks a bit cleaner to use the new create_genesis_header. - Create header from VM in opcode test (we need a proper London header at genesis)

Actually look at the clock, in UTC, and compare it to the parent. Related: - Fix doctests: set timestamp manually for reproducible results (new MiningChain API) - clique should verify timestamp in UTC (bug uncovered) - Only guess timestamp on non-genesis headers (when parent & VM are unavailable), so manually set in a test where headers are built manually

- Test London validation of sender sufficient funds

Each transaction object should now how to display itself, whether it's a legacy, access list, or dynamic fee transaction.

- add tests to verify - Push backwards-compatible usages of get_tip() to Frontier - Bonus fix: setting coinbase when mining The coinbase address was not correctly receiving transaction fees when the address is passed as a kwarg to MiningChain.mine_all or .mine_block - Bonus fix: Correctly decode dynamic-fee txns in London

This seems to be a commonly used name, and is definitely better than BaseGasFee.

The old test implementation was finding other problems, like a base_fee_per_gas that did not match the parent.

It just didn't show up because this is the first time that the VM header validation rules vary so significantly.

Not really getting anything useful from crashing there. It's an ugly crash that should be caught somewhere else.

Otherwise, get an infinite loop, where test hangs.

First steps for EIP-1559 support

5b205e6

- add initial EIP-1559 transaction validation - add initial EIP-1559 transaction execution - rename gas_target to gas_limit - add london VM in existing tests - update block according to latest EIP updates

carver force-pushed the eip-1559 branch 3 times, most recently from c0dd6e6 to b7daeac Compare August 11, 2021 21:43