Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for ContractEvent.processLog() #2124

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

smishy05
Copy link

@smishy05 smishy05 commented Aug 29, 2021

What was wrong?

Related to Issue #1648

For the python code (code-1.py),

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 = "0x6a3d3c28e34b1c3e3813A29374D1CE49FbeC9200"
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)

ContractEvent.processLog() produced the error :

Traceback (most recent call last):
  File "code-1.py", line 22, in <module>
    ev = acontract.events.Increment.processLog(log_to_process)
  File "/home/shail_hash_98/anaconda3/lib/python3.8/site-packages/eth_utils/decorators.py", line 20, in _wrapper
    return self.method(objtype, *args, **kwargs)
  File "/home/shail_hash_98/web3.py/web3/contract.py", line 1176, in processLog
    return get_event_data(self.web3.codec, self.abi, log)
  File "cytoolz/functoolz.pyx", line 254, in cytoolz.functoolz.curry.__call__
  File "cytoolz/functoolz.pyx", line 250, in cytoolz.functoolz.curry.__call__
  File "/home/shail_hash_98/web3.py/web3/_utils/events.py", line 205, in get_event_data
    if event_abi['anonymous']:
TypeError: 'NoneType' object is not subscriptable

ContractEvents.abi belonging to the subclass (Increment) of the ContractEvents class need to be used and referred to before the Increment object is initialised.

How was it fixed?

By changing every self.abi in the ContractEvent class to self._get_event_abi() (this was the # 2 as mentioned in the issue.)

I did try # 1 for solving the issue mentioned in #1648 . So, this is what I did while adding @property to .abi

The __init__ looked like:

def __init__(self, *argument_names: Tuple[str]) -> None:

        if argument_names is None:
            # https://github.com/python/mypy/issues/6283
            self.argument_names = tuple()  # type: ignore
        else:
            self.argument_names = argument_names

        self.abi = self._get_event_abi()

and my @property function looked like:

@property
def abi(self):
	return self._abi

@abi.setter
def abi(self, value=None):
	self._abi = find_matching_event_abi(
            cls.contract_abi,
            event_name=cls.event_name)

and the processLog() remained the same:

@combomethod
def processLog(self, log: HexStr) -> EventData:
        return get_event_data(self.web3.codec, self._get_event_abi(), log)

After using this, I get the following error:

Traceback (most recent call last):
  File "code-1.py", line 24, in <module>
    ev = acontract.events.Increment.processLog(log_to_process)
  File "/home/shail_hash_98/anaconda3/lib/python3.8/site-packages/eth_utils/decorators.py", line 20, in _wrapper
    return self.method(objtype, *args, **kwargs)
  File "/home/shail_hash_98/web3.py/web3/contract.py", line 1196, in processLog
    return get_event_data(self.web3.codec, self.abi, log)
  File "cytoolz/functoolz.pyx", line 254, in cytoolz.functoolz.curry.__call__
  File "cytoolz/functoolz.pyx", line 250, in cytoolz.functoolz.curry.__call__
  File "/home/shail_hash_98/web3.py/web3/_utils/events.py", line 205, in get_event_data
    if event_abi['anonymous']:
TypeError: 'property' object is not subscriptable

I think if we try to integrate @property with this, even then a person will have to create an object of ContractEvent. Thus, using this solution doesn't seem viable to achieve the target mentioned in #1648. Thus, I used the solution provided in # 2 and replaced all the self.abi with self._get_event_abi() in ContractEvent class. Please let me know if I have gone wrong somewhere. (I have elaborated the issue with # 1 using an example in the next comment)

In addition to this, I wanted some advice for writing tests. I did go through the documentation. I am not sure about how I should write it. It would be great if you could elaborate a bit on this.

Many thanks!

@smishy05 smishy05 changed the title Fix for ContractEvent.processLog() #1648 Fix for ContractEvent.processLog() Aug 30, 2021
@smishy05
Copy link
Author

What is the issue with # 1 while solving?

Consider this code.

# using property class
class Celsius:
    def __init__(self, temperature=0):
        self.temperature = temperature

    def to_fahrenheit(self):
        return (self.temperature * 1.8) + 32

    # getter
    def get_temperature(self):
        print("Getting value...")
        return self._temperature

    # setter
    def set_temperature(self, value):
        print("Setting value...")
        if value < -273.15:
            raise ValueError("Temperature below -273.15 is not possible")
        self._temperature = value

    # creating a property object
    temperature = property(fget=get_temperature, fset=set_temperature)

human = Celsius()
print(human.temperature)
print(human.to_fahrenheit())
print(human.temperature)

This is the output I get

Setting value...
Getting value...
0
Getting value...
32.0
Getting value...
0

But, what we want in our case is human = Celsius and the code to run the same way (correct me if I am wrong here). So, when I make human = Celsius, the code looks like:

# using property class
class Celsius:
    def __init__(self, temperature=0):
        self.temperature = temperature

    def to_fahrenheit(self):
        return (self.temperature * 1.8) + 32

    # getter
    def get_temperature(self):
        print("Getting value...")
        return self._temperature

    # setter
    def set_temperature(self, value):
        print("Setting value...")
        if value < -273.15:
            raise ValueError("Temperature below -273.15 is not possible")
        self._temperature = value

    # creating a property object
    temperature = property(fget=get_temperature, fset=set_temperature)

human = Celsius
print(human.temperature)
print(human().to_fahrenheit())
print(human.temperature)
human.temperature = -300

and my output is:

<property object at 0x7fe9803dcb30>
Setting value...
Getting value...
32.0
<property object at 0x7fe9803dcb30>

Hence, what we get is a property object (instead of a float value). Thus, when we are using the # 1 solution, we end up getting property object which makes it possible to work. I guess if we wish to make this work, then we will have to make some tweaks in the api that calls the events.

I hope this justifies the reason for not using # 1 solution. Please let me know if I have gone wrong somewhere.

Many thanks!

@kclowes
Copy link
Collaborator

kclowes commented Aug 30, 2021

Thanks for the PR @smishy05, and for such a detailed justification! I am wrapping up some things on py-evm, but I'll take a look once that's finished.

@smishy05
Copy link
Author

Thanks a lot for the reply!

By the way, this is the output after running make lint

tox -e lint
lint create: /home/shail_hash_98/git_repos/web3.py/.tox/lint
lint installdeps: .[dev]
lint develop-inst: /home/shail_hash_98/git_repos/web3.py
lint installed: aiohttp==3.7.4.post0,alabaster==0.7.12,async-timeout==3.0.1,atomicwrites==1.4.0,attrs==21.2.0,Babel==2.9.1,backports.entry-points-selectable==1.1.0,base58==2.1.0,bitarray==1.2.2,blake2b-py==0.1.4,bleach==4.1.0,bump2version==1.0.1,bumpversion==0.6.0,cached-property==1.5.2,certifi==2021.5.30,chardet==4.0.0,charset-normalizer==2.0.4,click==8.0.1,colorama==0.4.4,configparser==3.5.0,contextlib2==21.6.0,cytoolz==0.11.0,distlib==0.3.2,docopt==0.6.2,docutils==0.16,eth-abi==2.1.1,eth-account==0.5.5,eth-bloom==1.0.4,eth-hash==0.3.1,eth-keyfile==0.5.1,eth-keys==0.3.3,eth-rlp==0.2.1,eth-tester==0.5.0b4,eth-typing==2.2.2,eth-utils==1.10.0,execnet==1.9.0,filelock==3.0.12,flake8==3.8.3,flaky==3.7.0,hexbytes==0.2.2,hypothesis==5.49.0,idna==3.2,imagesize==1.2.0,incremental==21.3.0,ipfshttpclient==0.8.0a2,isort==4.3.4,Jinja2==3.0.1,jsonschema==3.2.0,lru-dict==1.1.7,MarkupSafe==2.0.1,mccabe==0.6.1,mock==4.0.3,more-itertools==8.8.0,multiaddr==0.0.9,multidict==5.1.0,mypy==0.812,mypy-extensions==0.4.3,netaddr==0.8.0,packaging==21.0,parsimonious==0.8.1,pkginfo==1.7.1,platformdirs==2.2.0,pluggy==0.13.1,protobuf==3.17.3,py==1.10.0,py-ecc==4.1.0,py-evm==0.4.0a4,py-geth==3.5.0,py-solc==3.2.0,pycodestyle==2.6.0,pycryptodome==3.10.1,pyethash==0.1.27,pyflakes==2.2.0,Pygments==2.10.0,pyparsing==2.4.7,pyrsistent==0.18.0,pytest==4.6.11,pytest-asyncio==0.10.0,pytest-forked==1.3.0,pytest-mock==1.13.0,pytest-pythonpath==0.7.3,pytest-watch==4.2.0,pytest-xdist==1.34.0,pytz==2021.1,readme-renderer==29.0,requests==2.26.0,requests-toolbelt==0.9.1,rlp==2.0.1,semantic-version==2.8.5,six==1.16.0,snowballstemmer==2.1.0,sortedcontainers==2.4.0,Sphinx==3.5.4,sphinx-better-theme==0.1.5,sphinx-rtd-theme==0.5.2,sphinxcontrib-applehelp==1.0.2,sphinxcontrib-devhelp==1.0.2,sphinxcontrib-htmlhelp==2.0.0,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.3,sphinxcontrib-serializinghtml==1.1.5,toml==0.10.2,toolz==0.11.1,toposort==1.6,towncrier==18.5.0,tox==3.24.3,tqdm==4.62.2,trie==2.0.0a5,twine==1.15.0,typed-ast==1.4.3,typing-extensions==3.10.0.1,urllib3==1.26.6,varint==1.0.2,virtualenv==20.7.2,watchdog==2.1.5,wcwidth==0.2.5,-e git+git@github.com:smishy05/web3.py.git@10328e35ae36548a0018a14c7094d04012ab1453#egg=web3,webencodings==0.5.1,websockets==9.1,when-changed==0.3.0,yarl==1.6.3
lint run-test-pre: PYTHONHASHSEED='1966815361'
lint run-test: commands[0] | flake8 /home/shail_hash_98/git_repos/web3.py/web3 /home/shail_hash_98/git_repos/web3.py/ens /home/shail_hash_98/git_repos/web3.py/ethpm /home/shail_hash_98/git_repos/web3.py/tests --exclude /home/shail_hash_98/git_repos/web3.py/ethpm/ethpm-spec
lint run-test: commands[1] | isort --recursive --check-only --diff /home/shail_hash_98/git_repos/web3.py/web3/ /home/shail_hash_98/git_repos/web3.py/ens/ /home/shail_hash_98/git_repos/web3.py/ethpm/ /home/shail_hash_98/git_repos/web3.py/tests/
Skipped 30 files
lint run-test: commands[2] | mypy -p web3 -p ethpm -p ens --config-file /home/shail_hash_98/git_repos/web3.py/mypy.ini
Success: no issues found in 174 source files
___________________________________ summary ____________________________________
  lint: commands succeeded
  congratulations :)

After running tox locally, I get the following output (the summary):

====== 1767 passed, 35 skipped, 6 xfailed, 181 warnings in 355.97 seconds ======
___________________________________ summary ____________________________________
  py38-core: commands succeeded
  congratulations :)

I am unable to understand why two of the tests failed. It would be great if you could help in this case!

@carver
Copy link
Collaborator

carver commented Aug 30, 2021

It could be nice to have it be a property still, so that ContractEvent.abi is accessible. (In order do make a class-level property, we could use a metaclass, like: https://stackoverflow.com/a/1800999/8412986 ). But I don't have an objection to this approach for now, it ought to solve the problem.

@smishy05
Copy link
Author

Hi @carver

Thanks a lot for the resource.

Should I move all the functions of the ContractEvent class to MetaContractEvent or should it be only the abi property that has to be in the metaclass?

@smishy05
Copy link
Author

smishy05 commented Aug 31, 2021

Hi @carver and @kclowes . After considering the advice from @carver , the code looks like this (please let me know if I have made a mistake)

class MetaContractEvent(type):
    """Base class for contract events

    An event accessed via the api contract.events.myEvents(*args, **kwargs)
    is a subclass of this class.
    """
    address: ChecksumAddress = None
    event_name: str = None
    web3: 'Web3' = None
    contract_abi: ABI = None
    abi: ABIEvent = None

    def __init__(self, *argument_names: Tuple[str]) -> None:

        if argument_names is None:
            # https://github.com/python/mypy/issues/6283
            self.argument_names = tuple()  # type: ignore
        else:
            self.argument_names = argument_names

        self._abi = None

    @property
    def abi(cls):
        return cls._abi

    @abi.setter
    def abi(cls, value):
        cls._abi = find_matching_event_abi(
            cls.contract_abi,
            event_name=cls.event_name)

    # Rest all contents of this is same as it was in the ContractEvent initially

class ContractEvent(metaclass=MetaContractEvent):
    pass

I am still getting the error:

Traceback (most recent call last):
  File "code-1.py", line 24, in <module>
    ev = acontract.events.Increment.processLog(log_to_process)
  File "/home/shail_hash_98/anaconda3/lib/python3.8/site-packages/eth_utils/decorators.py", line 20, in _wrapper
    return self.method(objtype, *args, **kwargs)
  File "/home/shail_hash_98/git_repos/web3.py/web3/contract.py", line 1186, in processLog
    return get_event_data(self.web3.codec, self.abi, log)
  File "cytoolz/functoolz.pyx", line 254, in cytoolz.functoolz.curry.__call__
  File "cytoolz/functoolz.pyx", line 250, in cytoolz.functoolz.curry.__call__
  File "/home/shail_hash_98/git_repos/web3.py/web3/_utils/events.py", line 205, in get_event_data
    if event_abi['anonymous']:
TypeError: 'property' object is not subscriptable

@carver
Copy link
Collaborator

carver commented Aug 31, 2021

@kclowes up to you what you want to do. I'm fine with just leaving the PR as-is.

@smishy05
Copy link
Author

smishy05 commented Sep 1, 2021

@carver thanks a lot for the reply!

@kclowes please do let me what I should do and also, please let me know if I can do something for the two failings.

Many thanks!

@smishy05
Copy link
Author

smishy05 commented Sep 6, 2021

Hi @kclowes , please let me know what improvements I need to make in the code so that the PR can be merged.

@kclowes
Copy link
Collaborator

kclowes commented Dec 17, 2021

Hi @smishy05, I'm sorry I missed this a few months ago. In regards to testing, you'd probably want to add something in tests/core/contracts/test_extracting_event_data.py. An example of how we test processLog can be seen here. A good test could be to copy/paste the test_event_rich_log and make sure the test still passes if you make event_instance = emitter.events[event_name]. Let me know if that doesn't make sense. Thanks again for all the work!

@tudorelu
Copy link

I think the initial TypeError can be avoided by adding () after Increment:

ev = acontract.events.Increment().processLog(log_to_process)

@kclowes kclowes self-assigned this Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants