search

ethereum / web3.py

A python interface for interacting with the Ethereum blockchain and ecosystem.
http://web3py.readthedocs.io
MIT License
4.95k stars 1.69k forks source link

Update test suite, update Solidity version for compiling test contracts, remove deprecated JSON-RPC methods #3307

Closed fselmo closed 2 months ago

fselmo commented 5 months ago

What was wrong?

This PR was meant to update Solidity contract versions but it revealed that geth --dev was broken. Geth fixed this and this PR brings those changes in (v1.14.3). Updated geth no longer has support for eth_mining, eth_hashrate, eth_submitWork, eth_getWork, eth_coinbase. This PR gets our test suite updated with all of these changes as well as removes support for those methods altogether. We will have to issue a backport PR for v6 to deprecate all of these.

The goal here was to compile test contracts with Solidity 0.8.25 which broke because we needed to update geth. This PR was open for a while and 0.8.26 came out as well. This build has everything working with 0.8.25 and then the latest version should use 0.8.26.

Todo:

Cute Animal Picture

0_o

fselmo commented 2 months ago

fyi: I updated my gpg key so I had to squash all old commits into one large commit that was signed by the new gpg key. I would've used a bit more separation of concerns in the commits / messages otherwise.

fselmo commented 2 months ago

Not sure what happened with the latest run in CI

Circle CI was down EOD yesterday. I meant to re-run today.

I'll have another look with the next py-geth release

I don't think we plan on releasing it while keeping this PR open. This is going in web3.py main which is also in beta so I think we should merge it as is and just make sure to update to stable before releasing v7 stable.

fselmo commented 2 months ago

LGTM! I left a few inline nits, and then the only other thing is I wonder if we should think of a better name for accounts[0] to reference it by. Maybe default_account? Feel free to take or leave, I don't think it's a huge improvement and I don't really have a better idea at the moment ¯(ツ)/¯

Yeah, agreed. I think that might be a nice pytest fixture abstraction to have if anything. I'll take a peek while I'm in there.

fselmo commented 2 months ago

@kclowes, I opted to make use of the w3.eth.default_account, in the most recent commit, by setting that to the first account for tests. I think that makes the most sense, does that feel like a good resolution to you? I added a test that makes sure w3.eth.default_account is empty by default since that was being checked in another test.