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

Migrate Python libraries to v3 #2284

Merged
merged 30 commits into from
Nov 6, 2019
Merged

Migrate Python libraries to v3 #2284

merged 30 commits into from
Nov 6, 2019

Conversation

feuGeneA
Copy link
Contributor

@feuGeneA feuGeneA commented Oct 23, 2019

Description

Fixes #2163 .

Latest build artifacts (for commit e472cee on Nov 4 at 9:51 PM EDT):

(If you're interested in reviewing generated wrappers for any other contracts, or docs for any other packages, let me know and I can dig up the direct links from the Circle CI runs for you.)

Testing instructions

0x-monorepo$ PKG=@0x/abi-gen yarn build
0x-monorepo/python-packages$ ./pre_install # generates contract wrappers, populates contract artifacts & json schemas
0x-monorepo/python-packages$ ./install
0x-monorepo/python-packages/sra_client$ ./setup.py start_test_relayer
0x-monorepo/python-packages$ ./parallel ./setup.py test
0x-monorepo/python-packages$ ./build_docs

Checklist:

Finish these in this PR:

  • Get existing tests & examples working with v3 contracts.
  • Change zero_ex.order_utils.sign_hash() to accept either a provider OR a web3 instance, so that a user can install a middleware (such as one to pre-sign messages) before invoking it.
  • Change generated contract wrappers to accept either a web3 object or a provider, so that people can install a local-signing middleware to be used by contract transactions.
  • Change contract's installation of rich-revert-handling middleware to not care if there's an error re-inserting it again. ValueError("You can't add the same un-named instance twice") is currently being raised if you instantiate an Exchange wrapper twice in the same Python process.
  • Clean up "web3 has no such attribute" # type: ignores. Need to expand mypy stubs rather than disabling linter checks in the code.
  • Migrate SRA client to v3.
  • Add new entries to the relevant CHANGELOG.jsons.

Raise new Issues for these discovered items so they can be addressed in a subsequent PR:

@coveralls
Copy link

coveralls commented Oct 23, 2019

Coverage Status

Coverage remained the same at 75.286% when pulling 19fac92 on feat/3.0/python into 3c6c412 on development.

@buildsize
Copy link

buildsize bot commented Oct 23, 2019

File name Previous Size New Size Change
init.py 7.04 KB 26.91 KB 19.87 KB (282%)
abi_gen_dummy.ts 184.68 KB [deleted]
lib_dummy.ts 5.23 KB [deleted]
test_lib_dummy.ts 14.8 KB [deleted]
environment.pickle [new file] 1.61 MB
index.doctree [new file] 193.72 KB
.buildinfo [new file] 230 bytes
genindex.html [new file] 5.6 KB
index.html [new file] 2.52 KB
objects.inv [new file] 375 bytes
py-modindex.html [new file] 3.07 KB
search.html [new file] 2.84 KB
searchindex.js [new file] 6.02 KB
index.rst.txt [new file] 415 bytes
alabaster.css [new file] 10.92 KB
basic.css [new file] 11.89 KB
custom.css [new file] 42 bytes
doctools.js [new file] 9.05 KB
documentation_options.js [new file] 303 bytes
file.png [new file] 286 bytes
jquery-[version].js [new file] 273.79 KB
jquery.js [new file] 86.08 KB
language_data.js [new file] 10.59 KB
minus.png [new file] 90 bytes
plus.png [new file] 90 bytes
pygments.css [new file] 4.69 KB
searchtools.js [new file] 15.61 KB
underscore-[version].js [new file] 34.34 KB
underscore.js [new file] 11.86 KB
contract_addresses.html [new file] 16.86 KB
contract_artifacts.html [new file] 8.24 KB
json_schemas.html [new file] 12.56 KB
order_utils.html [new file] 46.86 KB
asset_proxy_owner.html [new file] 350.44 KB
coordinator.html [new file] 133.77 KB
coordinator_registry.html [new file] 39.72 KB
dev_utils.html [new file] 525.67 KB
dutch_auction.html [new file] 59.6 KB
erc1155_mintable.html [new file] 288.23 KB
erc1155_proxy.html [new file] 129.7 KB
erc20_proxy.html [new file] 111 KB
erc20_token.html [new file] 93.31 KB
erc721_proxy.html [new file] 111.12 KB
erc721_token.html [new file] 148.29 KB
eth_balance_checker.html [new file] 24.65 KB
exchange.html [new file] 678.51 KB
forwarder.html [new file] 110.46 KB
i_asset_proxy.html [new file] 39.32 KB
i_validator.html [new file] 30.37 KB
i_wallet.html [new file] 27.63 KB
multi_asset_proxy.html [new file] 148.59 KB
order_validator.html [new file] 120.52 KB
static_call_proxy.html [new file] 39.44 KB
tx_params.html [new file] 9.41 KB
weth9.html [new file] 133.98 KB
zrx_token.html [new file] 111.93 KB
local_message_signer.html [new file] 15.07 KB
asset_data_utils.html [new file] 22.65 KB
types.html [new file] 8.68 KB
default_api.html [new file] 118.3 KB

@feuGeneA feuGeneA force-pushed the feat/3.0/python branch 5 times, most recently from 391a69f to 27e758a Compare October 25, 2019 21:41
These should have been added back when we started generating these
wrappers.
All of the contract artifacts were removed from the Python package
recently, because now they're copied from the monorepo/packages area as
an automated build step.  Somehow this one artifact slipped through the
cracks.
This was preventing the Exchange wrapper from ever importing its
validator!
- Capture stderr (and have it included in stdout) so that it doesn't
leak onto the console for commands that didn't actually fail.

- Include all error output in the Exception object (eliminate print
statement).
Newer versions care about this stuff.  Old versions didn't, and we don't
either.
@feuGeneA feuGeneA force-pushed the feat/3.0/python branch 5 times, most recently from 4102f93 to 13c5e4d Compare October 27, 2019 17:50
`bytes.fromhex(bytes.decode('utf-8')` is just plain wrong.  It would
work for some cases, but is not working when trying to fill orders with
the latest Exchange contract.
I swear the previous way was working before, but it wasn't working now,
so this fixes it.
@moodlezoup moodlezoup changed the base branch from 3.0 to development October 28, 2019 23:59
@feuGeneA feuGeneA requested a review from albrow as a code owner October 29, 2019 19:10
@feuGeneA feuGeneA force-pushed the feat/3.0/python branch 2 times, most recently from 1284fea to 47bfb72 Compare November 2, 2019 11:51
This allows easier consumption by other languages.  (Specifically, it
eliminates the overhead of keeping the Python addresses package in sync
with the TypeScript one.)
@feuGeneA feuGeneA changed the title Migrate Python libraries (except sra_client) to v3 Migrate Python libraries to v3 Nov 4, 2019
Copy link
Contributor

@PirosB3 PirosB3 left a comment

Choose a reason for hiding this comment

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

Sorry @feuGeneA I forgot to press the Submit button

.circleci/config.yml Show resolved Hide resolved
packages/abi-gen/templates/Python/contract.handlebars Outdated Show resolved Hide resolved
packages/contract-addresses/addresses.json Show resolved Hide resolved
packages/contract-addresses/src/index.ts Show resolved Hide resolved
python-packages/contract_addresses/setup.py Outdated Show resolved Hide resolved
Removed script that existed only to exclude runs of sra_client builds
(parallel_without_sra_client).  Now `parallel` is used by CI,
re-including sra_client in CI checks.
Copy link
Contributor

@PirosB3 PirosB3 left a comment

Choose a reason for hiding this comment

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

Sounds like you have addressed some of the changes that I suggested. I'm happy to sign off, but it's up to you if you would also rather have some second reviewer, given the size of this PR

... ),
... makerFeeAssetData='0x',
... takerFeeAssetData='0x',
... chain_id=Web3(ganache).eth.chainId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is chain_id really assignable to an Order?

When I run mypy on a file that looks like this:

order = Order(
    makerAddress='0x',
    takerAddress='0x0000000000000000000000000000000000000000',
    senderAddress='0x0000000000000000000000000000000000000000',
    feeRecipientAddress='0x0000000000000000000000000000000000000000',
    makerAssetData='0x',
    takerAssetData='0x',
    salt=1,
    makerFee=0,
    takerFee=0,
    makerAssetAmount=1,
    takerAssetAmount=1,
    expirationTimeSeconds=1,
    makerFeeAssetData='0x',
    takerFeeAssetData='0x',
    chain_id=42
)

I get the complaint:

error: Extra key 'chain_id' for TypedDict "Order"

Note, this is using contract-wrappers 2.0.0.dev9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @steveklebanoff ! Fixed in 19fac92

@feuGeneA feuGeneA requested a review from PirosB3 November 5, 2019 22:23
@feuGeneA feuGeneA merged commit e61f23d into development Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Python tools for v3
4 participants