hats-finance / Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd

Other
0 stars 0 forks source link

users can not repay their debts if `repay` is paused and would need to pay forced debt incured during pause #6

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

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

Description: Description\ BBBorrow.repay() function allows a borrower to repay debt. This function has implemented the optionNotPaused(PauseType.Repay) modifier, which will prevent the function from being used if the loan repayment is paused.

    function repay(address from, address to, bool, uint256 part)
        external
        optionNotPaused(PauseType.Repay)
        notSelf(to)
        returns (uint256 amount)
    {
        updateExchangeRate();

        _accrue();
        penrose.reAccrueBigBangMarkets();

        amount = _repay(from, to, part);
    }

The problem is that the usage of this function should not be prevented with the use of optionNotPaused(PauseType.Repay) modifier on repay() because if user is unable to repay their debts, they would need to pay higher debts when loan repayment is paused by Conservator by calling BigBang.updatePause()

repay() will call the _accrue() to get the interest over the passed time period. The interest rate depends on the time passed which means higher the time passed, then the higher interest has to be paid by the users.

Therefore, when the loan repayment is paused the user interest is still accuring interest and the user has to pay unnecessary high amount than he had to pay earlier. This introduces an unnecesary risk to users by preventing them to repay their debts.

Therefore, it is recommended to not accrue interest when the loan repayment is paused.

Impact\ Interest is still accuring when the loan repayment is paused, it will force user to incur debts and pay higher debts than he had to earlier due to repay pause.

Recommendation to fix\ Recommended to not accrue interest when the loan repayment is paused.

A similar approach as done in Singularity.updatePause() can be implemented to mitigate the issue.

    function updatePause(PauseType _type, bool val, bool resetAccrueTimestmap) external {
        if (msg.sender != conservator) revert NotAuthorized();
        if (val == pauseOptions[_type]) revert SameState();
        emit PausedUpdated(_type, pauseOptions[_type], val);
        pauseOptions[_type] = val;

        // In case of 'unpause', `lastAccrued` is set to block.timestamp
        // Valid for all action types that has an impact on debt or supply
        if (!val && (_type != PauseType.AddCollateral && _type != PauseType.RemoveCollateral)) {
            accrueInfo.lastAccrued = resetAccrueTimestmap ? block.timestamp.toUint64() : accrueInfo.lastAccrued;
        }
    }
0xRizwan commented 5 months ago

@cryptotechmaker Why the issue is labelled invalid?

cryptotechmaker commented 5 months ago

We added the repay pause option for the case when there are issues with repayment; if that's paused, it would be intended.

0xRizwan commented 5 months ago

@cryptotechmaker i understand the pause requirement on repay function. The issue is about continuous acrueing of interest even when the repay is paused. This should not be intended as the users are enforced to pay higher interest due to this issue.

As it can be seen in Singularity.updatePause() function,

In case of 'unpause', `lastAccrued` is set to block.timestamp

the last accrued timestamp is reset to current block.timestamp which means the interest generated in repay pause period is not being considered due to reset of accrue timestamp. It means in Singularity, users are not being to force to pay more interest due to pause of repay function and they would be paying the actual interest which are deemed to pay irrespective of pause or unpause of repay function, However, this is exactly opposite in BigBang.

A similar approach as done in Singularity.updatePause() can be implemented to mitigate the above issue to avoid higher payment of interest by users due to pause of repay function.

0xRizwan commented 5 months ago

@cryptotechmaker i understand the pause requirement on repay function. The issue is about continuous acrueing of interest even when the repay is paused. This should not be intended as the users are enforced to pay higher interest due to this issue.

As it can be seen in Singularity.updatePause() function,

In case of 'unpause', `lastAccrued` is set to block.timestamp

the last accrued timestamp is reset to current block.timestamp which means the interest generated in repay pause period is not being considered due to reset of accrue timestamp. It means in Singularity, users are not being to force to pay more interest due to pause of repay function and they would be paying the actual interest which are deemed to pay irrespective of pause or unpause of repay function, However, this is exactly opposite in BigBang.

A similar approach as done in Singularity.updatePause() can be implemented to mitigate the above issue to avoid higher payment of interest by users due to pause of repay function.

@maarcweiss Can you please check this issue