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

Can't call into contract method taking an array of structs #1574

Closed
feuGeneA opened this issue Feb 1, 2019 · 7 comments · Fixed by #1615
Closed

Can't call into contract method taking an array of structs #1574

feuGeneA opened this issue Feb 1, 2019 · 7 comments · Fixed by #1615
Assignees
Labels

Comments

@feuGeneA
Copy link
Contributor

feuGeneA commented Feb 1, 2019

Expected Behavior

Be able to call Exchange.marketBuyOrders() without raising an exception.

Current Behavior

Making this call results in an exception: ValueError: No matching entries for 'tuple' in encoder registry. (Interestingly, this is the same exception given when there is no tuple support.)

Solution

The problem is that our fork of web3.py isn't supporting arrays of tuples. We need to modify web3._utils.abi.get_abi_inputs() to be able to detect arrays of tuples in addition to single tuples.

Steps to Reproduce

Add the following line to zero_ex.contract_demo.test.test_exchange.test_get_order_info():

    fill_results = FillResults(*exchange.call().marketBuyOrders([order], 1, b""))

Context

Discord user frak, working for CoinAlpha, contacted me about this problem. They are getting signedOrders from RadarRelay and trying to fill them using marketBuyOrders. They have a workaround, calling fillOrder multiple times.

But this problem is more general than just supporting marketBuyOrders. The problem affects all methods that take or return arrays of structs. Skimming our contract docs, that's about 10 methods on the Exchange contract.

@feuGeneA feuGeneA added the python label Feb 1, 2019
@feuGeneA feuGeneA self-assigned this Feb 1, 2019
@officialcryptomaster
Copy link

officialcryptomaster commented Feb 9, 2019

This is a big problem for market-making and making a python DEX using 0x (e.g. pydex). If it is not fixed, we will have to abandon our python market maker... I would like to lobby for this to be fixed as soon as possible.
Specifically, this bug is preventing calling batchFillOrKillOrders due to ValueError: No matching entries for 'tuple' in encoder registry

@benjiqq
Copy link

benjiqq commented Feb 9, 2019

@officialcryptomaster I want to signal my support here, so that we can grow the python ecosystem. apparently this relates to web3.py and hopefully can be fixed there

https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#batchfillorkillorders

@michaelhly
Copy link
Contributor

michaelhly commented Feb 9, 2019

I agree with everyone here. Can't execute anything on-chain with an array of SignedOrders in python right now.

@feuGeneA
Copy link
Contributor Author

feuGeneA commented Feb 9, 2019

To be clear, the place to implement this support is in the Ethereum foundation's web3.py library. We can implement it in our fork, to get things working sooner rather than later, but ultimately, in order to make life easy, we need full tuple support (including support for arrays of them) in the mainline web3.py library. See ethereum/web3.py#829

@officialcryptomaster
Copy link

@feuGeneA Any workaround you can suggest would be great because honestly this is a pretty major roadblock for the python eco-system and we are very motivated to have some sort of workaround as python web3 moves too slow...

@feuGeneA
Copy link
Contributor Author

@officialcryptomaster @michaelhly @benjyz I've released a new version of the 0x-web3 package on PyPI, which includes support for tuple arrays, and you can go ahead and use that, without waiting for resolution of this Issue (or the linked PR).

The PR does contain one additional useful change, adding to order utils a dependency on 0x-web3 rather than web3, which should help manage the dependencies better when first installing it.

@officialcryptomaster
Copy link

@feuGeneA This is great news! thank you!

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

Successfully merging a pull request may close this issue.

4 participants