search

sherlock-audit / 2024-02-jala-swap-judging

6 stars 4 forks source link

cheatcode - Users are Unduly Penalized with High Gas Costs for Transactions that Fail due to variable conditions #241

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

cheatcode

medium

Users are Unduly Penalized with High Gas Costs for Transactions that Fail due to variable conditions

Summary

Users are Unduly Penalized with High Gas Costs for Transactions that Fail due to variable conditions.

Vulnerability Detail

In the _addLiquidity function, assert is used to ensure that the optimal amount of tokenA to be added to the liquidity pool amountAOptimal does not exceed the desired amount amountADesired specified by the user.

This condition relies on external inputs amountADesired, amountBDesired, and the current state of the reserves and the result of a calculation JalaLibrary.quote. Because these factors can vary widely and are not invariant, using assert here can be inappropriate.

Impact

If the assert condition fails, it will consume all remaining gas, which can be particularly punitive for users, especially in high-gas-price environments.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaRouter02.sol#L53

assert(amountAOptimal <= amountADesired);

Tool used

Manual Review

Recommendation

Replace assert with require to improve user experience and enure users are not penalized with high gas costs for transactions that fail

require(amountAOptimal <= amountADesired, "Insufficient amountA desired");
nevillehuang commented 6 months ago

Invalid, this is at most a gas optoimization finding not valid based on sherlock rules

  1. Gas optimizations: The user/protocol ends up paying a little extra gas because of this issue.