Gas Optimizations

[code423n4]

Vulnerability details:

Gas Optimizations

Use a more recent version of solidity

Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

1. File: external/MedianOracle.sol (line 1)

   pragma solidity 0.4.24;

2. File: external/StakedCitadelLocker.sol (line 2)

   pragma solidity 0.6.12;

Use a more recent version of solidity

Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

1. File: src/CitadelToken.sol (line 2)

   pragma solidity ^0.8.0;

2. File: src/GlobalAccessControl.sol (line 3)

   pragma solidity ^0.8.0;

3. File: src/lib/SafeERC20.sol (line 4)

   pragma solidity ^0.8.0;

Unused state variables should be removed

These unused state variables are wasting a lot of gas. The compiler automatically creates non-payable getter functions which is expensive and each variable, since it is given a value, costs a ton of gas to initialize. Variables initialized to zero cost G~sreset~ (2900 gas) and variables initialized to non-zero cost G~sset~ (20000 gas)

1. File: src/StakedCitadelLocker.sol (lines 91-99)

   // ========== Not used ==========
   //boost
   address public boostPayment = address(0);
   uint256 public maximumBoostPayment = 0;
   uint256 public boostRate = 10000;
   uint256 public nextMaximumBoostPayment = 0;
   uint256 public nextBoostRate = 10000;
   uint256 public constant denominator = 10000;
   // ==============================

Changing from immutable to just private wastes a lot of gas

One of the changes made after branching this file was to remove the immutable keyword. Doing this changes the variable from a deployment-time replacement to a state variable that gets assigned, costing G~sset~ (20000 gas). The variable's value does not change after initialization, so it should remain immutable

1. File: src/StakedCitadelLocker.sol (line 117)

   uint8 private _decimals;

Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping as well as a non-payable getter for the mapping. Depending on the circumstances and sizes of types, can avoid a G~sset~ (20000 gas). Reads and subsequent writes are also cheaper

1. File: external/StakedCitadelLocker.sol (lines 76-80)

   // user -> reward token -> amount
   mapping(address => mapping(address => uint256)) public userRewardPerTokenPaid;
   mapping(address => mapping(address => uint256)) public rewards;

2. File: external/StakedCitadelLocker.sol (lines 88-89)

   mapping(address => Balances) public balances;
   mapping(address => LockedBalance[]) public userLocks;

3. File: src/KnightingRound.sol (lines 47-51)

   mapping(address => uint256) public boughtAmounts;
   /// Whether an account has claimed tokens
   /// NOTE: can reset boughtAmounts after a claim to optimize gas
   ///       but we need to persist boughtAmounts
   mapping(address => bool) public hasClaimed;

4. File: src/StakedCitadel.sol (lines 94-95)

   mapping(address => uint256) public additionalTokensEarned;
   mapping(address => uint256) public lastAdditionalTokenAmount;

State variables can be packed into fewer storage slots

If variables occupying the same slot are both written the same function or by the constructor, avoids a separate G~sset~ (20000 gas). Reads of the variables are also cheaper

1. File: external/StakedCitadelLocker.sol (line 63)

   IERC20Upgradeable public stakingToken; // xCTDL token

   Variable ordering with two fewer slots: address:rewardTokens, mapping(32):rewardData, mapping(32):rewardDistributors, mapping(32):userRewardPerTokenPaid, mapping(32):rewards, uint256(32):lockedSupply, uint256(32):boostedSupply, user-defined:epochs, mapping(32):balances, mapping(32):userLocks, uint256(32):maximumBoostPayment, uint256(32):boostRate, uint256(32):nextMaximumBoostPayment, uint256(32):nextBoostRate, uint256(32):minimumStake, uint256(32):maximumStake, uint256(32):kickRewardPerEpoch, uint256(32):kickRewardEpochDelay, string(32):_name, string(32):_symbol, user-defined(20):stakingToken, bool(1):isShutdown, uint8(1):_decimals, address(20):boostPayment, address(20):stakingProxy

2. File: src/Funding.sol (line 32)

   IERC20 public citadel; /// token to distribute (in vested xCitadel form)

   Variable ordering with one fewer slots: uint256(32):citadelPriceInAsset, uint256(32):minCitadelPriceInAsset, uint256(32):maxCitadelPriceInAsset, uint256(32):assetDecimalsNormalizationValue, user-defined(20):citadel, bool(1):citadelPriceFlag, user-defined(20):xCitadel, user-defined(20):asset, address(20):citadelPriceInAssetOracle, address(20):saleRecipient, user-defined(*):funding

Pre-calculate repeatedly-checked offsets

Reading from storage is expensive, so it saves gas when only one variable has to be read versus multiple. If there is a calculation which requires multiple storage reads, the calculation should be optimized to pre-calculate as much as possible, and store the intermediate result in storage. Replace the saleDuration variable with a private pre-calculated saleEnd timestamp, and reference that rather than checking both saleStart and saleDuration in multiple places. The duration can be calculated by subtracting the start from the end in a view function. Making this change saves a G~coldsload~ (2100 gas) for each call to buy() and saleEnded()

1. File: src/KnightingRound.sol (lines 168-171)

       require(
           block.timestamp < saleStart + saleDuration,
           "KnightingRound: already ended"
       );

2. File: src/KnightingRound.sol (lines 259-261)

       hasEnded_ =
           (block.timestamp >= saleStart + saleDuration) ||
           (totalTokenIn >= tokenInLimit);

State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. With each of these individually, caching will replace a G~warmaccess~ (100 gas) with a much cheaper stack read. Less obvious optimizations flagged include having local storage variables of mappings within state variable mappings or mappings within state variable structs, having local storage variables of structs within mappings, or having local caches of state variable contracts/addresses.

1. File: external/MedianOracle.sol (line 144)

       providerReports[providerAddress][0].timestamp=1;

2. File: external/MedianOracle.sol (line 173)

               uint256 reportTimestampPast = providerReports[providerAddress][index_past].timestamp;

3. File: external/MedianOracle.sol (line 213)

       providerReports[provider][0].timestamp = 1;

4. File: external/StakedCitadelLocker.sol (line 770)

           stakingToken.safeTransfer(stakingProxy, increase);

5. File: external/StakedCitadelLocker.sol (line 166)

       rewardData[_rewardsToken].lastUpdateTime = uint40(block.timestamp);

6. File: external/StakedCitadelLocker.sol (line 229)

       uint256(rewardData[_rewardsToken].rewardPerTokenStored).add(

7. File: external/StakedCitadelLocker.sol (line 781)

               rewards[_account][_rewardsToken] = 0;

8. File: external/StakedCitadelLocker.sol (line 232)

               rewardData[_rewardsToken].rewardRate).mul(1e18).div(rewardData[_rewardsToken].useBoost ? boostedSupply : lockedSupply)

9. File: external/StakedCitadelLocker.sol (line 292)

       amount = balances[_user].boosted;

10. File: external/StakedCitadelLocker.sol (line 559)

        if (idx == 0 || userLocks[_account][idx - 1].unlockTime < unlockTime) {

11. File: external/StakedCitadelLocker.sol (line 537)

        uint256 boostRatio = boostRate.mul(_spendRatio).div(maximumBoostPayment==0?1:maximumBoostPayment);

12. File: external/StakedCitadelLocker.sol (line 509)

                maximumBoostPayment = nextMaximumBoostPayment;

13. File: external/StakedCitadelLocker.sol (line 506)

                boostRate = nextBoostRate;

14. File: external/StakedCitadelLocker.sol (line 762)

        uint256 min = MathUpgradeable.min(minimumStake, minimumStake - _offset);

15. File: external/StakedCitadelLocker.sol (line 761)

        uint256 max = maximumStake.add(_offset);

16. File: external/StakedCitadelLocker.sol (line 766)

            IStakingProxy(stakingProxy).withdraw(remove);

17. File: src/CitadelMinter.sol (line 217)

            IERC20Upgradeable(address(citadelToken)).safeTransfer(address(cachedXCitadel), stakingAmount);

18. File: src/CitadelMinter.sol (line 276)

            uint256 _newTotalWeight = totalFundingPoolWeight;

19. File: src/Funding.sol (line 134)

        assetDecimalsNormalizationValue = 10**asset.decimals();

20. File: src/Funding.sol (line 341)

        asset.safeTransfer(saleRecipient, amount);

21. File: src/Funding.sol (line 434)

                minCitadelPriceInAsset,

22. File: src/Funding.sol (line 461)

                minCitadelPriceInAsset,

23. File: src/Funding.sol (line 435)

                maxCitadelPriceInAsset

24. File: src/Funding.sol (line 462)

                maxCitadelPriceInAsset

25. File: src/KnightingRound.sol (line 148)

        tokenInNormalizationValue = 10**tokenIn.decimals();

26. File: src/KnightingRound.sol (line 169)

            block.timestamp < saleStart + saleDuration,

27. File: src/KnightingRound.sol (line 198)

        totalTokenIn = totalTokenIn + _tokenInAmount;

28. File: src/KnightingRound.sol (line 250)

            limitLeft_ = tokenInLimit - totalTokenIn;

29. File: src/KnightingRound.sol (line 250)

            limitLeft_ = tokenInLimit - totalTokenIn;

30. File: src/KnightingRound.sol (line 179)

            require(guestlist.authorized(msg.sender, _proof), "not authorized");

31. File: src/StakedCitadel.sol (line 774)

        token.safeTransferFrom(msg.sender, address(this), _amount);

32. File: src/StakedCitadel.sol (line 819)

            uint256 _after = token.balanceOf(address(this));

33. File: src/StakedCitadel.sol (line 795)

                guestList.authorized(_recipient, _amount, proof),

34. File: src/StakedCitadel.sol (line 507)

                IStrategy(strategy).balanceOf() == 0,

35. File: src/StakedCitadel.sol (line 723)

        IStrategy(strategy).earn();

36. File: src/StakedCitadel.sol (line 831)

        token.safeTransfer(vesting, _amount);

37. File: src/StakedCitadel.sol (line 909)

            ? (managementFee * (balance() - _harvestedAmount) * duration) /

38. File: src/SupplySchedule.sol (line 64)

        return (_timestamp - globalStartTimestamp) / epochLength;

39. File: src/SupplySchedule.sol (line 184)

            lastMintTimestamp > globalStartTimestamp,

Add unchecked {} for subtractions where the operands cannot underflow because of a previous require()

require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }

1. File: src/lib/SafeERC20.sol (line 79)

           uint256 newAllowance = oldAllowance - value;

Cheaper checks should be done first

Checking of equality to account is cheaper than looking up the gac role via static call, so that check should be on the left of the condition to shortcut the logic

1. File: src/lib/GlobalAccessControlManaged.sol (line 63)

           gac.hasRole(role, msg.sender) || msg.sender == account,

.length should not be looked up in every loop of a for-loop

Even memory arrays incur the overhead of bit tests and bit shifts to calculate the array length. Storage array length checks incur an extra G~warmaccess~ (100 gas) PER-LOOP.

1. File: external/MedianOracle.sol (line 226)

       for (uint256 i = 0; i < providers.length; i++) {

2. File: external/StakedCitadelLocker.sol (line 267)

       for (uint256 i = 0; i < userRewards.length; i++) {

3. File: external/StakedCitadelLocker.sol (line 459)

       for (uint i = nextUnlockIndex; i < locks.length; i++) {

4. File: external/StakedCitadelLocker.sol (line 777)

       for (uint i; i < rewardTokens.length; i++) {

5. File: external/StakedCitadelLocker.sol (line 838)

           for (uint i = 0; i < rewardTokens.length; i++) {

6. File: src/lib/GlobalAccessControlManaged.sol (line 48)

       for (uint256 i = 0; i < roles.length; i++) {

++i/i++ should be unchecked{++i}/unchecked{++i} when it is not possible for them to overflow, as is the case when used in for- and while-loops

1. File: external/MedianOracle.sol (line 164)

       for (uint256 i = 0; i < reportsCount; i++) {

2. File: external/MedianOracle.sol (line 226)

       for (uint256 i = 0; i < providers.length; i++) {

3. File: external/StakedCitadelLocker.sol (line 267)

       for (uint256 i = 0; i < userRewards.length; i++) {

4. File: external/StakedCitadelLocker.sol (line 296)

       for (uint i = nextUnlockIndex; i < locksLength; i++) {

5. File: external/StakedCitadelLocker.sol (line 325)

       for (uint i = locks.length - 1; i + 1 != 0; i--) {

6. File: external/StakedCitadelLocker.sol (line 363)

       for (uint i = locks.length - 1; i + 1 != 0; i--) {

7. File: external/StakedCitadelLocker.sol (line 391)

       for (uint i = epochindex - 1; i + 1 != 0; i--) {

8. File: external/StakedCitadelLocker.sol (line 409)

       for (uint i = _epoch; i + 1 != 0; i--) {

9. File: external/StakedCitadelLocker.sol (line 428)

       for (uint256 i = 0; i < 128; i++) {

10. File: external/StakedCitadelLocker.sol (line 459)

        for (uint i = nextUnlockIndex; i < locks.length; i++) {

11. File: external/StakedCitadelLocker.sol (line 659)

            for (uint i = nextUnlockIndex; i < length; i++) {

12. File: external/StakedCitadelLocker.sol (line 777)

        for (uint i; i < rewardTokens.length; i++) {

13. File: external/StakedCitadelLocker.sol (line 838)

            for (uint i = 0; i < rewardTokens.length; i++) {

14. File: src/CitadelMinter.sol (line 152)

        for (uint256 i = 0; i < numPools; i++) {

15. File: src/CitadelMinter.sol (line 344)

        for (uint256 i; i < length; ++i) {

16. File: src/lib/GlobalAccessControlManaged.sol (line 48)

        for (uint256 i = 0; i < roles.length; i++) {

17. File: src/SupplySchedule.sol (line 208)

        for (uint256 i = startingEpoch; i <= endingEpoch; i++) {

require()/revert() strings longer than 32 bytes cost extra gas

1. File: src/CitadelMinter.sol (lines 299-302)

       require(
           _fundingBps + _stakingBps + _lockingBps == MAX_BPS,
           "CitadelMinter: Sum of propvalues must be 10000 bps"
       );

2. File: src/CitadelMinter.sol (lines 319-322)

       require(
           lastMintTimestamp == 0,
           "CitadelMinter: last mint timestamp already initialized"
       );

3. File: src/CitadelMinter.sol (lines 326-329)

       require(
           globalStartTimestamp != 0,
           "CitadelMinter: supply schedule start not initialized"
       );

4. File: src/CitadelMinter.sol (lines 368-371)

       require(
           fundingPools.remove(_pool),
           "CitadelMinter: funding pool does not exist for removal"
       );

5. File: src/CitadelMinter.sol (lines 375-378)

       require(
           fundingPools.add(_pool),
           "CitadelMinter: funding pool already exists"
       );

6. File: src/Funding.sol (lines 146-149)

       require(
           citadelPriceFlag == false,
           "Funding: citadel price from oracle flagged and pending review"
       );

7. File: src/Funding.sol (lines 296-299)

       require(
           _assetCap > funding.assetCumulativeFunded,
           "cannot decrease cap below global sum of assets in"
       );

8. File: src/Funding.sol (lines 323-326)

       require(
           _token != address(asset),
           "cannot sweep funding asset, use claimAssetToTreasury()"
       );

9. File: src/Funding.sol (lines 388-391)

       require(
           _saleRecipient != address(0),
           "Funding: sale recipient should not be zero"
       );

10. File: src/GlobalAccessControl.sol (lines 116-119)

        require(
            keccak256(bytes(roleString)) == role,
            "Role string and role do not match"
        );

11. File: src/KnightingRound.sol (lines 120-123)

        require(
            _saleStart >= block.timestamp,
            "KnightingRound: start date may not be in the past"
        );

12. File: src/KnightingRound.sol (lines 124-127)

        require(
            _saleDuration > 0,
            "KnightingRound: the sale duration must not be zero"
        );

13. File: src/KnightingRound.sol (lines 128-131)

        require(
            _tokenOutPrice > 0,
            "KnightingRound: the price must not be zero"
        );

14. File: src/KnightingRound.sol (lines 132-135)

        require(
            _saleRecipient != address(0),
            "KnightingRound: sale recipient should not be zero"
        );

15. File: src/KnightingRound.sol (line 273)

        require(!finalized, "KnightingRound: already finalized");

16. File: src/KnightingRound.sol (lines 275-278)

        require(
            tokenOut.balanceOf(address(this)) >= totalTokenOutBought,
            "KnightingRound: not enough balance"
        );

17. File: src/KnightingRound.sol (lines 293-296)

        require(
            _saleStart >= block.timestamp,
            "KnightingRound: start date may not be in the past"
        );

18. File: src/KnightingRound.sol (line 297)

        require(!finalized, "KnightingRound: already finalized");

19. File: src/KnightingRound.sol (lines 312-315)

        require(
            _saleDuration > 0,
            "KnightingRound: the sale duration must not be zero"
        );

20. File: src/KnightingRound.sol (line 316)

        require(!finalized, "KnightingRound: already finalized");

21. File: src/KnightingRound.sol (lines 331-334)

        require(
            _tokenOutPrice > 0,
            "KnightingRound: the price must not be zero"
        );

22. File: src/KnightingRound.sol (lines 349-352)

        require(
            _saleRecipient != address(0),
            "KnightingRound: sale recipient should not be zero"
        );

23. File: src/KnightingRound.sol (line 384)

        require(!finalized, "KnightingRound: already finalized");

24. File: src/lib/GlobalAccessControlManaged.sol (lines 62-65)

        require(
            gac.hasRole(role, msg.sender) || msg.sender == account,
            "GAC: invalid-caller-role-or-address"
        );

25. File: src/lib/SafeERC20.sol (lines 55-58)

        require(
            (value == 0) || (token.allowance(address(this), spender) == 0),
            "SafeERC20: approve from non-zero to non-zero allowance"
        );

26. File: src/lib/SafeERC20.sol (line 78)

            require(oldAllowance >= value, "SafeERC20: decreased allowance below zero");

27. File: src/lib/SafeERC20.sol (line 98)

            require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed");

28. File: src/StakedCitadel.sol (lines 190-193)

        require(
            _feeConfig[0] <= PERFORMANCE_FEE_HARD_CAP,
            "performanceFeeGovernance too high"
        );

29. File: src/StakedCitadel.sol (lines 194-197)

        require(
            _feeConfig[1] <= PERFORMANCE_FEE_HARD_CAP,
            "performanceFeeStrategist too high"
        );

30. File: src/StakedCitadel.sol (lines 506-509)

            require(
                IStrategy(strategy).balanceOf() == 0,
                "Please withdrawToVault before changing strat"
            );

31. File: src/StakedCitadel.sol (lines 535-538)

        require(
            _fees <= PERFORMANCE_FEE_HARD_CAP,
            "performanceFeeStrategist too high"
        );

32. File: src/StakedCitadel.sol (lines 630-633)

        require(
            _performanceFeeStrategist <= maxPerformanceFee,
            "Excessive strategist performance fee"
        );

33. File: src/StakedCitadel.sol (lines 650-653)

        require(
            _performanceFeeGovernance <= maxPerformanceFee,
            "Excessive governance performance fee"
        );

34. File: src/StakedCitadelVester.sol (line 137)

        require(msg.sender == vault, "StakedCitadelVester: only xCTDL vault");

35. File: src/StakedCitadelVester.sol (line 138)

        require(_amount > 0, "StakedCitadelVester: cannot vest 0");

36. File: src/SupplySchedule.sol (lines 60-63)

        require(
            globalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

37. File: src/SupplySchedule.sol (lines 90-93)

        require(
            cachedGlobalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

38. File: src/SupplySchedule.sol (lines 94-97)

        require(
            block.timestamp > lastMintTimestamp,
            "SupplySchedule: already minted up to current block"
        );

39. File: src/SupplySchedule.sol (lines 137-140)

        require(
            globalStartTimestamp == 0,
            "SupplySchedule: minting already started"
        );

40. File: src/SupplySchedule.sol (lines 141-144)

        require(
            _globalStartTimestamp >= block.timestamp,
            "SupplySchedule: minting must start at or after current time"
        );

41. File: src/SupplySchedule.sol (lines 155-158)

        require(
            epochRate[_epoch] == 0,
            "SupplySchedule: rate already set for given epoch"
        );

42. File: src/SupplySchedule.sol (lines 179-182)

        require(
            globalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

43. File: src/SupplySchedule.sol (lines 183-186)

        require(
            lastMintTimestamp > globalStartTimestamp,
            "SupplySchedule: attempting to mint before start block"
        );

44. File: src/SupplySchedule.sol (lines 187-190)

        require(
            block.timestamp > lastMintTimestamp,
            "SupplySchedule: already minted up to current block"
        );

Not using the named return variables when a function returns, wastes deployment gas

1. File: external/StakedCitadelLocker.sol (line 277)

       return balances[_user].boosted;

2. File: external/StakedCitadelLocker.sol (line 282)

       return balances[_user].locked;

3. File: external/StakedCitadelLocker.sol (line 350)

           return locks[locksLength - 1].boosted;

4. File: external/StakedCitadelLocker.sol (line 353)

       return 0;

5. File: external/StakedCitadelLocker.sol (line 368)

               return locks[i].boosted;

6. File: external/StakedCitadelLocker.sol (line 375)

       return 0;

7. File: external/StakedCitadelLocker.sol (line 435)

               return mid;

8. File: external/StakedCitadelLocker.sol (line 442)

       return min;

9. File: external/StakedCitadelLocker.sol (line 471)

       return (userBalance.locked, unlockable, locked, lockData);

10. File: external/StakedCitadelLocker.sol (line 471)

        return (userBalance.locked, unlockable, locked, lockData);

11. File: external/StakedCitadelLocker.sol (line 471)

        return (userBalance.locked, unlockable, locked, lockData);

12. File: external/StakedCitadelLocker.sol (line 471)

        return (userBalance.locked, unlockable, locked, lockData);

Remove unused local variable

1. File: external/StakedCitadelLocker.sol (line 735)

       uint256 balance = stakingToken.balanceOf(address(this));

Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27

1. File: external/StakedCitadelLocker.sol (line 76)

   mapping(address => mapping(address => bool)) public rewardDistributors;

2. File: external/StakedCitadelLocker.sol (line 112)

   bool public isShutdown = false;

3. File: src/Funding.sol (line 39)

   bool public citadelPriceFlag; /// Flag citadel price for review by guardian if it exceeds min and max bounds;

4. File: src/GlobalAccessControl.sol (line 51)

   bool public transferFromDisabled; // Set to true in initialize

5. File: src/KnightingRound.sol (line 40)

   bool public finalized;

6. File: src/KnightingRound.sol (line 51)

   mapping(address => bool) public hasClaimed;

7. File: src/StakedCitadel.sol (line 75)

   bool public pausedDeposit; // false by default Allows to only block deposits, use pause for the normal pause state

Using > 0 costs more gas than != 0 when used on a uint in a require() statement

1. File: external/MedianOracle.sol (line 69)

       require(minimumProviders_ > 0);

2. File: external/MedianOracle.sol (line 109)

       require(minimumProviders_ > 0);

3. File: external/MedianOracle.sol (line 123)

       require(timestamps[0] > 0);

4. File: external/StakedCitadelLocker.sol (line 526)

       require(_amount > 0, "Cannot stake 0");

5. File: external/StakedCitadelLocker.sol (line 681)

       require(locked > 0, "no exp locks");

6. File: external/StakedCitadelLocker.sol (line 813)

       require(_reward > 0, "No reward");

7. File: src/CitadelMinter.sol (line 343)

       require(length > 0, "CitadelMinter: no funding pools");

8. File: src/Funding.sol (line 170)

       require(_assetAmountIn > 0, "_assetAmountIn must not be 0");

9. File: src/Funding.sol (line 322)

       require(amount > 0, "nothing to sweep");

10. File: src/Funding.sol (line 340)

        require(amount > 0, "nothing to claim");

11. File: src/Funding.sol (line 424)

        require(_citadelPriceInAsset > 0, "citadel price must not be zero");

12. File: src/Funding.sol (line 452)

        require(_citadelPriceInAsset > 0, "citadel price must not be zero");

13. File: src/interfaces/convex/BoringMath.sol (line 20)

        require(b > 0, "BoringMath: division by zero");

14. File: src/interfaces/convex/BoringMath.sol (line 102)

        require(b > 0, "BoringMath: division by zero");

15. File: src/interfaces/convex/BoringMath.sol (line 122)

        require(b > 0, "BoringMath: division by zero");

16. File: src/interfaces/convex/BoringMath.sol (line 142)

        require(b > 0, "BoringMath: division by zero");

17. File: src/KnightingRound.sol (lines 124-127)

        require(
            _saleDuration > 0,
            "KnightingRound: the sale duration must not be zero"
        );

18. File: src/KnightingRound.sol (lines 128-131)

        require(
            _tokenOutPrice > 0,
            "KnightingRound: the price must not be zero"
        );

19. File: src/KnightingRound.sol (line 172)

        require(_tokenInAmount > 0, "_tokenInAmount should be > 0");

20. File: src/KnightingRound.sol (line 215)

        require(tokenOutAmount_ > 0, "nothing to claim");

21. File: src/KnightingRound.sol (lines 312-315)

        require(
            _saleDuration > 0,
            "KnightingRound: the sale duration must not be zero"
        );

22. File: src/KnightingRound.sol (lines 331-334)

        require(
            _tokenOutPrice > 0,
            "KnightingRound: the price must not be zero"
        );

23. File: src/KnightingRound.sol (line 411)

        require(amount > 0, "nothing to sweep");

24. File: src/StakedCitadelVester.sol (line 138)

        require(_amount > 0, "StakedCitadelVester: cannot vest 0");

25. File: src/SupplySchedule.sol (lines 60-63)

        require(
            globalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

26. File: src/SupplySchedule.sol (lines 90-93)

        require(
            cachedGlobalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

27. File: src/SupplySchedule.sol (lines 179-182)

        require(
            globalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

It costs more gas to initialize variables to zero than to let the default of zero be applied

1. File: external/MedianOracle.sol (line 160)

       uint256 size = 0;

2. File: external/MedianOracle.sol (line 164)

       for (uint256 i = 0; i < reportsCount; i++) {

3. File: external/MedianOracle.sol (line 226)

       for (uint256 i = 0; i < providers.length; i++) {

4. File: external/StakedCitadelLocker.sol (line 267)

       for (uint256 i = 0; i < userRewards.length; i++) {

5. File: external/StakedCitadelLocker.sol (line 423)

       uint256 min = 0;

6. File: external/StakedCitadelLocker.sol (line 428)

       for (uint256 i = 0; i < 128; i++) {

7. File: external/StakedCitadelLocker.sol (line 634)

       uint256 reward = 0;

8. File: external/StakedCitadelLocker.sol (line 838)

           for (uint i = 0; i < rewardTokens.length; i++) {

9. File: src/CitadelMinter.sol (line 152)

       for (uint256 i = 0; i < numPools; i++) {

10. File: src/CitadelMinter.sol (line 180)

        uint256 lockingAmount = 0;

11. File: src/CitadelMinter.sol (line 181)

        uint256 stakingAmount = 0;

12. File: src/CitadelMinter.sol (line 182)

        uint256 fundingAmount = 0;

13. File: src/lib/GlobalAccessControlManaged.sol (line 48)

        for (uint256 i = 0; i < roles.length; i++) {

14. File: src/SupplySchedule.sol (line 103)

        uint256 mintable = 0;

15. File: src/SupplySchedule.sol (line 192)

        uint256 mintable = 0;

internal functions only called once can be inlined to save gas

1. File: external/StakedCitadelLocker.sol (line 747)

   function updateStakeRatio(uint256 _offset) internal {

2. File: external/StakedCitadelLocker.sol (line 796)

   function _notifyReward(address _rewardsToken, uint256 _reward) internal {

3. File: src/CitadelMinter.sol (line 338)

   function _transferToFundingPools(uint256 _citadelAmount) internal {

4. File: src/CitadelMinter.sol (line 362)

   function _removeFundingPool(address _pool) internal {

5. File: src/CitadelMinter.sol (line 374)

   function _addFundingPool(address _pool) internal {

6. File: src/StakedCitadel.sol (lines 764-766)

   function _depositFor(address _recipient, uint256 _amount)
       internal
       nonReentrant

7. File: src/StakedCitadel.sol (lines 859-862)

   function _calculatePerformanceFee(uint256 _amount)
       internal
       view
       returns (uint256, uint256)

8. File: src/StakedCitadel.sol (line 898)

   function _handleFees(uint256 _harvestedAmount, uint256 harvestTime)

9. File: src/SupplySchedule.sol (lines 169-170)

   function _setEpochRates() internal {
       epochRate[0] = 593962000000000000000000 / epochLength;

Using calldata instead of memory for read-only arguments in external functions saves gas

1. File: src/GlobalAccessControl.sol (line 109)

       string memory roleString,

2. File: src/StakedCitadel.sol (line 319)

   function deposit(uint256 _amount, bytes32[] memory proof)

3. File: src/StakedCitadel.sol (line 341)

   function depositAll(bytes32[] memory proof) external whenNotPaused {

4. File: src/StakedCitadel.sol (line 366)

       bytes32[] memory proof

++i costs less gas than ++i, especially when it's used in for-loops (--i/i-- too)

1. File: external/MedianOracle.sol (line 164)

       for (uint256 i = 0; i < reportsCount; i++) {

2. File: external/MedianOracle.sol (line 226)

       for (uint256 i = 0; i < providers.length; i++) {

3. File: external/StakedCitadelLocker.sol (line 267)

       for (uint256 i = 0; i < userRewards.length; i++) {

4. File: external/StakedCitadelLocker.sol (line 296)

       for (uint i = nextUnlockIndex; i < locksLength; i++) {

5. File: external/StakedCitadelLocker.sol (line 325)

       for (uint i = locks.length - 1; i + 1 != 0; i--) {

6. File: external/StakedCitadelLocker.sol (line 363)

       for (uint i = locks.length - 1; i + 1 != 0; i--) {

7. File: external/StakedCitadelLocker.sol (line 391)

       for (uint i = epochindex - 1; i + 1 != 0; i--) {

8. File: external/StakedCitadelLocker.sol (line 409)

       for (uint i = _epoch; i + 1 != 0; i--) {

9. File: external/StakedCitadelLocker.sol (line 428)

       for (uint256 i = 0; i < 128; i++) {

10. File: external/StakedCitadelLocker.sol (line 459)

        for (uint i = nextUnlockIndex; i < locks.length; i++) {

11. File: external/StakedCitadelLocker.sol (line 659)

            for (uint i = nextUnlockIndex; i < length; i++) {

12. File: external/StakedCitadelLocker.sol (line 777)

        for (uint i; i < rewardTokens.length; i++) {

13. File: external/StakedCitadelLocker.sol (line 838)

            for (uint i = 0; i < rewardTokens.length; i++) {

14. File: src/CitadelMinter.sol (line 152)

        for (uint256 i = 0; i < numPools; i++) {

15. File: src/lib/GlobalAccessControlManaged.sol (line 48)

        for (uint256 i = 0; i < roles.length; i++) {

16. File: src/SupplySchedule.sol (line 208)

        for (uint256 i = startingEpoch; i <= endingEpoch; i++) {

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

1. File: external/MedianOracle.sol (line 125)

       uint8 index_recent = timestamps[0] >= timestamps[1] ? 0 : 1;

2. File: external/MedianOracle.sol (line 126)

       uint8 index_past = 1 - index_recent;

3. File: external/MedianOracle.sol (line 168)

           uint8 index_recent = reports[0].timestamp >= reports[1].timestamp ? 0 : 1;

4. File: external/MedianOracle.sol (line 169)

           uint8 index_past = 1 - index_recent;

5. File: external/StakedCitadelLocker.sol (line 38)

       uint40 periodFinish;

6. File: external/StakedCitadelLocker.sol (line 39)

       uint208 rewardRate;

7. File: external/StakedCitadelLocker.sol (line 40)

       uint40 lastUpdateTime;

8. File: external/StakedCitadelLocker.sol (line 41)

       uint208 rewardPerTokenStored;

9. File: external/StakedCitadelLocker.sol (line 44)

       uint112 locked;

10. File: external/StakedCitadelLocker.sol (line 45)

        uint112 boosted;

11. File: external/StakedCitadelLocker.sol (line 46)

        uint32 nextUnlockIndex;

12. File: external/StakedCitadelLocker.sol (line 49)

        uint112 amount;

13. File: external/StakedCitadelLocker.sol (line 50)

        uint112 boosted;

14. File: external/StakedCitadelLocker.sol (line 51)

        uint32 unlockTime;

15. File: external/StakedCitadelLocker.sol (line 58)

        uint224 supply; //epoch boosted supply

16. File: external/StakedCitadelLocker.sol (line 59)

        uint32 date; //epoch start date

17. File: external/StakedCitadelLocker.sol (line 117)

    uint8 private _decimals;

18. File: external/StakedCitadelLocker.sol (line 142)

    function decimals() public view returns (uint8) {

19. File: external/StakedCitadelLocker.sol (line 538)

        uint112 lockAmount = _amount.sub(spendAmount).to112();

20. File: external/StakedCitadelLocker.sol (line 539)

        uint112 boostedAmount = _amount.add(_amount.mul(boostRatio).div(denominator)).to112();

21. File: external/StakedCitadelLocker.sol (line 631)

        uint112 locked;

22. File: external/StakedCitadelLocker.sol (line 632)

        uint112 boostedAmount;

23. File: external/StakedCitadelLocker.sol (line 658)

            uint32 nextUnlockIndex = userBalance.nextUnlockIndex;

24. File: src/KnightingRound.sol (line 78)

        uint8 indexed daoId,

25. File: src/KnightingRound.sol (line 164)

        uint8 _daoId,

Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

See this issue for a detail description of the issue

1. File: src/CitadelMinter.sol (lines 30-31)

   bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
       keccak256("CONTRACT_GOVERNANCE_ROLE");

2. File: src/CitadelMinter.sol (lines 32-33)

   bytes32 public constant POLICY_OPERATIONS_ROLE =
       keccak256("POLICY_OPERATIONS_ROLE");

3. File: src/CitadelToken.sol (lines 9-10)

   bytes32 public constant CITADEL_MINTER_ROLE =
       keccak256("CITADEL_MINTER_ROLE");

4. File: src/Funding.sol (lines 21-22)

   bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
       keccak256("CONTRACT_GOVERNANCE_ROLE");

5. File: src/Funding.sol (lines 23-24)

   bytes32 public constant POLICY_OPERATIONS_ROLE =
       keccak256("POLICY_OPERATIONS_ROLE");

6. File: src/Funding.sol (line 25)

   bytes32 public constant TREASURY_OPERATIONS_ROLE = keccak256("TREASURY_OPERATIONS_ROLE");

7. File: src/Funding.sol (lines 26-27)

   bytes32 public constant TREASURY_VAULT_ROLE =
       keccak256("TREASURY_VAULT_ROLE");

8. File: src/Funding.sol (line 28)

   bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");

9. File: src/GlobalAccessControl.sol (lines 25-26)

   bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
       keccak256("CONTRACT_GOVERNANCE_ROLE");

10. File: src/GlobalAccessControl.sol (lines 27-28)

    bytes32 public constant TREASURY_GOVERNANCE_ROLE =
        keccak256("TREASURY_GOVERNANCE_ROLE");

11. File: src/GlobalAccessControl.sol (lines 30-31)

    bytes32 public constant TECH_OPERATIONS_ROLE =
        keccak256("TECH_OPERATIONS_ROLE");

12. File: src/GlobalAccessControl.sol (lines 32-33)

    bytes32 public constant POLICY_OPERATIONS_ROLE =
        keccak256("POLICY_OPERATIONS_ROLE");

13. File: src/GlobalAccessControl.sol (lines 34-35)

    bytes32 public constant TREASURY_OPERATIONS_ROLE =
        keccak256("TREASURY_OPERATIONS_ROLE");

14. File: src/GlobalAccessControl.sol (line 37)

    bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");

15. File: src/GlobalAccessControl.sol (line 39)

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

16. File: src/GlobalAccessControl.sol (line 40)

    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");

17. File: src/GlobalAccessControl.sol (lines 42-43)

    bytes32 public constant BLOCKLIST_MANAGER_ROLE =
        keccak256("BLOCKLIST_MANAGER_ROLE");

18. File: src/GlobalAccessControl.sol (line 44)

    bytes32 public constant BLOCKLISTED_ROLE = keccak256("BLOCKLISTED_ROLE");

19. File: src/GlobalAccessControl.sol (lines 46-47)

    bytes32 public constant CITADEL_MINTER_ROLE =
        keccak256("CITADEL_MINTER_ROLE");

20. File: src/KnightingRound.sol (lines 19-20)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

21. File: src/KnightingRound.sol (lines 21-22)

    bytes32 public constant TREASURY_GOVERNANCE_ROLE =
        keccak256("TREASURY_GOVERNANCE_ROLE");

22. File: src/KnightingRound.sol (lines 24-25)

    bytes32 public constant TECH_OPERATIONS_ROLE =
        keccak256("TECH_OPERATIONS_ROLE");

23. File: src/KnightingRound.sol (lines 26-27)

    bytes32 public constant TREASURY_OPERATIONS_ROLE =
        keccak256("TREASURY_OPERATIONS_ROLE");

24. File: src/lib/GlobalAccessControlManaged.sol (line 15)

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

25. File: src/lib/GlobalAccessControlManaged.sol (line 16)

    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");

26. File: src/StakedCitadelVester.sol (lines 20-21)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

27. File: src/SupplySchedule.sol (lines 22-23)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code

1. File: external/StakedCitadelLocker.sol (line 70)

   uint256 public constant rewardsDuration = 86400; // 1 day

2. File: external/StakedCitadelLocker.sol (line 73)

   uint256 public constant lockDuration = rewardsDuration * 7 * 21; // 21 weeks

3. File: external/StakedCitadelLocker.sol (line 98)

   uint256 public constant denominator = 10000;

4. File: external/StakedCitadelLocker.sol (line 105)

   uint256 public constant stakeOffsetOnLock = 500; //allow broader range for staking when depositing

5. File: src/CitadelMinter.sol (lines 30-31)

   bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
       keccak256("CONTRACT_GOVERNANCE_ROLE");

6. File: src/CitadelMinter.sol (lines 32-33)

   bytes32 public constant POLICY_OPERATIONS_ROLE =
       keccak256("POLICY_OPERATIONS_ROLE");

7. File: src/CitadelToken.sol (lines 9-10)

   bytes32 public constant CITADEL_MINTER_ROLE =
       keccak256("CITADEL_MINTER_ROLE");

8. File: src/Funding.sol (lines 21-22)

   bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
       keccak256("CONTRACT_GOVERNANCE_ROLE");

9. File: src/Funding.sol (lines 23-24)

   bytes32 public constant POLICY_OPERATIONS_ROLE =
       keccak256("POLICY_OPERATIONS_ROLE");

10. File: src/Funding.sol (line 25)

    bytes32 public constant TREASURY_OPERATIONS_ROLE = keccak256("TREASURY_OPERATIONS_ROLE");

11. File: src/Funding.sol (lines 26-27)

    bytes32 public constant TREASURY_VAULT_ROLE =
        keccak256("TREASURY_VAULT_ROLE");

12. File: src/Funding.sol (line 28)

    bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");

13. File: src/Funding.sol (line 30)

    uint256 public constant MAX_BPS = 10000;

14. File: src/GlobalAccessControl.sol (lines 25-26)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

15. File: src/GlobalAccessControl.sol (lines 27-28)

    bytes32 public constant TREASURY_GOVERNANCE_ROLE =
        keccak256("TREASURY_GOVERNANCE_ROLE");

16. File: src/GlobalAccessControl.sol (lines 30-31)

    bytes32 public constant TECH_OPERATIONS_ROLE =
        keccak256("TECH_OPERATIONS_ROLE");

17. File: src/GlobalAccessControl.sol (lines 32-33)

    bytes32 public constant POLICY_OPERATIONS_ROLE =
        keccak256("POLICY_OPERATIONS_ROLE");

18. File: src/GlobalAccessControl.sol (lines 34-35)

    bytes32 public constant TREASURY_OPERATIONS_ROLE =
        keccak256("TREASURY_OPERATIONS_ROLE");

19. File: src/GlobalAccessControl.sol (line 37)

    bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");

20. File: src/GlobalAccessControl.sol (line 39)

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

21. File: src/GlobalAccessControl.sol (line 40)

    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");

22. File: src/GlobalAccessControl.sol (lines 42-43)

    bytes32 public constant BLOCKLIST_MANAGER_ROLE =
        keccak256("BLOCKLIST_MANAGER_ROLE");

23. File: src/GlobalAccessControl.sol (line 44)

    bytes32 public constant BLOCKLISTED_ROLE = keccak256("BLOCKLISTED_ROLE");

24. File: src/GlobalAccessControl.sol (lines 46-47)

    bytes32 public constant CITADEL_MINTER_ROLE =
        keccak256("CITADEL_MINTER_ROLE");

25. File: src/KnightingRound.sol (lines 19-20)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

26. File: src/KnightingRound.sol (lines 21-22)

    bytes32 public constant TREASURY_GOVERNANCE_ROLE =
        keccak256("TREASURY_GOVERNANCE_ROLE");

27. File: src/KnightingRound.sol (lines 24-25)

    bytes32 public constant TECH_OPERATIONS_ROLE =
        keccak256("TECH_OPERATIONS_ROLE");

28. File: src/KnightingRound.sol (lines 26-27)

    bytes32 public constant TREASURY_OPERATIONS_ROLE =
        keccak256("TREASURY_OPERATIONS_ROLE");

29. File: src/lib/GlobalAccessControlManaged.sol (line 15)

    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

30. File: src/lib/GlobalAccessControlManaged.sol (line 16)

    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");

31. File: src/StakedCitadel.sol (line 112)

    uint256 public constant MAX_BPS = 10_000;

32. File: src/StakedCitadel.sol (line 113)

    uint256 public constant SECS_PER_YEAR = 31_556_952; // 365.2425 days

33. File: src/StakedCitadel.sol (line 115)

    uint256 public constant WITHDRAWAL_FEE_HARD_CAP = 200; // Never higher than 2%

34. File: src/StakedCitadel.sol (line 116)

    uint256 public constant PERFORMANCE_FEE_HARD_CAP = 3_000; // Never higher than 30% // 30% maximum performance fee // We usually do 20, so this is insanely high already

35. File: src/StakedCitadel.sol (line 117)

    uint256 public constant MANAGEMENT_FEE_HARD_CAP = 200; // Never higher than 2%

36. File: src/StakedCitadelVester.sol (lines 20-21)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

37. File: src/StakedCitadelVester.sol (line 34)

    uint256 public constant INITIAL_VESTING_DURATION = 86400 * 21; // 21 days of vesting

38. File: src/SupplySchedule.sol (lines 22-23)

    bytes32 public constant CONTRACT_GOVERNANCE_ROLE =
        keccak256("CONTRACT_GOVERNANCE_ROLE");

39. File: src/SupplySchedule.sol (line 25)

    uint256 public constant epochLength = 21 days;

Don't compare boolean expressions to boolean literals

if ( == true) => if (), if ( == false) => if (!)

1. File: src/Funding.sol (line 147)

           citadelPriceFlag == false,

Duplicated require()/revert() checks should be refactored to a modifier or function

1. File: external/MedianOracle.sol (line 84)

       require(reportExpirationTimeSec_ <= MAX_REPORT_EXPIRATION_TIME);

2. File: external/MedianOracle.sol (line 109)

       require(minimumProviders_ > 0);

3. File: src/Funding.sol (line 452)

       require(_citadelPriceInAsset > 0, "citadel price must not be zero");

4. File: src/KnightingRound.sol (lines 293-296)

       require(
           _saleStart >= block.timestamp,
           "KnightingRound: start date may not be in the past"
       );

5. File: src/KnightingRound.sol (lines 312-315)

       require(
           _saleDuration > 0,
           "KnightingRound: the sale duration must not be zero"
       );

6. File: src/KnightingRound.sol (lines 331-334)

       require(
           _tokenOutPrice > 0,
           "KnightingRound: the price must not be zero"
       );

7. File: src/KnightingRound.sol (lines 349-352)

       require(
           _saleRecipient != address(0),
           "KnightingRound: sale recipient should not be zero"
       );

8. File: src/KnightingRound.sol (line 297)

       require(!finalized, "KnightingRound: already finalized");

9. File: src/StakedCitadel.sol (line 700)

       require(address(token) != _token, "No want");

10. File: src/StakedCitadel.sol (line 770)

        require(!pausedDeposit, "pausedDeposit"); // dev: deposits are paused

11. File: src/SupplySchedule.sol (lines 179-182)

        require(
            globalStartTimestamp > 0,
            "SupplySchedule: minting not started"
        );

12. File: src/SupplySchedule.sol (lines 187-190)

        require(
            block.timestamp > lastMintTimestamp,
            "SupplySchedule: already minted up to current block"
        );

Multiplication/division by two should use bit shifting

* 2 is equivalent to << 1 and / 2 is the same as >> 1

1. File: external/StakedCitadelLocker.sol (line 431)

           uint256 mid = (min + max + 1) / 2;

Stack variable used as a cheaper cache for a state variable is only used once

If the variable is only accessed once, it's cheaper to use the state variable directly that one time

1. File: external/StakedCitadelLocker.sol (line 489)

       uint256 epochindex = epochs.length;

require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables

1. File: external/StakedCitadelLocker.sol (line 813)

       require(_reward > 0, "No reward");

2. File: src/Funding.sol (lines 323-326)

       require(
           _token != address(asset),
           "cannot sweep funding asset, use claimAssetToTreasury()"
       );

3. File: src/GlobalAccessControl.sol (lines 116-119)

       require(
           keccak256(bytes(roleString)) == role,
           "Role string and role do not match"
       );

4. File: src/KnightingRound.sol (line 172)

       require(_tokenInAmount > 0, "_tokenInAmount should be > 0");

5. File: src/StakedCitadel.sol (line 441)

       require(address(token) != _token, "No want");

6. File: src/StakedCitadel.sol (line 487)

       require(_treasury != address(0), "Address 0");

7. File: src/StakedCitadel.sol (line 502)

       require(_strategy != address(0), "Address 0");

8. File: src/StakedCitadel.sol (line 523)

       require(_fees <= WITHDRAWAL_FEE_HARD_CAP, "withdrawalFee too high");

9. File: src/StakedCitadel.sol (lines 535-538)

       require(
           _fees <= PERFORMANCE_FEE_HARD_CAP,
           "performanceFeeStrategist too high"
       );

10. File: src/StakedCitadel.sol (line 550)

        require(_fees <= MANAGEMENT_FEE_HARD_CAP, "managementFee too high");

11. File: src/StakedCitadel.sol (line 562)

        require(_guardian != address(0), "Address cannot be 0x0");

12. File: src/StakedCitadel.sol (line 574)

        require(_vesting != address(0), "Address cannot be 0x0");

13. File: src/StakedCitadel.sol (line 588)

        require(_newToEarnBps <= MAX_BPS, "toEarnBps should be <= MAX_BPS");

14. File: src/StakedCitadel.sol (line 700)

        require(address(token) != _token, "No want");

15. File: src/StakedCitadelVester.sol (line 138)

        require(_amount > 0, "StakedCitadelVester: cannot vest 0");

16. File: src/SupplySchedule.sol (lines 94-97)

        require(
            block.timestamp > lastMintTimestamp,
            "SupplySchedule: already minted up to current block"
        );

17. File: src/SupplySchedule.sol (lines 141-144)

        require(
            _globalStartTimestamp >= block.timestamp,
            "SupplySchedule: minting must start at or after current time"
        );

18. File: src/SupplySchedule.sol (lines 187-190)

        require(
            block.timestamp > lastMintTimestamp,
            "SupplySchedule: already minted up to current block"
        );

Superfluous event fields

block.timestamp and block.number are added to event information by default so adding them manually wastes gas

1. File: external/MedianOracle.sol (line 38)

   event ProviderReportPushed(address indexed provider, uint256 payload, uint256 timestamp);

2. File: src/StakedCitadel.sol (line 125)

       uint256 indexed blockNumber,

3. File: src/StakedCitadel.sol (line 126)

       uint256 timestamp

4. File: src/StakedCitadel.sol (line 133)

       uint256 indexed blockNumber,

5. File: src/StakedCitadel.sol (line 134)

       uint256 timestamp

Use custom errors rather than revert()/require() strings to save deployment gas

1. File: src/CitadelMinter.sol (Various lines throughout the file)
2. File: src/Funding.sol (Various lines throughout the file)
3. File: src/interfaces/convex/BoringMath.sol (Various lines throughout the file)
4. File: src/KnightingRound.sol (Various lines throughout the file)
5. File: src/lib/GlobalAccessControlManaged.sol (Various lines throughout the file)
6. File: src/lib/SettAccessControl.sol (Various lines throughout the file)
7. File: src/StakedCitadel.sol (Various lines throughout the file)
8. File: src/StakedCitadelVester.sol (Various lines throughout the file)
9. File: src/SupplySchedule.sol (Various lines throughout the file)

Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

1. File: external/MedianOracle.sol (lines 80-82)

   function setReportExpirationTimeSec(uint256 reportExpirationTimeSec_)
       external
       onlyOwner

2. File: external/MedianOracle.sol (lines 93-95)

   function setReportDelaySec(uint256 reportDelaySec_)
       external
       onlyOwner

3. File: external/MedianOracle.sol (lines 105-107)

   function setMinimumProviders(uint256 minimumProviders_)
       external
       onlyOwner

4. File: external/MedianOracle.sol (lines 207-209)

   function addProvider(address provider)
       external
       onlyOwner

5. File: external/MedianOracle.sol (lines 221-223)

   function removeProvider(address provider)
       external
       onlyOwner

6. File: external/StakedCitadelLocker.sol (lines 158-162)

   function addReward(
       address _rewardsToken,
       address _distributor,
       bool _useBoost
   ) public onlyOwner {

7. File: external/StakedCitadelLocker.sol (lines 173-177)

   function approveRewardDistributor(
       address _rewardsToken,
       address _distributor,
       bool _approved
   ) external onlyOwner {

8. File: external/StakedCitadelLocker.sol (line 183)

   function setStakingContract(address _staking) external onlyOwner {

9. File: external/StakedCitadelLocker.sol (line 190)

   function setStakeLimits(uint256 _minimum, uint256 _maximum) external onlyOwner {

10. File: external/StakedCitadelLocker.sol (line 200)

    function setBoost(uint256 _max, uint256 _rate, address _receivingAddress) external onlyOwner {

11. File: external/StakedCitadelLocker.sol (line 210)

    function setKickIncentive(uint256 _rate, uint256 _delay) external onlyOwner {

12. File: external/StakedCitadelLocker.sol (line 218)

    function shutdown() external onlyOwner {

13. File: external/StakedCitadelLocker.sol (line 825)

    function recoverERC20(address _tokenAddress, uint256 _tokenAmount) external onlyOwner {

14. File: src/CitadelMinter.sol (lines 169-173)

    function mintAndDistribute()
        external
        onlyRole(POLICY_OPERATIONS_ROLE)
        gacPausable
        nonReentrant

15. File: src/CitadelMinter.sol (lines 250-254)

    function setFundingPoolWeight(address _pool, uint256 _weight)
        external
        onlyRole(POLICY_OPERATIONS_ROLE)
        gacPausable
        nonReentrant

16. File: src/CitadelMinter.sol (lines 294-298)

    function setCitadelDistributionSplit(
        uint256 _fundingBps,
        uint256 _stakingBps,
        uint256 _lockingBps
    ) external onlyRole(POLICY_OPERATIONS_ROLE) gacPausable nonReentrant {

17. File: src/CitadelMinter.sol (lines 314-317)

    function initializeLastMintTimestamp()
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)
        gacPausable

18. File: src/CitadelToken.sol (lines 40-43)

    function mint(address dest, uint256 amount)
        external
        onlyRole(CITADEL_MINTER_ROLE)
        gacPausable

19. File: src/Funding.sol (lines 163-168)

    function deposit(uint256 _assetAmountIn, uint256 _minCitadelOut)
        external
        onlyWhenPriceNotFlagged
        gacPausable
        nonReentrant
        returns (uint256 citadelAmount_)

20. File: src/Funding.sol (lines 265-268)

    function setDiscount(uint256 _discount)
        external
        gacPausable
        onlyRoleOrAddress(POLICY_OPERATIONS_ROLE, funding.discountManager)

21. File: src/Funding.sol (lines 278-281)

    function clearCitadelPriceFlag()
        external
        gacPausable
        onlyRole(POLICY_OPERATIONS_ROLE)

22. File: src/Funding.sol (lines 291-294)

    function setAssetCap(uint256 _assetCap)
        external
        gacPausable
        onlyRole(POLICY_OPERATIONS_ROLE)

23. File: src/Funding.sol (lines 315-319)

    function sweep(address _token)
        external
        gacPausable
        nonReentrant
        onlyRole(TREASURY_OPERATIONS_ROLE)

24. File: src/Funding.sol (lines 334-337)

    function claimAssetToTreasury()
        external
        gacPausable
        onlyRole(TREASURY_OPERATIONS_ROLE)

25. File: src/Funding.sol (lines 356-359)

    function setDiscountLimits(uint256 _minDiscount, uint256 _maxDiscount)
        external
        gacPausable
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

26. File: src/Funding.sol (lines 373-376)

    function setDiscountManager(address _discountManager)
        external
        gacPausable
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

27. File: src/Funding.sol (lines 383-386)

    function setSaleRecipient(address _saleRecipient)
        external
        gacPausable
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

28. File: src/Funding.sol (lines 397-400)

    function setCitadelAssetPriceBounds(uint256 _minPrice, uint256 _maxPrice)
        external
        gacPausable
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

29. File: src/Funding.sol (lines 414-417)

    function updateCitadelPriceInAsset()
        external
        gacPausable
        onlyRole(KEEPER_ROLE)

30. File: src/Funding.sol (lines 447-450)

    function updateCitadelPriceInAsset(uint256 _citadelPriceInAsset)
        external
        gacPausable
        onlyCitadelPriceInAssetOracle

31. File: src/KnightingRound.sol (line 272)

    function finalize() external onlyRole(CONTRACT_GOVERNANCE_ROLE) {

32. File: src/KnightingRound.sol (lines 289-291)

    function setSaleStart(uint256 _saleStart)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

33. File: src/KnightingRound.sol (lines 308-310)

    function setSaleDuration(uint256 _saleDuration)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

34. File: src/KnightingRound.sol (lines 327-329)

    function setTokenOutPrice(uint256 _tokenOutPrice)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

35. File: src/KnightingRound.sol (lines 345-347)

    function setSaleRecipient(address _saleRecipient)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

36. File: src/KnightingRound.sol (lines 367-369)

    function setGuestlist(address _guestlist)
        external
        onlyRole(TECH_OPERATIONS_ROLE)

37. File: src/KnightingRound.sol (lines 380-382)

    function setTokenInLimit(uint256 _tokenInLimit)
        external
        onlyRole(TECH_OPERATIONS_ROLE)

38. File: src/KnightingRound.sol (line 402)

    function sweep(address _token) external gacPausable nonReentrant onlyRole(TREASURY_OPERATIONS_ROLE) {

39. File: src/lib/GlobalAccessControlManaged.sol (lines 27-29)

    function __GlobalAccessControlManaged_init(address _globalAccessControl)
        public
        onlyInitializing

40. File: src/StakedCitadelVester.sol (lines 163-165)

    function setVestingDuration(uint256 _duration)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)

41. File: src/SupplySchedule.sol (lines 132-135)

    function setMintingStart(uint256 _globalStartTimestamp)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)
        gacPausable

42. File: src/SupplySchedule.sol (lines 150-153)

    function setEpochRate(uint256 _epoch, uint256 _rate)
        external
        onlyRole(CONTRACT_GOVERNANCE_ROLE)
        gacPausable

[liveactionllama]

Warden created this issue as a placeholder, because their submission was too large for the contest form. They then emailed their md file to our team. I've updated this issue with their md file content.

[liveactionllama]

Note: my original copy/paste did not capture all of the warden's content. I've updated this issue to now contain the entirety of the original submission. I've also notified the sponsor. Contest has not yet moved to judging.