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

feat(tests) Precompile Checks #1120

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions converted-ethereum-tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,6 @@ GeneralStateTests/VMTests/vmTests/push.json

([#1067](https://github.com/ethereum/execution-spec-tests/pull/1067))
GeneralStateTests/stPreCompiledContracts/blake2B.json

([#1067](https://github.com/ethereum/execution-spec-tests/pull/1120))
GeneralStateTests/stPreCompiledContracts/idPrecomps.json
128 changes: 128 additions & 0 deletions tests/frontier/precompiles/test_precompiles.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
"""Tests supported precompiled contracts."""

from typing import Iterator, Tuple

import pytest

from ethereum_test_forks import Fork
from ethereum_test_tools import (
Account,
Alloc,
Environment,
StateTestFiller,
Transaction,
)
from ethereum_test_tools.code.generators import Conditional
from ethereum_test_tools.vm.opcode import Opcodes as Op

UPPER_BOUND = 0xFF
NUM_UNSUPPORTED_PRECOMPILES = 1


def precompile_addresses(fork: Fork) -> Iterator[Tuple[str, bool]]:
"""
Yield the addresses of precompiled contracts and their support status for a given fork.

Args:
fork (Fork): The fork instance containing precompiled contract information.

Yields:
Iterator[Tuple[str, bool]]: A tuple containing the address in hexadecimal format and a
boolean indicating whether the address is a supported precompile.

"""
supported_precompiles = fork.precompiles()

num_unsupported = NUM_UNSUPPORTED_PRECOMPILES
for address in range(1, UPPER_BOUND + 1):
if address in supported_precompiles:
yield (hex(address), True)
elif num_unsupported > 0:
# Check unsupported precompiles up to NUM_UNSUPPORTED_PRECOMPILES
yield (hex(address), False)
num_unsupported -= 1
Copy link
Contributor

@winsvega winsvega Feb 11, 2025

Choose a reason for hiding this comment

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

what if precompiles are [1-15], [20-25] then 17,18,19 won't be checked for not being a precompile ?
its not the case, just analyzing. I see the original test verified first 0xff addresses. so if a fork will define precompiles with gaps could be an issue here.
I think now we have separate range of special addresses starting from 0x100 too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there are other tests for no precompile in test_precompile_absence, having those checks here as well would be redundant. I think that's why @marioevz said we should just check the first address that a precompile does not exist. I could change the upper bound to 0xff if need be, but I think the fork.precompiles for the forks being checked in this test are mostly just 0x1 - 0x9, and for cancun it includes 0xa as well.

I think now we have separate range of special addresses starting from 0x100 too.

Should the fork that supports this be included here? I don't see those specific ranges in the fork classes.



@pytest.mark.valid_from("Berlin")
@pytest.mark.parametrize_by_fork("address,precompile_exists", precompile_addresses)
def test_precompiles(
state_test: StateTestFiller, address: str, precompile_exists: bool, pre: Alloc
):
"""
Tests the behavior of precompiled contracts in the Ethereum state test.

Args:
state_test (StateTestFiller): The state test filler object used to run the test.
address (str): The address of the precompiled contract to test.
precompile_exists (bool): A flag indicating whether the precompiled contract exists at the
given address.
pre (Alloc): The allocation object used to deploy the contract and set up the initial
state.

This test deploys a contract that performs two CALL operations to the specified address and a
fixed address (0x10000), measuring the gas used for each call. It then stores the difference
in gas usage in storage slot 0. The test verifies the expected storage value based on
whether the precompiled contract exists at the given address.

"""
env = Environment()

slot_precompile_call_gas = 0x00
slot_empty_address_call_gas = 0x20

account = pre.deploy_contract(
Op.MSTORE(slot_precompile_call_gas, Op.GAS)
+ Op.CALL(
address=address,
value=0,
args_offset=0,
args_size=32,
output_offset=32,
output_size=32,
)
+ Op.MSTORE(slot_precompile_call_gas, Op.SUB(Op.MLOAD(slot_precompile_call_gas), Op.GAS))
+ Op.MSTORE(slot_empty_address_call_gas, Op.GAS)
+ Op.CALL(
address=0x10000,
value=0,
args_offset=0,
args_size=32,
output_offset=32,
output_size=32,
)
+ Op.MSTORE(
slot_empty_address_call_gas, Op.SUB(Op.MLOAD(slot_empty_address_call_gas), Op.GAS)
)
+ Op.SSTORE(
0,
Op.LT(
Conditional(
condition=Op.GT(
Op.MLOAD(slot_precompile_call_gas), Op.MLOAD(slot_empty_address_call_gas)
),
if_true=Op.SUB(
Op.MLOAD(slot_precompile_call_gas), Op.MLOAD(slot_empty_address_call_gas)
),
if_false=Op.SUB(
Op.MLOAD(slot_empty_address_call_gas), Op.MLOAD(slot_precompile_call_gas)
),
),
0x1A4,
),
)
+ Op.STOP,
storage={0: 0xDEADBEEF},
)

tx = Transaction(
to=account,
sender=pre.fund_eoa(),
gas_limit=1_000_000,
protected=True,
)

# A high gas cost will result from calling a precompile
# Expect 0x00 when a precompile exists at the address, 0x01 otherwise
post = {account: Account(storage={0: "0x00" if precompile_exists else "0x01"})}
Copy link
Contributor

@winsvega winsvega Feb 11, 2025

Choose a reason for hiding this comment

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

here I don't clearly understand why abs(empty_call_gas - precompile_call_gas) < 0x1a4 means this logic.
if you can rewrite it more clean like
if precompile_exists then precompile_call_gas is x else y if it is possible.

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'm not clear on this, since the actual gas value varies by precompile call. I'm not sure what logic could be set outside of the contract code. Could you explain a bit more on this?


state_test(env=env, pre=pre, post=post, tx=tx)