Incorrect Balance Update in `FurnaceP1::melt()` Will Lead to Reduced RToken Melting Over Time

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Furnace.sol#L65-L79

Vulnerability details

Description

The FurnaceP1 contract implements a mechanism to continuously melt RTokens based on a predefined ratio and elapsed time. The melt() function is responsible for calculating and executing this melting process. However, there is a critical issue in the implementation where the lastPayoutBal is updated incorrectly, leading to a compounding error that results in less RToken being melted over time than intended.

The melt() function calculates the amount of RTokens to be melted based on the lastPayoutBal, the elapsed time since the last payout, and the melting ratio. The function then updates the lastPayoutBal and calls rToken.melt() to burn the calculated amount of tokens. The issue arises from the order of these operations:

function melt() public {
    // ... (calculation of amount to melt)

    lastPayout += numPeriods;
    lastPayoutBal = rToken.balanceOf(address(this)) - amount; // Incorrect update
    if (amount != 0) rToken.melt(amount);
}

The lastPayoutBal is updated before the actual melting occurs, which means it doesn't accurately reflect the balance after melting. This leads to a compounding error where each subsequent call to melt() uses an inflated balance as its starting point, resulting in progressively less RToken being melted than intended.

Impact

The incorrect balance update in the melt() function leads to a systematic undermelting of RTokens over time. This deviation from the intended melting rate can have significant implications for the protocol's tokenomics and overall economic model. As the error compounds with each melt() call, the discrepancy between the intended and actual melting rates will grow larger over time.

This could lead to an oversupply of RTokens in circulation compared to what the protocol design intended, potentially affecting the token's value and the overall stability of the ecosystem.

Proof of Concept

To illustrate the impact of this bug, consider the following scenario:

1. Initial RToken balance in the Furnace: 1,000,000 tokens
2. Melting ratio: 1% per period
3. Assume melt() is called once per period

Period 1:

• Expected melt amount: 10,000 tokens (1% of 1,000,000)
• Actual melt amount: 10,000 tokens
• lastPayoutBal is set to 990,000 (1,000,000 - 10,000) before melting

Period 2:

• Expected melt amount: 9,900 tokens (1% of 990,000)
• Actual melt amount: 9,900 tokens
• lastPayoutBal is set to 980,100 (990,000 - 9,900) before melting

Period 3:

• Expected melt amount: 9,801 tokens (1% of 980,100)
• Actual melt amount: 9,801 tokens
• lastPayoutBal is set to 970,299 (980,100 - 9,801) before melting

As this process continues, the lastPayoutBal used for calculations becomes increasingly higher than the actual balance after melting, leading to less RToken being melted in each subsequent period compared to what was intended.

Tools Used

Manual review

Recommended Mitigation Steps

To fix this issue, the lastPayoutBal should be updated after the rToken.melt() call to ensure it accurately reflects the balance after melting. Here's the recommended fix:

function melt() public {
    // ... (existing code for calculations)

    lastPayout += numPeriods;
-   lastPayoutBal = rToken.balanceOf(address(this)) - amount;
    if (amount != 0) rToken.melt(amount);
+   lastPayoutBal = rToken.balanceOf(address(this));
}

This change ensures that lastPayoutBal always represents the actual balance after melting, preventing the compounding error and maintaining the intended melting rate over time.

Assessed type

Other