hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

GNU General Public License v3.0
0 stars 0 forks source link

missing revert reason messages #34

Open hats-bug-reporter[bot] opened 2 days ago

hats-bug-reporter[bot] commented 2 days ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xef009bd8fdcf6d6be29e78d75ba6432831992897b61af724fda562f4f3d66527 Severity: low

Description: Description

StableSwapRouter.sol has a couple of require statements without any message in case of revert. This will lead to confused users, not knowing what the reason for their failed tx is(bad UX). This might lead to user frustration, more support demand for Thorn, hard debbuging on why the failed txs failed(due to the missing revert messages)

    function exactInputStableSwap(
        address[] calldata path,
        uint256[] calldata flag,
        uint256 amountIn,
        uint256 amountOutMin,
        address to
    ) external payable nonReentrant returns (uint256 amountOut) {
    ...
        require(amountOut >= amountOutMin); <@ users wont know the exact reason why the tx reverted
    ...
}
    function _swap(address[] memory path, uint256[] memory flag) private {
        uint256 amountIn_;
        require(path.length - 1 == flag.length);<@ users wont know the exact reason why the tx reverted

Attack Scenario\

Attachments

  1. Proof of Concept (PoC) File

StableSwapRouter::_swap()and StableSwapRouter::exactInputStableSwap()

  1. Revised Code File (Optional)

Add the following revert reasons for the failed txs

    function _swap(address[] memory path, uint256[] memory flag) private {
        uint256 amountIn_;
        require(path.length - 1 == flag.length, "path.lenght != flag.length");
    function exactInputStableSwap(
        address[] calldata path,
        uint256[] calldata flag,
        uint256 amountIn,
        uint256 amountOutMin,
        address to
    ) external payable nonReentrant returns (uint256 amountOut) {
    ...
        require(amountOut >= amountOutMin, "amountOut < amountOutMin"); 
    ...
}
Ghoulouis commented 1 day ago

Not important enough

dinkrass commented 1 day ago

Hi @Ghoulouis,

How would the user know if his tx has failed due to a slippage error? There is no way currently, since there will be no info on the blockchain. And this info is important. That's why protocols like Curve and UniSwap put revert reason messages.

Imagine bots or other smart contracts integrating with the protocol. If they know the concrete reason for the failure they can retry the swap with bigger slippage immediately(even in the same tx, in case of contracts). Without the revert msg it's not possible to know the root cause of the failure.

That's why I reported it as Low