in `RTokenP1::issueTo` wrong Dev assumptions about new RToken holders getting revenue from the past

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L124-L128

Vulnerability details

Impact

New RToken issuers will get rewards from the past due to outdated Melt state, leading to loss of funds for old users

Proof of Concept

in RTokenP1issueTo whenever a user calls issueTo Furnace.melt is not called

Although, Devs assume here that the Furnace is Up-To-Date

This will lead to a state where:

• Rewards are staying in Furnace and slowly getting vested(Good)
• users are holding RTokens for 1 week
• Furnace.melt last called is 1 day before the new issue operation
• a new user issue RToken and gets rewards like if he was holding them from the day before(Bad)

Although the same case and logic of vesting RSR rewards is the same of melt operations, yet RSR contract calls _payoutRewards in every stake operation (which is expected and the good behavior)

File: StRSR.sol
227:     function stake(uint256 rsrAmount) public {
228:         _notZero(rsrAmount);
229: 
230:         _payoutRewards();

yet we don't do that in RToken::issueTo which will lead to the described behavior above and loss of funds for old genuine users

• Yes melt operation is permissionless but we can't assume that users will keep calling it like Bots, outdated melt state is not rare
• the Dev comments confirm that the intention (invariant) is to have Up-To-Date melt but not implemented (invariant broken)

  Tools Used

  manual review

  Recommended Mitigation Steps

  Call Furnace.melt in every RToken issuance, the operation is not gas extensive and won't cause bad experience for issuer and will cause fair economic state for RToken holders

Assessed type

Context

[Al-Qa-qa]

My initial concern about this was how the comments assumed that melting is up-to-date on his comments

But this is harmful and unfair on its own to old holders and profitable to new issuers due to not calling melt during issuance

the logic is implemented correctly on RSR rewards payout during staking, but on the contrary its not for issuance, although its clearly mentioned in the comments that its assumed as invariant that its up-to-date

[thereksfour]

rToken will be added to AssetRegistry as an asset, and assetRegistry.refresh() will call RTokenAsset.refresh(), thereby calling melt to make it up to date.

        assets[0] = new RTokenAsset(components.rToken, params.rTokenMaxTradeVolume);
        assets[1] = rsrAsset;

        // Init Asset Registry
        components.assetRegistry.init(main, assets);

    function refresh() public virtual override {
        // No need to save lastPrice; can piggyback off the backing collateral's saved prices

        furnace.melt();
        if (msg.sender != address(assetRegistry)) assetRegistry.refresh();

        cachedOracleData.cachedAtTime = 0; // force oracle refresh
    }