Unable to use try/catch to catch local reverts in extra code generated for high-level external calls

[drortirosh]

How can we call an external code, and reliably catch errors?

Consider the following code, which tries to create "safeBalance" method, which calls balanceOf and never reverts.
(spoiler: try/catch doesn't catch a lot of cases)

• It returns a proper balance of an external token
• It properly catches revert in the external balanceOf method

But....

• it crashes if calling non-existent address (e.g. address(0))
• it crashes if the target contract doesn't have that method.
• it crashes if the target contract returns wrong number of arguments.

So basically, we can't rely on try/catch ...

The only alternative is to resort to low-level call (address.call()) and manually parse the result - in realSafeBalance()
This solution is error-prone, type-unsafe and more expensive in its gas usage.

pragma solidity ^0.8.17;
//SPDX-License-Identifier: MIT

interface IERC20 {
  function balanceOf(address) external returns (uint);
}

contract ATestSafeBalance {

    event Debug(uint bal);
    constructor () {
        IERC20 a;
        //  a = IERC20(address(this));
        //  a = IERC20(address(0));
        // a = new Token();
        //  a = new RevertToken();
         a = IERC20(address(new NoReturnValue()));
        uint bal = pseudoSafeBalance(a,address(this));
        emit Debug(bal);
    }

    function pseudoSafeBalance(IERC20 token, address addr) public returns (uint) {
        try token.balanceOf(addr) returns (uint ret) {
            return ret;
        }
        catch {
            return 11111;
        }
    }

    function realSafeBalance(IERC20 token, address addr) public returns (uint retBalance) {
       (bool success, bytes memory ret) = address(token).call(abi.encodeCall(IERC20.balanceOf, addr));
       if (!success || ret.length != 32) return 11111;
       (retBalance) = abi.decode(ret, (uint));
    }
}

contract NoReturnValue {
  function balanceOf(address) external {
  }
}

contract RevertToken is IERC20 {
  function balanceOf(address) external override returns (uint) {
      revert("just because");
  }
}

contract Token is IERC20 {
  function balanceOf(address) external override returns (uint) {
      return 1;
  }
}

[drortirosh]

This issue is a major use-case of the #12725 issue.
I do think this issue requires language support and not require developers to resort to assembly.

[chriseth]

I think we should extend the try/catch statement in the following way (which is backwards-compatible):

Currently, we allow the following catch statements:

• catch ErorrName(...) {}
• catch (bytes memory ...) {}
• catch {}

We can easily extend these to capture the two additional cases of "contract does not exist" and "abi decoding failure" by adding more catch statements:

• catch NoContract {}
• catch DecodingFailure {}
  (note the missing parentheses, they are required for custom errors)

In order to make this easier to use and also backwards-compatible, we could also add

• catch Other {}
  That one would be invoked if there is any kind of failure (including non-existing contract, abi-decoding failure of error data and of return data), but there is no clause that would match for that failure.

In the end, this could at some point maybe be replaced by some kind of destructuring / match expression, and both NoContract and DecodingFailure could be specific identifiers that need to be defined in the standard library.

[cameel]

Here's my proposal from the call, adapted a bit to cover also all @chriseth is proposing above:

• Allow using external after catch to make it very clear that it only catches reverts from external calls:
  • catch external ErorrName(...) {}
  • catch external (bytes memory ...) {}
  • catch external {}
• Introduce a new form that can catch the reverts performed by the current contract:
  • catch internal {}
• Optionally, allow distinguishing between different internal failures. Instead of introducing identifiers for them, we could instead implement Consider to encode certain reverts (e.g. abi decoding / input validation errors) as Error(uint256) with defined error codes #11664 and then use the error signature that they would revert with:
  • catch internal Error(0x01) {}
  • catch internal Error(0x02) {}
  • catch internal (bytes memory ...) {}
• Having names for these codes is not bad in itself but they would pollute the global namespace. Maybe instead we could also introduce enums in stdlib to cover them?
  • catch external Panic(PanicCode.Assert) {}
  • catch internal Error(ErrorCode.DecodingFailure) {}
  • or maybe even catch internal ErrorCode.DecodingFailure {}?
• In a breaking release disallow all catch forms without explicit internal or external.
  • Or we could make it catch both kinds.
  • Consider adding deprecation warnings for catch without internal/external.

[hrkrshnn]

I think we should avoid extending try-catch with new annotations--try-catch is already probably the most confusing part of Solidity.

I would rather prefer re-adding the extcodesize check for try I(...).f() in all cases and going into catch all than further extending try-catch.

---

Alternatively, we should create a primitive that allows completely getting rid of try-catch. One such primitive could be try_decode for decoding abi routines. This way, a user can explicitly reason about what would be caught.

[cameel]

I would rather prefer re-adding the extcodesize check for try I(...).f() in all cases and going into catch all than further extending try-catch.

But that's breaking, isn't it? I think the idea of bringing it up now was to patch it to make it actually usable without breaking its current semantics.

Alternatively, we should create a primitive that allows completely getting rid of try-catch. One such primitive could be try_decode for decoding abi routines. This way, a user can explicitly reason about what would be caught.

That's a good idea too if we're fine deprecating try/catch. I don't think the proposals above are very extreme though. You have to be aware of distinction between "external" and "internal" reverts either way and the altered try/catch syntax wraps it in some nice syntax sugar.

[ekpyron]

Just generally: We can still decide to only change this for 0.9, doing anything here non-breakingly would be nice, but we shouldn't constrain ourselves by it. A "silent" change in behaviour for the default clause without any prior change or notice is harsh even for a breaking release though (by no means a no-go, but it is a heavy thing to do).

Something like try_decode is a separate issue we're not unlikely to do anyways (#10381), but it's not a good reason for deprecating try/catch.

[cameel]

We talked about try/catch in depth on the last design call. For anyone interested in the topic, I made a write-up based on what I took out of that discussion. It goes over the current problem and various design proposals we considered to address this and also several other related try/catch issues (#10381, #11886, #11278, #12654).

[call for feedback] The future of try/catch in Solidity.

The decision on the call was basically to wait for user feedback on these proposals and maybe implement the short term ones, but also keep in mind that in the long term we'll likely want to replace try/catch with generic language primitives once we have them.

[hellwolf]

I guess we still don't have any update on this in 2024?