feat(test-benchmark): implement opcode count verification

[LouisTsai-Csie]

🗒️ Description

There are several enhancements in this PR:

• Implement opcode-count verification for --fixed-opcode-count mode using the opcount field provided when generating tests with evmone with 5% deviation.

• Re-label the worst-case scenarios for the repricing marker based on the Grafana dashboard data and this script that sort the cases based on the MGas result.

  • The repricing marker under tests/benchmark/compute/instruction/ are udpated.

Based on our discussion about the verification mechanism during sync up call, specifically whether to remove the target_opcode field. I would suggest not removing it for now, since this feature depends on it, which is very helpful for me to show coverage to others, without manually update the Notion page.

🔗 Related Issues or PRs

Issue #1835

✅ Checklist

• All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:

  uvx tox -e static

• All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
• All: Considered adding an entry to CHANGELOG.md.
• All: Considered updating the online docs in the ./docs/ directory.
• All: Set appropriate labels for the changes (only maintainers can apply labels).
• Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
• Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
• Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (forks/amsterdam@8b089c9). Learn more about missing BASE report.
⚠️ Report is 6 commits behind head on forks/amsterdam.

Additional details and impacted files

@@                Coverage Diff                 @@
##             forks/amsterdam    #1869   +/-   ##
==================================================
  Coverage                   ?   86.33%           
==================================================
  Files                      ?      538           
  Lines                      ?    34557           
  Branches                   ?     3222           
==================================================
  Hits                       ?    29835           
  Misses                     ?     4148           
  Partials                   ?      574           

Flag	Coverage Δ
unittests	86.33% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[spencer-tb]

Awesome work on implementing this, nice solution to a tricky problem!

Added some questions below. I think we should rebase once #1790 is merged, as there will be some conflicts. :D

[spencer-tb]

Approved from my end (as going ooo)! :D

[LouisTsai-Csie]

Solving failing benchmark scenario due to typing issue

[fselmo]

I wonder if instead of manual coercion / handling here, we can make use of pydantic more directly. It wasn't immediately obvious (and I couldn't find) what test was giving issues here but it would be really nice if this worked:

@validate_call(config=ConfigDict(arbitrary_types_allowed=True))
def fund_eoa(
    self,
    ...
) -> EOA:
    ...

What this does is it will validate all the inputs as the models that they are defined, with the same type coercion that pydantic gives to model instantiation. I wouldn't spend a ton of time on it but it would be nice to use pydantic for all of it's bells and whistles if we can.

Do you know what test was giving issues here?

[marioevz]

I really like this idea, although this would basically touch every single test created, so perhaps for a follow up PR.

What I would fix in this PR though is to make it explicit that this can receive a Dict in that argument.

[LouisTsai-Csie]

This breaks the test_ext_account_query_warm benchmark, its behavior is different under fill and execute remote. Now it passes during fill but fails when using execute remote, so now our CI cannot catch the bug!

[fselmo]

From an implementation standpoint, this looks good on my end. I tested the tolerance and the counts, CALL vs STATICCALL, looks like it works as intended. The review looks good from an outsider's perspective, I just left some things to think about and some things for me to understand a bit better as I'm not so used to this flow yet.

I will take a look again tomorrow with fresh eyes. Really nice work 👌🏼

[fselmo]

I wonder if instead of manual coercion / handling here, we can make use of pydantic more directly. It wasn't immediately obvious (and I couldn't find) what test was giving issues here but it would be really nice if this worked:

@validate_call(config=ConfigDict(arbitrary_types_allowed=True))
def fund_eoa(
    self,
    ...
) -> EOA:
    ...

What this does is it will validate all the inputs as the models that they are defined, with the same type coercion that pydantic gives to model instantiation. I wouldn't spend a ton of time on it but it would be nice to use pydantic for all of it's bells and whistles if we can.

Do you know what test was giving issues here?

[marioevz]

Rebased to latest forks/amsterdam to make review easier.

[marioevz]

Created a PR on top of this to use pydantic: LouisTsai-Csie#1

Other than that it looks good to me! @LouisTsai-Csie

[danceratopz]

This is looking good. A few very minor nits below!

[danceratopz]

Thanks @LouisTsai-Csie! One more minor nit!

[danceratopz]

Thanks @LouisTsai-Csie! LGTM!