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

Fix Invalid Transaction Behavior #4

Merged
merged 9 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ The Blockchain tests span multiple blocks which can contain or not transactions,

- Verify system-level operations such as coinbase balance updates or withdrawals
- Verify fork transitions
- Verify blocks with invalid transactions/properties are rejected

### Adding a New Test

Expand Down Expand Up @@ -231,6 +232,19 @@ check that test is effective. E.g. when a transaction is supposed to fail, it
is necessary to check that the failure error is actually the one expected by
the test.

### Failing or invalid transactions

Transactions included in a StateTest are expected to be intrinsically valid,
i.e. the account sending the transaction must have enough funds to cover the
gas costs, the max fee of the transaction must be equal or higher than the
base fee of the block, etc.

An intrinsically valid transaction can still revert during its execution.

Blocks in a BlockchainTest can contain intrinsically invalid transactions but
in this case the block is expected to be completely rejected, along with all
transactions in it, including other valid transactions.

[t8n]: https://github.com/ethereum/go-ethereum/tree/master/cmd/evm
[b11r]: https://github.com/ethereum/go-ethereum/pull/23843
[yul]: https://docs.soliditylang.org/en/latest/yul.html
14 changes: 10 additions & 4 deletions src/ethereum_test/blockchain_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,14 @@ def make_block(
txs_rlp = txs_rlp_file.read().decode().strip('"')

rejected_txs = verify_transactions(block.txs, result)
if len(rejected_txs) > 0:
# TODO: This block is invalid because it contains intrinsically
# invalid transactions
pass
if len(rejected_txs) > 0 and block.exception is None:
raise Exception(
"one or more transactions in `BlockchainTest` are "
+ "intrinsically invalid, but the block was not expected "
+ "to be invalid. Please verify whether the transaction "
+ "was indeed expected to fail and add the proper "
+ "`block.exception`"
)

header = FixtureHeader.from_dict(
result
Expand Down Expand Up @@ -186,6 +190,7 @@ def make_block(
FixtureBlock(
rlp=rlp,
block_header=header,
block_number=header.number,
),
env.apply_new_parent(header),
next_alloc,
Expand All @@ -196,6 +201,7 @@ def make_block(
FixtureBlock(
rlp=rlp,
expected_exception=block.exception,
block_number=header.number,
),
previous_env,
previous_alloc,
Expand Down
36 changes: 24 additions & 12 deletions src/ethereum_test/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,22 @@
def test_from_until(
fork_from: str,
fork_until: str,
) -> Callable[[StateTestSpec], Callable[[str], Mapping[str, Fixture]]]:
) -> Callable[
[StateTestSpec], Callable[[Any, Any, str], Mapping[str, Fixture]]
]:
"""
Decorator that takes a test generator and fills it for all forks after the
specified fork.
"""
fork_from = fork_from.capitalize()
fork_until = fork_until.capitalize()

def decorator(fn: StateTestSpec) -> Callable[[str], Mapping[str, Fixture]]:
def inner(engine) -> Mapping[str, Fixture]:
def decorator(
fn: StateTestSpec,
) -> Callable[[Any, Any, str], Mapping[str, Fixture]]:
def inner(t8n, b11r, engine) -> Mapping[str, Fixture]:
return fill_test(
fn, forks_from_until(fork_from, fork_until), engine
t8n, b11r, fn, forks_from_until(fork_from, fork_until), engine
)

cast(Any, inner).__filler_metadata__ = {
Expand All @@ -38,16 +42,20 @@ def inner(engine) -> Mapping[str, Fixture]:

def test_from(
fork: str,
) -> Callable[[StateTestSpec], Callable[[str], Mapping[str, Fixture]]]:
) -> Callable[
[StateTestSpec], Callable[[Any, Any, str], Mapping[str, Fixture]]
]:
"""
Decorator that takes a test generator and fills it for all forks after the
specified fork.
"""
fork = fork.capitalize()

def decorator(fn: StateTestSpec) -> Callable[[str], Mapping[str, Fixture]]:
def inner(engine) -> Mapping[str, Fixture]:
return fill_test(fn, forks_from(fork), engine)
def decorator(
fn: StateTestSpec,
) -> Callable[[Any, Any, str], Mapping[str, Fixture]]:
def inner(t8n, b11r, engine) -> Mapping[str, Fixture]:
return fill_test(t8n, b11r, fn, forks_from(fork), engine)

cast(Any, inner).__filler_metadata__ = {
"fork": fork,
Expand All @@ -61,16 +69,20 @@ def inner(engine) -> Mapping[str, Fixture]:

def test_only(
fork: str,
) -> Callable[[StateTestSpec], Callable[[str], Mapping[str, Fixture]]]:
) -> Callable[
[StateTestSpec], Callable[[Any, Any, str], Mapping[str, Fixture]]
]:
"""
Decorator that takes a test generator and fills it only for the specified
fork.
"""
fork = fork.capitalize()

def decorator(fn: StateTestSpec) -> Callable[[str], Mapping[str, Fixture]]:
def inner(engine) -> Mapping[str, Fixture]:
return fill_test(fn, [fork], engine)
def decorator(
fn: StateTestSpec,
) -> Callable[[Any, Any, str], Mapping[str, Fixture]]:
def inner(t8n, b11r, engine) -> Mapping[str, Fixture]:
return fill_test(t8n, b11r, fn, [fork], engine)

cast(Any, inner).__filler_metadata__ = {
"fork": fork,
Expand Down
28 changes: 15 additions & 13 deletions src/ethereum_test/fill.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@


def fill_test(
test_spec: TestSpec, forks: List[str], engine: str
t8n: TransitionTool,
b11r: BlockBuilder,
test_spec: TestSpec,
forks: List[str],
engine: str,
) -> Mapping[str, Fixture]:
"""
Fills fixtures for certain forks.
"""
fixtures: List[Fixture] = []
for fork in forks:
b11r = BlockBuilder()
t8n = TransitionTool()

for test in test_spec(fork):

Expand All @@ -37,17 +39,17 @@ def fill_test(
b11r, t8n, genesis, fork, reward=2000000000000000000
)

fixtures.append(
Fixture(
blocks=blocks,
genesis=genesis,
head=head,
fork=fork,
pre_state=test.pre,
post_state=None,
seal_engine=engine,
)
fixture = Fixture(
blocks=blocks,
genesis=genesis,
head=head,
fork=fork,
pre_state=test.pre,
post_state=None,
seal_engine=engine,
)
fixture.fill_info(t8n, b11r)
fixtures.append(fixture)

out = {}
for fixture in fixtures:
Expand Down
9 changes: 6 additions & 3 deletions src/ethereum_test/state_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,12 @@ def make_blocks(

rejected_txs = verify_transactions(self.txs, result)
if len(rejected_txs) > 0:
# TODO: This block is invalid because it contains intrinsically
# invalid transactions
pass
raise Exception(
"one or more transactions in `StateTest` are "
+ "intrinsically invalid, which are not allowed. "
+ "Use `BlockchainTest` to verify rejection of blocks "
+ "that include invalid transactions."
)

verify_post_alloc(self.post, alloc)

Expand Down
21 changes: 20 additions & 1 deletion src/ethereum_test/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
from dataclasses import dataclass, field
from typing import Any, Dict, List, Mapping, Optional, Tuple, Type, Union

from evm_block_builder import BlockBuilder
from evm_transition_tool import TransitionTool

from .code import Code, code_to_hex
from .common import AddrAA, TestPrivateKey

Expand Down Expand Up @@ -572,6 +575,8 @@ class FixtureBlock:
rlp: str
block_header: Optional[FixtureHeader] = None
expected_exception: Optional[str] = None
block_number: Optional[int] = None
chain_name: Optional[str] = None


@dataclass(kw_only=True)
Expand All @@ -587,6 +592,14 @@ class Fixture:
pre_state: Mapping[str, Account]
post_state: Optional[Mapping[str, Account]]
seal_engine: str
info: Dict[str, str] = field(default_factory=dict)

def fill_info(self, t8n: TransitionTool, b11r: BlockBuilder):
"""
Fill the info field for this fixture
"""
self.info["filling-transition-tool"] = t8n.version()
self.info["filling-block-build-tool"] = b11r.version()


class JSONEncoder(json.JSONEncoder):
Expand Down Expand Up @@ -696,10 +709,16 @@ def default(self, obj):
b["blockHeader"] = json.loads(
json.dumps(obj.block_header, cls=JSONEncoder)
)
if obj.expected_exception is not None:
b["expectException"] = obj.expected_exception
if obj.block_number is not None:
b["blocknumber"] = str(obj.block_number)
if obj.chain_name is not None:
b["chainname"] = obj.chain_name
return b
elif isinstance(obj, Fixture):
f = {
"_info": {},
"_info": obj.info,
"blocks": [
json.loads(json.dumps(b, cls=JSONEncoder))
for b in obj.blocks
Expand Down
7 changes: 6 additions & 1 deletion src/ethereum_test_filler/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import setuptools

from ethereum_test.types import JSONEncoder
from evm_block_builder import EvmBlockBuilder
from evm_transition_tool import EvmTransitionTool


class Filler:
Expand Down Expand Up @@ -105,6 +107,9 @@ def fill(self) -> None:

os.makedirs(self.options.output, exist_ok=True)

t8n = EvmTransitionTool()
b11r = EvmBlockBuilder()

for filler in fillers:
name = filler.__filler_metadata__["name"]
output_dir = os.path.join(
Expand All @@ -115,7 +120,7 @@ def fill(self) -> None:
path = os.path.join(output_dir, f"{name}.json")

self.log.debug(f"filling {name}")
fixture = filler("NoProof")
fixture = filler(t8n, b11r, "NoProof")

with open(path, "w", encoding="utf-8") as f:
json.dump(
Expand Down
52 changes: 51 additions & 1 deletion src/evm_block_builder/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,47 @@

import json
import subprocess
from abc import abstractmethod
from pathlib import Path
from shutil import which
from typing import Any, Optional, Tuple


class BlockBuilder:
"""
Block builder frontend.
Generic Block builder frontend.
"""

@abstractmethod
def build(
self,
header: Any,
txs: Any,
ommers: Any,
clique: Optional[Any] = None,
ethash: bool = False,
ethashMode: str = "normal",
) -> Tuple[str, str]:
"""
Build a block with specified parameters and return RLP and hash
"""
pass

@abstractmethod
def version(self) -> str:
"""
Return name and version of tool used to build the block
"""
pass


class EvmBlockBuilder(BlockBuilder):
"""
Go-ethereum `evm` Block builder frontend.
"""

binary: Path
cached_version: Optional[str] = None

def __init__(self, binary: Optional[Path] = None):
if binary is None:
Expand Down Expand Up @@ -78,3 +108,23 @@ def build(
Exception("malformed result")

return (output["rlp"], output["hash"])

def version(self) -> str:
"""
Gets `evm` binary version.
"""
if self.cached_version is None:

result = subprocess.run(
[str(self.binary), "-v"],
stdout=subprocess.PIPE,
)

if result.returncode != 0:
raise Exception(
"failed to evaluate: " + result.stderr.decode()
)

self.cached_version = result.stdout.decode().strip()

return self.cached_version
Loading