Assessment reports>WOOFi Swap>Discussion>Throwing Panic is not recommended

Throwing Panic is not recommended

The receive hook for WooPPV2 uses assert to ensure that it does not accept native ETH from an unknown source:

receive() external payable {
    // only accept ETH from WETH or whitelisted external swaps.
    assert(msg.sender == WETH || isWhitelisted[msg.sender]);
}

However, in Solidity, a failed assert will throw a Panic exception. According to the Solidity docs,

Assert should only be used to test for internal errors, and to check invariants. Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix. Language analysis tools can evaluate your contract to identify the conditions and function calls which will cause a Panic.

So, to ensure compatibility with automated language-analysis tools and to better align with the standard for error types, this check should be replaced with a require, with or without a specific error message.

This issue has been acknowledged by WOOFI, and a fix was implemented in commit 39deb1cd.

Zellic © 2024Back to top ↑