Skip to content

Latest commit

 

History

History
201 lines (136 loc) · 6.19 KB

File metadata and controls

201 lines (136 loc) · 6.19 KB

Glamorous Canvas Camel

Medium

Rounding Errors in Price Calculation May Violate LMSR Invariant

Summary

The ReputationMarket contract is designed to maintain the LMSR (Logarithmic Market Scoring Rule) invariant that the sum of the prices of "trust" and "distrust" votes equals the basePrice. However, due to inconsistent and improper rounding in the _calcVotePrice function, this invariant can be violated. The function applies rounding methods that, in certain scenarios, cause the sum of the calculated prices to be less than 1.

Root Cause

Broken Invariant

From the contract's documentation:

Must maintain LMSR invariant (yes + no price sum to 1)


  • Incorrect Rounding in Price Calculation:

    The _calcVotePrice function calculates the price of trust and distrust votes using the odds multiplied by the basePrice. However, it applies rounding inconsistently and opposite to the intended way, as per the comments in the code.

https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L992C1-L1004C4

  function _calcVotePrice(Market memory market, bool isPositive) private pure returns (uint256) {
      // odds are in a ratio of N / 1e18
      uint256 odds = LMSR.getOdds(
          market.votes[TRUST],
          market.votes[DISTRUST],
          market.liquidityParameter,
          isPositive
      );
      // multiply odds by base price to get price; divide by 1e18 to get price in wei
      // round up for trust, down for distrust so that prices always equal basePrice
      return
          odds.mulDiv(
              market.basePrice,
              1e18,
              isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil
          );
  }
  • Inconsistent Rounding:

    • The comment says:

      "round up for trust, down for distrust so that prices always equal basePrice"

    • However, in the code:

      • For trust votes (isPositive == true), it uses Math.Rounding.Floor (rounds down).
      • For distrust votes (isPositive == false), it uses Math.Rounding.Ceil (rounds up).

    This is the opposite of what the comment indicates and what is needed to ensure the prices sum to basePrice.

  • Violation of Invariant Due to Rounding Errors:

    Because of the incorrect rounding, the sum of trustPrice and distrustPrice may become less than the basePrice. This violates the invariant that the sum must always equal basePrice.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

Because of the incorrect rounding, the sum of trustPrice and distrustPrice may become less than the basePrice. This violates the invariant that the sum must always equal basePrice.

PoC

Market Parameters:

  • trustVotes = 1000
  • distrustVotes = 2000
  • basePrice = 1 ether (1e18 wei)
  • liquidityParameter = 1000

Calculating Odds

Using the LMSR.getOdds function:

uint256 oddsTrust = LMSR.getOdds(
    trustVotes,          // 1000
    distrustVotes,       // 2000
    liquidityParameter,  // 1000
    true                 // isPositive = true for trust votes
);

uint256 oddsDistrust = LMSR.getOdds(
    trustVotes,          // 1000
    distrustVotes,       // 2000
    liquidityParameter,  // 1000
    false                // isPositive = false for distrust votes
);

Assuming getOdds function returns odds in UD60x18 format (scaled by 1e18). Suppose the calculated odds are:

  • oddsTrust = 268941421369995120 (approximately 0.2689 * 1e18)
  • oddsDistrust = 731058578630004880 (approximately 0.7310 * 1e18)

Calculating Prices

Using the _calcVotePrice function:

// Calculating trust price
uint256 trustPrice = oddsTrust.mulDiv(
    basePrice,             // 1 ether
    1e18,
    Math.Rounding.Floor    // Since isPositive is true, uses Floor (should be Ceil as per the comment)
);

// Calculating distrust price
uint256 distrustPrice = oddsDistrust.mulDiv(
    basePrice,             // 1 ether
    1e18,
    Math.Rounding.Ceil     // Since isPositive is false, uses Ceil (should be Floor as per the comment)
);

Values after Calculation:

  • Trust Price:

    trustPrice = (268941421369995120 * 1e18) / 1e18 = 268941421369995120 wei (no rounding needed)
  • Distrust Price:

    distrustPrice = (731058578630004880 * 1e18 + (1e18 - 1)) / 1e18 = 731058578630004881 wei (rounding up)

Summing Prices

uint256 sumPrices = trustPrice + distrustPrice;
// sumPrices = 268941421369995120 + 731058578630004881 = 999999999999999999 wei

Expected Sum:

  • basePrice = 1 ether = 1e18 wei

Difference:

  • basePrice - sumPrices = 1e18 - 999999999999999999 = 1 wei

Violation of Invariant

The sum of trustPrice and distrustPrice is less than basePrice by 1 wei, thereby violating the invariant that the sum must equal basePrice.

Cause of the Violation

  • The incorrect rounding directions applied in the _calcVotePrice function cause this discrepancy.
  • By rounding down for trustPrice and up for distrustPrice, the inevitable loss due to integer division accumulates, leading to a total sum less than basePrice.

Mitigation

To fix the issue and ensure the sum of the prices always equals basePrice, the rounding directions in the _calcVotePrice function should be corrected to match the intention stated in the comment:

  • Round up for trust votes and round down for distrust votes.

Corrected Code:

function _calcVotePrice(Market memory market, bool isPositive) private pure returns (uint256) {
    // odds are in a ratio of N / 1e18
    uint256 odds = LMSR.getOdds(
        market.votes[TRUST],
        market.votes[DISTRUST],
        market.liquidityParameter,
        isPositive
    );
    // multiply odds by base price to get price; divide by 1e18 to get price in wei
    // round up for trust, down for distrust so that prices always equal basePrice
    return
        odds.mulDiv(
            market.basePrice,
            1e18,
            isPositive ? Math.Rounding.Ceil : Math.Rounding.Floor
        );
}