From ba7a1998870bc2bf5221fce01fa97e8f35eaaf7c Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 3 Apr 2023 13:10:09 -0600 Subject: [PATCH 1/6] fillers: 4844 - more invalid tx tests --- fillers/eips/eip4844/excess_data_gas.py | 166 ++++++++++++++++-------- 1 file changed, 110 insertions(+), 56 deletions(-) diff --git a/fillers/eips/eip4844/excess_data_gas.py b/fillers/eips/eip4844/excess_data_gas.py index d48ff1795b1..a86078964f8 100644 --- a/fillers/eips/eip4844/excess_data_gas.py +++ b/fillers/eips/eip4844/excess_data_gas.py @@ -5,7 +5,13 @@ from dataclasses import dataclass from typing import List, Mapping, Optional -from ethereum_test_forks import Fork, ShardingFork +from ethereum_test_forks import ( + Fork, + Shanghai, + ShanghaiToShardingForkAtTime15k, + ShardingFork, + is_fork, +) from ethereum_test_tools import ( Account, Block, @@ -15,6 +21,7 @@ TestAddress, Transaction, test_from, + test_only, to_address, to_hash_bytes, ) @@ -426,12 +433,33 @@ def test_invalid_excess_data_gas_in_header(_: Fork): yield tc.generate() -def ignore_test_fork_transition_excess_data_gas_in_header(_: Fork): +@test_only(fork=ShanghaiToShardingForkAtTime15k) +def test_fork_transition_excess_data_gas_in_header(_: Fork): """ Test excess_data_gas calculation in the header when the fork is activated. """ - # TODO! - pass + env = Environment() + + # Generate some blocks to reach Sharding fork + FORK_TIMESTAMP = 15_000 + blocks: List[Block] = [] + for t in range(999, FORK_TIMESTAMP, 1_000): + blocks.append(Block(timestamp=t)) + + pre = { + TestAddress: Account(balance=10**40), + } + post: Mapping[str, Account] = {} + + # Test N blocks until excess data gas after fork reaches data gas cost > 1 + + yield BlockchainTest( + pre=pre, + post=post, + blocks=blocks, + genesis_environment=env, + tag="correct_initial_data_gas_calc", + ) @dataclass(kw_only=True) @@ -446,10 +474,10 @@ class InvalidBlobTransactionTestCase: """ tag: str - parent_excess_blobs: int blobs_per_tx: int tx_error: str tx_count: int = 1 + parent_excess_blobs: Optional[int] = None tx_max_data_gas_cost: Optional[int] = None account_balance_modifier: int = 0 block_base_fee: int = 7 @@ -458,14 +486,18 @@ def generate(self) -> BlockchainTest: """ Generate the test case. """ - parent_excess_data_gas = self.parent_excess_blobs * DATA_GAS_PER_BLOB - env = Environment(excess_data_gas=parent_excess_data_gas) - + env = Environment() + data_gasprice = 1 destination_account = to_address(0x100) - data_gasprice = get_data_gasprice( - excess_data_gas=parent_excess_data_gas - ) + if self.parent_excess_blobs is not None: + parent_excess_data_gas = ( + self.parent_excess_blobs * DATA_GAS_PER_BLOB + ) + env = Environment(excess_data_gas=parent_excess_data_gas) + data_gasprice = get_data_gasprice( + excess_data_gas=parent_excess_data_gas + ) total_account_minimum_balance = 0 @@ -522,8 +554,8 @@ def generate(self) -> BlockchainTest: ) -@test_from(fork=ShardingFork) -def test_invalid_blob_txs(_: Fork): +@test_from(fork=Shanghai) +def test_invalid_blob_txs(fork: Fork): """ Reject blocks with invalid blob txs due to: - The user cannot afford the data gas specified (but max_fee_per_gas @@ -534,49 +566,71 @@ def test_invalid_blob_txs(_: Fork): - blob count > MAX_BLOBS_PER_BLOCK in type 5 transaction - block blob count > MAX_BLOBS_PER_BLOCK """ - test_cases: List[InvalidBlobTransactionTestCase] = [ - InvalidBlobTransactionTestCase( - tag="insufficient_max_fee_per_data_gas", - parent_excess_blobs=15, # data gas cost = 2 - tx_max_data_gas_cost=1, # less than minimum - tx_error="insufficient max fee per data gas", - blobs_per_tx=1, - ), - InvalidBlobTransactionTestCase( - tag="insufficient_balance_sufficient_fee", - parent_excess_blobs=15, # data gas cost = 1 - tx_max_data_gas_cost=100, # > data gas cost - account_balance_modifier=-1, - tx_error="insufficient max fee per data gas", - blobs_per_tx=1, - ), - InvalidBlobTransactionTestCase( - tag="zero_max_fee_per_data_gas", - parent_excess_blobs=0, # data gas cost = 1 - tx_max_data_gas_cost=0, # invalid value - tx_error="insufficient max fee per data gas", - blobs_per_tx=1, - ), - InvalidBlobTransactionTestCase( - tag="blob_overflow", - parent_excess_blobs=10, # data gas cost = 1 - tx_error="too_many_blobs", - blobs_per_tx=5, - ), - InvalidBlobTransactionTestCase( - tag="multi_tx_blob_overflow", - parent_excess_blobs=10, # data gas cost = 1 - tx_error="too_many_blobs", - tx_count=5, - blobs_per_tx=1, - ), - # InvalidBlobTransactionTestCase( - # tag="blob_underflow", - # parent_excess_blobs=10, # data gas cost = 1 - # tx_error="too_few_blobs", - # blobs_per_tx=0, - # ), - ] + test_cases: List[InvalidBlobTransactionTestCase] = [] + if is_fork(fork, ShardingFork): + test_cases = [ + InvalidBlobTransactionTestCase( + tag="insufficient_max_fee_per_data_gas", + parent_excess_blobs=15, # data gas cost = 2 + tx_max_data_gas_cost=1, # less tha than minimum + tx_error="insufficient max fee per data gas", + blobs_per_tx=1, + ), + InvalidBlobTransactionTestCase( + tag="insufficient_balance_sufficient_fee", + parent_excess_blobs=15, # data gas cost = 1 + tx_max_data_gas_cost=100, # > data gas cost + account_balance_modifier=-1, + tx_error="insufficient max fee per data gas", + blobs_per_tx=1, + ), + InvalidBlobTransactionTestCase( + tag="zero_max_fee_per_data_gas", + parent_excess_blobs=0, # data gas cost = 1 + tx_max_data_gas_cost=0, # invalid value + tx_error="insufficient max fee per data gas", + blobs_per_tx=1, + ), + InvalidBlobTransactionTestCase( + tag="blob_overflow", + parent_excess_blobs=10, # data gas cost = 1 + tx_error="too_many_blobs", + blobs_per_tx=5, + ), + InvalidBlobTransactionTestCase( + tag="multi_tx_blob_overflow", + parent_excess_blobs=10, # data gas cost = 1 + tx_error="too_many_blobs", + tx_count=5, + blobs_per_tx=1, + ), + # TODO: Enable, at the time of writing this test case, the EIP does not + # specify a minimum blob amount for the type 5 transaction. + # InvalidBlobTransactionTestCase( + # tag="blob_underflow", + # parent_excess_blobs=10, # data gas cost = 1 + # tx_error="too_few_blobs", + # blobs_per_tx=0, + # ), + ] + else: + # Pre-Sharding, blocks with type 5 txs must be rejected + test_cases = [ + InvalidBlobTransactionTestCase( + tag="type_5_tx_pre_fork", + parent_excess_blobs=None, + tx_max_data_gas_cost=1, + tx_error="tx_type_5_not_allowed_yet", + blobs_per_tx=1, + ), + InvalidBlobTransactionTestCase( + tag="empty_type_5_tx_pre_fork", + parent_excess_blobs=None, + tx_max_data_gas_cost=1, + tx_error="tx_type_5_not_allowed_yet", + blobs_per_tx=0, + ), + ] for tc in test_cases: yield tc.generate() From 59d48c4ab121655fc30afdf294dd576dca2e76b1 Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 3 Apr 2023 16:37:30 -0500 Subject: [PATCH 2/6] forks: fix shanghai->sharding transition fork name --- fillers/eips/eip4844/excess_data_gas.py | 4 ++-- src/ethereum_test_forks/__init__.py | 11 ++++++++--- src/ethereum_test_forks/forks/transition.py | 13 +++++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/fillers/eips/eip4844/excess_data_gas.py b/fillers/eips/eip4844/excess_data_gas.py index a86078964f8..ef2e89cb0c4 100644 --- a/fillers/eips/eip4844/excess_data_gas.py +++ b/fillers/eips/eip4844/excess_data_gas.py @@ -8,7 +8,7 @@ from ethereum_test_forks import ( Fork, Shanghai, - ShanghaiToShardingForkAtTime15k, + ShanghaiToShardingAtTime15k, ShardingFork, is_fork, ) @@ -433,7 +433,7 @@ def test_invalid_excess_data_gas_in_header(_: Fork): yield tc.generate() -@test_only(fork=ShanghaiToShardingForkAtTime15k) +@test_only(fork=ShanghaiToShardingAtTime15k) def test_fork_transition_excess_data_gas_in_header(_: Fork): """ Test excess_data_gas calculation in the header when the fork is activated. diff --git a/src/ethereum_test_forks/__init__.py b/src/ethereum_test_forks/__init__.py index 7d57f6a56a7..ca2e77691b3 100644 --- a/src/ethereum_test_forks/__init__.py +++ b/src/ethereum_test_forks/__init__.py @@ -18,7 +18,11 @@ MuirGlacier, Shanghai, ) -from .forks.transition import BerlinToLondonAt5, MergeToShanghaiAtTime15k +from .forks.transition import ( + BerlinToLondonAt5, + MergeToShanghaiAtTime15k, + ShanghaiToShardingAtTime15k, +) from .forks.upcoming import ShardingFork from .helpers import ( fork_only, @@ -33,6 +37,7 @@ "Fork", "ArrowGlacier", "Berlin", + "BerlinToLondonAt5", "Byzantium", "Constantinople", "ConstantinopleFix", @@ -42,10 +47,10 @@ "Istanbul", "London", "Merge", + "MergeToShanghaiAtTime15k", "MuirGlacier", "Shanghai", - "BerlinToLondonAt5", - "MergeToShanghaiAtTime15k", + "ShanghaiToShardingAtTime15k", "ShardingFork", "fork_only", "forks_from", diff --git a/src/ethereum_test_forks/forks/transition.py b/src/ethereum_test_forks/forks/transition.py index 009d4789918..6223057e83e 100644 --- a/src/ethereum_test_forks/forks/transition.py +++ b/src/ethereum_test_forks/forks/transition.py @@ -35,6 +35,19 @@ def header_withdrawals_required(cls, _: int, timestamp: int) -> bool: return timestamp >= 15_000 +class ShanghaiToShardingAtTime15k(Shanghai): + """ + Shanghai to Sharding transition at Timestamp 15k + """ + + @classmethod + def header_excess_data_gas_required(cls, _: int, timestamp: int) -> bool: + """ + Excess data gas is required if transitioning to Sharding. + """ + return timestamp >= 15_000 + + # Test-only transition forks From 9123852c8efaf4ef0abdcf5287341ff970ae6e70 Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 3 Apr 2023 16:38:14 -0500 Subject: [PATCH 3/6] fillers: 4844: add fork transition tests --- fillers/eips/eip4844/excess_data_gas.py | 58 ++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/fillers/eips/eip4844/excess_data_gas.py b/fillers/eips/eip4844/excess_data_gas.py index ef2e89cb0c4..56e37585a68 100644 --- a/fillers/eips/eip4844/excess_data_gas.py +++ b/fillers/eips/eip4844/excess_data_gas.py @@ -439,6 +439,10 @@ def test_fork_transition_excess_data_gas_in_header(_: Fork): Test excess_data_gas calculation in the header when the fork is activated. """ env = Environment() + pre = { + TestAddress: Account(balance=10**40), + } + dest = to_address(0x100) # Generate some blocks to reach Sharding fork FORK_TIMESTAMP = 15_000 @@ -446,12 +450,51 @@ def test_fork_transition_excess_data_gas_in_header(_: Fork): for t in range(999, FORK_TIMESTAMP, 1_000): blocks.append(Block(timestamp=t)) - pre = { - TestAddress: Account(balance=10**40), - } - post: Mapping[str, Account] = {} - # Test N blocks until excess data gas after fork reaches data gas cost > 1 + BLOBS_TO_DATA_GAS_COST_INCREASE = 12 + assert get_data_gasprice_from_blobs( + BLOBS_TO_DATA_GAS_COST_INCREASE - 1 + ) != get_data_gasprice_from_blobs(BLOBS_TO_DATA_GAS_COST_INCREASE) + + parent_excess_data_gas = 0 + dest_value = 0 + for i in range( + BLOBS_TO_DATA_GAS_COST_INCREASE + // (MAX_BLOBS_PER_BLOCK - TARGET_BLOBS_PER_BLOCK) + + 1 + ): + blocks.append( + Block( + txs=[ + Transaction( + ty=5, + nonce=i, + to=dest, + value=1, + gas_limit=3000000, + max_fee_per_gas=1000000, + max_priority_fee_per_gas=10, + max_fee_per_data_gas=get_data_gasprice( + excess_data_gas=parent_excess_data_gas + ), + access_list=[], + blob_versioned_hashes=[ + to_hash_bytes(x) + for x in range(MAX_BLOBS_PER_BLOCK) + ], + ) + ], + ) + ) + dest_value += 1 + parent_excess_data_gas = calc_excess_data_gas( + parent_excess_data_gas, + MAX_BLOBS_PER_BLOCK, + ) + + post: Mapping[str, Account] = { + dest: Account(balance=dest_value), + } yield BlockchainTest( pre=pre, @@ -604,8 +647,9 @@ def test_invalid_blob_txs(fork: Fork): tx_count=5, blobs_per_tx=1, ), - # TODO: Enable, at the time of writing this test case, the EIP does not - # specify a minimum blob amount for the type 5 transaction. + # TODO: Enable, at the time of writing this test case, the EIP + # does not specify a minimum blob amount for the type 5 + # transaction. # InvalidBlobTransactionTestCase( # tag="blob_underflow", # parent_excess_blobs=10, # data gas cost = 1 From 278645c24967083c6179eb9154dd49e7c1665633 Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 3 Apr 2023 17:20:06 -0500 Subject: [PATCH 4/6] fillers: 4844: incomplete list of tox fixes --- fillers/eips/eip4844/excess_data_gas.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fillers/eips/eip4844/excess_data_gas.py b/fillers/eips/eip4844/excess_data_gas.py index 56e37585a68..9657247a00f 100644 --- a/fillers/eips/eip4844/excess_data_gas.py +++ b/fillers/eips/eip4844/excess_data_gas.py @@ -442,7 +442,7 @@ def test_fork_transition_excess_data_gas_in_header(_: Fork): pre = { TestAddress: Account(balance=10**40), } - dest = to_address(0x100) + destination_account = to_address(0x100) # Generate some blocks to reach Sharding fork FORK_TIMESTAMP = 15_000 @@ -457,7 +457,7 @@ def test_fork_transition_excess_data_gas_in_header(_: Fork): ) != get_data_gasprice_from_blobs(BLOBS_TO_DATA_GAS_COST_INCREASE) parent_excess_data_gas = 0 - dest_value = 0 + destination_account_value = 0 for i in range( BLOBS_TO_DATA_GAS_COST_INCREASE // (MAX_BLOBS_PER_BLOCK - TARGET_BLOBS_PER_BLOCK) @@ -469,7 +469,7 @@ def test_fork_transition_excess_data_gas_in_header(_: Fork): Transaction( ty=5, nonce=i, - to=dest, + to=destination_account, value=1, gas_limit=3000000, max_fee_per_gas=1000000, @@ -486,14 +486,14 @@ def test_fork_transition_excess_data_gas_in_header(_: Fork): ], ) ) - dest_value += 1 + destination_account_value += 1 parent_excess_data_gas = calc_excess_data_gas( parent_excess_data_gas, MAX_BLOBS_PER_BLOCK, ) post: Mapping[str, Account] = { - dest: Account(balance=dest_value), + destination_account: Account(balance=destination_account_value), } yield BlockchainTest( @@ -615,7 +615,7 @@ def test_invalid_blob_txs(fork: Fork): InvalidBlobTransactionTestCase( tag="insufficient_max_fee_per_data_gas", parent_excess_blobs=15, # data gas cost = 2 - tx_max_data_gas_cost=1, # less tha than minimum + tx_max_data_gas_cost=1, # less than minimum tx_error="insufficient max fee per data gas", blobs_per_tx=1, ), From 78a2818b651c2aa9bb7c62c4d744b4df0d802a87 Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 17 Apr 2023 17:51:20 -0500 Subject: [PATCH 5/6] tools/types: fix environment setup on excess data --- src/ethereum_test_tools/common/types.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ethereum_test_tools/common/types.py b/src/ethereum_test_tools/common/types.py index add4be9c194..aae0264176d 100644 --- a/src/ethereum_test_tools/common/types.py +++ b/src/ethereum_test_tools/common/types.py @@ -588,7 +588,10 @@ def set_fork_requirements(self, fork: Fork) -> "Environment": if fork.header_zero_difficulty_required(self.number, self.timestamp): res.difficulty = 0 - if fork.header_excess_data_gas_required(self.number, self.timestamp): + if ( + fork.header_excess_data_gas_required(self.number, self.timestamp) + and res.excess_data_gas is None + ): res.excess_data_gas = 0 return res From 4ac55151bde0ebef425a662face8ea07638ad812 Mon Sep 17 00:00:00 2001 From: marioevz Date: Mon, 17 Apr 2023 18:06:37 -0500 Subject: [PATCH 6/6] all: fix transition fork naming --- src/ethereum_test_forks/base_fork.py | 7 +++++++ src/ethereum_test_forks/forks/transition.py | 3 ++- src/ethereum_test_forks/tests/test_forks.py | 5 ++++- src/ethereum_test_forks/transition_base_fork.py | 9 ++++++++- src/ethereum_test_tools/filling/fill.py | 6 +++--- src/evm_transition_tool/__init__.py | 2 +- 6 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/ethereum_test_forks/base_fork.py b/src/ethereum_test_forks/base_fork.py index ea2575c624a..82136d5d4e5 100644 --- a/src/ethereum_test_forks/base_fork.py +++ b/src/ethereum_test_forks/base_fork.py @@ -70,6 +70,13 @@ def get_reward(cls, block_number: int, timestamp: int) -> int: """ pass + @classmethod + def name(cls) -> str: + """ + Returns the name of the fork. + """ + return cls.__name__ + # Fork Type Fork = Type[BaseFork] diff --git a/src/ethereum_test_forks/forks/transition.py b/src/ethereum_test_forks/forks/transition.py index 6223057e83e..abd27ee1b23 100644 --- a/src/ethereum_test_forks/forks/transition.py +++ b/src/ethereum_test_forks/forks/transition.py @@ -3,7 +3,7 @@ """ from ..transition_base_fork import transition_fork from .forks import Berlin, London, Merge, Shanghai -from .upcoming import TestOnlyUpcomingFork +from .upcoming import ShardingFork, TestOnlyUpcomingFork # Transition Forks @@ -35,6 +35,7 @@ def header_withdrawals_required(cls, _: int, timestamp: int) -> bool: return timestamp >= 15_000 +@transition_fork(to_fork=ShardingFork) class ShanghaiToShardingAtTime15k(Shanghai): """ Shanghai to Sharding transition at Timestamp 15k diff --git a/src/ethereum_test_forks/tests/test_forks.py b/src/ethereum_test_forks/tests/test_forks.py index de12a417c7e..f99feca9927 100644 --- a/src/ethereum_test_forks/tests/test_forks.py +++ b/src/ethereum_test_forks/tests/test_forks.py @@ -34,7 +34,10 @@ def test_forks(): ] assert forks_from(Merge) == [Merge, Shanghai] - assert London.__name__ == "London" + # Test fork names + assert London.name() == "London" + assert TestOnlyUpcomingFork.name() == "TestOnlyUpcomingFork" + assert MergeToShanghaiAtTime15k.name() == "MergeToShanghaiAtTime15k" # Test some fork properties assert Berlin.header_base_fee_required(0, 0) is False diff --git a/src/ethereum_test_forks/transition_base_fork.py b/src/ethereum_test_forks/transition_base_fork.py index f53c4e39dbf..a5f812b73fb 100644 --- a/src/ethereum_test_forks/transition_base_fork.py +++ b/src/ethereum_test_forks/transition_base_fork.py @@ -25,8 +25,15 @@ def transition_fork(to_fork: Fork): """ def decorator(cls) -> Type[TransitionBaseClass]: + transition_name = cls.__name__ + class NewTransitionClass(cls, TransitionBaseClass): # type: ignore - pass + @classmethod + def name(cls) -> str: + """ + Returns the name of the transition fork. + """ + return transition_name NewTransitionClass.transitions_to = lambda: to_fork # type: ignore diff --git a/src/ethereum_test_tools/filling/fill.py b/src/ethereum_test_tools/filling/fill.py index 6cd35466cd1..621e46f53ea 100644 --- a/src/ethereum_test_tools/filling/fill.py +++ b/src/ethereum_test_tools/filling/fill.py @@ -55,15 +55,15 @@ def fill_test( name_tag = f"{name} {test.tag}" if test.tag else name print(f"Exception during test '{name_tag}'") raise e - + fork_name = fork.name() fixture = Fixture( blocks=blocks, genesis=genesis, genesis_rlp=genesis_rlp, head=head, - fork="+".join([fork.__name__] + [str(eip) for eip in eips]) + fork="+".join([fork_name] + [str(eip) for eip in eips]) if eips is not None - else fork.__name__, + else fork_name, pre_state=copy(test.pre), post_state=alloc_to_accounts(alloc), seal_engine=engine, diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index 0705af8c1a2..6d3a8a7d9bd 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -180,7 +180,7 @@ def evaluate( """ Executes `evm t8n` with the specified arguments. """ - fork_name = fork.__name__ + fork_name = fork.name() if eips is not None: fork_name = "+".join([fork_name] + [str(eip) for eip in eips])