Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

eth-keys, eth-tester, Web3.py update #1725

Merged
merged 6 commits into from May 18, 2020
Merged

Conversation

carver
Copy link
Contributor

@carver carver commented May 14, 2020

What was wrong?

Fixes #1724

How was it fixed?

Upgraded various libraries. web3.py seems to have started exporting types since the last version, so a lot of new type warnings were triggered.

To-Do

Cute Animal Picture

put a cute animal picture link inside the parentheses

@carver carver mentioned this pull request May 14, 2020
2 tasks
@@ -120,7 +122,7 @@ def get_logs(self, block_number: BlockNumber) -> Tuple[DepositLog, ...]:
}
)
processed_logs = tuple(
self._deposit_contract.events.DepositEvent().processLog(log) for log in logs
self._deposit_contract.events.DepositEvent.processLog(log) for log in logs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This raised a "DepositEvent is not callable" error in mypy. This is a bug in web3's typing I think: ethereum/web3.py#1646

But since processLog is callable as a classmethod, and that usage makes more sense here anyhow, I just switched it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, oof, there's no clean way out of this. Will just have to type: ignore, because: ethereum/web3.py#1648

@carver carver force-pushed the web3-update branch 3 times, most recently from e2bb7c0 to 5171988 Compare May 14, 2020 21:30
@carver
Copy link
Contributor Author

carver commented May 14, 2020

The wheel cli was the missing >>> ping issue, but I'll just wait to rebase on #1681 instead of re-running the test.

web3.py and eth-tester both had stricter old eth-keys dependencies,
so they also need an update.
The old eth-account had an eth-keys version that was incompatible.
@carver carver force-pushed the web3-update branch 2 times, most recently from b570345 to 858452f Compare May 15, 2020 23:07
@carver
Copy link
Contributor Author

carver commented May 15, 2020

Hey, sorry to bother you with something so minor @hwwhww but since it touches eth2 code I thought I'd at least send a heads-up.

@carver carver requested a review from hwwhww May 15, 2020 23:49
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

One exception handling suggestion, otherwise LGTM 👍

trinity/components/eth2/eth1_monitor/eth1_monitor.py Outdated Show resolved Hide resolved
@@ -189,7 +172,9 @@ def get_block(self, arg: Union[Hash32, int, str]) -> Optional[Eth1Block]:
)
# If `arg` is block hash
elif isinstance(arg, bytes):
block_number = int.from_bytes(arg, byteorder="big")
block_hash = Hash32(arg)
# Why are we interpreting the block hash as a block number here?
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I'm not certain, but this is FakeEth1DataProvider, the real one is Web3Eth1DataProvider, so it can be filled with whatever block_number for testing.

processed_logs = tuple(
self._deposit_contract.events.DepositEvent().processLog(log) for log in logs
processed_logs = self._deposit_contract.events.DepositEvent.getLogs(
fromBlock=block_number, toBlock=block_number
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double confirm: address & topics arguments can be removed because it has been initialized in __init__: self._deposit_contract = self.w3.eth.contract(address=deposit_contract_address, abi=deposit_contract_abi)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! Exactly

carver and others added 3 commits May 18, 2020 13:16
- Require 5.10.0 for IPCProvider(pathlib.Path) type support
- Ignore a few event log parsing type warnings, will fix them next, by
using a new v5 API to simplify the logic and avoid the warnings.
Has the side benefit of avoiding type issues
No need to raise a `ValueError`, just break and allow the `Eth1BlockNotFound` to be raised below.

Co-authored-by: Hsiao-Wei Wang <hwwang156@gmail.com>
@carver carver merged commit 93d23da into ethereum:master May 18, 2020
@carver carver deleted the web3-update branch May 18, 2020 22:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Old eth-keys shows a deprecation warning
2 participants