-
Notifications
You must be signed in to change notification settings - Fork 415
fix(tests): fix EIP-7934 tests logic accuracy #2078
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
fix(tests): fix EIP-7934 tests logic accuracy #2078
Conversation
- extradata field is 32 bytes so we can adjust using the block's extradata up to +/- 15 bytes or so (we will need to add one more for rlp at limit + 1 byte invalid tests). This makes it so we should selfomly even hit the binary search logic.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2078 +/- ##
================================================
Coverage 86.14% 86.14%
================================================
Files 599 599
Lines 39472 39472
Branches 3780 3780
================================================
Hits 34002 34002
Misses 4848 4848
Partials 622 622
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:
|
gurukamath
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.
Just a minor nit about the values returned by the exact_size_transactions. Looks good to me otherwise
🗒️ Description
We had some issues filling EIP-7934 tests for Amsterdam after introducing tests for EIP-7843, that we very quickly patched (here) so we could get the tests released. This "fix" was more of a bandaid fix that was likely going to leak issues out again eventually. This PR aims to fix this in a more proper way as this is still currently failing one test in
forks/amsterdamsince we haven't introduced this fix here.extraDatato half of what it can be.extraDatais a 32-byte field, so if we start somewhere in the middle we can have even better tolerance. It turns out that our calldata estimate for the last tx is good enough where we're always within ~5 bytes of what we need. This actually removes the need for the binary search entirely as we have plenty of tolerance to adjust with blockextraData.This PR brings the
filltime for these tests down significantly to where I don't believe these tests are a burden onfillanymore.Bonus:
Fix typo in EIP-7251 BAL tests that was just merged to
forks/amsterdamand slipped by PR reviews.✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.Cute Animal Picture