Could not decode contract function call {function_identifier} return data {return_data!r} for output_types {output_types}

[Uxio0]

• Version: 5.21.0 (detecting this since 5.20)
• Python: 3.9
• OS: linux

What was wrong?

Previously when a contract reached a Solidity require the message was returned in the exception. Currently I just see the placeholders.

Errors can be seen in our library (gnosis-py) tests:

• Bump web3 from 5.19.0 to 5.23.0 safe-global/safe-eth-py#43
• https://github.com/gnosis/gnosis-py/pull/43/checks?check_run_id=3058214761

When 5.19.0 version is used, no problems:

• https://github.com/gnosis/gnosis-py/runs/3058176262?check_suite_focus=true

Is this a bug or do I need to change anything? I couldn't find anything in the changelog

[fselmo]

@Uxio0 Thanks for reporting this! Something did change in the string interpolation here near 5.20... I'm going to check if any other files were affected and I'll submit a quick fix.

[fselmo]

@Uxio0 I didn't see any other affected files and I pushed a hotfix for this directly to master since it was quite a small change. It should be included in the next release but you can use master for now if this helps.

I'm going to close this. Feel free to re-open if you are still having issues.

[Uxio0]

Thanks for the quick response and fix @fselmo

[Uxio0]

Hi @fselmo , I've just tried the new release and there's still different behaviour. Revert messages are not decoded. For example, now I get:

Could not decode contract function call execTransaction return data HexBytes('0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000195369676e617475726573206461746120746f6f2073686f727400000000000000') for output_types ['bool']

If use str instead of repr I can see the revert reason Signatures data too short, that was the previous behaviour. As a fstring is used to raise the message is not even easy to extract the parameter from the exception unless using a regular expression

[Uxio0]

I think the issue is the !r in the string interpolation:

In [3]: str(HexBytes('0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000195369676e617475726573206461746120746f6f2073686f727400000000000000'))
Out[3]: "b'\\x08\\xc3y\\xa0\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00 \\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x19Signatures data too short\\x00\\x00\\x00\\x00\\x00\\x00\\x00'"

In [4]: repr(HexBytes('0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000195369676e617475726573206461746120746f6f2073686f727400000000000000'))
Out[4]: "HexBytes('0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000195369676e617475726573206461746120746f6f2073686f727400000000000000')"

[fselmo]

Hey @Uxio0 You are right that we can get the bytes returned without the !r (repr). Ideally we'd like to extract a better message out of this but putting the return data in the exception for now seems like a happy medium with all the possible parsing that might need to take place. I'm assuming that's what you are trying to do on your end?

If we were to return this for now, is this what you are looking for?:

b'\\x08\\xc3y\\xa0\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00 \\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x19Signatures data too short\\x00\\x00\\x00\\x00\\x00\\x00\\x00'

From what I gathered, you used to be able to extract the meaningful message out of the return_data and this broke with some recent string interpolation updates. If the above will fix your issue, I would be glad to remove the !r / repr() from the result_data and return the bytes instead.

Let me know! Sorry about this seemingly small thing that has taken a bit to resolve. Thanks for reporting on it!

[Uxio0]

I'm assuming that's what you are trying to do on your end?

That's right

If we were to return this for now, is this what you are looking for?:

Yes, that would work

From what I gathered, you used to be able to extract the meaningful message out of the return_data and this broke with some recent string interpolation updates. If the above will fix your issue, I would be glad to remove the !r / repr() from the result_data and return the bytes instead.

Yes, we have some logic to check for some messages inside the error string and return proper exceptions. I would be also ok if you pass the HexBytes as an argument of the exception, so I can extract it using exception.args[x], but removing !r would make everything like before and our code will work again. Even both things, removing !r and passing the HexBytes as a parameter of the exception would be awesome!

Thanks for your quick response and understanding!

[fselmo]

@Uxio0 sounds good... this was a breaking change that was never intended so thanks for the feedback. The only reason for the change (adding !r) was due to some dependency updates that led to linting errors. This section outlines why this is bad practice and one reason why this will eventually change in a future major release.

We also talked about improving our exception pattern in a future major release, since it will clearly introduce breaking changes. Ideally this will mean a more user-friendly way to extract meaningful information from these types of exceptions. Hopefully no more regex parsing on the user end will be needed!

Thanks again. This is merged to master and will be out in the next release... for now you may pull the latest master if that helps your team at all. Cheers.

[Uxio0]

@Uxio0 sounds good... this was a breaking change that was never intended so thanks for the feedback. The only reason for the change (adding !r) was due to some dependency updates that led to linting errors. This section outlines why this is bad practice and one reason why this will eventually change in a future major release.

Good to know! I was not aware.

We also talked about improving our exception pattern in a future major release, since it will clearly introduce breaking changes. Ideally this will mean a more user-friendly way to extract meaningful information from these types of exceptions. Hopefully no more regex parsing on the user end will be needed!

Nice! Thank you for everything. Cheers!

[fselmo]

@Uxio0 fyi, web3 v5.23.0 was just released and includes this change

[Uxio0]

Thank you very much @fselmo! Everything is working on our side now