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

minimal changes to allow for custom error decoding #2795

Merged
merged 5 commits into from
Apr 14, 2023

Conversation

joseluu
Copy link
Contributor

@joseluu joseluu commented Jan 26, 2023

What was wrong?

In case of reverting in a smart contract using the "custom error" solidity construct available since solidity 0.8.4 (https://blog.soliditylang.org/2021/04/21/custom-errors/) , the details of the error were not available to the python application code

Related to Issue #
#2793
#2336

How was it fixed?

defining a new type of exception: ContractCustomError
raising this exception along with the data allows the application code to take it into account

example code that can be used by an application to decode a failed tx_hash for a contract whose abi is in variable DT_abi:

from eth_utils.abi import function_abi_to_4byte_selector, collapse_if_tuple

def decode_custom_error(contract_abi, error_data):
    for error in [abi for abi in contract_abi if abi["type"] == "error"]:
        # Get error signature components
        name = error["name"]
        data_types = [collapse_if_tuple(abi_input) for abi_input in error.get("inputs", [])]
        error_signature_hex = function_abi_to_4byte_selector(error).hex()
        # Find match signature from error_data
        if error_signature_hex.casefold() == str(error_data)[2:10].casefold():
            params = ','.join([str(x) for x in w3.codec.decode(data_types,bytes.fromhex(str(error_data)[10:]))])
            decoded = "%s(%s)" % (name , str(params))
            return decoded
    return None #try other contracts until result is not None since error may be raised from another called contract 

def diagnose_error_if_needed(receipt):
    if receipt.status != 1:
        tx = w3.eth.get_transaction(receipt.transactionHash)
        # build a new transaction to replay:
        replay_tx = {
            'to': tx['to'],
            'from': tx['from'],
            'value': tx['value'],
            'data': tx['input'],
        }
        # replay the transaction locally:
        w3.eth.call(replay_tx, tx.blockNumber - 1)

try:
    diagnose_error_if_needed(w3.eth.waitForTransactionReceipt(tx_hash))
except web3.exceptions.ContractCustomError as e:
    print ('Custom Error: %s' % e)
    print (decode_custom_error(DT_abi,str(e)))
except web3.exceptions.ContractLogicError as e:
    print ('Logic Error: %s' % e)

Todo:

Cute Animal Picture

@kclowes
Copy link
Collaborator

kclowes commented Jan 30, 2023

Thanks for the PR @joseluu! We are focused on getting v6 stable out right now, so we don't have the bandwidth to review right now but please ping me if you haven't heard anything in ~2 weeks!

@joseluu
Copy link
Contributor Author

joseluu commented Mar 10, 2023

@kclowes gentle reminder

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

Thank you @joseluu! I started looking at this again. We'll need a test before we merge. I think the best thing to do is add a custom error to the existing revert contract. The source can be found under web3/_utils/contract_sources/RevertContract.sol, and then you'll need to recompile the contract, and put the bytecode and new ABI in web3/_utils/contract_sources/contract_data/revert_contract.py. Then some tests can be added in web3/_utils/module_testing/eth_module.py, similar to the test_eth_call_revert_with_msg/test_eth_call_revert_without_msg tests in that file. Let me know if you want me to take over, or if you have any questions! Thanks again!

@Shchepetov
Copy link

Hoping this PR can be merged soon
@joseluu Let me know if there's anything I can do to help

joseluu and others added 3 commits April 10, 2023 14:16
…lity with existing exception handlers, sending to a wrong selector leads to an error with no data, this should be a ContractLogicError as before
@kclowes
Copy link
Collaborator

kclowes commented Apr 12, 2023

@Shchepetov - does this PR fix your issue? As in - are you trying to get the raw data from a custom error revert? Or just a regular one?

@fselmo - would love to get your eyes/opinions here when you get a chance!

@kclowes
Copy link
Collaborator

kclowes commented Apr 13, 2023

I cherry-picked these commits to a new branch - #2916 - that I made (and therefore have CI permissions on), and CI is passing there.

@joseluu
Copy link
Contributor Author

joseluu commented Apr 14, 2023

Thanks @kclowes for filling-in the test requirements, I have had little time recently to devote for this.

@fselmo : interpreting the error using the current contract ABI is certainly an improvement, it will ease simple cases where the error is raised by the contract itself and this will help a lot debug cases where one needs the extra information. However, in the general case, the error may be raised by another contract down the call chain, in which case the unformatted error should be passed as it is now for the application code to deal with it.

@kclowes
Copy link
Collaborator

kclowes commented Apr 14, 2023

Thanks @joseluu!

Thanks @kclowes for filling-in the test requirements, I have had little time recently to devote for this.

No problem! Thanks for getting it rolling.

However, in the general case, the error may be raised by another contract down the call chain, in which case the unformatted error should be passed as it is now for the application code to deal with it.

Thanks for this info. It sounds like the ideal case is a option (maybe a flag passed to call) that can switch between retrieving the raw error and the decoded error. I'm going to merge this as-is, and I'll add an issue to figure out a nice way to do that in a separate PR.

edit: there are already a couple issues to track, I don't think I need to add yet another :)

@kclowes kclowes mentioned this pull request Apr 14, 2023
1 task
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

CI is passing on #2916, so this is okay to merge!

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