Precision loss in contracts/fee/FeeCalculations.sol#_calculateMintAmountForStreamingFees

[hats-bug-reporter[bot]]

Github username: -- Twitter username: rnemes4 Submission hash (on-chain): 0xcae1fd8054e74e1235a2a97fca61254f2c15306448f84b46c0593e0136ff6a79 Severity: medium

Description: Description\ _calculateMintAmountForStreamingFees is susceptable to precision loss errors when calculating tokensToMint due to instances of dividing before m,ultiplication over multiple functions. The first precision loss comes from the _calculateStreamingFee where the following calculation is made:

streamingFee =
80:       (_totalSupply * _feePercentage * timeElapsed) /
81:       ONE_YEAR_IN_SECONDS /
82:       TOTAL_WEIGHT;

the loss is then compounded by multiplying the result streamingFees by ONE_ETH_IN_WEI on the following line:

uint256 feeReceiverShare = (streamingFees * ONE_ETH_IN_WEI) / _totalSupply;

finaly the feeReceiverShare is multipliplied again _userShare in:

return (_userShare * _totalSupply) / remainingShare;

All three functions are listed below for clarity of the call sequence.

File: contracts/fee/FeeCalculations.sol

93:   function _calculateMintAmountForStreamingFees(
94:     uint256 _totalSupply,
95:     uint256 _lastChargedTime,
96:     uint256 _feePercentage,
97:     uint256 _currentTime
98:   ) internal pure returns (uint256 tokensToMint) {
99:     if (_lastChargedTime >= _currentTime) {
100:       return 0;
101:     }
102: 
103:     uint256 streamingFees = _calculateStreamingFee(
104:       _totalSupply,
105:       _lastChargedTime,
106:       _feePercentage,
107:       _currentTime
108:     );
109: 
110:     // Calculates the share of the asset manager after minting
111:     uint256 feeReceiverShare = (streamingFees * ONE_ETH_IN_WEI) / _totalSupply;
112: 
113:     tokensToMint = _calculateMintAmount(feeReceiverShare, _totalSupply);
114:   }

File: contracts/fee/FeeCalculations.sol

72:   function _calculateStreamingFee(
73:     uint256 _totalSupply,
74:     uint256 _lastChargedTime,
75:     uint256 _feePercentage,
76:     uint256 _currentTime
77:   ) internal pure returns (uint256 streamingFee) {
78:     uint256 timeElapsed = _currentTime - _lastChargedTime;
79:     streamingFee =
80:       (_totalSupply * _feePercentage * timeElapsed) /
81:       ONE_YEAR_IN_SECONDS /
82:       TOTAL_WEIGHT;
83:   }

File: contracts/core/calculations/TokenCalculations.sol

26:   function _calculateMintAmount(
27:     uint256 _userShare,
28:     uint256 _totalSupply
29:   ) internal pure returns (uint256) {
30:     uint256 remainingShare = ONE_ETH_IN_WEI - _userShare;
31:     if (remainingShare == 0) revert ErrorLibrary.DivisionByZero();
32:     return (_userShare * _totalSupply) / remainingShare;
33:   }

Attack Scenario\ The following foundry Fuzzing test was used to highlight this precision loss wherby the tokensToMint was calulated using the current functions and then compared to a more precise method wherby the as many divisions before multiplications were removed by rearranging the equations into the following form:

function _calculateMintAmountForStreamingFeesPrecise(
    uint256 _totalSupply,
    uint256 _lastChargedTime,
    uint256 _feePercentage,
    uint256 _currentTime
  ) internal view returns (uint256 tokensToMint) {
    if (_lastChargedTime >= _currentTime) {
      return 0;
    }

    uint256 timeElapsed = _currentTime - _lastChargedTime;

    uint256 feeReceiverShare = ((_totalSupply * _feePercentage * timeElapsed) *
      ONE_ETH_IN_WEI) /
      ONE_YEAR_IN_SECONDS /
      TOTAL_WEIGHT /
      _totalSupply;

    console.log("Precise feeReceiverShare", feeReceiverShare);

    uint256 remainingShare = ONE_ETH_IN_WEI - feeReceiverShare;
    if (remainingShare == 0) revert ErrorLibrary.DivisionByZero();
    tokensToMint = (feeReceiverShare * _totalSupply) / remainingShare;
  }

Attachments

1. Proof of Concept (PoC) File Copy the following Foundry test into the test suit which shows it is possible to get a precision loss of over 1_000_000 fee tokens.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import { AccessController } from "../../contracts/access/AccessController.sol";
import { ErrorLibrary } from "../../contracts/library/ErrorLibrary.sol";

contract ScratchTest is Test {
  uint256 public constant TOTAL_WEIGHT = 10_000; // Represents 100% in basis points.
  uint256 public constant ONE_YEAR_IN_SECONDS = 365 days; // Used for annualized fee calculations.
  uint256 internal constant ONE_ETH_IN_WEI = 10 ** 18;

  function setUp() public {}

  function _calculateMintAmountForStreamingFees(
    uint256 _totalSupply,
    uint256 _lastChargedTime,
    uint256 _feePercentage,
    uint256 _currentTime
  ) internal view returns (uint256 tokensToMint) {
    if (_lastChargedTime >= _currentTime) {
      return 0;
    }

    uint256 streamingFees = _calculateStreamingFee(
      _totalSupply,
      _lastChargedTime,
      _feePercentage,
      _currentTime
    );

    // Calculates the share of the asset manager after minting
    uint256 feeReceiverShare = (streamingFees * ONE_ETH_IN_WEI) / _totalSupply;

    console.log("feeReceiverShare", feeReceiverShare);

    tokensToMint = _calculateMintAmount(feeReceiverShare, _totalSupply);
  }

  function _calculateMintAmountForStreamingFeesPrecise(
    uint256 _totalSupply,
    uint256 _lastChargedTime,
    uint256 _feePercentage,
    uint256 _currentTime
  ) internal view returns (uint256 tokensToMint) {
    if (_lastChargedTime >= _currentTime) {
      return 0;
    }

    uint256 timeElapsed = _currentTime - _lastChargedTime;

    uint256 feeReceiverShare = ((_totalSupply * _feePercentage * timeElapsed) *
      ONE_ETH_IN_WEI) /
      ONE_YEAR_IN_SECONDS /
      TOTAL_WEIGHT /
      _totalSupply;

    console.log("Precise feeReceiverShare", feeReceiverShare);

    uint256 remainingShare = ONE_ETH_IN_WEI - feeReceiverShare;
    if (remainingShare == 0) revert ErrorLibrary.DivisionByZero();
    tokensToMint = (feeReceiverShare * _totalSupply) / remainingShare;
  }

  function _calculateStreamingFee(
    uint256 _totalSupply,
    uint256 _lastChargedTime,
    uint256 _feePercentage,
    uint256 _currentTime
  ) internal view returns (uint256 streamingFee) {
    uint256 timeElapsed = _currentTime - _lastChargedTime;
    console.log("timeElapsed", timeElapsed);
    streamingFee =
      (_totalSupply * _feePercentage * timeElapsed) /
      ONE_YEAR_IN_SECONDS /
      TOTAL_WEIGHT;
    console.log("streamingFee", streamingFee);
  }

  function _calculateMintAmount(
    uint256 _userShare,
    uint256 _totalSupply
  ) internal view returns (uint256) {
    uint256 remainingShare = ONE_ETH_IN_WEI - _userShare;
    if (remainingShare == 0) revert ErrorLibrary.DivisionByZero();
    return (_userShare * _totalSupply) / remainingShare;
  }

  function testMintFeesPrecisionLoss(
    uint256 _totalSupply,
    uint256 _lastChargedTime,
    uint256 _feePercentage
  ) public {
    uint _currentTime = block.timestamp + 28 days;

    _totalSupply = bound(_totalSupply, 1e18, 1_000_000e18);

    _lastChargedTime = bound(
      _lastChargedTime,
      block.timestamp,
      block.timestamp + 28 days - 1
    );

    _feePercentage = bound(_feePercentage, 100, 1_000);

    console.log("totalSupply", _totalSupply);
    console.log("currentTime", _currentTime);
    console.log("lastChargedTime", _lastChargedTime);
    console.log("feePercentage", _feePercentage);

    uint256 tokensToMint = _calculateMintAmountForStreamingFees(
      _totalSupply,
      _lastChargedTime,
      _feePercentage,
      _currentTime
    );

    console.log("tokensToMint", tokensToMint);

    uint tokensToMintPrecise = _calculateMintAmountForStreamingFeesPrecise(
      _totalSupply,
      _lastChargedTime,
      _feePercentage,
      _currentTime
    );

    console.log("tokensToMintPrecise", tokensToMintPrecise);
    console.log("diff", tokensToMintPrecise - tokensToMint);

    assert(tokensToMint != 0);
    assert(tokensToMintPrecise - tokensToMint < 1_000_000);
  }
}

Output

  totalSupply 999999000077845163104130
  currentTime 2419201                                                                                    
  lastChargedTime 1092718
  feePercentage 705                                                                                      
  timeElapsed 1326483
  streamingFee 2965403284824592533201                                                                    
  feeReceiverShare 2965406249999999
  tokensToMint 2974223064488921130002                                                                    
  Precise feeReceiverShare 2965406250000000
  tokensToMintPrecise 2974223064488922135958                                                             
  diff 1005956

2. Revised Code File (Optional)

It is recommended to reduce the precision loss by using the following function to calculate tokensToMint

  function _calculateMintAmountForStreamingFees(
    uint256 _totalSupply,
    uint256 _lastChargedTime,
    uint256 _feePercentage,
    uint256 _currentTime
  ) internal view returns (uint256 tokensToMint) {
    if (_lastChargedTime >= _currentTime) {
      return 0;
    }

    uint256 timeElapsed = _currentTime - _lastChargedTime;

    uint256 feeReceiverShare = ((_totalSupply * _feePercentage * timeElapsed) *
      ONE_ETH_IN_WEI) /
      ONE_YEAR_IN_SECONDS /
      TOTAL_WEIGHT /
      _totalSupply;

    uint256 remainingShare = ONE_ETH_IN_WEI - feeReceiverShare;
    if (remainingShare == 0) revert ErrorLibrary.DivisionByZero();
    tokensToMint = (feeReceiverShare * _totalSupply) / remainingShare;
  }

[langnavina97]

It's true that a single, precise calculation might be more accurate, but our current implementation is more readable and has practical accuracy. We defined a uint256 internal constant MIN_MINT_FEE = 1_000_000, considering anything below this as dust. The difference between a more precise solution and our current readable code is negligible, only affecting the dust value. Therefore, we believe no changes are required.