Skip to content

feat: Add abi property for ContractFunction #3664

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

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

Conversation

tibor-reiss
Copy link

What was wrong?

Before: foo_contract.functions.setBar.abi["stateMutability"] == "nonpayable"
After: foo_contract.functions.setBar.state_mutability == "nonpayable"

Related to Issue #3626

How was it fixed?

Add state_mutability property.

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes
  • Add entry to the release notes

Cute Animal Picture

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

@tibor-reiss tibor-reiss force-pushed the i3626_add_abi_attributes branch from 72d1592 to a0cfd6d Compare April 17, 2025 06:30
@property
def state_mutability(self) -> Optional[str]:
if self.abi is None:
return None
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This or raise Web3TypeError?

@@ -596,6 +596,12 @@ def __init__(self, abi: Optional[ABIFunction] = None) -> None:
self.argument_names = tuple([input.get("name", None) for input in event_inputs])
self.argument_types = tuple([input["type"] for input in event_inputs])

@property
def state_mutability(self) -> Optional[str]:
if self.abi is None:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On l595, there is self.abi.get(...) which will fail if abi == None. Is this intended please? Because if we accept that abi cannot be None, I do not need this check here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout here, it is expected that the class instance will always have a value for abi. This works because the contract initialization uses the ContractFunction factory method. I think it should be fine to remove this check and implement better error handling in the __init__ function itself.

Right now, if someone attempts to create a ContractFunction() without passing an abi, it will throw an AttributeError. While tangential to your change, it would be better to check if self.abi is None and raise a Web3AttributeError at that time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work in __init__ - let me know please if I should include it in this PR:

if abi:
    self.abi = abi
if self.abi is None:
    raise Web3AttributeError

@tibor-reiss
Copy link
Author

tibor-reiss commented Apr 17, 2025

Hi @reedsa, is this implementation on the right track please? If yes, I can try to add the others, except payable, because that is deprecated according to https://github.com/ethereum/eth-typing/blob/main/eth_typing/abi.py#L78.

@reedsa
Copy link
Contributor

reedsa commented Apr 17, 2025

Hi @reedsa, is this implementation on the right track please? If yes, I can try to add the others, except payable, because that is deprecated according to https://github.com/ethereum/eth-typing/blob/main/eth_typing/abi.py#L78.

Looking good! Appreciate your contribution and it will be great to see all the other properties added.

@tibor-reiss tibor-reiss force-pushed the i3626_add_abi_attributes branch from a0cfd6d to 72e9265 Compare April 18, 2025 06:40
@tibor-reiss tibor-reiss force-pushed the i3626_add_abi_attributes branch from 72e9265 to 8675245 Compare April 18, 2025 06:46
@tibor-reiss tibor-reiss force-pushed the i3626_add_abi_attributes branch 2 times, most recently from 0df8cd1 to b16de95 Compare April 18, 2025 06:58
@tibor-reiss
Copy link
Author

tibor-reiss commented Apr 18, 2025

Hi @reedsa, is this implementation on the right track please? If yes, I can try to add the others, except payable, because that is deprecated according to https://github.com/ethereum/eth-typing/blob/main/eth_typing/abi.py#L78.

Looking good! Appreciate your contribution and it will be great to see all the other properties added.

Hi @reedsa, thanks for feedback, I added the non-deprecated attributes.

The failing tests work locally - could this be a CI problem?

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.

2 participants