search

code-423n4 / 2021-10-union-findings

0 stars 0 forks source link

Function getFrozenCoinAge Can Be Made More Efficient (UserManager.sol) #26

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

ye0lde

Vulnerability details

Impact

Removing unnecessary initializations and branches can result in gas savings and code clarity.

Proof of Concept

The function that can be rewritten is here: https://github.com/code-423n4/2021-10-union/blob/4176c366986e6d1a6b3f6ec0079ba547b040ac0f/contracts/user/UserManager.sol#L794-L814

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

I suggest changing getFrozenCoinAge as follows:

function getFrozenCoinAge(address staker, uint256 pastBlocks) public view override returns (uint256) { uint256 totalFrozenCoinAge; 

address[] memory borrowerAddresses = getBorrowerAddresses(staker);

for (uint256 i = 0; i < borrowerAddresses.length; i++) {
    address borrower = borrowerAddresses[i];
    uint256 blocks = block.number - uToken.getLastRepay(borrower);
    if (uToken.checkIsOverdue(borrower)) {
        (, , uint256 lockedStake) = getStakerAsset(borrower, staker);               
        totalFrozenCoinAge +=  lockedStake * (pastBlocks >= blocks ? blocks : pastBlocks);                    
    }
}

return totalFrozenCoinAge;

}

GalloDaSballo commented 3 years ago

Neat usage of ternary operator to simplify the code, I believe this will save gas on the deployment but not on the execution

GalloDaSballo commented 3 years ago

A bigger "red flag" seems to be the usage of the same function as both public and private, the public function can already be used in the contract, rendering the private function superfluous