try/catch doesn't catch some call errors.

[drortirosh]

The following code is expected (by a developer using any other language) to catch failures related to the call.
But instead, it reverts.
This is due to the very confusing semantics of the "try/catch" construct in solidity: the catch block will only be called if the actual external code was reverted, but it might still revert in some compiler-generated code

pragma solidity ^0.8.12;

interface Xface {
    function func() external;
}

contract Test {
    function run() external {
        try Xface(address(0)).func()  {
            console.log('successfully called');
        } catch {
            console.log('catch everything');
        }
    }
}

Suggestion: solidity should not generate a "revert" code between the try and catch. Instead, it should jump into the catch block.

Currently, the following checks are done by solidity, and it reverts if any of these validation fails:

• if the called method is "void", it first performs extcodesize, to validate the target indeed has some code
• if the called method is expected to return something, it validates that returndatasize is not zero
• if there are returned values, it decodes them and of course validates they were encoded correctly.

[chriseth]

Do I understand you correctly that you find it confusing that the catch block is only executed when the call reverted? Would you want the catch block to be executed for a successful call to the function but a failure to decode the return data?

[drortirosh]

Yes. I think its confusing that the code that is covered syntactically by try/catch causes a revert.
The assumption of a developer is that this is a "safe-zone".
The purpose of try/catch is catch errors caused by things outside of the control of the developer and continue the normal path of the code.
With the current implementation, it is nearly impossible to achieve that, without resorting to low level calls and assembly - since the compiler explicitly generates "revert" calls before calling the external function (to handle "no-contract-at-target") and after the external function returns (when it fails to parse the response)
Note that both of these cases fall under "not under the contact developer's control" category - just like reverts in the called external function

I understand that you can't add a real try/catch, since the EVM only support reverting an external call.
But I DO expect the code-generator to "honor" the developer's intention that reverts inside the try/catch range will be captured.

[chriseth]

If you send ether to the called function in the try/catch, how do you distinguish the ether being gone or not in the catch clause?

The try/catch statement was not meant as something like a protection against malicious called contracts, it's rather a way to handle errors that come with reverts. Can you share a bit more about your use-case?

[drortirosh]

1. the major issue is a revert done by the compiler even before the call (target address is invalid), which obviously doesn't send any funds
2. even in the lesser case (called a function that returns a value, and failed to decode its return value) I think its better to treat it as "exception" and get into the "catch" block.
   If I really expect to transfer value in this call, i'd have to save my balance before the call, and compare the saved balance to current value inside the catch block:
   Yes, its not a nice solution - but that's the edge case.
   Currently, the major case (=target address is mis-configured) goes the wrong path, and require me (the developer) to use a low-level, error-prone "call" API instead of using try/catch block

[chriseth]

Can you please share a bit more about your use-case? It is difficult to discuss breaking invariants at such an abstraction level without any examples.

[drortirosh]

Here is a reference to our code, which is the implementation of "account abstraction" EIP-4337: https://bit.ly/3sMBirB
Some context: account-abstraction is a mechanism that "abstracts" EOA account as a wallet contract. specifically, the signature and nonce validations are done by the wallet itself.
The singleton "EntryPoint" calls the validateUserOp() method on the wallet contract.
Since we attempt to create a "batch" of multiple requests, we need to find out which (if any) causes the entire batch to fail.
For this purpose we wrap the request with try...catch, with the expectation to catch any failure case, and properly handle it (in our case, it is reverting with a custom "FailedOp" structure)

However, the current code is not enough, in case the target address is not a contract, it reverts.
We're very conscious to the gas cost, as this is an overhead of each request that goes through the system.

using address.call is not an option, since its neither safe, nor efficient (cost ~2000 gas more)

The only option we currently have is add an assembly code to do extcodesize(sender) just before calling the method - mimicking the code the compiler inserts when calling the method.

[chriseth]

Ok, from your example, I see that the problem is rather the excodesize check and not the decoding failure (as you already said above).
I think changing the behaviour of try/catch with respect to the extcodesize check could be a good change.

I'm less convinced to execute the catch block in case of a successful call and return data decoding. I think we should not change the invariant that if the catch code is executed, the call did not have any effect. Note that ether transfer is not the only state change that can happen and there are state changes that are impossible to detect.

[drortirosh]

I still think that it should be possible to "catch" (that is, not throw) inside the try...catch block
A possible way is have a maybe a different catch mode, e.g. using s different catch signature. At first I thought of Panic(), but it is not thrown on such cases, so maybe something like:

try func() 
catch DecodeError() {

}

I think that handling of decoding errors is highly required in complex contracts interactions.
Currently, developers are required to use the error-prone (and gas-inefficient) "address.call".

[chriseth]

@hrkrshnn noted that if we go to the catch block on excodesizecheck failure, we have to re-add the extcodesizecheck that was recently removed due to a decoding error resulting in a revert anyway #12204

[drortirosh]

I still think that its worthwhile handling the general case of "void" function: in that case, you already call extcodesize to validate it exists, and it should redirect to the global catch block (if there is one) instead of directly reverting.

[aathan]

Just to chime in here. I have a use case related to ERC165-like "light" interface checking. Basically, try/catch guards could be used to experimentally call an external contract function and act on the basis of which catch block executes. This would have many useful applications. One in particular would be to check if one of the transfer addresses in a token transfer appears to be something that smells like a particular kind of token wallet etc.

Of course, something as simple as:

    function respondsTo(address c, bytes4 sel) internal view returns(bool result) {
        assembly {
            let freeMemory := mload(0x40)
            mstore(freeMemory,sel)
            result := staticcall(gas(),c,freeMemory,4,0,0)
        }
    }

serves such a purpose for view functions that take no parameters. Perhaps Solidity does not want to encourage this kind of usage, but a gas-efficient mechanism that could query external contracts in this way seems like a good thing. I know more efficient dispatchers are being discussed, and this consideration might be one to include in that implementation.

[bogdoslavik]

Hello! I often have troubles detecting and catch calls to non contract accounts.

There is one simple example:

  function _getPairName(IUniswapV2Pair pair) 
  internal view returns (string memory) {
    try pair.name{gas : 3000}() returns (string memory name) {
      return name;
    } catch {}
    return _NONAME;
  }

If I face with some non-standart pair without name() method or it is not contract address - all my code reverts while I expect what it should catched in tr..catch block.