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

[WIP] feat(fw): verkle t8n post state verification #485

Conversation

spencer-tb
Copy link
Collaborator

@spencer-tb spencer-tb commented Mar 25, 2024

🗒️ Description

Creating a draft PR here for visibility and discussion.

This implements a method for checking the post state of the verkle tree after t8n execution.

Currently it calls an additonal subcommand: evm t8n verkle, responsible for generating the verkle tree keys for account addresses and storage slots. These can be used for each account to generate an expected verkle post state using our existing post alloc, and compared against the actual vkt from t8n execution.


For example, once we have the account address key:

evm t8n verkle 0x0000000000000000000000000000000000000100
> 0x31b6...6300   # address

We can create the account balance, nonce and code hash keys, by adding 0x01, 0x02 and 0x03 respectively to get their keys for the vkt:

> 0x31b6...6301  # balance
> 0x31b6...6302  # nonce
> 0x31b6...6303  # code hash

For account storage keys we simply add the storage slot to the sub-command:

evm t8n verkle 0x0000000000000000000000000000000000000100 0x01
> 0x31b6...6341   # storage slot 1

Using these keys we can create our own vkt representation, adding there appropriate values from the post alloc. Note the address value is Hash32(0). Numerical values are in little endian (balance/nonce). The code hash value is the keccak256 of the account code bytes.


Currently this method is not ideal - but it does work. As we call the subcommand around 260 times it takes around 120s to fill a single test. One proposed solution is update the subcommand, passing the entire post alloc to the subcommand - expecting an entire vkt conversion back. More comments are below.

Usage

Use the following geth branch: gballet/t8n-merge-from-shanghai-to-prague

Run with:

fill --from Prague --until Prague --t8n-dump-dir=./tmp -k test_verkle_from_mpt_conversion[fork_ShanghaiToPragueVerkleTransition-blockchain_test]

Check the output folders with vkt.json for any debugging.

Note it takes around 2 mins to fill the test.

🔗 Related Issues

N/A

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

Comment on lines +87 to +88
# TODO: how to determine what is unexpected, i.e TestAddress is expected but not in post state
unexpected_keys = [key for key in got_vkt if key not in expected_vkt]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does not check for unexpected keys within the vkt. For example, as we don't specify the TestAddress account in the post state (and another account added during the test) we don't have a way of determining whether those additions within the tree are unexpected even though they are expected. This is something we don't check currently, so its a questions of if we need to be this granular when checking the post state vkt

Whatever option we choose to finalize with, we need to determine how we check for unexpected keys in the vkt. The previous solution involved using Account.NON_EXISTENT: https://github.com/ethereum/execution-spec-tests/blob/main/src/ethereum_test_tools/spec/base/base_test.py#L61-L63

Can we do something similar here?

@spencer-tb
Copy link
Collaborator Author

spencer-tb commented Mar 25, 2024

The question now that pops up - how do we best optimize this?

  1. We update the evm t8n verkle subcommand to take in our post allocation as an input, where geth creates and returns our expected verkle tree instead of us creating it manually. This would solve the runtime issue. We'd only call the subcommand once.

  2. We do 1), but entirely within t8n and not an additional subcommand. Essentially we'd provide additional optional flags --input.postAlloc and --output.postVkt. We'd provide the input post alloc on our last t8n call. T8n would do the same conversion above and provide it as an output.

  3. Can t8n provide alloc alongside the vkt during each transition? If so, why don't we use the alloc here. Maybe it could be prone to errors as we are not using the vkt for comparison?

If the long term plan is to use the pedersen hash in python within EEST, then the current implementation style would be the way forward. Although, 1) or 2) using the latter in EELS seems more optimal.


After more discussion, option 1) seems to be the viable option. This is implemented now in the same geth branch linked above.

@spencer-tb
Copy link
Collaborator Author

To allow for checking specific erroneous key/values in the post vkt on our end, essentially using our post state alloc expectation (the one we will now pass to the evm t8n verkle sub-command) we also maintain a mapping to what keys each item has within the vkt representation. So now the subcommand will take our expected postAlloc.json as input, and output 2 files:

    1. the verkle representation of the alloc: postVkt.json
    1. a mapping.json which maps the vkt keys to each account item in the tree

The potential mismatch verification flow on our end could now be:

    1. there is a mismatch between the value for the vkt key 0x31b6...6347 when comparing the expected and actual VKTs,
    1. we find the item this key is mapped to in mapping.json,
    1. we determine that the value mismatch is from storage slot 0x0...04 for account 0x0...100. This would give us the same behaviour we have currently (pre-Verkle).

@spencer-tb spencer-tb changed the title feat(fw): verkle t8n post state verification [WIP] feat(fw): verkle t8n post state verification Apr 12, 2024
@spencer-tb
Copy link
Collaborator Author

CIFO: #507

All relavent changes are included there. Most code is however removed, as we are going to verify the post state vkt slightly differently.

@spencer-tb spencer-tb closed this Apr 12, 2024
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.

None yet

1 participant