Open
Description
The problem:
try/catch construct was added to allow easily handle reverts when calling functions.
However, it gives false sense of security, since there are cases where reverts are "leaked" through
Example
The following code looks safe, since the call is covered by catch-all try/catch.
But in fact, it reverts
interface Istr {
function retstr() external view returns(string memory);
}
contract Test {
function asd() external {
try Istr(address(this)).retstr() returns(string memory ret) {
emit Debug(ret);
} catch {
emit Debug('reverted');
}
}
function retstr() external returns (uint) {
return 1;
}
event Debug(string mesg);
}Root cause
while try/catch does catch all reverts by the external call, it doesn't handle any local handling of the response.
The above example receives a "uint" return value, but tries to parse it as dynamic content (string), and the decoder reverts.
There are other cases where the user might use checked arithmetics between the try/catch, which assumes it get caught by "catch"
Suggested fix
- At a minimum, the return value decoding should not revert in a try/catch block. otherwise, try/catch can't be trusted when calling unknown contracts, and developers must resort to low-level
address.call(abi.encodeWithSelector(...))(and manually validate the returned structure before decoding)
In addition, the try/catch model should be updated:
- in case a user specifies a
catch {}block, it is his expectation that no revert will leak from the "try" block. - The entire code after the "try" until the "catch" should be "no-revert" - must not generate revert by the compiler itself.
- Instead of reverting, this code should jump to the
catch {}block - Implementing a full "stack unwinding" for revert handling seems unlikely for solidity, so instead I suggest:
- when compiling methods, mark each method as "may-revert" or "no-revert"
- Is case the try/catch block calls "may-revert" method, a compilation error should be generated:
cannot call method X from a try/catch block: the method might revert - the user should then alter his code and call that method outside the try/catch block, or add "unchecked" markers to that method