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

0 stars 0 forks source link

Avoid unnecessary code execution can save gas #48

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

The variable epochs is only used when endEpoch != 0, move it into the block of if (endEpoch != 0){ ... } can avoid unnecessary code execution and save some gas.

https://github.com/code-423n4/2021-10-covalent/blob/ded3aeb2476da553e8bb1fe43358b73334434737/contracts/DelegatedStaking.sol#L101-L115

function takeOutRewardTokens(uint128 amount) public onlyOwner {
    require(amount > 0, "Amount is 0");
    uint128 currentEpoch = uint128(block.number);
    uint128 epochs = amount / allocatedTokensPerEpoch;
    if (endEpoch != 0){
        require(endEpoch - epochs > currentEpoch, "Cannot takeout rewards from past");
        endEpoch = endEpoch - epochs;
    }
    else{
        require(rewardsLocked >= amount, "Amount is greater than available");
        rewardsLocked -= amount;
    }
    transferFromContract(owner(), amount);
    emit AllocatedTokensTaken(amount);
}

Recommendation

Change to:

function takeOutRewardTokens(uint128 amount) public onlyOwner {
    require(amount > 0, "Amount is 0");
    uint128 currentEpoch = uint128(block.number);
    if (endEpoch != 0){
        uint128 epochs = amount / allocatedTokensPerEpoch;
        require(endEpoch - epochs > currentEpoch, "Cannot takeout rewards from past");
        endEpoch = endEpoch - epochs;
    }
    else{
        require(rewardsLocked >= amount, "Amount is greater than available");
        rewardsLocked -= amount;
    }
    transferFromContract(owner(), amount);
    emit AllocatedTokensTaken(amount);
}
kitti-katy commented 2 years ago

duplicate of #26

GalloDaSballo commented 2 years ago

Duplicate of #26