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

Run ci 2795 #2916

Closed
wants to merge 6 commits into from
Closed

Run ci 2795 #2916

wants to merge 6 commits into from

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Apr 12, 2023

What was wrong?

Related to Issue #

How was it fixed?

Todo:

Cute Animal Picture

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

joseluu and others added 2 commits April 12, 2023 15:39
…lity with existing exception handlers, sending to a wrong selector leads to an error with no data, this should be a ContractLogicError as before
Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'm wondering if we can't catch the ContractCustomError at the Contract level, whether call or transact, and use the contract's ABI to decode the data for the defined custom error too...

That could maybe be an improvement for another time but any thoughts on that?

@kclowes
Copy link
Collaborator Author

kclowes commented Apr 14, 2023

@fselmo - I like that idea, and explored doing that for a little bit yesterday. I don't think it would be too complicated, but will take a some plumbing since we don't have the utility functions built out to handle custom error abi's yet. Our call function gets passed the function abi but we'd have to pull the error abi from the contract abi and match the 4byte identifier that comes back in the response. The other thing that would be sort of weird is that while it would be straightforward to plug into the contract's call method, if someone were to do a raw w3.eth.call, we don't have access to the contract at all, so I don't think we could parse the return value into something human-readable. But seeing as you have to call a contract function to get the custom error response and message, I think that would be a pretty small edge case. I'm going to add a separate issue to track returning the raw error data that comes back, because that sounds like it would be a useful thing. See this comment.

@kclowes kclowes closed this Apr 14, 2023
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.

3 participants