-
Notifications
You must be signed in to change notification settings - Fork 553
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
Generalized RPC logic / basic support for Geth dev #998
Conversation
@@ -98,9 +98,6 @@ def test_rpc_project_cmd_settings(devnetwork, testproject, config, settings_proj | |||
assert web3.eth.chainId == 666 | |||
assert web3.net.version == "777" | |||
|
|||
# Test if evm version is returned properly | |||
assert devnetwork.rpc.evm_version() == cmd_settings_proj["evm_version"] |
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.
why not check this anymore?
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 was already challenging with only ganache because if the instance was already running, we had to extract it from the commandline args. In generalizing the RPC we'd now have to find a way to determine this from multiple clients, and I feel like the ++ to complexity weighed against the frequency that somebody actually has a fail from this.. simpler to just remove the check.
What I did
Generalize the logic for RPC clients and add basic support for using
geth --dev
viaethnode
.How I did it
brownie/networks/rpc.py
has become a subdirectory, with the__init__.py
holding a base class that can be pointed at different specific backend logic.Geth dev support is added. Brownie expects to launch or attach using
ethnode
. Unfortunately, when using Geth it is not possible to time travel or rewind the chain, effectively making test isolation impossible. For some limited cases this is still useful (hello, Vyper) so I've still included it. It's not documented for now due to a combination of it's limited usefulness and overall beta-ness.This also lays the groundwork for
hardhat
support! Once NomicFoundation/hardhat#1328 merges, it should be fairly quick on the Brownie side to make them play nice together. Then we'll have no blockers to flood poor @alcuadrado with annoying and meaningless support requests until he regrets the day he met me.