8000 Prague updates by fselmo · Pull Request #3659 · ethereum/web3.py · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Prague updates #3659

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Apr 28, 2025
Merged

Prague updates #3659

merged 15 commits into from
Apr 28, 2025

Conversation

fselmo
Copy link
Collaborator
@fselmo fselmo commented Apr 10, 2025 •
edited
Loading

What was wrong?

We need to add some additional support for Prague-related fields.

How was it fixed?

  • Add formatters for authorization list fields and block requests field in block headers
  • Add integration test for 7702 so both geth and py-evm interactions are tested with expected results

Significant change to integration tests:

Change integration tests so that each test has its own context. We are having way too many issues with muddied contexts and tests needing to wait on the right moment to test transactions between a mining state. This isolates each integration test into its own state which makes issues a lot easier to track and should give less CI headaches.

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes --> This probably needs to be better... future PR + blog post?
  • Add at least one good 7702 tx test to the integration tests so we can test across both geth and eth-tester <-> py-evm
  • Will need Main updates for Prague eth-tester#317 and a release which is waiting on a py-evm Prague release. Update eth-tester lower pin when release is ready.
  • Add entry to the release notes

Cute Animal Picture

Screenshot 2025-04-23 at 13 19 00

@fselmo fselmo force-pushed the prague-updates branch 2 times, most recently from ca0cb1e to 844e01e Compare April 21, 2025 23:00
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 23, 2025
@fselmo
newsfragment for ethereum#3659
d2c98a8
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 23, 2025
@fselmo
newsfragment for ethereum#3659
bca57bf
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 23, 2025
@fselmo
newsfragment for ethereum#3659
f238fd2
@fselmo fselmo force-pushed the prague-updates branch 2 times, most recently from 36a4376 to 7b6ccc9 Compare April 23, 2025 22:11
@fselmo fselmo marked this pull request as ready for review April 23, 2025 22:26
@fselmo fselmo requested review from kclowes, reedsa and pacrob April 23, 2025 22:37
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 24, 2025
@fselmo
newsfragment for ethereum#3659
1999e72
@fselmo fselmo force-pushed the prague-updates branch 14 times, most recently from ed72356 to 44dba6c Compare April 24, 2025 23:05
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 24, 2025
@fselmo
bump eth-tester to newly released version with prague changes
39ff0a2 
- newsfragment for ethereum#3659
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 28, 2025
@fselmo
Changes from comments on PR ethereum#3659
4b89e5d
fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 28, 2025
@fselmo
Changes from comments on PR ethereum#3659
a5d3c58
@fselmo fselmo force-pushed the prague-updates branch from 4b89e5d to a5d3c58 Compare April 28, 2025 18:42 8000
@fselmo
Copy link
Collaborator Author
fselmo commented Apr 28, 2025 •
edited
Loading

For my own learning, is the fixture scope from module -> function for ease of parallelization, or is there another reason?

I think there are maybe a few fixtures we can change back to module but this is so that each test has its own instance of geth. This is the change that makes them run in isolation as each function is a single test.

edit: Even still, the overhead of not changing only one or two fixtures to module or session is probably pretty low

@fselmo fselmo force-pushed the prague-updates branch 4 times, most recently from 6ed2505 to 969c993 Compare April 28, 2025 19:22
@fselmo
Copy link
Collaborator Author
fselmo commented Apr 28, 2025 •
edited
Loading

@kclowes all changes from your comments accounted for. I don't think the dev.period is going to affect test times other than the ones that get a / wait for a transaction receipt, which should be 1 or 2 tests. Aside from those, tests should finish as soon as the test logic passes the test, which for most tests doesn't require block processing. I raised the dev.period to 4 in this PR since -n auto increased the number of workers to ~ 36 and slowed down processing time for each test, which ended up causing some issues due to mining.

In the end -n auto with increased dev.period seems like the right way to go. So far, running the CI with --dev.period=4 seems to be plenty of time for the pending / replace transaction tests to run their logic before the block is processed.

I'm going to re-run the CI a few more times and make sure the success rate good and then I'll merge this.

reedsa
reedsa approved these changes Apr 28, 2025
View reviewed changes
Copy link
Contributor
@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

👍

pacrob
pacrob approved these changes Apr 28, 2025
View reviewed changes
Copy link
Contributor
@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

🐑

@fselmo fselmo force-pushed the prague-updates branch 5 times, most recently from d31c523 to 1b700e4 Compare April 28, 2025 20:33
fselmo added 2 commits April 28, 2025 14:39
@fselmo
Add upper bound on number of processes to run for CI runs as this can
1fcd983 
  help control flakiness. ``36`` workers (unbounded ``-n auto``) seems
  to be too much to account for tests that rely on the ``dev.period``
  block processing time.
@fselmo
Silence warnings on async iterator by properly disconnecting
787307f
@fselmo fselmo merged commit 042b761 into ethereum:main Apr 28, 2025
85 checks passed
fselmo added a commit that referenced this pull request Apr 28, 2025
@fselmo
bump eth-tester to newly released version with prague changes
73edd9d 
- newsfragment for #3659
fselmo added a commit that referenced this pull request Apr 28, 2025
@fselmo
Changes from comments on PR #3659
923bc43
@fselmo fselmo deleted the prague-updates branch April 28, 2025 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kclowes kclowes kclowes approved these changes

@reedsa reedsa reedsa approved these changes

@pacrob pacrob pacrob approved these changes

Assignees

@fselmo fselmo

Labels
priority: p0 urgent
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fselmo @reedsa @pacrob @kclowes
0