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

#1002 Handle error when interacting with a bad contract #3194

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

Conversation

MRLab12
Copy link

@MRLab12 MRLab12 commented Jan 19, 2024

What was wrong?

When a contract error happens during a call, usually we let the eth-abi exception raise through. This makes users think that web3.py is broken.

Related to Issue #1002

How was it fixed?

A ContractLogicError is now raised in call_contract_function when a contract has no code.

Todo:

@MRLab12
Copy link
Author

MRLab12 commented Jan 19, 2024

Created this draft PR to initiate a conversation on this fix.

I was able to reproduce the error when connecting to a Base Sepolia test wallet and setting up an invalid contract address. In order to not have the test actually connect to the network, I have mocked the functionalities.

This is my first contribution to this repo so any suggestions on contributing and code reviewing are welcome!

@MRLab12
Copy link
Author

MRLab12 commented Jan 19, 2024

The test_call_undeployed_contract was failing and I noticed this was returning the eth_abi.exceptions.InsufficientDataBytes and web3.exceptions.BadFunctionCallOutput errors, so maybe we just keep this one instead of the one I added.

@MRLab12 MRLab12 marked this pull request as ready for review January 22, 2024 20:31
@kclowes
Copy link
Collaborator

kclowes commented Jan 23, 2024

@MRLab12 Thanks for the PR! I haven't had a chance to look at this yet, but I hope to be able to check it out in the next few days.

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