ethereum / web3.py

A python interface for interacting with the Ethereum blockchain and ecosystem.
http://web3py.readthedocs.io
MIT License
4.95k stars 1.69k forks source link

ContractEvent.processLog (and others?) break when used as classmethod #1648

Open carver opened 4 years ago

carver commented 4 years ago

What was wrong?

When ContractEvent.processLog() is called (instead of ContractEvent().processLog()), you get this:

trinity/components/eth2/eth1_monitor/eth1_data_provider.py:126: in <genexpr>
    self._deposit_contract.events.DepositEvent.processLog(log) for log in logs
venv-eth2-trio/lib/python3.6/site-packages/eth_utils/decorators.py:20: in _wrapper
    return self.method(objtype, *args, **kwargs)
venv-eth2-trio/lib/python3.6/site-packages/web3/contract.py:1163: in processLog
    return get_event_data(self.web3.codec, self.abi, log)
cytoolz/functoolz.pyx:254: in cytoolz.functoolz.curry.__call__
    ???
cytoolz/functoolz.pyx:250: in cytoolz.functoolz.curry.__call__
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

abi_codec = <eth_abi.codec.ABICodec object at 0x7fac05cae2e8>, event_abi = None
log_entry = {'address': '0xF2E246BB76DF876Cef8b38ae84130F4F55De395b', 'blockHash': HexBytes('0xdf2ab0b165b14772ed33090fb6a9c20dfbf...0000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000', ...}

    @curry
    def get_event_data(abi_codec: ABICodec, event_abi: ABIEvent, log_entry: LogReceipt) -> EventData:
        """
        Given an event ABI and a log entry for that event, return the decoded
        event data
        """
>       if event_abi['anonymous']:
E       TypeError: 'NoneType' object is not subscriptable

How can it be fixed?

The event ABI is only set on initialization, which doesn't make sense to me.

Maybe:

  1. make .abi a @property that caches the result of _get_event_abi()
  2. drop .abi altogether and use _get_event_abi() everywhere internally
  3. set .abi when ContractEvents builds all its event attrs

I think my preference is # 1

Most importantly: add a test showing that this is broken, and make sure the test works by seeing it fail before implementing the fix.

wolovim commented 4 years ago

Poking around this and your other couple issues now 👀

wolovim commented 4 years ago

Was able to reproduce this, but I'm unclear on expected behavior. Should ContractEvent.processLog() and ContractEvent().processLog() have identical output?

carver commented 4 years ago

Should ContractEvent.processLog() and ContractEvent().processLog() have identical output?

So, there seems to be some kind of half-baked feature to filter events by argument name. I don't really understand that use case, and it doesn't seem tested at the contract API level.

Maybe it's worth looking a little deeper about what how the argument_names input to the ContractEvent init method work. (and maybe removing it until there's a properly supported feature there?)

But yeah, my naive expectation was for ContractEvent.processLog() to work without instantiating the event. Without looking at the web3 API, I sort of expected an instantiated event to be like a bound function call, where the event would have some values attached, and you could do something like encode it from there. But that's not a current need I have.

smishy05 commented 3 years ago

Hey there, @carver ! Is this issue still open? I would love to contribute to this. Please let me know. Thanks!

carver commented 3 years ago

I believe so. A contribution would be great! I would ask Marc, but he's out right now. Maybe @kclowes knows for sure.

kclowes commented 3 years ago

@smishy05 Yep, it's still open! Feel free to open a PR and tag me if you need any direction! Thanks!

smishy05 commented 3 years ago

Thanks @carver and @kclowes! I am new to open source and I have been looking at other issues as well in web3py! Will open a PR in one which seems more reachable now.

Maybe:

1. make .abi a @property that caches the result of _get_event_abi()
2. drop .abi altogether and use _get_event_abi() everywhere internally
3. set .abi when ContractEvents builds all its event attrs
I think my preference is # 1

Most importantly: add a test showing that this is broken, and make sure the test works by seeing it fail before implementing the fix.

@carver - Are the three steps mentioned by you a good start for this?

kclowes commented 3 years ago

@smishy05 Those were options on how to solve the problem. So you'd want to pick one, write a test and then implement a fix. I'd go with option #1. Thanks!

smishy05 commented 3 years ago

Hey @carver and @kclowes !

I have gone through the contract.py and I was able regenerate the error.

This is the code that I used for getting the error.

from web3 import Web3
import json
json_file_path = "./abi.json"
with open(json_file_path, 'r') as j:
    contents = json.loads(j.read())
network_url = 'http://localhost:8545'
web3 = Web3(Web3.HTTPProvider(network_url))
address = "0x1f942eF89297cC2A0eF5e5FC9c52D617B3f34011"
acontract = web3.eth.contract(address=address, abi=contents)
tx_hash = acontract.functions.getValue(5, 10).transact({'from': web3.eth.accounts[0]})
receipt = web3.eth.get_transaction_receipt(tx_hash)
log_to_process = receipt['logs'][0]
ev = acontract.events.Increment.processLog(log_to_process)
print(ev)

I have migrated the contract using Truffle and used Ganache for deployment (I think this information is superfluous, however I have mentioned it if it has any significance). Hence, I wanted to ask if this code is fine?

What should be the parameter that would indicate if I have resolved the issue? That is, what should be expected from my side? Should the output of ContractEvent.processLog() and ContractEvent().processLog() be the same?

As suggested by @carver , do I have to simply add the @property that catches the result of _get_event_abi()? Also by the term caches the result, do you mean that _get_event_abi() is the getter function for the @property?

It would be great if you could clarify these doubts. Also, sorry for multiple questions!

Waiting for your response and many thanks!

smishy05 commented 3 years ago

Hi @carver and @kclowes !

I have opened a PR and I used # 2 for solving this issue. I have put the reason for not using # 1 for solving the issue. Please let me know if I have gone wrong somewhere.

Hoping to hear from you soon,

Many thanks!