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.