Omniscia Native Audit

ConstantSumPricer Manual Review Findings

ConstantSumPricer Manual Review Findings

CSP-01M: Unfair Market Dynamics

TypeSeverityLocation
Mathematical OperationsConstantSumPricer.sol:L8-L17, L19-L27

Description:

The way the getAmountOut and getAmountIn implementations are designed indicates that the underlying assets should be equivalent in a one-to-one fashion, however, the system currently incentivizes getAmountIn conversions as the truncation uses a smaller denominator.

Impact:

As users are incentivized to trade using the getAmountIn method, the getAmountOut method and consequent trade path in the Router implementation will incur an unfair loss to its users.

Example:

contracts/ConstantSumPricer.sol
8function getAmountOut(
9 uint256 amountIn,
10 uint256 reserveIn,
11 uint256 reserveOut,
12 uint256 fee
13) public pure override returns (uint amountOut) {
14 uint256 amountInWithFee = amountIn * (10000 - fee);
15 amountOut = amountInWithFee / 10000;
16 require(amountOut <= reserveOut, "amountOut too large");
17}
18
19function getAmountIn(
20 uint256 amountOut,
21 uint256 reserveIn,
22 uint256 reserveOut,
23 uint256 fee
24) public pure override returns (uint amountIn) {
25 require(amountOut <= reserveOut, "amountOut too large");
26 amountIn = amountOut * (10000) / (10000 - fee);
27}

Recommendation:

We advise the fee to be applied identically to getAmountOut in getAmountIn, applying it to the amountOut value as the amountIn and amountOut assets are meant to be equivalent in value.

Alleviation:

The calculations now make use of the FullMath::mulDivRoundingUp function, ensuring that the calculations are rounding in the correct direction on each operation and alleviating this exhibit.