Badger Citadel contest - fatherOfBlocks's results

CitadelMiner.sol

• L250: setFundingPoolWeight() only raises events on the if() and else if() cases, there is no final else case. It is not correct that a setting method does not notify what happened, if the change was made or not.

StakedCitadel.sol

• L808: the0 _withdraw() function has variables like "b" and "r" that are very loosely declarative.

StakedCitadelLocker.sol

• L762: MathUpgradeable.min(minimumStake, minimumStake - _offset); This line could be written more simply, since the minimum between a and a-b will always be a-b, even if b is zero.

CitadelMiner.sol

• L364: this operation can be wrapped in unckecked, since totalFundingPoolWeight can only be >= fundingPoolWeights[_pool]

Funding.sol

• L79: if it were a function, instead of a modifier, the gas cost would be lower.

• L145: if it were a function, instead of a modifier, the gas cost would be lower.

• L236: this operation can be wrapped in unckecked, since the if() validates the underflow.

• L419 / L420: Instead of creating the variables and then setting them, if you create and set them at the same time, you spend less gas.

KnightingRound.sol

• L218: this operation can be wrapped in unckecked, It shouldn't be possible to overflow totalTokenOutClaimed + tokenOutAmount_, since they depend on all the validations above.

• L250: this operation can be wrapped in unckecked, since the if() validates the underflow.

StakedCitadel.sol

• L452: instead of creating a variable and then using it, it could be put directly into L461.

• L456: instead of creating a variable and then using it, it could be put directly into L464.

• L822: this operation can be wrapped in unckecked, _diff= _after - b and r = b + _diff, if L820 does not revert, then neither does L822.

• L852: directly return the operation and not create a variable, this generates an extra expense of gas.

• L874: directly return the _calculateFee() calls and not create a variable, this generates an extra expense of gas.

StakedCitadelVester.sol

• L111: esting[recipient].unlockEnd could be put as a local variable, since it is used more than once.

• L115: esting[recipient].unlockBegin could be put as a local variable, since it is used more than once.

StakedCitadelLocker.sol

• L265: take the i++ out of the for() and make it unckecked.

• L407: take the i-- out of the for() and make it unckecked.

• L426: take the i-- out of the for() and make it unckecked.

• L421: the return variable uint256 "epoch" is not used, should it be used or not create that variable.