-
Notifications
You must be signed in to change notification settings - Fork 19
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
EOFv0 for packaging legacy code in Verkle Trees #58
Conversation
spec/eofv0_verkle.md
Outdated
|
||
### Super-dense metadata encoding | ||
|
||
Follow the original Verkle Tree idea to provide the metadata of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another variant of this could be storing chunk_id => offset
map only for those chunks that have non-zero offset
("number of leading pushdata bytes in a chunk"). Intuitively it seems that most of the chunks have 0 there in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is actually true, the PUSH
instructions are quite frequent. But we can measure this of course.
But the main disadvantage is that you need to preprocess the section to use it.
I did some quick calculations: for the max number of chunks 768 the size of the 5-bit encoded section is 480 bytes. Assuming 3 bytes per entry of the map encoding, this encoding brings savings only if only 20% of chunks have non-zero entry.
Alternative design I would suggest to explore is not packaging encoded jumpdest table into bytecode, but rather have it as a separate new field of the account (which will be chunked as well in a Verkle trie structure). Upsides of this are no need to parse the container, not changing the spec of (EXT)CODE* and jumps. (and in general I see this problem as not related to EOF at all.) Probably Verkle trie structure would reserve some number of jumpdest table chunks in 0th account subtree (similar to how now current spec reserves 128 code chunks there), then the downside might be less code chunks fitting there, and wasted reserved chunks for EOF accounts, which will have empty jumpdest table. |
I think the end effect of moving more stuff to the Vekle Tree's account is similar. E.g. reserving some additional chunks for code metadata in 0th subtree give similar effect to reserving more such chunks to the EOF container. Considering this, my preference is still leaving the container mess to the code itself. It is also easier to take such container outside of the Verkle Tree world. |
It's not the same, when the jumptable is inside the code, it's either in front of code section, then all jumptable chunks go into 0th subtree (worst case bitset jumptable occupies 96 chunks, then only ~32 left for code), or it's after code section, then often all of them are out of 0th subtree and expensive. With a separate field it's more flexible design space, for example reserve 112 code chunks + 16 jumptable chunks in 0th subtree, or other similar combinations. |
Outside world can also continue with doing jumpdest analysis 🙃 |
Let's assume the max jumpdest section size is 16 chunks (this is the case for 5-bit encoding). I think what you propose is less flexible because you need to make single decision for all contracts. E.g. reserve 16 chunks [112:128] in the account for jumpdest analysis. With EOF you can have the same by defining Anyway, I'm very much for collecting ideas. We can add yours as a separate PR or you can just push to this branch.
This includes testing and initcode execution. |
I agree it is not flexible in the sense of single decision for all contracts. But I was arguing from bitset encoding, which seemed to be your preferred encoding. For 5-bit encoding my argument doesn't really stand and when worst case is 16 chunks it might be okay to have it inside the code.
Good point, it would be great to get rid of jumpdest analysis in initcode execution too, but I think we'll still need to have it for existing factory contracts, so maybe it's okay to have it for all initcode executions. |
spec/eofv0_verkle.md
Outdated
|
||
Follow the original Verkle Tree idea to provide the metadata of | ||
"number of leading pushdata bytes in a chunk". However, instead of including | ||
this metadata as a single byte in the chunk itself, place the value as a 5-bit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As pointed out by @gballet, we need 6 bits to encode the max value 32.
spec/eofv0_verkle.md
Outdated
|
||
## Extensions | ||
|
||
### Data section |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it doesn't make sense for the given example heuristic. What it does is searches for the unreachable code at the end without any valid jumpdests in it. We can achieve the same or better effect by trimming the jumpdest section to the latest bit set.
The design draft that proposes the use of EOF for storing code in Verkle Trees. An alternative to the existing method of executing 31-byte code chunks accompanied by 1 byte of metadata.
spec/eofv0_verkle.md
Outdated
1. Put the code in the single *code* EOF section. | ||
2. Use the EOF container format proposed by [EIP-3540](https://eips.ethereum.org/EIPS/eip-3540) with | ||
version 0 and following modifications to "Changes to execution semantics": | ||
1. `CODECOPY`/`CODESIZE`/`EXTCODECOPY`/`EXTCODESIZE`/`EXTCODEHASH` operates on the *code* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on our meeting, this is an important change. This a separate set of semantics to what EOFv1 proposes.
spec/eofv0_verkle.md
Outdated
1. `CODECOPY`/`CODESIZE`/`EXTCODECOPY`/`EXTCODESIZE`/`EXTCODEHASH` operates on the *code* | ||
section only. | ||
2. `JUMP`/`JUMPI`/`PC` relates code positions to the *code* section only. | ||
3. Perform the jumpdest analysis of the code at deploy time (during contract creation). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually makes a big semantical change here: we are locking in the EVM version at the time of contract deployment (or verkle transition).
Currently in mainnet we rely on the fact that semantics of contracts can change. This is both a negative (and maybe a positive?)
Example: if a new opcode is introduced, the jumpdest analysis result of a contract may change. This is not the case after this proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not been following 100%, but could we mitigate this:
Example: if a new opcode is introduced, the jumpdest analysis result of a contract may change. This is not the case after this proposal.
by versioning the jumpdest analysis result and updating it on first use after the new opcode was introduced? Similar to how we do "packaging" (point 5), we do "re-packaging".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean -- I mean that if the jumpdest analysis result is locked it at the time of contract creation / verkle transition, then introduction of new legacy opcodes will need to have special considerations for such contracts.
Currently we can do an analysis on chain to see what effect a new opcode may be (does it change semantics of contracts?), but with the addition of the jumpdest table this analysis is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that when you store the jumpdest analysis result during verkle transition, you can prepend a version 1
to it. If there's a new opcode introduced you would need to re-do the analysis, overwrite the jumpdest result and bump the version. That would happen the next time the code is touched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is unfeasible. Would need to update the entire chain at every hardfork. This is one of the reasons any transition like verkle or flat-tree proposed earlier hits the roadblock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting topic. Locking jumpdest analysis allows introducing instructions with immediate data. But this only works for deployed code but not for initcode (where analysis runs before execution).
5. The packaging process is done for every deployed code during Verkle Tree migration | ||
and also for every contract creation later | ||
(i.e. becomes the part of the consensus forever). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed a special handling for data contracts: one could check if the first byte is a terminating instruction (STOP, REVERT, etc.) or an unassigned instruction.
However, marking it as a data contract based on an unassigned instruction, then the "problem" listed in https://github.com/ipsilon/eof/pull/58/files#r1484267443 comes up. The only "use case" here I can see is that someone deploys a contract with a soon-to-be-introduced instruction, and that will not work. Think about those merge NFTs.
31-byte code chunks accompanied by 1 byte of metadata. | ||
|
||
## Goal | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd summarise the main goal as the following:
Have a unified way of handling chunking (by not having chunking).
With reusing basic EOF constructs, this allows a more simplified verkle implementation supporting both "eof0 legacy" and eof1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Secondary objective: can this result in a better "code-to-data" ratio (by avoiding chunking)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by your term "not having chunking" or "avoiding chunking". The chunking will be still present. Do you mean the chunking scheme with 31-byte code payload and additional metadata byte?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simplified verkle implementation supporting both "eof0 legacy" and eof1.
I agree "simplifying verkle impl" and "better code-to-data" (to be verified with data!) are the ultimate goals and benefits here, and should be listed in the doc to keep focused on that.
5. The packaging process is done for every deployed code during Verkle Tree migration | ||
and also for every contract creation later | ||
(i.e. becomes the part of the consensus forever). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what happens to initcode? Do creation transaction and CREATE/CREATE2 accept the container? I assume you didn't intend it, as it's a big change, so they kinda accept only stripped-down "code section". And deployed container is also not returned from initcode as container. This should be mentioned I think.
I think I personally would prefer not to package it into EOF container and not to frame it as "container" and anything related to EOF at all. Just prepend bytecode with jumptable in code
field of the account. None of the instructions change their semantics, we only change how bytecode is fetched from the trie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a fix to Verkle trie structure, it's not like we're inventing yet another EOF to make Verkle compatible with EOFv1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general, just a bunch of discussion points
31-byte code chunks accompanied by 1 byte of metadata. | ||
|
||
## Goal | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simplified verkle implementation supporting both "eof0 legacy" and eof1.
I agree "simplifying verkle impl" and "better code-to-data" (to be verified with data!) are the ultimate goals and benefits here, and should be listed in the doc to keep focused on that.
### Container | ||
|
||
1. Re-use the EOF container format defined by [EIP-3540](https://eips.ethereum.org/EIPS/eip-3540). | ||
2. Set the EOF version to 0. I.e. the packaged legacy code will be referenced as EOFv0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if EOF is chosen as the format, with version 00
, I would not refer to this as EOFv0, to avoid confusion and be clear that this is not "the EOF" we've been working on. Adherence to EOF sections format should just be treated as a fortunate coincidence imho and mentioned in fine print.
4. The header must contain information about the sizes of these sections. | ||
For that the EIP-3540 header or a simplified one can be used. | ||
5. The legacy code is placed in the *code* section without modifications. | ||
6. The *jumpdest* section contains the set of all valid jump destinations matching the positions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has the alternative of storing invalid destinations been considered, that is 0x5b
bytes not being JUMPDESTs? possibly as one functioning alongside the fallback storage of valid destinations, only intended to protect maliciously crafted code 0x7f5b5b5b5b5b...
etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By invalid destinations do you mean push data? There are some alternative encodings in the "Jumpdest section encoding" section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By invalid destinations do you mean push data?
Not exactly - I mean the invalid 5b
bytes, which are inside the push data, I think the encoding section doesn't cover this. This isn't about encoding that much as it is about the high-level concept. Which encoding would best suit this approach - I do not know.
- `EXTCODECOPY`, | ||
- `EXTCODESIZE`, | ||
- `EXTCODEHASH`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these instructions are invalid.
These 3 are valid when called in legacy and targeting EOF, they have semantics equaling to treating EOF code as being "just two bytes EF00
". So the difference should be analyzed here. My guess here that it is fine except being a tiny bit more complex to think about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should not mention "original EIP-3540" because it is no longer relevant.
spec/eofv0_verkle.md
Outdated
without EOF wrapping. However, because the EOF containers are not visible to any EVM program, | ||
implementations may decide to wrap initcodes with EOFv0 and execute it the same way as | ||
EOFv0 deployed codes. | ||
2. The initcode size limit remains defined by [EIP-3860](https://eips.ethereum.org/EIPS/eip-3860). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. The initcode size limit remains defined by [EIP-3860](https://eips.ethereum.org/EIPS/eip-3860). | |
2. The initcode size limit and cost remains defined by [EIP-3860](https://eips.ethereum.org/EIPS/eip-3860). | |
The size limit and cost refers to plain initcode without wrapping |
- encode the jumpdest analysis result as the *jumpdest* section, | ||
- put the plain code in the *code* section, | ||
- create EOFv0 container with the *jumpdest* and *code* sections. | ||
5. The code deployment cost is calculated from the total EOFv0 size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't we concerned that this de-facto cost increase will break existing CREATE/CREATE2 contracts?
- `EXTCODEHASH`. | ||
5. To execute a `JUMP` or `JUMPI` instruction the jump target position must exist | ||
in the *jumpdest* set. The *jumpdest* guarantees that the target instruction is `JUMPDEST`. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6. `DELEGATECALL2` (from EOFv1) must also check the version to disallow delegating from EOFv1 to legacy ("EOFv0") | |
7. `CREATE4` implementation must ensure that legacy code packaged with `EF00` magic isn't mistakenly taken as something CREATE4 can deploy (might be the case if some EVM implementation code is reused) |
Applies only if EF00
is chosen as the magic to indicate such packaging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should skip all details about interaction with the new EOF calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These points are to mostly to remember to be careful about "reusing" the EF00
magic, the details can be skipped if you prefer.
spec/eofv0_verkle.md
Outdated
EOFv0 deployed codes. | ||
2. The initcode size limit remains defined by [EIP-3860](https://eips.ethereum.org/EIPS/eip-3860). | ||
3. The initcode still returns a plain deploy code. | ||
The plain code size limit is defined by [EIP-170](https://eips.ethereum.org/EIPS/eip-170). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plain code size limit is defined by [EIP-170](https://eips.ethereum.org/EIPS/eip-170). | |
The plain code size limit and cost is defined by [EIP-170](https://eips.ethereum.org/EIPS/eip-170). |
spec/eofv0_verkle.md
Outdated
2. The initcode size limit remains defined by [EIP-3860](https://eips.ethereum.org/EIPS/eip-3860). | ||
3. The initcode still returns a plain deploy code. | ||
The plain code size limit is defined by [EIP-170](https://eips.ethereum.org/EIPS/eip-170). | ||
4. The plain code is not empty it must be wrapped with EOFv0 before put in the state: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4. The plain code is not empty it must be wrapped with EOFv0 before put in the state: | |
4. If the plain code is not empty, it must be wrapped with EOFv0 before put in the state: |
This seems like it increases code overhead from ~3% to ~12.5%, and increases the number of chunks you need to read to access a piece of code from 1 to 2 in many cases. The latter particularly feels pretty worrying, especially since one of the main existing downsides of Verkle gas cost accounting is that accessing code becomes more expensive. Given that you can already very easily compute the jumpdest table from a chunk just by knowing the 31 bytes of code and the 1 byte offset, are the benefits of this really worth the downsides? |
spec/eofv0_verkle.md
Outdated
This provides the following benefits over the original Verkle Tree design: | ||
|
||
1. The code executes by full 32-byte chunks. | ||
2. The *metadata* size overhead slightly smaller `1/32` instead of `1/31`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess better to mention how much this is in % for easier comparison with other options.
Not exactly. As described in the "Jumpdest section encoding", we can tune the code overhead from ~12.5% to ~3% or even 2%. The main idea here is: move the information encoded by the "1 byte offset" to separate section and allow code chunks to be full 32 bytes. This obviously breaks the locality of the information: a jump may need to load additional chunk for the jumpdest section. However, we save one byte of code when going from one code chunk to the next one (we don't need the 1 byte offset then). Deeper analysis of the execution of real transactions should reveal the true numbers. Finally, we may try to extend the analysis to be able to mark most of the contracts as "all jumps are valid" and therefore skip the jumpdest section at all. |
5. The code deployment cost is calculated from the total EOFv0 size. | ||
This is a breaking change so the impact must be analysed. | ||
6. During Verkle Tree migration perform the above EOFv0 wrapping of all deployed code. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also worth to mention than verkle account's code_size and code_hash fields should refer to the code without wrapping. And how to calculate full contract size from that.
3. The *metadata* lookup is only needed for executing jumps | ||
(not needed when following through to the next chunk). | ||
|
||
### Super-dense metadata encoding (6-bit numbers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New proposal discussed in person:
Encode only "invalid pushdata" list. The goal is that we only encode invalid pushdatas (blocklist) as opposed to allowlist, and the hunch is that majority of the contracts have none so this header will be empty, and average contracts will only have a few entries.
The list is basically a hashmap: invalid_jumpdest[chunk_no] = position_in_chunk
. This could be encoded as a delta-encoding of chunk_no
and 6-bit encoding of position_in_chunk
.
Any dense encoding is sufficient if we want to reduce the header size the most.
Analysis is still needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple calculation with 4-bit encoding of chunk_no
, for worst case scenario where each 32-byte chunk has an invalid jumpdest:
total_chunk_count = 24576 / 32 = 768
total_chunk_count * (4 + 6) / 8 = 960 # bytes for the header
number_of_verkle_leafs = total_chunk_count / 32 = 30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need a basic "RLE-style" encoding here with a skip mode (lets call this scheme 1), i.e.:
- 1-bit mode (skip, value)
- For skip:
- 10-bit chunk number delta (could be 9-bit, then need two skips in worst case if there's a single invalid jumpdest in the last chunk)
- For value:
- 4-bit chunk number delta
- 6-bit jump position offset
The size of chunk number deltas must / can be tuned based on actual data.
Worst case with this becomes:
total_chunk_count = 24576 / 32 = 768
total_chunk_count * (1 + 4 + 6) / 8 = 1056 # bytes for the header
number_of_verkle_leafs = total_chunk_count / 32 = 33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also simplify with not having delta in the value encoding (let's call this scheme 2):
- 1-bit mode
- skip: 10-bit chunk delta
- value: 6-bit offset
Here's a calculation we did, these are the "invalid pushdatas" from a 2147 byte long contract:
(chunk offset, chunk number, pushdata offset)
malicious push byte: 85 2 21
malicious push byte: 95 2 31
malicious push byte: 116 3 20
malicious push byte: 135 4 7
malicious push byte: 216 6 24
malicious push byte: 1334 41 22
Encoding with scheme 1:
[skip, 2]
[value, 0, 21]
[value, 0, 31]
[value, 1, 20]
[value, 1, 7]
[value, 2, 24]
[skip, 35, 22]
2 skips (2 * 11 bits), 5 values (5 * 11 bits) = 10-bytes header (0.465%)
Encoding with scheme 2:
[skip, 2]
[value, 21]
[value, 31]
[skip, 1]
[value, 20]
[skip, 1]
[value, 7]
[skip, 2]
[value, 24]
[skip, 35]
[value, 22]
5 skips (5 * 11 bits), 6 values (6 * 7 bits) = 13-bytes header (0.605%)
Another example of the uniswap router contract (17958 bytes):
malicious push byte: 1646 51 14
malicious push byte: 1989 62 5
malicious push byte: 4239 132 15
malicious push byte: 4533 141 21
malicious push byte: 7043 220 3
malicious push byte: 8036 251 4
malicious push byte: 8604 268 28
malicious push byte: 12345 385 25
malicious push byte: 15761 492 17
Encoding using scheme 1:
[skip, 51]
[value, 0, 14]
[value, 11, 5]
[skip, 70]
[value, 0, 15]
[value, 9, 21]
[skip, 79]
[value, 0, 3]
[skip, 31]
[value, 0, 4]
[skip, 17]
[value, 0, 28]
[skip, 117]
[value, 0, 25]
[skip, 107]
[value, 0, 17]
22-bytes header (0.122%)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documented in #98.
The design draft that proposes the use of EOF for storing code in Verkle Trees.
An alternative to the existing method of executing 31-byte code chunks accompanied by 1 byte of metadata.