ethereumjs-monorepo icon indicating copy to clipboard operation
ethereumjs-monorepo copied to clipboard

Implement Big EOF EIPs

Open jochem-brouwer opened this issue 2 years ago • 7 comments

Implements the EIPs according to https://github.com/ethereum/EIPs/tree/4e5cd9fe6dd168165a7a5cfef68ead439ef27f17

The EIPs in question are:

  • 3540 (EOF header format)
  • 4750 (EOF functions)
  • 5450 (EOF Stack validation)
  • 3670 (EOF code validation)
  • 4200 (EOF static relative jumps)

Note that 3860 (limit and meter initcode) is also part of this one, but it does not seem that we need changes to that one here.

This PR aims to create code which passes the consensus tests (not yet available). The code is not optimized. Optimizing the code will make it less readable and this is not part of this PR.

Some notes regarding optimization:

  • checkOpcodes (essentially 3670 + 5450 checks) passes the code twice
  • When creating an EOFContainer the container is always verified. This is only necessary when checking initcode (e.g. create transaction, CREATE opcode), or when deploying a new EOF contract.

TODOS

  • [ ] Write tests for 5450
  • [ ] Write tests for 4750
  • [ ] Rewrite tests for 4200 (or at helper methods to convert old tests)
  • [ ] Ensure that EOF1 contracts cannot deploy legacy contracts
  • [ ] Ensure that EOF1 checks on initcode/code on both txs and CREATE/CREATE2 consume the right amount of gas when the validation fails (see EIP 4200, 3540)
  • [ ] Stack delta stuff should be part of Opcode class (codes.ts)
  • [ ] Change stack delta stuff to only update inputs/outputs. For swap/dup, (e.g. swap16) mark swap16 as 16 inputs and 16 outputs (delta = 0, but min stack height of 16 is now implicitly verified)
  • [ ] Ensure the newly introduced opcodes (RJUMP, RJUMPV, RJUMPI, CALLF, RETF) are INVALID in legacy contracts
  • [ ] Await until ethereum/tests gets a final spec. Note: we should wait until the tests are also updated with this (currently unmerged) PR: https://github.com/ethereum/EIPs/pull/6249. Currently we fail the EIP 3860 tests, and it makes no sense currently to update those, as we are not know if the tests are incorrect, or our implementation is incorrect. (We need 3860 to test EOF).

NOTE: since Big EOF is highly evolving, always check above ^ URL to see to which version of the EIPs it points

Version updates:

  1. ~~EIP commit 4e5cd9fe6dd168165a7a5cfef68ead439ef27f17 (so https://github.com/ethereum/EIPs/tree/4e5cd9fe6dd168165a7a5cfef68ead439ef27f17)~~
  2. EIPs are now up to date with 932574d7581de03764eb00b9e2c34b2d7c5ce265 (so https://github.com/ethereum/EIPs/tree/932574d7581de03764eb00b9e2c34b2d7c5ce265). At time of writing (Jan 7th) this is also up-to-date with the versions on https://eips.ethereum.org/

jochem-brouwer avatar Dec 16 '22 22:12 jochem-brouwer

Will reserve this comment for some general gotcha's on the EOF:

There are only a few cases where EOF code should be validated:

  • In a create transaction. Check if initcode is valid EOF (if it starts with EOF magic, i.e. 0xEF00)
  • In a CREATE or CREATE2 opcode: validate that initcode is valid EOF
  • If a contract is being deployed (so either after a create transaction, or after a CREATE or CREATE2 opcode)

If EOF is validated and deployed, it is valid.

Note that EOF disallows JUMP and JUMPI. Since we verify in EOF that all RJUMPs are within the code section, this means we can just treat the entire code section as a buffer and just execute it, we already know that we do not go out-of-bounds.

jochem-brouwer avatar Dec 28 '22 05:12 jochem-brouwer

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.38%. Comparing base (70d93fd) to head (a1dfd53). Report is 776 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 90.13% <ø> (ø)
blockchain 90.29% <ø> (ø)
client 87.70% <ø> (ø)
common 95.83% <100.00%> (+0.01%) :arrow_up:
devp2p 91.82% <ø> (+0.05%) :arrow_up:
ethash ∅ <ø> (∅)
evm ?
rlp ?
statemanager 89.61% <ø> (ø)
trie 90.36% <ø> (ø)
tx 93.71% <ø> (ø)
util 84.50% <ø> (ø)
vm ?

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar Dec 28 '22 06:12 codecov[bot]

The 4200 tests now show that we can run EOF contracts and that 4200 is still correct.

jochem-brouwer avatar Dec 28 '22 14:12 jochem-brouwer

Interesting, failing consensus tests.

One of them (Merge, --excludeDir=./stTimeConsuming):

2023-01-07T17:48:42.0870323Z Errors thrown in file: GeneralStateTests/stCreate2/CREATE2_EOF1.json test: CREATE2_EOF1_d0g0v0_Merge:
2023-01-07T17:48:42.0871537Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xd97636deb71cbb6e6a451ef04fc089fff1de118840783e19960a0e3380971418 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0872071Z 	correct last header block
2023-01-07T17:48:42.0872612Z Errors thrown in file: GeneralStateTests/stCreate2/CREATE2_EOF1.json test: CREATE2_EOF1_d1g0v0_Merge:
2023-01-07T17:48:42.0873790Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xd2e2cf10c89149c53f706b9829f09e0c0c643cdd298851a748a7c73ba58c21e5 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0874286Z 	correct last header block
2023-01-07T17:48:42.0874831Z Errors thrown in file: GeneralStateTests/stCreate2/CREATE2_EOF1.json test: CREATE2_EOF1_d2g0v0_Merge:
2023-01-07T17:48:42.0875671Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0x5759428e6b0aa09b5d1226e56b8c030a16d3082e122467ccac657491d9fb7436 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0876150Z 	correct last header block
2023-01-07T17:48:42.0876682Z Errors thrown in file: GeneralStateTests/stCreate2/CREATE2_EOF1.json test: CREATE2_EOF1_d3g0v0_Merge:
2023-01-07T17:48:42.0877525Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0x417a7de5c1cb64745566b1e2d351696f08e5c9952d0b75a0dcbf42cb0291d9b1 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0878009Z 	correct last header block
2023-01-07T17:48:42.0878604Z Errors thrown in file: GeneralStateTests/stCreate2/CREATE2_FirstByte_loop.json test: CREATE2_FirstByte_loop_d0g0v0_Merge:
2023-01-07T17:48:42.0879503Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0x7938ad92e18e9f4dbabc6fafbf673c39548b57fbd3e7cbe3c2c3a981273fa4a2 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0879995Z 	correct last header block
2023-01-07T17:48:42.0880640Z Errors thrown in file: GeneralStateTests/stCreateTest/CREATE_EOF1.json test: CREATE_EOF1_d0g0v0_Merge:
2023-01-07T17:48:42.0881464Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xe1e367d792973c56dad669a6c34d6401e2939895ded90e06721a4f69273c1446 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0881917Z 	correct last header block
2023-01-07T17:48:42.0882433Z Errors thrown in file: GeneralStateTests/stCreateTest/CREATE_EOF1.json test: CREATE_EOF1_d1g0v0_Merge:
2023-01-07T17:48:42.0883256Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0x1f2a44619920bef3dc85086e735ec5a666b2f86519a8d901dda6aa66488fbf59 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0883727Z 	correct last header block
2023-01-07T17:48:42.0884252Z Errors thrown in file: GeneralStateTests/stCreateTest/CREATE_EOF1.json test: CREATE_EOF1_d2g0v0_Merge:
2023-01-07T17:48:42.0885102Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xab7bda61a3d9b390cf5ad5136a8404fbbf01b1a536af582f5155fefcba6c5e40 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0885636Z 	correct last header block
2023-01-07T17:48:42.0886155Z Errors thrown in file: GeneralStateTests/stCreateTest/CREATE_EOF1.json test: CREATE_EOF1_d3g0v0_Merge:
2023-01-07T17:48:42.0886980Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xefb2ccf0b85a135c4af022674c5d390252b6ec920bc7e184d2bd721cb244e2a7 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0887465Z 	correct last header block
2023-01-07T17:48:42.0888059Z Errors thrown in file: GeneralStateTests/stCreateTest/CREATE_FirstByte_loop.json test: CREATE_FirstByte_loop_d0g0v0_Merge:
2023-01-07T17:48:42.0888964Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xc03dba8594b4234ef2b3d1e6c4c99dca346315798eea56bd4aed649612459226 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0889445Z 	correct last header block
2023-01-07T17:48:42.0890123Z Errors thrown in file: GeneralStateTests/stCreateTest/CreateAddressWarmAfterFailureEF.json test: CreateAddressWarmAfterFailureEF_d0g0v0_Merge:
2023-01-07T17:48:42.0891064Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0x16ebef67681c0eb06531b3af74c5c03ab7d8a33be6551db9d46825405f1be1b8 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0891541Z 	correct last header block
2023-01-07T17:48:42.0892214Z Errors thrown in file: GeneralStateTests/stCreateTest/CreateAddressWarmAfterFailureEF.json test: CreateAddressWarmAfterFailureEF_d0g0v1_Merge:
2023-01-07T17:48:42.0893641Z 	Error: invalid receiptTrie (vm hf=merge -> block number=1 hash=0xf6d16a227f8de0ba995d48ccded6a644e434214bc9ad0e6053d5e3ea87b01022 hf=merge baseFeePerGas=10 txs=1 uncles=0)
2023-01-07T17:48:42.0894159Z 	correct last header block

These are likely related to EIP 3541.

jochem-brouwer avatar Jan 07 '23 18:01 jochem-brouwer

Will stop for today.

jochem-brouwer avatar Jan 07 '23 18:01 jochem-brouwer

Outdated, when EOF is on roadmap we can re-address.

jochem-brouwer avatar Mar 07 '24 21:03 jochem-brouwer

This is a dramatic amount of work (a month+?), can we maybe have a closer look at times if we eventually can take this as a basis for a re-implementation at some point? Often things look more tedious at first glance in such situations (because it's mentally straining to bring the old and the new stuff together, map the simliarities and differences,...), but once one is getting it it might still be the case that the amount of work on top here might be dramatically lower than to start from scratch.

Will reopen, let me know if you strongly disagree, but I fear this huge code base will get completely lost if we take this to the closed state.

holgerd77 avatar Mar 08 '24 11:03 holgerd77

Closed in favour of #3440

Note: 3440 has taken a lot of code from here and updated it to the newest EOF spec. In some cases, the changes were so huge that I found it more effective to start from scratch (especially specific parts of container validation)

jochem-brouwer avatar May 29 '24 21:05 jochem-brouwer