Yieldy contest - ElKu's results

1) affiliateFee should have an upper limit to avoid abuse.

affiliateFee

2) Grammatical mistakes in natspec comments:

a) this is function is called

b) @dev this allows us to change how much time before the end of the cycle we send the withdraw requests should be "we can send" and not "we send"

3) Grammatical errors in normal comments:

a) asdf

b) // try to claim withdraw if user has withdraws to claim function will check if withdraw is valid

1. Use custom error reporting instead of require statements. This reduces both deployment and runtime gas usage. See this link for an example. Also many of these errors are of the same type, which means we can use the same custom error for them, reducing the gas usage even further.

References where this could be done are:

a. Staking.sol: on lines 143 , 408 , 410 , 527 , 572 , 574 , 586 , 604-614 , 644 and 676.

b. Yieldy.sol: on lines 58-59 , 83 , 96 , 187, 190 , 210 , 249 , 257 , 279 and 286.

c. LiquidityReserve.sol: on lines 25, 44, 61, 62, 68, 94, 105, 163, 170, 192 and 215.

2. When all elements of a structure are not used, avoid reading the whole structure from storage.

a. Claim memory info = warmUpInfo[_recipient]; return epoch.number >= info.expiry && info.expiry != 0;

Three slots(struct Claim contains three uint256) are read here while only one slot is used in the function. This will save 200 gas.

b. Claim memory info = warmUpInfo[_recipient]; The whole structure is read while only the Claim.credits element is needed. Also this statement should be put inside the if statement, just above the delete statement on line 468.

3. If the same storage variable is read more than once inside a function, cache it in memory first and use the cached variable. Reading from memory costs 3 gas and storage reads cost 100 gas.

a. withdrawalAmount.

b. Use currentTotalSupply instead of _totalSupply. It will save 100-3=97 gas. require(_totalSupply > 0, "Can't rebase if not circulating");

c. Cache _totalSupply in _storeRebase function

d. Cache creditBalances[msg.sender] in transfer function

4. A return statement with multiple conditions inside to return a bool can be split into multiple if statements. This will avoid doing unnecessary calculations in some cases, thus saving gas. Also the order of these if statements should be thought off, as it increases the probability of using less gas.

a. _isClaimWithdrawAvailable function in Staking.sol

    function _isClaimWithdrawAvailable(address _recipient)
        internal
        returns (bool)
    {
        Claim memory info = coolDownInfo[_recipient];
        ITokeManager tokeManager = ITokeManager(TOKE_MANAGER);
        ITokePool tokePoolContract = ITokePool(TOKE_POOL);
        RequestedWithdrawalInfo memory requestedWithdrawals = tokePoolContract
            .requestedWithdrawals(address(this));
        uint256 currentCycleIndex = tokeManager.getCurrentCycleIndex();
        return
            epoch.number >= info.expiry &&
            info.expiry != 0 &&
            info.amount != 0 &&
            ((requestedWithdrawals.minCycle <= currentCycleIndex &&
                requestedWithdrawals.amount + withdrawalAmount >=
                info.amount) || withdrawalAmount >= info.amount);
    }

Can be rewritten as:

    function _isClaimWithdrawAvailable(address _recipient)
        internal
        returns (bool)
    {
        Claim memory info = coolDownInfo[_recipient];
	//by bringing some statements up here, we might save a large amount of gas in some cases.
	if(epoch.number < info.expiry ||
            info.expiry == 0 ||
            info.amount == 0) {
		return false;
	}
        ITokeManager tokeManager = ITokeManager(TOKE_MANAGER);
        ITokePool tokePoolContract = ITokePool(TOKE_POOL);
        RequestedWithdrawalInfo memory requestedWithdrawals = tokePoolContract
            .requestedWithdrawals(address(this));
        uint256 currentCycleIndex = tokeManager.getCurrentCycleIndex();
	uint256 w = withdrawalAmount;   //cache the storage variable.
        return  //notice how I have swapped the 1st and 2nd condition of OR. 
	   //If first condition is true, then we will save gas on 2nd condition.
            (w >= info.amount || (requestedWithdrawals.minCycle <= currentCycleIndex &&
                requestedWithdrawals.amount + w >=
                info.amount));
    }

b. Function canBatchTransactions() can be rewritten as:

function canBatchTransactions() public view returns (bool) {
        if (requestWithdrawalAmount == 0) {  //least dependent on calculations.
            return false;
        }
        ITokeManager tokeManager = ITokeManager(TOKE_MANAGER);
        uint256 currentCycleIndex = tokeManager.getCurrentCycleIndex();
        if (currentCycleIndex <= lastTokeCycleIndex) {  //needs relatively less calculation than next one.
            return false;
        }
        uint256 duration = tokeManager.getCycleDuration();
        uint256 currentCycleStart = tokeManager.getCurrentCycle();
        uint256 nextCycleStart = currentCycleStart + duration;
        return block.timestamp + timeLeftToRequestWithdrawal >= nextCycleStart;  //depends on many calculations
}

5. Avoid redundant usage of temporary variables in functions. If a variable doesn’t get used more than once and not doing so doesn’t affect readability, then its better to use them directly.

References include: a. ITokeReward tokeRewardContract = ITokeReward(TOKE_REWARD); b. ITokeManager tokeManager = ITokeManager(TOKE_MANAGER); ITokePool tokePoolContract = ITokePool(TOKE_POOL); c. ITokeManager tokeManager = ITokeManager(TOKE_MANAGER); d. ITokePool tokePoolContract = ITokePool(TOKE_POOL); e. ITokePool tokePoolContract = ITokePool(TOKE_POOL); f. ITokePool tokePoolContract = ITokePool(TOKE_POOL); g. ITokePool tokePoolContract = ITokePool(TOKE_POOL); h. ITokeManager tokeManager = ITokeManager(TOKE_MANAGER); i. uint256 currentCycleIndex = tokeManager.getCurrentCycleIndex();

6. When emitting messages, if the variable is available in memory, use it, rather than the storage variable(which in most cases is just assigned from the memory variable)

   emit LogSetCurvePool(CURVE_POOL, curvePoolTo, curvePoolFrom);

7. Redundant code logic should be removed. This if statement will never return true.

if (_amount >= warmUpBalance)

Because the require statement on line 528 says _amount <= walletBalance + warmUpBalance.

8. These two lines could be re-written to avoid a storage write in some cases. Note that in some cases it will use 100 gas extra rather than saving you 20000 gas for avoiding storage write.

rebasingCreditsPerToken = rebasingCredits / updatedTotalSupply;
require(rebasingCreditsPerToken > 0, "Invalid change in supply");

to

require(rebasingCredits  > updatedTotalSupply, "Invalid change in supply");
rebasingCreditsPerToken = rebasingCredits / updatedTotalSupply;

9. This for loop will run through the whole length of array even if the address to be deleted was found at index 0. Add a break statement after the delete statement to resolve this.

for (uint256 i; i < contractsLength; ) {
            if (contracts[i] == _address) {
                delete contracts[i];
            }
            unchecked {
                ++i;
            }
        }