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

core: fix call to post-validation evm message hook #437

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

protolambda
Copy link
Collaborator

Description

The post-validation check was running even when the EVM aborted without returning an execution-result. E.g. this may happen during block-building, when a tx does not pass state-transition pre-checks, and inclusion is aborted.

This post-validation hook was only set when the experimental Interop fork functionality is enabled, and thus not a production issue for mainnet.

Tests

E2E tests hit block-building code-path, although with flakes. I'll look into regression-testing it, but creating a more deliberate version of it e.g. with an action-test might mean it ignores the tx before even running the pre-checks.

Additional context

Reported by @mslipper, example in: https://app.circleci.com/pipelines/github/ethereum-optimism/optimism/72874/workflows/2ec0d66a-c736-4e19-ad98-031fb11428db/jobs/2986089/steps

@protolambda protolambda requested a review from a team as a code owner November 26, 2024 15:31
@protolambda
Copy link
Collaborator Author

Hit an unrelated flake in the test-run of the fix. It was a small test fix, so I'm including it here in this PR. The tx that was being checked for rejection wasn't always making it into the tx-pool quick enough for consideration during block-building. That was fixed by making the tx-pool insertion synchronous for this one tx.

@axelKingsley axelKingsley merged commit b84907b into optimism Nov 26, 2024
10 checks passed
@axelKingsley axelKingsley deleted the fix-post-validation-call branch November 26, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants