-
Notifications
You must be signed in to change notification settings - Fork 374
feat(tests): Check minimum gaslimit (5000) is validated #1731
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
base: forks/osaka
Are you sure you want to change the base?
Conversation
66222f7 to
131daee
Compare
|
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1731 +/- ##
============================================
Coverage 86.07% 86.07%
============================================
Files 743 743
Lines 44078 44078
Branches 3894 3894
============================================
Hits 37938 37938
Misses 5659 5659
Partials 481 481
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Sentry. π New features to boost your workflow:
|
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, i do not know the frontier gas limit is so low: https://ethereum.stackexchange.com/questions/28486/why-is-block-gas-limit-set-to-5000
nit: maybe combine test_gas_limit_at_minimum & test_gas_limit_below_minimum but personally i think it is fine.
131daee to
118e40c
Compare
I got complaints about |
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Good from my side, but needs others to merge
| from execution_testing.base_types.base_types import ZeroPaddedHexNumber | ||
| from execution_testing.base_types.composite_types import Alloc | ||
| from execution_testing.exceptions.exceptions import BlockException | ||
| from execution_testing.specs.blockchain import ( | ||
| Block, | ||
| BlockchainTestFiller, | ||
| Header, | ||
| ) | ||
| from execution_testing.test_types.block_types import Environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems after the weld we could do import like this:
from execution_testing import (
Alloc,
Block,
BlockchainTestFiller,
BlockException,
Environment,
Header,
)
from execution_testing.base_types.base_types import ZeroPaddedHexNumberI tried locally and it is working, but i assume we could further simplify the import for ZeroPaddedHexNumber?
Maybe @danceratopz has more context on this.
ποΈ Description
One legacy block validation check has been left uncovered, filling the gap.
Also I'm pulling in a bunch of missing exception mappers I've come across.
π Related Issues or PRs
N/A.
β Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.