higher demurrage on minted tokens compare to non minted tokens

[hats-bug-reporter[bot]]

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0xf5ede84a0a2c253769c18567548ebbbdc66f1cb39f7c7f2ecc7ebeabb28f6095 Severity: medium

Description:

Description

The calculateIssuance() function in the Circles contract factors in demurrage when determining the amount of new currency to mint for an individual. The expected behavior should be that the demurrage amount over a day should be equal for both minted and non-minted tokens.

Consider the following scenario:

• User A mints tokens now, and after one day, they have an amount of X.
• User B mints tokens after one day, and they will gain an amount of more than X.

The issue here is that users who mint tokens frequently incur more demurrage compared to users who do not mint as often.

In the provided test case, the code illustrates that if a user mints now, after two days, they will have fewer tokens compared to if they had not minted for two days:

    function testetstsset2days() public {
        // Alice register in v2
        ITokenV1 tokenAlice = signupInV1(addresses[0]);
        vm.prank(addresses[0]);
        tokenAlice.stop();
        vm.prank(addresses[0]);
        mockHub.registerHuman(address(0), bytes32(0));
        console.log("day", day(block.timestamp));
        skipTime(2 days);
        console.log("day", day(block.timestamp));

        vm.prank(addresses[0]);
        mockHub.personalMint();
        skipTime(2 days);
        console.log("day", day(block.timestamp));
        uint256 aliceAmountMintet = mockHub.balanceOf(addresses[0], uint160(addresses[0]));

        (uint256 AlicEamountv2,,) = mockHub.calculateIssuance(addresses[0]);
        console.log("1", aliceAmountMintet);
        console.log("1", AlicEamountv2);
        assertTrue(relativeApproximatelyEqual(AlicEamountv2, aliceAmountMintet, ONE_IN_HUNDRED_THOUSAND));
    }

[FAIL. Reason: assertion failed] testetstsset2days() (gas: 1257245)
Logs:
  day 18971
  day 18973
  day 18975
  1 47966632301801410078
  1 47985696851874424310
  Error: Assertion Failed

The user mints on day 18973 and, after two days, has 47966632301801410078 tokens. However, over the time between day 18975 and day 18973, they would have gained more tokens if they hadn't minted.

This results in users who mint frequently losing more share compared to users who mint every two weeks. Demurrage is applied more heavily on minted tokens compared to non-minted tokens.

Impact

demurrage is not equal on minted and non-minted tokens.

Issue

balanceOf use _calculateDiscountedBalance:

    function _calculateDiscountedBalance(uint256 _balance, uint256 _daysDifference) internal view returns (uint256) {
        if (_daysDifference == 0) {
            return _balance;
        }
        int128 r = _calculateDemurrageFactor(_daysDifference);
        return Math64x64.mulu(r, _balance);
    }

that multiply tokens with R[n].

however in minting process, the contract user another formula which result in this issue.

[benjaminbollen]

Thank you for your report on the potential discrepancy in demurrage application between minted and non-minted tokens. After careful review, we've determined this is not an issue.

Both calculateIssuance and calculateDiscountedBalance functions use the same tabulated R values and mathematical principles to apply demurrage. The calculateIssuance function employs a more gas-efficient calculation method, which is an intentional optimization.

We appreciate the unit test you've provided and will review it. However, any potential numerical discrepancies resulting from this optimization are an acceptable trade-off for improved gas efficiency.

Thank you for your thorough examination of our demurrage calculations and your contribution to this security review.

[benjaminbollen]

see https://github.com/aboutcircles/circles-contracts-v2/blob/rc-v0.3.6-alpha/specifications/TCIP009-demurrage.md#calculating-mint

[benjaminbollen]

Hey, on second reading there is also a correction to be considered in your provided test:

        mockHub.personalMint();
        skipTime(2 days);
        console.log("day", day(block.timestamp));
        uint256 aliceAmountMintet = mockHub.balanceOf(addresses[0], uint160(addresses[0]));

when you query the balance of alice after two days, a demurrage factor for those two days will be applied to the amount she had two days ago, so rather you want to query her balance before you skip 2 days, and then compare that

[0xmahdirostami]

oh thanks, yes you are right, i compared 4 days of demurrage with 2 days of demurrage.