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

Python contract wrapper methods return raw tuples #2298

Closed
feuGeneA opened this issue Oct 28, 2019 · 0 comments · Fixed by #2345
Closed

Python contract wrapper methods return raw tuples #2298

feuGeneA opened this issue Oct 28, 2019 · 0 comments · Fixed by #2345
Assignees

Comments

@feuGeneA
Copy link
Contributor

Expected Behavior

Contract method wrappers should return TypedDicts that match the Python method definitions.

Current Behavior

Contract method wrappers are returning raw tuples instead of typed dictionaries. For example, the Python definition for Exchange.fillOrder() is:

    def call(
        self,
        order: Tuple0x6ca34a6f,
        taker_asset_fill_amount: int,
        signature: Union[bytes, str],
        tx_params: Optional[TxParams] = None,
    ) -> Union[Tuple0x735c43e3, Union[HexBytes, bytes]]:
    ...

And Tuple0x735c43e3 is defined as:

class Tuple0x735c43e3(TypedDict):
...
    makerAssetFilledAmount: int
    takerAssetFilledAmount: int
    makerFeePaid: int
    takerFeePaid: int
    protocolFeePaid: int

Currently, executing a fillOrder eth_call returns a raw tuple:

>>> exchange.fill_order.call(
...     order=order,
...     taker_asset_fill_amount=order["takerAssetAmount"],
...     signature=maker_signature,
...     tx_params=TxParams(from_=taker_address)
... )
(100000000000000000, 100000000000000000, 0, 0, 0)

Whereas one would expect that return value to look more like:

>>> exchange.fill_order.call(...)
{'makerAssetFilledAmount': 100000000000000000,
 'takerAssetFilledAmount': 100000000000000000,
 'makerFeePaid': 0,
 'takerFeePaid': 0,
 'protocolFeePaid': 0}

Possible Solution

Modify @0x/abi-gen to have generated Python methods populate the TypedDict structure with the data return data tuple given by the method invocation. This will require changes to both the templates and the TypeScript helper code.

Steps to Reproduce (for bugs)

Just run a contract method that returns a struct and look at the return value.

Context

We're generating method wrappers to return structured, typed data, but we're not living up to those expectations we're setting.

@feuGeneA feuGeneA self-assigned this Oct 28, 2019
feuGeneA added a commit that referenced this issue Nov 15, 2019
feuGeneA added a commit that referenced this issue Nov 15, 2019
feuGeneA added a commit that referenced this issue Nov 15, 2019
…2345)

* .gitignore gen'd Python staking contract wrappers

* abi-gen/test-cli: check Python type hints in lint

* sra_client.py: Update doc for replicating examples

* abi-gen/Py: fix call() return type incl. tx hash

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.

* abi-gen: fix test_cli:lint checking wrong code

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

* contract_wrappers.py: fix misplaced doc

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.

* abi-gen/Py: stop documenting return types for TXs

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.

* abi-gen/Py: stop gen'ing send_tx for const methods

* abi-gen/Py: add build_tx to contract methods

* abi-gen/Py: fix incorrect method return types

Fixes #2298 .

* abi-gen/Py: rm validator arg to no-input methods

* abi-gen: mv Py Handlebars helpers to own module

Move all existing Python-related Handlebars helpers to the newly created
python_handlebars_helpers module.

* abi-gen: refactor internal interface

No functionality is changed.  Sole purpose of this commit is to
facilitate an upcoming commit.

* abi-gen: refactor internal interface

No functionality is changed.  Sole purpose of this commit is to
facilitate an upcoming commit.

* abi-gen/Py: name tuples w/internalType, not hash

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.

* Fix CI errors

* abi-gen/Py/wrapper: make internal member private

* Update CHANGELOGs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant