Skip to content

Latest commit

 

History

History
59 lines (43 loc) · 3.55 KB

049.md

File metadata and controls

59 lines (43 loc) · 3.55 KB

Ambitious Cotton Hyena

Medium

Rounding Directions can result in Taking the initial liquidity

Summary

Rounding to Ceil every time we buy or sell disVotes can result in selling with a price greater than the amount of buying.

Description

When calculating the cost of votes/disVotes to buy or sell we round to Ceil in case of disVotes.

ReputationMarket.sol#L1054-L1058

  function _calcCost( ... ) private pure returns (uint256 cost) {
    ...
    cost = positiveCostRatio.mulDiv(
      market.basePrice,
      1e18,
>>    isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil
    );
  }

In most cases Rounding to Ceil in case of disVotes and Floor in case of Votes should be OK. But the problem of rounding issues can result in selling with price greater than buying in some situations.

In case we bought a bunch of votes at a time like 4, 5, 6, ... where the total price we pay makes the last number end with 0, we will not pay an excess amount from rounding that case.

Since we multiply the basePrice by the costRatio, then divide by 1e18 in case of ending costRation by 0 this can make Rounding to Ciel process not occur as the number will end up being zero. This will make user not pay much when buying disVotes

Now in the case of selling where we sold only part of disVotes we bought the price of each can not end with 0, so when calculating the price the Rounding will go up, resulting in user taking more money. If the user sold the amount of disVotes he bought one by one, rounding direction will be to Ciel which will make the total amount taken when selling > the amount of buying results in taking a part of initial liquidity which should not occur according to README.

The contract must never pay out the initial liquidity deposited as part of trading. The only way to access those funds is to graduate the market.

The process can occur in the opposite direction for votes. Buying votes 1 by 1 where the rounding is Floor favors the user, and when selling them as one punch, in case the total value ends with zero, no Rounding effect will occur, which will result in taking money more than paid for these votes.

The Impact is not only excess wei/s is removed from the contract, breaking invariant. The initial liquidity can be zero in some situations, where markets created by admins do not check msg.value against creationCost.

ReputationMarket::_createMarket()

    uint256 creationCost = marketConfigs[marketConfigIndex].creationCost;

    // Handle creation cost, refunds and market funds for non-admin users
    if (!hasRole(ADMIN_ROLE, msg.sender)) {
      if (msg.value < creationCost) revert InsufficientLiquidity(creationCost);
      marketFunds[profileId] = creationCost;
      if (msg.value > creationCost) {
        _sendEth(msg.value - creationCost);
      }
    } else {
      // when an admin creates a market, there is no minimum creation cost; use whatever they sent
>>    marketFunds[profileId] = msg.value;
    }

So in case markets are created with Admins with 0 initial liquidity, this issue can result in the last withdrawal being unable to withdraw as subtracting MarketFunds this way will result in an underflow error.

Recommendations

Make rounding buy/sell dependent instead of vote/disVote dependent, where when buying we round UP and when selling we round DOWN, so that users pay much when buying and earn less when selling, this will ensure no money getting out of the contract.