hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

`GAMMA_64x64` and `BETA_64x64` considers leap year which results in incorrect value #33

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

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

Description: Description\

As per the documentation, a registered user can mint Cicle for every hour.

Issuance Rate: Each registered human can mint one Circle per hour, which translates to 24 Circles per day or 8760 Circles per year (not accounting for leap years).

The accounting here does not consider leap years. Here the demurrage rate of 7% annually which is applied on daily basis.

Demurrage Rate: All Circles undergo a 7% annual demurrage, applied on a daily basis.

The issue is that, the reduction factors like GAMMA_64x64 and BETA_64x64 have considered 362.25 as days in their calculations which is implemented as:

    /**
     * @dev Reduction factor GAMMA for applying demurrage to balances
     *   demurrage_balance(d) = GAMMA^d * inflationary_balance
     * where 'd' is expressed in days (DEMURRAGE_WINDOW) since demurrage_day_zero,
     * and GAMMA < 1.
     * GAMMA_64x64 stores the numerator for the signed 128bit 64.64
     * fixed decimal point expression:
     *   GAMMA = GAMMA_64x64 / 2**64.
     * To obtain GAMMA for a daily accounting of 7% p.a. demurrage
@>>>     *   => GAMMA = (0.93)^(1/365.25)
     *            = 0.99980133200859895743...
     * and expressed in 64.64 fixed point representation:
     *   => GAMMA_64x64 = 18443079296116538654
     * For more details, see ./specifications/TCIP009-demurrage.md
     */
    int128 internal constant GAMMA_64x64 = int128(18443079296116538654);

    /**
     * @dev For calculating the inflationary mint amount on day `d`
     * since demurrage_day_zero, a person can mint
     *   (1/GAMMA)^d CRC / hour
     * As GAMMA is a constant, to save gas costs store the inverse
     * as BETA = 1 / GAMMA.
     * BETA_64x64 is the 64.64 fixed point representation:
@>>>      *   BETA_64x64 = 2**64 / ((0.93)^(1/365.25))
     *              = 18450409579521241655
     * For more details, see ./specifications/TCIP009-demurrage.md
     */
    int128 internal constant BETA_64x64 = int128(18450409579521241655);

In Demurrage.sol, GAMMA_64x64 and BETA_64x64 are used in following functions:

1) convertInflationaryToDemurrageValue() 2) _calculateDemurrageFactor() 3) _calculateDemurrageFactorAndCache()

4) convertDemurrageToInflationaryValue() 5) _calculateInflationaryBalance()

In BatchedDemurrage.sol, GAMMA_64x64 and BETA_64x64 are used in following functions:

1) convertBatchInflationaryToDemurrageValues() 2) convertBatchDemurrageToInflationaryValues()

Due to the descripancy in using 365.25 days instead 365 days, the above functions would always return incorrect values which would break the contracts intended design.

Recommendations\ Consider using 365 days while calculating GAMMA_64x64 and BETA_64x64 values as the 7% demurrage rate is applied annually which considers 1 year as 365 days and does not count leap years as stated in documentation.

benjaminbollen commented 2 days ago

Decided to keep the definition of 365.25 days in a year. This is both consistent with the existing definition of Circles v1 and avoids creating an impedance.