<!-- canonical: https://0xsimao.com/findings/mento-provider-mint-price-rounding -->

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

Medium · Sherlock · Stablecoin · 24th October 2024

Finding M-2 of the Mento competition.

- Protocol: https://audits.sherlock.xyz/contests/598
- Report: /reports/mento
- Codebase: https://github.com/0xsimao/2024-10-mento-update/tree/098b17fb32d294145a7f000d96917d13db8756cc
- Source: https://github.com/sherlock-audit/2024-10-mento-update-judging/issues/21

---

### Summary

[GoodDollarExchangeProvider::mintFromExpansion()](https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/goodDollar/GoodDollarExchangeProvider.sol#L147) 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()](https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/goodDollar/GoodDollarExchangeProvider.sol#L179-L181) due to using `mul` and then `div`, as `mul` divides by `1e18` unnecessarily in this case.

Note2: [GoodDollarExchangeProvider::updateRatioForReward()](https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/goodDollar/GoodDollarExchangeProvider.sol#L205) 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`:
```solidity
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.
```solidity
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);
  ...
}
```

---

Related findings:

- [It's possible to mint an infinite number of shares without increasing quote or base amounts, due to rounding down](https://0xsimao.com/findings/nftperp-ii-mint-infinite-shares-rounding): Nftperp Matching Engine
- [Significant rounding error in whitelist discount that could be avoided](https://0xsimao.com/findings/1inch-rounding-whitelist-discount-avoided): 1inch Fee Extension
- [Rounding error in Aave Lending Pool](https://0xsimao.com/findings/gaib-rounding-error-aave-lending): GAIB Pre-Vaults
- [Unhandled rounding error in DistributionVault.getClaimable leads to locked dust](https://0xsimao.com/findings/m-0-rounding-claimable-locked-dust): M^0 Minter Gateway