Skip to content

Latest commit

 

History

History
71 lines (47 loc) · 3.62 KB

021.md

File metadata and controls

71 lines (47 loc) · 3.62 KB
Raw

Fit Menthol Sawfish

Medium

GoodDollarExchangeProvider::mintFromExpansion() will change the price due to a rounding error in the new ratio

Summary

GoodDollarExchangeProvider::mintFromExpansion() mints supply tokens while keeping the current price constant. To achieve this, a certain formula is used, but in the process it scales the reserveRatioScalar * exchange.reserveRatio to 1e8 precision (the precision of exchange.reserveRatio) down from 1e18.

However, the calculation of the new amount of tokens to mint is based on the full ratio with 1e18, which will mint more tokens than it should and change the price, breaking the readme.

Note1: there is also a slight price change in GoodDollarExchangeProvider::mintFromInterest() due to using mul and then div, as mul divides by 1e18 unnecessarily in this case.

Note2: GoodDollarExchangeProvider::updateRatioForReward() also has precision loss as it calculates the ratio using the formula and then scales it down, changing the price.

Root Cause

In GoodDollarExchangeProvider:147, newRatio is calculated with full 1e18 precision and used to calculate the amount of tokens to mint, but exchanges[exchangeId].reserveRatio is stored with the downscaled value, newRatio / 1e10, causing an error and price change.

This happens because the price is reserve / (supply * reserveRatio). As supply is increased by a calculation that uses the full precision newRatio, but reserveRatio is stored with less precision (1e8), the price will change due to this call.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

  1. GoodDollarExchangeProvider::mintFromExpansion() is called and a rounding error happens in the calculation of newRatio.

Impact

The current price is modified due to the expansion which goes against the readme:

What properties/invariants do you want to hold even if breaking them has a low/unknown impact?

Bancor formula invariant. Price = Reserve / Supply * reserveRatio

PoC

Add the following test to GoodDollarExchangeProvider.t.sol:

function test_POC_mintFromExpansion_priceChangeFix() public {
  uint256 priceBefore = exchangeProvider.currentPrice(exchangeId);
  vm.prank(expansionControllerAddress);
  exchangeProvider.mintFromExpansion(exchangeId, reserveRatioScalar);
  uint256 priceAfter = exchangeProvider.currentPrice(exchangeId);
  assertEq(priceBefore, priceAfter, "Price should remain exactly equal");
}

If the code is used as is, it fails. but if it is fixed by dividing and multiplying by 1e10, eliminating the rounding error, the price matches exactly (exact fix show below).

Mitigation

Divide and multiply newRatio by 1e10 to eliminate the rounding error, keeping the price unchanged.

function mintFromExpansion(
  bytes32 exchangeId,
  uint256 reserveRatioScalar
) external onlyExpansionController whenNotPaused returns (uint256 amountToMint) {
  require(reserveRatioScalar > 0, "Reserve ratio scalar must be greater than 0");
  PoolExchange memory exchange = getPoolExchange(exchangeId);

  UD60x18 scaledRatio = wrap(uint256(exchange.reserveRatio) * 1e10);
  UD60x18 newRatio = wrap(unwrap(scaledRatio.mul(wrap(reserveRatioScalar))) / 1e10 * 1e10);
  ...
}