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

abi-gen/Py: fix incorrect method return types and other small issues #2345

Merged
merged 18 commits into from
Nov 15, 2019

Conversation

feuGeneA
Copy link
Contributor

@feuGeneA feuGeneA commented Nov 15, 2019

Description

Fixes #2298

Testing instructions

It's all in CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Fix generated methods returning simple tuples rather than the dictionaries that their type hints suggest. Fixes Python contract wrapper methods return raw tuples #2298 . Fixed in 931a8f2
  • Add to .gitignore wrappers for staking and staking_proxy.
  • Change SRA client docs to just say "see our docker-compose.yml" instead of prescribing specific docker commands, since the prescriptions just end up falling out of sync with what we really use to test the examples. 7e91613
  • Fix: The call() interface to a method is currently stated to have return type like Union[X, Union[HexBytes, bytes]], when really it should just be X. b42d8e0
  • Fix doc for zero_ex.contract_wrappers.exchange.types explaining how it contains conversion routines. They used to be in that module, but those routines got moved to contract_wrappers.order_conversions. Move that doc to that module. 8c3008c
  • Add a new method to generated inheritors of zero_ex.contract_wrappers.bases.ContractMethod, buildTransaction(), which simply does a validation and delegation to the underlying web3.py implementation (just like our estimateGas() method currently does). 99a4794
  • Don't generate a send_transaction method for constant methods. b99396f
  • In abi-gen Move all Python handlebars into the new module created to hold them. 2de9637
  • Utilize new internalType field in ABI to know when tuples are Solidity structs vs when they're literal tuples, and use that information to give meaningful names to the generated tuple classes. 17b70cf
  • Fix: ContractMethod subclasses shouldn't have a validator parameter on their constructor when the underlying methods don't have any inputs. 58270e1
  • rename zero_ex.contract_wrappers.[GENERATED_CONTRACT].underlying_method to _underlying_method.
  • Update CHANGELOGs.

Previously, the routines `order_to_jsdict()` and `jsdict_to_order()`
were moved from contract_wrappers.exchange.types to
contract_wrappers.order_conversions.  However, the module-level
docstring describing those routines was accidentally left behind in
exchange.types.
test_cli:lint is meant to be a rudimentary test of the code generated by
abi-gen.  However, previously, this script was incorporated into `yarn
lint`, and in CircleCI `static-tests` runs independently of `build`.
Consequently, the runs of test_cli:lint were checking the OLD code,
which was previously generated and checked in to git, NOT the code
generated with the version of abi-gen represented by the git repo.  Now,
test_cli:lint happens during `yarn test` rather than `yarn lint`,
because `yarn test` IS dependent on `yarn build`.
Previously the send_transaction() interface included docstring
documentation for the return types of the contract method, but that
doesn't make any sense because send_transaction() returns a transaction
hash rather than any actual return values.
Previously, generated wrappers for contract methods were including type
hints that suggested that a call() (as opposed to a send_transaction())
might return either the underlying return type or a transaction hash.
This doesn't make sense because a call() will never return a TX hash.
Now, the type hint just has the return type of the underlying method.
Move all existing Python-related Handlebars helpers to the newly created
python_handlebars_helpers module.
No functionality is changed.  Sole purpose of this commit is to
facilitate an upcoming commit.
No functionality is changed.  Sole purpose of this commit is to
facilitate an upcoming commit.
Use the new `internalType` field on the `DataItem`s in the contract
artifact to give generated tuple classes a better name than just hashing
their component field names.
@buildsize
Copy link

buildsize bot commented Nov 15, 2019

File name Previous Size New Size Change
init.py 60.47 KB 60.25 KB -216 bytes (0%)
abi_gen_dummy.ts 112.85 KB [deleted]
lib_dummy.ts 7.04 KB [deleted]
test_lib_dummy.ts 12.09 KB [deleted]
environment.pickle 1.61 MB 1.61 MB 0 bytes (0%)
index.doctree 184.8 KB 184.11 KB -701 bytes (0%)
.buildinfo 230 bytes 230 bytes 0 bytes (0%)
genindex.html 5.6 KB 5.6 KB 0 bytes (0%)
index.html 2.52 KB 2.52 KB 0 bytes (0%)
objects.inv 375 bytes 375 bytes 0 bytes (0%)
py-modindex.html 3.07 KB 3.07 KB 0 bytes (0%)
search.html 2.84 KB 2.84 KB 0 bytes (0%)
searchindex.js 6.28 KB 5.98 KB -300 bytes (5%)
index.rst.txt 415 bytes 415 bytes 0 bytes (0%)
alabaster.css 10.92 KB 10.92 KB 0 bytes (0%)
basic.css 11.89 KB 11.89 KB 0 bytes (0%)
custom.css 42 bytes 42 bytes 0 bytes (0%)
doctools.js 9.05 KB 9.05 KB 0 bytes (0%)
documentation_options.js 303 bytes 303 bytes 0 bytes (0%)
file.png 286 bytes 286 bytes 0 bytes (0%)
jquery-[version].js 273.79 KB 273.79 KB 0 bytes (0%)
jquery.js 86.08 KB 86.08 KB 0 bytes (0%)
language_data.js 10.59 KB 10.59 KB 0 bytes (0%)
minus.png 90 bytes 90 bytes 0 bytes (0%)
plus.png 90 bytes 90 bytes 0 bytes (0%)
pygments.css 4.69 KB 4.69 KB 0 bytes (0%)
searchtools.js 15.61 KB 15.61 KB 0 bytes (0%)
underscore-[version].js 34.34 KB 34.34 KB 0 bytes (0%)
underscore.js 11.86 KB 11.86 KB 0 bytes (0%)
contract_addresses.html 16.8 KB 16.8 KB 0 bytes (0%)
contract_artifacts.html 8.24 KB 8.24 KB 0 bytes (0%)
json_schemas.html 12.55 KB 12.55 KB 0 bytes (0%)
order_utils.html 46.85 KB 46.85 KB 0 bytes (0%)
erc20_token.html 93.31 KB 93.54 KB 239 bytes (0%)
exchange.html 678.51 KB 718.37 KB 39.87 KB (6%)
tx_params.html 9.41 KB 9.41 KB 0 bytes (0%)
local_message_signer.html 15.07 KB 15.07 KB 0 bytes (0%)
asset_data_utils.html 22.65 KB 22.65 KB 0 bytes (0%)
default_api.html 113.16 KB 113.16 KB 0 bytes (0%)
asset_proxy_owner.html 350.44 KB 337.37 KB -13.06 KB (4%)
coordinator.html 133.77 KB 121.42 KB -12.35 KB (9%)
coordinator_registry.html 39.72 KB 39.24 KB -492 bytes (1%)
dutch_auction.html 59.6 KB 66.12 KB 6.53 KB (11%)
erc20_proxy.html 111 KB 109.06 KB -1.94 KB (2%)
erc721_proxy.html 111.12 KB 109.18 KB -1.94 KB (2%)
erc721_token.html 148.29 KB 150.19 KB 1.9 KB (1%)
forwarder.html 110.46 KB 121.63 KB 11.17 KB (10%)
i_asset_proxy.html 39.32 KB 40.17 KB 871 bytes (2%)
i_validator.html 30.37 KB 27.05 KB -3.32 KB (11%)
i_wallet.html 27.63 KB 24.88 KB -2.74 KB (10%)
multi_asset_proxy.html 148.59 KB 144.02 KB -4.57 KB (3%)
order_validator.html 120.52 KB 107.67 KB -12.85 KB (11%)
weth9.html 133.98 KB 132.08 KB -1.9 KB (1%)
zrx_token.html 111.93 KB 107.6 KB -4.33 KB (4%)
dev_utils.html 580.48 KB 491.09 KB -89.39 KB (15%)
types.html 8.68 KB 8.54 KB -138 bytes (2%)
erc1155_mintable.html 288.23 KB 276.5 KB -11.74 KB (4%)
erc1155_proxy.html 129.7 KB 130.37 KB 688 bytes (1%)
static_call_proxy.html 39.44 KB 34.02 KB -5.41 KB (14%)

Copy link
Contributor

@fabioberger fabioberger left a comment

Choose a reason for hiding this comment

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

LGTM once CI passes

@coveralls
Copy link

coveralls commented Nov 15, 2019

Coverage Status

Coverage decreased (-0.1%) to 75.612% when pulling 70de33b on fix/python/return-types into 9e3cc37 on development.

@feuGeneA feuGeneA merged commit df97b20 into development Nov 15, 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.

Python contract wrapper methods return raw tuples
3 participants