Skip to content

Conversation

@LouisTsai-Csie
Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie commented Jul 4, 2025

🗒️ Description

Add different gas cost test cases for CLZ opcode

🔗 Related Issues or PRs

Issue #1795

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
  • 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).

@LouisTsai-Csie LouisTsai-Csie self-assigned this Jul 4, 2025
@LouisTsai-Csie LouisTsai-Csie added fork:osaka Osaka hardfork type:test Type: Add/refactor fw unit tests; no fw or el client test case changes labels Jul 4, 2025
Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

Thanks! Just some small changes. I wondered if it was worth combining this test with test_clz_gas initially, but happy to have this as its own case.

Could we also rename test_clz_gas to test_clz_gas_cost?

@LouisTsai-Csie
Copy link
Collaborator Author

Thanks! Just some small changes. I wondered if it was worth combining this test with test_clz_gas initially, but happy to have this as its own case.

I explored this idea before implementation, but based on my understanding, the CodeGasMeasure object is responsible for verifying the gas usage specified in its code field. Therefore, we can’t use it to test cases involving insufficient or excessive gas.

Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

Nice! LGTM :)

@spencer-tb spencer-tb added scope:tests Scope: Changes EL client test cases in `./tests` type:feat type: Feature and removed type:test Type: Add/refactor fw unit tests; no fw or el client test case changes labels Jul 4, 2025
@LouisTsai-Csie LouisTsai-Csie force-pushed the clz-gas-cost branch 2 times, most recently from b9ca764 to 1313b65 Compare July 7, 2025 12:02
@spencer-tb
Copy link
Collaborator

Thanks for updating and the comment @jochem-brouwer!

@spencer-tb spencer-tb merged commit e88a211 into ethereum:main Jul 7, 2025
13 checks passed
kclowes pushed a commit to kclowes/execution-spec-tests that referenced this pull request Oct 20, 2025
…#1859)

* feat(tests): add test for CLZ opcode with varying gas costs

* refactor(tests): rename and update CLZ gas cost test to handle boundary conditions

* refactor(tests): rename CLZ gas cost test for clarity

* refactor(tests): parameterize for varying leading zero lengths

* refactor(tests): update contract deployment to include storage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork:osaka Osaka hardfork scope:tests Scope: Changes EL client test cases in `./tests` type:feat type: Feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants