This repository has been archived by the owner on Jul 9, 2021. It is now read-only.
Migrate to Web3.py v5 #2038
Merged
Migrate to Web3.py v5 #2038
Conversation
This file contains 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
|
feuGeneA
force-pushed
the
fix/python/migrate-to-web3-v5
branch
2 times, most recently
from
c00f47b
to
88b25df
Compare
Install Python packages in dependency order, not in parallel.
...and include a script to produce the proper ordering.
feuGeneA
force-pushed
the
fix/python/migrate-to-web3-v5
branch
2 times, most recently
from
4295625
to
adca5f0
Compare
feuGeneA
force-pushed
the
fix/python/migrate-to-web3-v5
branch
7 times, most recently
from
26f0b2f
to
039ab78
Compare
feuGeneA
force-pushed
the
fix/python/migrate-to-web3-v5
branch
4 times, most recently
from
ec93726
to
590c0d8
Compare
This was discovered while minimizing CircleCI steps to dianose a problem with running the Launch Kit Backend in CircleCI. These classes should be imported via the zero_ex.contract_wrappers.exchange and zero_ex.contract_wrappers.erc20_token modules, respectively. We permitted importing them from just zero_ex.contract_wrappers back when they were the only wrappers we had, but now that we have so many different contracts being wrapped, this is just another list to keep manually updated (which, obviously is error prone, since it slipped through the cracks already), so it's better to just not support this type of import.
Without this, generated documentation was not including the class members that represent the contract methods, rendering the usage unclear.
feuGeneA
force-pushed
the
fix/python/migrate-to-web3-v5
branch
from
590c0d8
to
d81927a
Compare
...except for the dummy tokens.
Previously these teses were using 0xorg/launch-kit-ci, but that was a one-off thing created just for CI, back before there was a regularly maintained docker image of Launch Kit. Changed to use 0xorg/launch-kit-backend since it's regularly maintained/updated. Because the -backend image is using a different Linux distribution (Alpine), the commands used to wait for ganache startup also had to change. The tag used, 74bcc39, is provisional due to the pending Issue at 0xProject/0x-launch-kit-backend#73 . When that issue is resolved, the tag suffix on the imag name should be removed.
Due to problem with launch-kit-backend, documented at 0xProject/0x-launch-kit-backend#73 , we need to checksum the makerAddress, in the order retrieved from the relayer, before filling it, otherwise Web3.py gives this error: InvalidAddress('Web3.py only accepts checksum addresses. The software that gave you this non-checksum address should be considered unsafe, please file it as a bug on their platform. Try using an ENS name instead. Or, if you must accept lower safety, use Web3.toChecksumAddress(lower_case_address).', '0x5409ed021d9299bf6814279a6a1411a7e866a631')
Formerly CHANGELOG was in chronological order. Now it's in reverse chronological order.
xianny
reviewed
Aug 8, 2019
| @@ -446,8 +446,8 @@ def validate_and_normalize_inputs(self, _hash: bytes, v: int, r: bytes, s: bytes | |||
| def call(self, _hash: bytes, v: int, r: bytes, s: bytes, tx_params: Optional[TxParams] = None) -> str: | |||
| """Execute underlying contract method via eth_call. | |||
|
|
|||
| test that devdocs will be generated and that multiline devdocs will | |||
| look okay | |||
| test that devdocs will be generated andthat multiline devdocs will look | |||
There was a problem hiding this comment.
looks like we need 1 space here!
xianny
reviewed
Aug 8, 2019
| @@ -246,11 +246,9 @@ def run(self): | |||
| "0x-contract-artifacts", | |||
| "0x-json-schemas", | |||
| "0x-order-utils", | |||
| "0x-web3", | |||
| "web3", | |||
xianny
suggested changes
Aug 8, 2019
xianny
approved these changes
Aug 8, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #2029
This PR started as simply a migration, but turned into a bit more.
First of all, the migration exposed a lack of a
cleancommand in thesra_clientpackage, which was causing headaches and which I've fixed here. Also, a recent change I made to build components in parallel simply doesn't make sense; they need to be built in dependency order, and I've fixed that here. And, it turns out that the dependency order stored in the monorepo scripts was mixed up, so I fixed it and also provided a script to automatically produce that dependency order by inspecting the packages' dependencies.The migration also turned up two problems with the SRA client tests. One of those problems is that currently Launch Kit Backend does not support EIP-55 checksummed addresses. This is a problem because Web3.py v5 enforces all addresses coming through its interfaces to be checksummed, which throws a kink in the SRA tests. I've raised 0xProject/0x-launch-kit-backend#73 to track this problem, and @dekz has created a custom build (docker image, used in CI here) that allows my tests to post an order with a checksummed address, but because retrieving an order still conveys a
toLower()ed address, the SRA example has been temporarily modified to checksum the maker address in the order retrieved from launch kit before it proceeds to fill that order.The other problem with the SRA tests is with the migration to the new Launch Kit Backend docker image. Previously these tests were using
0xorg/launch-kit-ci, but that was a one-off thing created just for CI, back before there was a regularly maintained docker image of Launch Kit. In this PR I changed the setup to use0xorg/launch-kit-backendinstead, since it's regularly maintained/updated. However, there is an outstanding problem with this setup in CircleCI, which I will need to raise with their support team. If you look at the commit where I changed docker images, you'll see that there are two places the image has changed: in.circleci/config.yml, and inpython-packages/sra_client/test/relayer/docker-compose.yml. Note that both before and after that commit, those two files have an effectively identical configuration. However, while the newdocker-compose.ymlfile works great for local development, the.circleci/config.ymlfile is simply not working. The docker image never produces any output in the build run, and the tests time out trying to connect to the relayer endpoint. For this reason, this PR disables the SRA client tests for CI. (They remain enabled for local development.)Finally, I happened to notice some problems with the generated docs for the contract wrappers, which I've also fixed in this PR. The latest build of the contract wrapper documentation is here. The problems were that (a) we weren't generating docs for all of the wrappers (just Exchange and ERC20Token), and (b) the docs weren't including the contract wrappers' member variables, which are the instantiated, named method classes.
UpdatePunted on this, and raised EIP-55 checksummed addresses not supported 0x-launch-kit-backend#73 .launch-kit-cidocker image with recent changes to JSON schema foraddressdata type. Web3.py 5.x is now handling all addresses with checksum formatting, and the currentlaunch-kit-ciimage is still using the old JSON schema which only allows lower-case hex digits in an address.