refactor(exceptions,specs): Support general block exceptions - Pydantic context to parse using mapper

[marioevz]

🗒️ Description

Pydantic Mapper Parsing

This changes introduces the exception types right in the model when reading external tool responses.

Instead of parsing all exceptions to a str, and then using the mapper to manually parse the exception when it's time to use it, the mapper is passed as a context parameter to model_validate and picked up by a BeforeValidator defined in a field so it is automatically converted to BlockException, EOFException, TransactionException or UndefinedException.

If the field is parsed as UndefinedException it means that the mapper does not know about the exception and it could not be parsed.

Mapper Optimizations/Improvements

Instead of defining a _mapping_data property, which must be re-computed each time the property is accessed, two new ClassVar have been defined and are used to parse exceptions in the mapper.

mapping_substring

Similar behavior to _mapping_data but in the form of a dictionary: the substring is searched within the message returned by the external tool and if found the exception is returned.

mapping_regex

Dictionary containing regex strings mapped to exceptions: re.search is used on the message returned by the external tool and if found the exception is returned.

Block.exception Matched Against Transition Tool Response

Block.exception is now matched against a new optional field returned by the transition tool in the Result object: blockException.

Previously we could produced an invalid block test by one of several methods:

• An invalid transaction included in the block, detected in the Result.rejected_transactions field of the returned object by the transition tool.
• Mock a block header or body field by sending a correct set of transactions to the transition tool and then modifying the field in the response before exporting it to the fixture.

None of these methods required the transition tool to inform us that a block was invalid, but with new EIP changes like:
ethereum/EIPs#9508
ethereum/EIPs#9582

There's a block failure type that we cannot mock in the tests: system contract failures.

This new field is used the tests for both of these EIP changes: #1394

reliable Class Variable in the Exception Mapper

Added a new property to the exception mappers that when set to False means that the exception mapper cannot reliably discriminate between types of exceptions.

This needs to be set back to True once the EELS's t8n returns different exceptions for each type.

Global default_t8n Fixture for Unit Tests

New global fixture that contains an already-running instance of the default t8n tool, which significantly speeds up unit test runs.

🔗 Related Issues

✅ 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: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
• 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.

[marioevz]

Hey @gurukamath, I have a branch for the blockException field returned by EELS's T8N here: marioevz/execution-specs@d615cb5

Wanted to ask your opinion on this and whether you think it's a good idea to have this.

[gurukamath]

Wanted to ask your opinion on this and whether you think it's a good idea to have this.

Is there any particular scenario you have in mind? Because the system transactions are supposed to fail silently iirc, so I am not sure you would expect to catch anything here.

[marioevz]

Wanted to ask your opinion on this and whether you think it's a good idea to have this.

Is there any particular scenario you have in mind? Because the system transactions are supposed to fail silently iirc, so I am not sure you would expect to catch anything here.

Yes that was the way they worked but with ethereum/EIPs#9508 and ethereum/EIPs#9582 they are no longer supposed to fail silently, so instead the block should be invalid, even when the call does not fail but instead the address contains no code.

The problem as I could debug it is that T8N throws at some point when the block is invalid, and a zero length response is returned instead of the usual JSON response, so we cannot parse anything.

The branch I pointed out uses a try-except block to catch the exception, and when caught it populates a new field in the t8n response to signal a block-wide issue, which is what we parse in this PR. Please feel free to suggest anything else since I'm not super familiar with T8N in EELS so there's potentially a better solution 👍

[gurukamath]

Ah. I see. In that case this sounds reasonable to me.

[winsvega]

looks like a good approach.
can you add a unit test where

1. transaction expected exception but it didn't happen
2. transaction expected exception but another exception happen
3. transaction expected exception and it happen, so the validation is green
4. transaction didn't expect exception but it happen

as well as the error strings in case.
there was a helper message in exception. that error string is not defined in mapper. exception not found.
if error string is found it was also fetching the exception CODE_NAME (so you know what type of exception you hit)
and if excpected exception CODE_NAME is not defined in mapper it was also hinting where in code you should define the error string.

just check that this hint logic is not affected.

[danceratopz]

Really nice refactor! This is a much nicer approach. Just a couple of comments below, respectively in #1404.

[marioevz]

looks like a good approach. can you add a unit test where

1. transaction expected exception but it didn't happen

2. transaction expected exception but another exception happen

3. transaction expected exception and it happen, so the validation is green

4. transaction didn't expect exception but it happen

as well as the error strings in case. there was a helper message in exception. that error string is not defined in mapper. exception not found. if error string is found it was also fetching the exception CODE_NAME (so you know what type of exception you hit) and if excpected exception CODE_NAME is not defined in mapper it was also hinting where in code you should define the error string.

just check that this hint logic is not affected.

Thanks! This unit test actually already existed here but it contained an xfail for it to pass:

execution-spec-tests/src/ethereum_test_specs/tests/test_expect.py

Lines 261 to 354 in 58a2ec1

	# Transaction result mismatch tests
	@pytest.mark.parametrize(
	"tx,exception_type",
	[
	pytest.param(
	Transaction(
	secret_key=TestPrivateKey,
	gas_limit=20_999,
	error=TransactionException.SENDER_NOT_EOA,
	),
	ExecutionExceptionMismatchError,
	id="TransactionExecutionExceptionMismatchError",
	),
	pytest.param(
	Transaction(
	secret_key=TestPrivateKey,
	error=TransactionException.INTRINSIC_GAS_TOO_LOW,
	expected_receipt=TransactionReceipt(gas_used=21_000),
	),
	UnexpectedExecutionSuccessError,
	id="TransactionUnexpectedExecutionSuccessError",
	),
	pytest.param(
	Transaction(
	secret_key=TestPrivateKey,
	gas_limit=20_999,
	expected_receipt=TransactionReceipt(gas_used=21_000),
	),
	UnexpectedExecutionFailError,
	id="TransactionUnexpectedExecutionFailError",
	),
	pytest.param(
	Transaction(
	secret_key=TestPrivateKey,
	expected_receipt=TransactionReceipt(gas_used=21_001),
	),
	TransactionReceiptMismatchError,
	id="TransactionReceiptMismatchError",
	),
	pytest.param(
	Transaction(
	secret_key=TestPrivateKey,
	gas_limit=20_999,
	expected_receipt=TransactionReceipt(gas_used=21_001),
	),
	UnexpectedExecutionFailError,
	id="TransactionUnexpectedExecutionFailError+TransactionReceiptMismatchError",
	),
	pytest.param(
	Transaction(
	secret_key=TestPrivateKey,
	error=TransactionException.INTRINSIC_GAS_TOO_LOW,
	expected_receipt=TransactionReceipt(gas_used=21_001),
	),
	UnexpectedExecutionSuccessError,
	id="TransactionUnexpectedExecutionSuccessError+TransactionReceiptMismatchError",
	),
	],
	)
	@pytest.mark.parametrize(
	"fixture_format",
	[
	StateFixture,
	BlockchainFixture,
	],
	)
	def test_transaction_expectation(
	state_test,
	default_t8n: TransitionTool,
	fork: Fork,
	exception_type: Type[Exception] | None,
	fixture_format: FixtureFormat,
	):
	"""
	Test a transaction that has an unexpected error, expected error, or expected a specific
	value in its receipt.
	"""
	if (
	exception_type == ExecutionExceptionMismatchError
	and not default_t8n.exception_mapper.reliable
	):
	pytest.xfail(
	reason="Exceptions need to be better described in the t8n tool "
	f"({default_t8n.__class__.__name__})."
	)
	if exception_type is None:
	state_test.generate(
	request=None, t8n=default_t8n, fork=fork, fixture_format=fixture_format
	)
	else:
	with pytest.raises(exception_type) as _:
	state_test.generate(
	request=None, t8n=default_t8n, fork=fork, fixture_format=fixture_format
	)

And actually the test was in correct but I fixed in this PR.

Also it was extremely slow, and it turns out that every time we do ExecutionSpecsTransitionTool() (instantiating the t8n for an unit test) it took many seconds, and in this test it was doing it once per parametrized value.

So I implemented the default_t8n fixture globally and sped up this and many other tests!

@danceratopz I added the comment you requested in the blobs test 👍

[danceratopz]

Looks good, just one small question and a possible improvement to src/ethereum/test_spec/helpers.py::verify().

[danceratopz]

LGTM!