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 TypeError in ContractFunction.call when args and kwargs properties are None #2545

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iwiwi
Copy link

@iwiwi iwiwi commented Jun 28, 2022

Fix #2541.

I confirmed that the example code in #2541 passes with this change.

If you want me to add related tests in the same PR, as I am new to this project, I would appreciate if you could give some hints, e.g., similar existing tests, where to put tests (a new file or existing file?), etc.

Cute Animal Picture

My cats 😺

Put a link to a cute animal picture inside the parenthesis-->

@kclowes
Copy link
Collaborator

kclowes commented Jun 29, 2022

Your cats are very cute!

We will need to add a test before merging. I don't really see an obvious home for where the test, but I think if you model it after the tests in tests/core/contracts/test_ambiguous_functions.py but put the test in tests/core/contracts/test_contract_call_interface.py that would be best. I haven't looked too closely, but it looks like there is a test that is similar to what you are running into, but the test is passing here. Can you tell why that test is passing but you're running into the TypeError? Thanks!

@iwiwi
Copy link
Author

iwiwi commented Jul 4, 2022

Thanks for useful pointers!

def test_contract_function_methods(string_contract):
set_value_func = string_contract.get_function_by_signature('setValue(string)')
get_value_func = string_contract.get_function_by_signature('getValue()')
assert isinstance(set_value_func('Hello').transact(), HexBytes)
assert get_value_func().call() == 'Hello'

The difference between my snippet and this test is that, in this test, ContractFunction.__call__ is called before calling ContractFunction.call. That is, if we change L183 from assert get_value_func().call() == 'Hello' to assert get_value_func.call() == 'Hello', it will cause the same type errors.

What do you think?

  • My opinion is that, even before calling ContractFunction.__call__, as it is indeed a ContractFunction instance, I think it should be callable. So I think we should apply my fix.
  • Another option is to consider this as a design. That is, we distinguish two kinds of ContractFunction: (1) instances before invoking __call__ (which we cannot call), and (2) instances after invoking __call__ (which we can call). We may improve error messages and documents.

@kclowes kclowes self-assigned this Apr 15, 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.

ContractFunction.call may fail as args and kwargs properties may be None
2 participants