ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
22.76k stars 5.64k forks source link

Try/catch can still error if Solidity's own pre-call or post-call checks fail; should jump to catch block instead #10311

Closed haltman-at closed 1 year ago

haltman-at commented 3 years ago

Description

(Note: I rewrote this issue after realizing it's more general than I originally thought.)

The try/catch construction is supposed to allow one to catch errors from external calls and handle them. But, Solidity also makes various checks of its own both before and after making an external call, and will error if these checks fail. If the external call in question has an associated try/catch, and an applicable catch block exists, then Solidity should jump to the catch block instead of erroring.

For post-call checks, which catch block to jump to should be determined the normal way, based on the data returned. (E.g., if no data was returned, you would jump to the catch (bytes memory x) block if it exists, and revert if not.)

For pre-call checks, you have to go based on what the returned data would be, I guess. Fortunately as best I can tell the only pre-call check Solidity makes is whether the called has any code. (I'm unclear why, it doesn't seem like this would save on gas when no ether is being transferred?) So in this case, if the check fails, you can say that the returned data would have been an empty hex"". So this is still handleable. I suspect if any other pre-call checks are ever added, they will likely also have this property, that if they can fail you can say the returned data would have been hex"".

Environment

Steps to Reproduce

Pre-call example

pragma solidity ^0.7.4;

contract TryTest {
  function run() public returns (bytes memory) {
    Called called;
    try called.test() returns (uint n) {
      return abi.encode(n);
    } catch (bytes memory x) {
      return x;
    }
  }
}

contract Called {
  function test() public returns (uint) {
    return 1;
  }
}

The run() function should return hex"", but instead it reverts.

Post-call example

pragma solidity ^0.7.4;

contract TryTest {
  function run() public returns (bytes memory) {
    Called called = new Called();
    try called.boom() returns (uint n) {
      return abi.encode(n);
    } catch (bytes memory x) {
      return x;
    }
  }
}

contract Called {
  function boom() public returns (uint) {
    selfdestruct(tx.origin);
  }
}

The run() function should return hex"", but instead it reverts.

haltman-at commented 3 years ago

Note: Significantly rewrote the original issue after determining it was more general than I originally thought.

chriseth commented 3 years ago

The design choice behind this is that if you reach the catch block, then the call reverted. Other reasons for failure might not lead to the catch block. This is in line with try-catch not being available on internal calls.

github-actions[bot] commented 1 year ago

Hi everyone! This issue has been closed due to inactivity. If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen. However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

cameel commented 1 year ago

Note that while this has been closed, we're aware the underlying problem did not disappear. We just have too many duplicate issues about this. Here's a recent one with updated syntax proposals that would address this: https://github.com/ethereum/solidity/issues/13869#issuecomment-1422879881. Feedback is welcome.