Skip to content
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

Update EIP-7612: Update func implementations and add further clarifications #8754

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

jsign
Copy link
Contributor

@jsign jsign commented Jul 23, 2024

This PR proposes:

  • Updating the spec to align with the latest EIP-6800 changes regarding multiple account fields being packed in BASIC_DATA.
  • Add a missing new function to save code-chunks in the state. This is required not only for contract creations, but also for the EIP that explains how to do the MPT->VKT conversion.
  • Explain block header changes.

Signed-off-by: Ignacio Hagopian <[email protected]>
Signed-off-by: Ignacio Hagopian <[email protected]>
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 23, 2024

✅ All reviewers have approved.

@eth-bot eth-bot changed the title EIP-7612: Update func implementations and add further clarifications Update EIP-7612: Update func implementations and add further clarifications Jul 23, 2024
@@ -13,15 +13,15 @@ requires: 4762, 6800, 7545

## Abstract

This EIP proposes a method to switch the state tree tree format from hexary Merkle Patricia Tree (MPT) to a verkle tree: the MPT tree is frozen, and new writes to the state are stored in a verkle tree “laid over” the hexary MPT. The historical MPT state is left untouched and its eventual migration is handled at a later time.
This EIP proposes a method to switch the state tree tree format from hexary Merkle Patricia Tree (MPT) to a Verkle Tree (VKT): the MPT tree is frozen, and new writes to the state are stored in a VKT “laid over” the hexary MPT. The historical MPT state is left untouched and its eventual migration is handled at a later time.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For "Verkle Tree" do the same as we do for "Merkle Patricia Tree".

@@ -31,9 +31,9 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S

### Constants

|Parameter|value|Description|
|-|-|-|
|`FORK_TIME`|`TDB`|Time at which the verkle, overlay tree is activated.|
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think TDB was a typo.

Comment on lines -57 to +55
ckkey[31] = CODE_KECCAK_LEAF_KEY
ckkey[31] = CODEHASH_LEAF_KEY
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align naming to EIP-4762/EIP-6800

Comment on lines +48 to +52
basicdata = bytes(0) # Version
basicdata += bytes(4) # Reserved
basicdata += len(account.code).to_bytes(3, 'big')
basicdata += account.nonce.to_bytes(8, 'big')
basicdata += account.balance.to_bytes(16, 'big')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to do BASIC_DATA packing. This code assumes this PR will be accepted where we switch to big-endian encoding.

```python3
def state_set_codechunk(state: State, addr: Address, chunk_num: int, chunk: Bytes):
state._overlay_tree.set(get_tree_key_for_code_chunk(addr, chunk_num), chunk)
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't used anywhere, so imo we should remove it unless it is used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not referenced in this EIP, but it's referenced in another one which has this one as a dependency, see here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would then put it the referring eip, but I will let the allmighty editors decide on this one. @g11tech ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this code should be part of 6800, but that EIP doesn't have this "Python-style". But yep, let's see what @g11tech says and I can remove/move/add as needed.

@jsign jsign marked this pull request as ready for review July 24, 2024 13:16
@jsign jsign requested a review from eth-bot as a code owner July 24, 2024 13:16
@eth-bot eth-bot enabled auto-merge (squash) July 24, 2024 15:53
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 4545a92 into ethereum:master Jul 24, 2024
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants