`try`/`catch` doesn't catch reverts during decoding or inside the `try` block
- Dominant language
- C++
- Stars
- 25.7k
- Forks
- 6.2k
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 29
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
```solidity
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
Contributor guide
Research direction
Start by reproducing the supplied Solidity example and the described revert from return-value decoding. Then trace how the compiler handles try/catch boundaries, decoding, and statements inside the try block; the issue is done only when the intended behavior is defined and covered by regression tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, solidity
- Domain
- blockchain, compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100