0xProject / 0x-monorepo

0x protocol monorepo - includes our smart contracts and many developer tools
Other
1.41k stars 466 forks source link

Refactor Python wrapper templates to use nested methods like TypeScript does #1893

Closed feuGeneA closed 4 years ago

feuGeneA commented 5 years ago

Current Behavior

Generated wrapper methods distinguish a call from a sendTransaction via a parameter to the contract wrapper method, eg erc20Wrapper.transferFrom(..., send_tx=True).

TypeScript wrappers have a different convention, more like erc20Wrapper.transferFrom.sendTransaction(...).

Expected Behavior

Python wrappers should follow the same convention as TypeScript.

Possible Solution

Context

This will be important in order to add other functionality besides just call vs. sendTransaction, from example estimateGas.

feuGeneA commented 5 years ago

@fabioberger Here's my latest thinking on this. Note the (doctested! :smile:) usage examples in the class docstring, showing the notation in action. As a demo it's far from polished but it does demonstrate the idea of using instances of named classes to provide the desired notation. And Python permits nested class definitions, so I took advantage of that.

Multi-line lambdas are simply not possible in Python, so declaring object literals with inline method definitions is not really possible. I explored several different obscure ways to try to model this, and this is the only one that really made any sense to me. It looks weird, but the weirdness isn't overly clever, and the weirdness is well encapsulated, and overall I think it's the lesser of many evils. We'll see what I think at the end of the weekend...

"""Demonstrate use of nested classes for dot-notation nested objects."""

class Exchange:  # pylint: disable=too-few-public-methods
    """Wrapper for the 0x Exchange contract.

    >>> exchange = Exchange('Web3.py')

    >>> exchange.fill_order.call('{makerAddress: ...}')
    Trace: inside FillOrderMethod.call({makerAddress: ...}). provider=Web3.py
    'fill results'

    >>> exchange.fill_order.send_transaction('{makerAddress: ...}')
    Trace: inside FillOrderMethod.send_transaction({makerAddress: ...}). provider=Web3.py
    'transaction_hash'
    """

    class FillOrderMethod:
        """A class interface to the variety of ways to call fillOrder."""

        def __init__(self, abi: str, provider: str, contract_address: str):
            """Persist instance data."""
            self.abi = abi
            self.provider = provider
            self.contract_address = contract_address

        def call(self, order: str):
            """Call fillOrder & return its return value but don't transact."""
            print(f"Trace: inside FillOrderMethod.call({order}). provider={self.provider}")
            return 'fill results'

        def send_transaction(self, order: str):
            """What goes here?"""
            print(
                f"Trace: inside FillOrderMethod.send_transaction({order}). "
                + f"provider={self.provider}"
            )
            return 'transaction_hash'

    fill_order: FillOrderMethod

    def __init__(self, provider: str):
        self.provider = provider

        self.fill_order = self.FillOrderMethod(
            "{Exchange.fillOrder ABI}", self.provider, "0xContractAddress"
        )
feuGeneA commented 5 years ago

@PirosB3 can you weigh in on my design in the comment above? Is there any other practical way to contrive this "nested method" notation? I considered things like dynamic attributes and namedtuples, but none of it seemed to play nice with type checking...

PirosB3 commented 5 years ago

Hi @feuGeneA it actually looks pretty nice. While I've seen several patterns in Django's source code that are similar, I'm confused as to why you would still want to nest the two classes together. Is there any way you could un-nest them instead? (could Exchange and FillOrderMethod be on the same level)?