-
Notifications
You must be signed in to change notification settings - Fork 124
EIP-7748 tests #1312
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
Draft
jsign
wants to merge
24
commits into
ethereum:verkle/main
Choose a base branch
from
jsign:jsign-eip7748
base: verkle/main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
EIP-7748 tests #1312
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
3f4b674
test
jsign c5205f6
temp: disable system contract addition
jsign c97902a
eip7748: add eoa test cases
jsign 3944076
hack the hack
jsign 88c37f1
eip7748: generalize test
jsign 0bbd4f6
eip7748: fixes and more cases
jsign 08bd4d1
enable problematic test case
jsign 31f350d
fixes
jsign 4a6a8d0
more test coverage and dynamic prepared accounts
jsign 9678dba
add partial conversion test cases
jsign 4f64795
code chunk stride overflow tests
jsign 48056a9
revert hack
jsign ddc9209
hack fix
jsign c4bb35b
conversion: generalize non-partial and add empty code dimension
jsign 240e626
conversion: add test case for empty-code with non-empty storage non-p…
jsign d01e5ad
refactor
jsign 69315cd
stale keys tests
jsign 0134e5c
add modified contract test & refactor
jsign 06f3ba3
add modified EOA test and refactor
jsign c5ccd54
add stride stale contract storage slots
jsign 8cc7f9e
fix
jsign 55af989
cleanup
jsign fd7c0ae
eip-7748: add tests for stale data happening in the same block as the…
jsign 0354ccd
eip7748: cover tx revertions in tx generating stale data accounts
jsign File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -582,7 +582,7 @@ def generate_block_data( | |
) | ||
transition_tool_output.alloc = previous_alloc | ||
# TODO: hack for now, replace with actual witness output once available from t8n | ||
if transition_tool_output.result.verkle_conversion_ended: | ||
if transition_tool_output.result.verkle_conversion_ended and False: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs a proper fix! |
||
witness_parent_root = transition_tool_output.result.parent_state_root | ||
transition_tool_output.witness = Witness( | ||
verkle_proof=transition_tool_output.result.verkle_proof, | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,7 +57,7 @@ def test_balance(blockchain_test: BlockchainTestFiller, fork: Fork, target, warm | |
], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From this point forward, the changes are mostly the new tests so nothing framework related. |
||
) | ||
@pytest.mark.parametrize( | ||
" gas, exp_target_basic_data", | ||
"gas, exp_target_basic_data", | ||
[ | ||
(21_203 + 2099, False), | ||
(21_203 + 2100, True), | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
""" | ||
abstract: Tests [EIP-7748: State conversion to Verkle Tree] | ||
(https://eips.ethereum.org/EIPS/eip-7748) | ||
Tests for [EIP-7748: State conversion to Verkle Tree] | ||
(https://eips.ethereum.org/EIPS/eip-7748). | ||
""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,239 @@ | ||
import pytest | ||
|
||
from typing import Optional | ||
from ethereum_test_tools import BlockchainTestFiller, Transaction, Account, TestAddress | ||
from ethereum_test_tools.vm.opcode import Opcodes as Op | ||
from .utils import stride, _state_conversion, accounts, ConversionTx | ||
|
||
REFERENCE_SPEC_GIT_PATH = "EIPS/eip-7748.md" | ||
REFERENCE_SPEC_VERSION = "TODO" | ||
|
||
|
||
class StaleAccountTx: | ||
""" | ||
Class to represent a transaction that modifies an account to be converted, making it completely/partially stale. | ||
It can be configured to: | ||
- be in the same block as the conversion transaction or in a previous block | ||
- revert or not revert | ||
""" | ||
|
||
def __init__(self, same_block_as_conversion: bool, revert: bool): | ||
self.same_block_as_conversion = same_block_as_conversion | ||
self.revert = revert | ||
|
||
|
||
@pytest.mark.valid_from("EIP6800Transition") | ||
@pytest.mark.parametrize( | ||
"storage_slot_write", | ||
[ | ||
None, | ||
0, | ||
1, | ||
300, | ||
301, | ||
], | ||
ids=[ | ||
"No storage slot modified", | ||
"Stale storage slot in the header", | ||
"New storage slot in the account header", | ||
"Stale storage slot outside the header", | ||
"New storage slot outside the header", | ||
], | ||
) | ||
@pytest.mark.parametrize( | ||
"tx_send_value", | ||
[True, False], | ||
) | ||
@pytest.mark.parametrize( | ||
"tx_stale_account_config", | ||
[ | ||
StaleAccountTx(False, False), | ||
StaleAccountTx(True, False), | ||
StaleAccountTx(True, True), | ||
], | ||
ids=[ | ||
"Tx generating stale account in previous block", | ||
"Tx generating stale account in the same block", | ||
"Reverted tx generating stale account in the same block", | ||
], | ||
) | ||
def test_modified_contract( | ||
blockchain_test: BlockchainTestFiller, | ||
storage_slot_write: int, | ||
tx_send_value: bool, | ||
tx_stale_account_config: StaleAccountTx, | ||
): | ||
""" | ||
Test converting a modified contract where a previous transaction writes to: | ||
- Existing storage slots (i.e., storage slots that must not be converted (stale)) | ||
- New storage slots (i.e., storage slots that must not be converted (not overriden with zeros)) | ||
- Basic data (i.e., balance/nonce which must not be converted (stale)) | ||
The conversion transaction can be in the same block or previous block as the conversion. | ||
""" | ||
_convert_modified_account( | ||
blockchain_test, | ||
tx_send_value, | ||
ContractSetup(storage_slot_write), | ||
tx_stale_account_config, | ||
) | ||
|
||
|
||
@pytest.mark.valid_from("EIP6800Transition") | ||
@pytest.mark.parametrize( | ||
"tx_stale_account_config", | ||
[ | ||
StaleAccountTx(False, False), | ||
StaleAccountTx(True, False), | ||
], | ||
ids=[ | ||
"Tx generating stale account in previous block", | ||
"Tx generating stale account in the same block", | ||
], | ||
) | ||
def test_modified_eoa( | ||
blockchain_test: BlockchainTestFiller, tx_stale_account_config: StaleAccountTx | ||
): | ||
""" | ||
Test converting a modified EOA in the same block or previous block as the conversion. | ||
""" | ||
_convert_modified_account(blockchain_test, True, None, tx_stale_account_config) | ||
|
||
|
||
class ContractSetup: | ||
def __init__(self, storage_slot_write: Optional[int]): | ||
self.storage_slot_write = storage_slot_write | ||
|
||
|
||
def _convert_modified_account( | ||
blockchain_test: BlockchainTestFiller, | ||
tx_send_value: bool, | ||
contract_setup: Optional[ContractSetup], | ||
tx_stale_account_config: StaleAccountTx, | ||
): | ||
pre_state = {} | ||
pre_state[TestAddress] = Account(balance=1000000000000000000000) | ||
|
||
expected_conversion_blocks = 1 | ||
accounts_idx = 0 | ||
if not tx_stale_account_config.same_block_as_conversion: | ||
expected_conversion_blocks = 2 | ||
# TODO(hack): today the testing-framework does not support us signaling that we want to | ||
# put the `ConversionTx(tx, **0**)` at the first block after genesis. To simulate that, we have | ||
# to do this here so we "shift" the target account to the second block in the fork. | ||
# If this is ever supported, remove this. | ||
for i in range(stride): | ||
pre_state[accounts[accounts_idx]] = Account(balance=100 + 1000 * i) | ||
accounts_idx += 1 | ||
|
||
target_account = accounts[accounts_idx] | ||
if contract_setup is not None: | ||
pre_state[target_account] = Account( | ||
balance=1_000, | ||
nonce=0, | ||
code=( | ||
( | ||
Op.SSTORE(contract_setup.storage_slot_write, 9999) | ||
if contract_setup.storage_slot_write is not None | ||
else Op.RETURN | ||
) | ||
+ Op.REVERT | ||
if tx_stale_account_config.revert | ||
else Op.RETURN | ||
), | ||
storage={0: 100, 300: 200}, | ||
) | ||
else: | ||
if tx_stale_account_config.revert: | ||
raise Exception("Invalid test case -- EOA transfers can't revert") | ||
pre_state[target_account] = Account(balance=10_000, nonce=0) | ||
|
||
tx = Transaction( | ||
ty=0x0, | ||
chain_id=0x01, | ||
to=target_account, | ||
value=500 if tx_send_value else 0, | ||
gas_limit=100_000, | ||
gas_price=10, | ||
) | ||
|
||
_state_conversion( | ||
blockchain_test, | ||
pre_state, | ||
stride, | ||
expected_conversion_blocks, | ||
[ConversionTx(tx, 0)], | ||
) | ||
|
||
|
||
@pytest.mark.valid_from("EIP6800Transition") | ||
def test_modified_eoa_conversion_units(blockchain_test: BlockchainTestFiller): | ||
""" | ||
Test stale EOA are properly counted as used conversion units. | ||
""" | ||
pre_state = {} | ||
pre_state[TestAddress] = Account(balance=1000000000000000000000) | ||
|
||
# TODO(hack): today the testing-framework does not support us signaling that we want to | ||
# put the `ConversionTx(tx, **0**)` at the first block after genesis. To simulate that, we have | ||
# to do this here so we "shift" the target account to the second block in the fork. | ||
# If this is ever supported, remove this. | ||
for i in range(stride): | ||
pre_state[accounts[i]] = Account(balance=100 + 1000 * i) | ||
|
||
txs = [] | ||
# Add stride+3 extra EOAs, and invalidate the first stride ones. This is to check that | ||
# the conversion units are properly counted, and the last 3 aren't converted in that block. | ||
for i in range(stride + 3): | ||
pre_state[accounts[stride + i]] = Account(balance=1_000, nonce=0) | ||
if i < stride: | ||
tx = Transaction( | ||
ty=0x0, | ||
chain_id=0x01, | ||
nonce=i, | ||
to=accounts[stride + i], | ||
value=100, | ||
gas_limit=100_000, | ||
gas_price=10, | ||
) | ||
txs.append(ConversionTx(tx, 0)) | ||
|
||
_state_conversion(blockchain_test, pre_state, stride, 3, txs) | ||
|
||
|
||
@pytest.mark.valid_from("EIP6800Transition") | ||
def test_modified_contract_conversion_units(blockchain_test: BlockchainTestFiller): | ||
""" | ||
Test stale contract storage slots are properly counted as used conversion units. | ||
""" | ||
pre_state = {} | ||
pre_state[TestAddress] = Account(balance=1000000000000000000000) | ||
|
||
# TODO(hack): today the testing-framework does not support us signaling that we want to | ||
# put the `ConversionTx(tx, **0**)` at the first block after genesis. To simulate that, we have | ||
# to do this here so we "shift" the target account to the second block in the fork. | ||
# If this is ever supported, remove this. | ||
for i in range(stride): | ||
pre_state[accounts[i]] = Account(balance=100 + 1000 * i) | ||
|
||
target_account = accounts[stride] | ||
pre_state[target_account] = Account( | ||
balance=1_000, | ||
nonce=0, | ||
code=sum([Op.SSTORE(ss, 10_000 + i) for i, ss in enumerate(range(stride))]), | ||
storage={ss: 100 + i for i, ss in enumerate(range(stride))}, | ||
) | ||
for i in range(3): | ||
pre_state[accounts[stride + 1 + i]] = Account(balance=1_000 + i, nonce=0) | ||
|
||
# Send tx that writes all existing storage slots. | ||
tx = Transaction( | ||
ty=0x0, | ||
chain_id=0x01, | ||
nonce=0, | ||
to=target_account, | ||
value=100, | ||
gas_limit=100_000, | ||
gas_price=10, | ||
) | ||
|
||
_state_conversion(blockchain_test, pre_state, stride, 3, [ConversionTx(tx, 0)]) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@spencer-tb, this is a quick hack to disable the automatic inclusion of the EIP-2935 system contract. Ideally, I want this feature to avoid having this extra address in the genesis state since we want full predictability on which accounts exist for the tree conversion tests.
I think once we talked about an extra flag to do this more cleanly.