fix - shutdown anvil-zksync zombies, review tests and fork method for latest anvil-zksync
#46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed deprecated anvil-zksync arguments - Updated test to use "safe" block_identifier by default - Updated environment.py to use new fork_rpc method - Updated node.py to handle new fork mode arguments - Ensured compatibility with latest anvil-zksync changes Now anvil-zksync can be shut down gracefully without leaving zombie processes. And the tests are updated to reflect the new behavior of the fork method. Everything has been adapted to pytest xdist workers with detection of env variable to run tests in parallel, without port conflicts. Suggested anvil port is now by default 8011 to allow adding the network to MetaMask. If 8011 is already in use, then we apply free port logic.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
s3bc40
commented
Jul 2, 2025
| "<unknown>).get_name_of(address) -> ['string'])", | ||
| " <Unknown contract 0x0000000000000000000000000000000000008009>", | ||
| " <Unknown contract 0x000000000000000000000000000000000000800b>", | ||
| # " <Unknown contract 0x000000000000000000000000000000000000800b>", |
There was a problem hiding this comment.
I let this here, but maybe we can remove it if we follow along with the latest?
s3bc40
commented
Jul 2, 2025
Comment on lines
+85
to
+86
| # @dev deprecated in boa, use AnvilZKsync fork mode | ||
| # return super().fork(url, reset_traces, block_identifier, debug, **kwargs) |
There was a problem hiding this comment.
I can remove it after the review
Suggested change
| # @dev deprecated in boa, use AnvilZKsync fork mode | |
| # return super().fork(url, reset_traces, block_identifier, debug, **kwargs) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue: #45
What I did
The main goal was to kill the remaining
anvil-zksyncprocess, since they were not garbage collected. But I ended up adaptingboa-zksyncto the latestanvil-zksyncand reviewed the fork system.Key changes include the refactoring of environment setup functions, enhancements to the
AnvilZKsyncclass for managing test nodes (lifecycle), and updates to dependencies and test configurations.I tried at first to go for a context manager method inside
AnvilZKsyncbut it was not adapted to our need, and it would have brought too many changes in other tools like Moccasin.How I did it
Improvements to
AnvilZKsynctest node management:WeakSetto track activeAnvilZKsyncinstances and ensure proper cleanup using anatexithandler. (boa_zksync/node.py, boa_zksync/node.pyR1-R31)pytest-xdistand fallback to a suggested port when available. (boa_zksync/node.py, boa_zksync/node.pyR76-R221)_build_commandmethod for constructing commands and improved error handling during node shutdown. (boa_zksync/node.py, boa_zksync/node.pyR76-R221)Enhancements to zkSync environment setup:
set_zksync_env,set_zksync_test_env, andset_zksync_forkto improve clarity and ensure consistent environment initialization. Added detailed docstrings for better documentation. (boa_zksync/__init__.py, boa_zksync/init.pyR12-R48)node_argsinset_zksync_forkand updated the default behavior forblock_identifier. (tests/conftest.py, tests/conftest.pyL33-R35)Updates to forking and RPC handling:
forkandfork_rpcmethods to improve handling of RPC connections, ensure proper cleanup of existing nodes, and support new configurations. (boa_zksync/environment.py, [1] [2]is_port_freeto check port availability, improving the robustness of test node setup. (boa_zksync/util.py, boa_zksync/util.pyR23-R47)Dependency and configuration updates:
pyproject.tomltemporarily sincezkvyperdoes not supportvyper>=0.4.2(I'll go handle this).Test updates:
zksync_sepolia_forkfixture. (tests/conftest.py, tests/conftest.pyL33-R35)test_boa_loads.pyto reflect changes in error messages and contract behavior. (tests/test_boa_loads.py, tests/test_boa_loads.pyL134-R151)How to verify it
Run
make testand see it all pass. You should have the latest anvil-zksync, and it should pass with the latest blocks.You can also run
make lintto be sure that the code complies.Description for the changelog
Enhanced AnvilZKsync Management: implemented automatic cleanup of test nodes, enabled dynamic port allocation for parallel testing, and improved overall node startup/shutdown robustness.
Streamlined zkSync Environment: refactored environment setup functions (set_zksync_env, set_zksync_fork, etc.) for clearer usage and updated forking defaults.
Improved Forking Logic: enhanced fork and fork_rpc methods for better RPC connection handling and more reliable node setup.
Dependency & Test Adjustments: included a temporary dependency fix for zkvyper and updated test configurations/assertions to match the new system.
Cute Animal Picture