Skip to content
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

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

Merged
merged 5 commits into from
Jun 26, 2024
Merged

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

merged 5 commits into from
Jun 26, 2024

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Mar 26, 2024 •
edited
Loading

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 added a commit to fselmo/web3.py that referenced this pull request Mar 29, 2024
@fselmo
newsfragment for ethereum#3307
b69d66a
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch from 1989108 to b69d66a Compare March 29, 2024 20:58
@fselmo fselmo changed the title Recompile test contracts with 0.8.25 Recompile test contracts with Solidity v0.8.25 Mar 29, 2024
fselmo added a commit to fselmo/web3.py that referenced this pull request May 13, 2024
@fselmo
newsfragment for ethereum#3307
044fa8b
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch from 4465127 to 1994dfe Compare May 13, 2024 23:03
@fselmo fselmo changed the title Recompile test contracts with Solidity v0.8.25 Update test suite, update Solidity contract version, remove deprecated JSON-RPC methods May 13, 2024
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch 2 times, most recently from 747e932 to 8c52a4c Compare May 16, 2024 17:22
fselmo added a commit to fselmo/web3.py that referenced this pull request Jun 19, 2024
@fselmo
newsfragment for ethereum#3307
0d9f69b
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch 4 times, most recently from 48b345a to cd23243 Compare June 20, 2024 20:16
@fselmo fselmo changed the title Update test suite, update Solidity contract version, remove deprecated JSON-RPC methods Update test suite, update Solidity version for compiling test contracts, remove deprecated JSON-RPC methods Jun 20, 2024
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch from cd23243 to bb82062 Compare June 20, 2024 20:23
fselmo added a commit to fselmo/web3.py that referenced this pull request Jun 20, 2024
@fselmo
Update test fixture to use Sol v0.8.25; Update geth:
bb82062 
- Remove pending block reliance; set dev period to 1 second
- Use the latest geth 1.14.5 for integration; re-generate fixture
- Tweaks to the fixture generation based on geth updates
- re-compile test contracts with Solidity v0.8.25
- Remove support for non-existent client properties:
  - ``eth_coinbase``
  - ``eth_mining``
  - ``eth_hashrate``
  - ``eth_submitHashrate``
  - ``eth_submitWork``
  - ``eth_getWork``
- Update py-geth to use the newly released beta
- quick lint fixes
- Fix remaining tests that rely on mining:
- newsfragment for ethereum#3307
@fselmo
Copy link
Collaborator Author

fselmo commented Jun 20, 2024

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 added a commit to fselmo/web3.py that referenced this pull request Jun 20, 2024
@fselmo
Update test fixture to use Sol v0.8.25; Update geth:
371b3a0 
- Remove pending block reliance; set dev period to 1 second
- Use the latest geth 1.14.5 for integration; re-generate fixture
- Tweaks to the fixture generation based on geth updates
- re-compile test contracts with Solidity v0.8.25
- Remove support for non-existent client properties:
  - ``eth_coinbase``
  - ``eth_mining``
  - ``eth_hashrate``
  - ``eth_submitHashrate``
  - ``eth_submitWork``
  - ``eth_getWork``
- Update py-geth to use the newly released beta
- quick lint fixes
- Fix remaining tests that rely on mining:
- newsfragment for ethereum#3307
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch 3 times, most recently from 50edf39 to ab0436f Compare June 20, 2024 21:47
@fselmo fselmo marked this pull request as ready for review June 20, 2024 21:50
@fselmo fselmo requested review from reedsa, kclowes and pacrob June 20, 2024 21:50
reedsa
reedsa reviewed Jun 21, 2024
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.

Looks good so far. Not sure what happened with the latest run in CI but I'll have another look with the next py-geth release.

@fselmo
Copy link
Collaborator Author

fselmo commented Jun 21, 2024 •
edited
Loading

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 fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch from 2d44a6f to 832dd0f Compare June 22, 2024 17:00
kclowes
kclowes approved these changes Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

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 ¯_(ツ)_/¯

tests/integration/generate_fixtures/go_ethereum.py Outdated Show resolved Hide resolved
tests/integration/generate_fixtures/go_ethereum.py Outdated Show resolved Hide resolved
@fselmo
Copy link
Collaborator Author

fselmo commented Jun 24, 2024

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
Update test fixture to use Sol v0.8.25; Update geth:
efdf7fc 
- Remove pending block reliance; set dev period to 1 second
- Use the latest geth 1.14.5 for integration; re-generate fixture
- Tweaks to the fixture generation based on geth updates
- re-compile test contracts with Solidity v0.8.25
- Remove support for non-existent client properties:
  - ``eth_coinbase``
  - ``eth_mining``
  - ``eth_hashrate``
  - ``eth_submitHashrate``
  - ``eth_submitWork``
  - ``eth_getWork``
- Update py-geth to use the newly released beta
- quick lint fixes
- Fix remaining tests that rely on mining:
- newsfragment for ethereum#3307
@fselmo
Compile test contracts with Solidity v0.8.26
1df1333
@fselmo
Give more stability to tests using either "pending" or "latest":
f6cd136 
- Set a timeout and loop over the latest block a few times until desired
  tx is included.

- Some processing, such as async IPC, goes faster than others. There'stability
  not a great way to ensure if we are in the "pending" or "latest" state for
  blocks using geth --dev. The important thing for testing on our end is that the
  JSON-RPC method works the way it's expected. Whether or not it returns the
  expected result based on block number, etc, is on Geth's side to make sure.
  Test the JSON-RPC method and give it flexibility in this commit.
@fselmo
Simplify old contract state test
6e6e608
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch from 832dd0f to 5096d35 Compare June 24, 2024 22:58
@fselmo
Use eth.default_account for tests, in the age of no eth_coinbase
46a717a 
This commit updates the tests to set ``w3.eth.default_account`` to the
first account and use it in place of ``w3.eth.coinbase``, since
``eth_coinbase`` was removed in clients like geth. Using
``w3.eth.accounts[0]`` is not a very user-friendly API to keep calling
across all tests.
@fselmo fselmo force-pushed the recompile-test-contracts-with-0.8.25 branch from 5096d35 to 46a717a Compare June 24, 2024 23:10
@fselmo
Copy link
Collaborator Author

fselmo commented Jun 24, 2024 •
edited
Loading

@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.

kclowes
kclowes approved these changes Jun 26, 2024
View reviewed changes
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

Yeah, I like that! 🚢

@fselmo fselmo merged commit 5f5e056 into ethereum:main Jun 26, 2024
71 checks passed
fselmo added a commit that referenced this pull request Jun 26, 2024
@fselmo
Update test fixture to use Sol v0.8.25; Update geth:
eacc6aa 
- Remove pending block reliance; set dev period to 1 second
- Use the latest geth 1.14.5 for integration; re-generate fixture
- Tweaks to the fixture generation based on geth updates
- re-compile test contracts with Solidity v0.8.25
- Remove support for non-existent client properties:
  - ``eth_coinbase``
  - ``eth_mining``
  - ``eth_hashrate``
  - ``eth_submitHashrate``
  - ``eth_submitWork``
  - ``eth_getWork``
- Update py-geth to use the newly released beta
- quick lint fixes
- Fix remaining tests that rely on mining:
- newsfragment for #3307
@fselmo fselmo deleted the recompile-test-contracts-with-0.8.25 branch June 26, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reedsa reedsa reedsa approved these changes

@kclowes kclowes kclowes approved these changes

@pacrob pacrob Awaiting requested review from pacrob

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Recompile test contracts with new Solidity version: v0.8.26
3 participants
@fselmo @reedsa @kclowes