Badger Citadel contest - IllIllI's results

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L202-L216

Vulnerability details

The amount of citadel bought when there is no discount is always zero. If the user doesn't specify, or specifies zero as the _minCitadelOut, then the user will get no xCitadel and will still have to pay the full price.

Proof of Concept

If funding.discount is equal to zero, citadelAmount_ will remain at the uninitialized value of zero rather than the correct value of citadelAmountWithoutDiscount

File: src/Funding.sol (lines 202-216)

    function getAmountOut(uint256 _assetAmountIn)
        public
        view
        returns (uint256 citadelAmount_)
    {
        uint256 citadelAmountWithoutDiscount = _assetAmountIn * citadelPriceInAsset;

        if (funding.discount > 0) {
            citadelAmount_ =
                (citadelAmountWithoutDiscount * MAX_BPS) /
                (MAX_BPS - funding.discount);
        }

        citadelAmount_ = citadelAmount_ / assetDecimalsNormalizationValue;
    }

Tools Used

Code inspection

Recommended Mitigation Steps

Initialize citadelAmount_ to citadelAmountWithoutDiscount

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/Funding.sol#L430-L437

Vulnerability details

During the video it was explained that the policy operations team was meant to be a nimble group that could change protocol values considered to be safe. Further, it was explained that since pricing comes from an oracle, and there would have to be unusual coordination between the two to affect outcomes, the group was given the ability to clear the pricing flag to get things moving again once the price was determined to be valid

Impact

If an oracle price falls out of the valid min/max range, the citadelPriceFlag is set to true, but the out-of-bounds value is not stored. If the policy operations team calls clearCitadelPriceFlag(), the stale price from before the flag will be used. Not only is it an issue because of stale prices, but this means the policy op team now has a way to affect pricing not under the control of the oracle (i.e. no unusual coordination required to affect an outcome). Incorrect pricing leads to incorrect asset valuations, and loss of funds.

Proof of Concept

The flag is set but the price is not stored File: src/Funding.sol (lines 427-437)

        if (
            _citadelPriceInAsset < minCitadelPriceInAsset ||
            _citadelPriceInAsset > maxCitadelPriceInAsset
        ) {
            citadelPriceFlag = true;
            emit CitadelPriceFlag(
                _citadelPriceInAsset,
                minCitadelPriceInAsset,
                maxCitadelPriceInAsset
            );
        } else {

Tools Used

Code inspection

Recommended Mitigation Steps

Always set the citadelPriceInAsset

Vulnerability details:

Low Risk Issues

New min/max values should be checked against the current stored value

If citadelPriceInAsset is above the new max or below the new min, the next update will likely have a similar value and immediately cause problems. The code should require that the current value falls within the new range

1. File: src/Funding.sol (lines 402-403)

        minCitadelPriceInAsset = _minPrice;
        maxCitadelPriceInAsset = _maxPrice;

Loss of precision

If tokenOutPrice is less than tokenInNormalizationValue, then the amount will be zero for some amounts. The caller of getAmountOut() should revert if tokenOutAmount ends up being zero

1. File: src/KnightingRound.sol (lines 239-241)

        tokenOutAmount_ =
            (_tokenInAmount * tokenOutPrice) /
            tokenInNormalizationValue;

Unsafe calls to optional ERC20 functions

decimals(), name() and symbol() are optional parts of the ERC20 specification, so there are tokens that do not implement them. It's not safe to cast arbitrary token addresses in order to call these functions. If IERC20Metadata is to be relied on, that should be the variable type of the token variable, rather than it being address, so the compiler can verify that types correctly match, rather than this being a runtime failure. See this prior instance of this issue which was marked as Low risk. Do this to resolve the issue.

1. File: src/interfaces/erc20/IERC20.sol (lines 14-18)

    function name() external view returns (string memory);

    function symbol() external view returns (string memory);

    function decimals() external view returns (uint256);

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

        tokenInNormalizationValue = 10**tokenIn.decimals();

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

                abi.encodePacked(_defaultNamePrefix, namedToken.name())

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

                abi.encodePacked(_symbolSymbolPrefix, namedToken.symbol())

Missing checks for address(0x0) when assigning values to address state variables

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

        stakingProxy = _staking;

2. File: src/lib/SettAccessControl.sol (line 39)

        strategist = _strategist;

3. File: src/lib/SettAccessControl.sol (line 46)

        keeper = _keeper;

4. File: src/lib/SettAccessControl.sol (line 53)

        governance = _governance;

initialize functions can be front-run

See this finding from a prior badger-dao contest for details

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

    function initialize(

2. File: src/KnightingRound.sol (line 119)

    ) external initializer {

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

    ) external initializer {

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

    ) public initializer whenNotPaused {

now is deprecated

Use block.timestamp instead

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

        require(timestamps[index_recent].add(reportDelaySec) <= now);

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

        reports[index_past].timestamp = now;

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

        emit ProviderReportPushed(providerAddress, payload, now);

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

        uint256 minValidTimestamp =  now.sub(reportExpirationTimeSec);

5. File: external/MedianOracle.sol (line 162)

        uint256 maxValidTimestamp =  now.sub(reportDelaySec);

safeApprove() is deprecated

Deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance()

1. File: src/CitadelMinter.sol (line 133)

        IERC20Upgradeable(_citadelToken).safeApprove(_xCitadel, type(uint256).max);

2. File: src/CitadelMinter.sol (line 136)

        IERC20Upgradeable(_xCitadel).safeApprove(_xCitadelLocker, type(uint256).max);

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

        IERC20(_citadel).safeApprove(address(_xCitadel), type(uint256).max);

Open TODOs

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment

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

 * TODO: Better revert strings

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

    // TODO: we should conform to some interface here

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

        // TODO: Check gas costs. How does this relate to market buying if you do want to deposit to xCTDL?

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

    /// TODO: Add string -> hash EnumerableSet to a new RoleRegistry contract for easy on-chain viewing.

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

 * TODO: Better revert strings

6. File: src/SupplySchedule.sol (line 159)

        // TODO: Require this epoch is in the future. What happens if no data is set? (It just fails to mint until set)

Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

See this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

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

contract StakedCitadelLocker is Initializable, ReentrancyGuardUpgradeable, OwnableUpgradeable {

2. File: src/CitadelMinter.sol (lines 23-25)

contract CitadelMinter is
    GlobalAccessControlManaged,
    ReentrancyGuardUpgradeable

3. File: src/CitadelToken.sol (line 8)

contract CitadelToken is GlobalAccessControlManaged, ERC20Upgradeable {

4. File: src/Funding.sol (line 17)

contract Funding is GlobalAccessControlManaged, ReentrancyGuardUpgradeable {

5. File: src/GlobalAccessControl.sol (lines 19-21)

contract GlobalAccessControl is
    AccessControlEnumerableUpgradeable,
    PausableUpgradeable

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

contract KnightingRound is GlobalAccessControlManaged, ReentrancyGuardUpgradeable {

7. File: src/lib/GlobalAccessControlManaged.sol (line 12)

contract GlobalAccessControlManaged is PausableUpgradeable {

8. File: src/StakedCitadel.sol (lines 59-63)

contract StakedCitadel is
    ERC20Upgradeable,
    SettAccessControl,
    PausableUpgradeable,
    ReentrancyGuardUpgradeable

9. File: src/StakedCitadelVester.sol (lines 14-16)

contract StakedCitadelVester is
    GlobalAccessControlManaged,
    ReentrancyGuardUpgradeable

Misleading comment

The value of transferFromDisabled is never updated, let alone in an initialize() function. I don't see any bugs related to this, but this comment makes it seem as though something was overlooked when branching.

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

    bool public transferFromDisabled; // Set to true in initialize

Unbounded loop

If there are too many pools, the function may run out of gas while returning them. It's best to allow for a start offset and maximum length, so data can be returned in batches that don't run out of gas

1. File: src/CitadelMinter.sol (lines 143-147)

    function getFundingPoolWeights()
        external
        view
        returns (address[] memory pools, uint256[] memory weights)
    {

Non-critical Issues

Multiple definitions of an interface

These are the only two differences between IEmptyStrategy and IStrategy. IEmptyStrategy should be changed to be is IStrategy and remove the duplicate functions

1. File: src/interfaces/badger/IEmptyStrategy.sol (lines 12-14)

    function initialize(address vault, address want) external;

    function getName() external view returns (string memory);

Unused file

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

// SPDX-License-Identifier: MIT

Contract header not updated after branching

1. File: src/GlobalAccessControl.sol (lines 12-17)

/**
 * @title Badger Geyser
 @dev Tracks stakes and pledged tokens to be distributed, for use with
 @dev BadgerTree merkle distribution system. An arbitrary number of tokens to
 distribute can be specified.
 */

Comment not moved when function was moved

1. File: src/SupplySchedule.sol (lines 52-53)

    // @dev duplicate of getMintable() with debug print added
    // @dev this function is out of scope for reviews and audits

Comments not updated after branching

There are a lot of references to the old owner-related code. The comments should be updated to talk about the new RBAC system

1. File: src/KnightingRound.sol

$ grep owner src/KnightingRound.sol
     * @notice Finalize the sale after sale duration. Can only be called by owner
     * @notice Update the sale start time. Can only be called by owner
     * @notice Update sale duration. Can only be called by owner
     * @notice Modify the tokenOut price in. Can only be called by owner
     * @notice Update the `tokenIn` receipient address. Can only be called by owner
     * @notice Update the guestlist address. Can only be called by owner
     * @notice Modify the max tokenIn that this contract can take. Can only be called by owner
     * @notice Transfers out any tokens accidentally sent to the contract. Can only be called by owner

The price calulation seems inverted since this comment was first written, so it should be updated to reflect the new calculation: 2. File: src/KnightingRound.sol (line 43)

    /// eg. 1 WBTC (8 decimals) = 40,000 CTDL ==> price = 10^8 / 40,000

Remove include for ds-test

Test code should not be mixed in with production code. The production version should be extended and have its functions overridden for testing purposes

1. File: src/SupplySchedule.sol (line 4)

import "ds-test/test.sol";

The nonReentrant modifier should occur before all other modifiers

This is a best-practice to protect against reentrancy in other modifiers

1. File: src/CitadelMinter.sol (line 173)

        nonReentrant

2. File: src/CitadelMinter.sol (line 254)

        nonReentrant

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

    ) external onlyRole(POLICY_OPERATIONS_ROLE) gacPausable nonReentrant {

4. File: src/Funding.sol (line 167)

        nonReentrant

5. File: src/Funding.sol (line 318)

        nonReentrant

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

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

Solidity versions greater than the current version should not be included in the pragma range

The below pragmas should be < 0.9.0, not <=

$ grep '<= 0.9.0' src/*/*/*
src/interfaces/badger/IBadgerGuestlist.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/badger/IBadgerVipGuestlist.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/badger/IEmptyStrategy.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/badger/IStrategy.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/badger/IVault.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/citadel/ICitadelToken.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/citadel/IGac.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/citadel/IStakedCitadelLocker.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/citadel/ISupplySchedule.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/citadel/IVesting.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/convex/BoringMath.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/convex/IRewardStaking.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/convex/IStakingProxy.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/convex/MathUtil.sol:pragma solidity >= 0.5.0 <= 0.9.0;
src/interfaces/erc20/IERC20.sol:pragma solidity >= 0.5.0 <= 0.9.0;

Adding a return statement when the function defines a named return variable, is redundant

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

        return userRewards;

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

        return amount;

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

        return amount;

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

        return supply;

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

        return supply;

require()/revert() statements should have descriptive reason strings

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

        require(reportExpirationTimeSec_ <= MAX_REPORT_EXPIRATION_TIME);

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

        require(minimumProviders_ > 0);

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

        require(reportExpirationTimeSec_ <= MAX_REPORT_EXPIRATION_TIME);

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

        require(minimumProviders_ > 0);

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

        require(timestamps[0] > 0);

6. File: external/MedianOracle.sol (line 129)

        require(timestamps[index_recent].add(reportDelaySec) <= now);

7. File: external/MedianOracle.sol (line 143)

        require (providerReports[providerAddress][0].timestamp > 0);

8. File: external/MedianOracle.sol (line 211)

        require(providerReports[provider][0].timestamp == 0);

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

        require(_stakingToken != address(0)); // dev: _stakingToken address should not be zero

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

        require(rewardData[_rewardsToken].lastUpdateTime == 0);

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

        require(rewardData[_rewardsToken].lastUpdateTime > 0);

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

        require(rewardDistributors[_rewardsToken][msg.sender]);

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

        require(gac.hasRole(PAUSER_ROLE, msg.sender));

14. File: src/lib/GlobalAccessControlManaged.sol (line 86)

        require(gac.hasRole(UNPAUSER_ROLE, msg.sender));

15. File: src/StakedCitadel.sol (line 180)

        require(_token != address(0)); // dev: _token address should not be zero

16. File: src/StakedCitadel.sol (line 181)

        require(_governance != address(0)); // dev: _governance address should not be zero

17. File: src/StakedCitadel.sol (line 182)

        require(_keeper != address(0)); // dev: _keeper address should not be zero

18. File: src/StakedCitadel.sol (line 183)

        require(_guardian != address(0)); // dev: _guardian address should not be zero

19. File: src/StakedCitadel.sol (line 184)

        require(_treasury != address(0)); // dev: _treasury address should not be zero

20. File: src/StakedCitadel.sol (line 185)

        require(_strategist != address(0)); // dev: _strategist address should not be zero

21. File: src/StakedCitadel.sol (line 186)

        require(_badgerTree != address(0)); // dev: _badgerTree address should not be zero

22. File: src/StakedCitadel.sol (line 187)

        require(_vesting != address(0)); // dev: _vesting address should not be zero

public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

1. File: external/StakedCitadelLocker.sol (lines 121-125)

    function initialize(
        address _stakingToken,
        string calldata name,
        string calldata symbol
    ) public initializer {

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

    function decimals() public view returns (uint8) {

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

    function name() public view returns (string memory) {

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

    function symbol() public view returns (string memory) {

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

    function version() public view returns(uint256){

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

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

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

    function lastTimeRewardApplicable(address _rewardsToken) public view returns(uint256) {

8. File: src/CitadelToken.sol (lines 22-26)

    function initialize(
        string memory _name,
        string memory _symbol,
        address _gac
    ) public initializer {

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

    function getStakedCitadelAmountOut(uint256 _assetAmountIn) public view returns (uint256 xCitadelAmount_) {

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

    function __GlobalAccessControlManaged_init(address _globalAccessControl)
        public
        onlyInitializing

11. File: src/lib/SettAccessControl.sol (line 51)

    function setGovernance(address _governance) public {

12. File: src/StakedCitadel.sol (lines 167-179)

    function initialize(
        address _token,
        address _governance,
        address _keeper,
        address _guardian,
        address _treasury,
        address _strategist,
        address _badgerTree,
        address _vesting,
        string memory _name,
        string memory _symbol,
        uint256[4] memory _feeConfig
    ) public initializer whenNotPaused {

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

    function getPricePerFullShare() public view returns (uint256) {

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

    function initialize(address _gac) public initializer {

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

    function getEmissionsForCurrentEpoch() public view returns (uint256) {

constants should be defined rather than using magic numbers

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

        _decimals = 18;

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

        require(_max < 1500, "over max payment"); //max 15%

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

        require(_rate < 30000, "over max rate"); //max 3x

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

        require(_rate <= 500, "over max rate"); //max 5% per epoch

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

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

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

        ).div(1e18).add(rewards[_user][_rewardsToken]);

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

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

8. File: src/CitadelMinter.sol (line 272)

            require(_weight <= 10000, "exceed max funding pool weight");

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

        uint256[4] memory _feeConfig

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

            _feeConfig[3] <= MANAGEMENT_FEE_HARD_CAP,

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

        managementFee = _feeConfig[3];

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

        toEarnBps = 9_500; // initial value of toEarnBps // 95% is invested to the strategy, 5% for cheap withdrawals

13. File: src/SupplySchedule.sol (line 170)

        epochRate[0] = 593962000000000000000000 / epochLength;

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

        epochRate[1] = 591445000000000000000000 / epochLength;

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

        epochRate[2] = 585021000000000000000000 / epochLength;

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

        epochRate[3] = 574138000000000000000000 / epochLength;

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

        epochRate[3] = 574138000000000000000000 / epochLength;

18. File: src/SupplySchedule.sol (line 174)

        epochRate[4] = 558275000000000000000000 / epochLength;

19. File: src/SupplySchedule.sol (line 174)

        epochRate[4] = 558275000000000000000000 / epochLength;

20. File: src/SupplySchedule.sol (line 175)

        epochRate[5] = 536986000000000000000000 / epochLength;

21. File: src/SupplySchedule.sol (line 175)

        epochRate[5] = 536986000000000000000000 / epochLength;

Numeric values having to do with time should use time units for readability

There are units for seconds, minutes, hours, days, and weeks

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

    uint256 public constant rewardsDuration = 86400; // 1 day

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

    uint256 public constant rewardsDuration = 86400; // 1 day

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

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

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

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

Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated

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

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

seen in src/CitadelMinter.sol

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

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

seen in src/CitadelMinter.sol

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

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

seen in src/Funding.sol

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

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

seen in src/Funding.sol

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

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

seen in src/Funding.sol

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

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

seen in src/Funding.sol

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

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

seen in src/CitadelToken.sol

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

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

seen in src/GlobalAccessControl.sol

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

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

seen in src/GlobalAccessControl.sol

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

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

seen in src/GlobalAccessControl.sol

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

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

seen in src/GlobalAccessControl.sol

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

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

seen in src/GlobalAccessControl.sol

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

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

seen in src/GlobalAccessControl.sol

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

    uint256 public constant MAX_BPS = 10_000;

seen in src/Funding.sol

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

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

seen in src/KnightingRound.sol

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

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

seen in src/StakedCitadelVester.sol

Non-library/interface files should use fixed compiler versions, not floating ones

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/GlobalAccessControlManaged.sol (line 3)

pragma solidity ^0.8.12;

Typos

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

                //stop now as no futher checks are needed

futher

2. File: src/CitadelMinter.sol (line 102)

     * @dev this contract is intended to be the only way citadel is minted, with the expection of the initial minting event

expection

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

     * @param _assetCap New max cumulatiive amountIn

cumulatiive

4. File: src/Funding.sol (line 333)

    /// @dev We let assets accumulate and batch transfer to treasury (rather than transfer atomically on each deposi)t for user gas savings

deposi)t

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

     * @notice Update the `tokenIn` receipient address. Can only be called by owner

receipient

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

     * @dev this is assumed to be used in the initializer of the inhereiting contract

inhereiting

7. File: src/lib/GlobalAccessControlManaged.sol (line 60)

    // @dev used to faciliate extra contract-specific permissioned accounts

faciliate

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

    address public badgerTree; // Address we send tokens too via reportAdditionalTokens

too -> to

File does not contain an SPDX Identifier

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

pragma solidity 0.4.24;

File is missing NatSpec

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

// SPDX-License-Identifier: MIT

2. File: src/interfaces/badger/IBadgerGuestlist.sol (line 0)

// SPDX-License-Identifier: MIT

3. File: src/interfaces/badger/IBadgerVipGuestlist.sol (line 0)

// SPDX-License-Identifier: MIT

4. File: src/interfaces/badger/IEmptyStrategy.sol (line 0)

// SPDX-License-Identifier: MIT

5. File: src/interfaces/badger/IStrategy.sol (line 0)

// SPDX-License-Identifier: MIT

6. File: src/interfaces/badger/IVault.sol (line 0)

// SPDX-License-Identifier: MIT

7. File: src/interfaces/citadel/ICitadelToken.sol (line 0)

// SPDX-License-Identifier: MIT

8. File: src/interfaces/citadel/IGac.sol (line 0)

/// SPDX-License-Identifier: MIT

9. File: src/interfaces/citadel/IMedianOracle.sol (line 0)

/// SPDX-License-Identifier: MIT

10. File: src/interfaces/citadel/IStakedCitadelLocker.sol (line 0)

// SPDX-License-Identifier: MIT

11. File: src/interfaces/citadel/ISupplySchedule.sol (line 0)

// SPDX-License-Identifier: MIT

12. File: src/interfaces/citadel/IVesting.sol (line 0)

// SPDX-License-Identifier: MIT

13. File: src/interfaces/convex/IRewardStaking.sol (line 0)

// SPDX-License-Identifier: MIT

14. File: src/interfaces/convex/IStakingProxy.sol (line 0)

// SPDX-License-Identifier: MIT

NatSpec is incorrect

Wrong parameter description

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

     * @param _minCitadelOut ID of DAO to vote for

NatSpec is incomplete

1. File: src/Funding.sol (lines 95-112)

    /**
     * @notice Initializer.
     * @param _gac Global access control
     * @param _citadel The token this contract will return in a trade
     * @param _asset The token this contract will receive in a trade
     * @param _xCitadel Staked citadel, citadel will be granted to funders in this form
     * @param _saleRecipient The address receiving the proceeds of the sale - will be citadel multisig
     * @param _assetCap The max asset that the contract can take
     */
    function initialize(
        address _gac,
        address _citadel,
        address _asset,
        address _xCitadel,
        address _saleRecipient,
        address _citadelPriceInAssetOracle,
        uint256 _assetCap
    ) external initializer {

Missing: @param _citadelPriceInAssetOracle

2. File: src/KnightingRound.sol (lines 98-119)

    /**
     * @notice Initializer.
     * @param _tokenOut The token this contract will return in a trade (citadel)
     * @param _tokenIn The token this contract will receive in a trade
     * @param _saleStart The time when tokens can be first purchased
     * @param _saleDuration The duration of the token sale
     * @param _tokenOutPrice The tokenOut per tokenIn price
     * @param _saleRecipient The address receiving the proceeds of the sale - will be citadel multisig
     * @param _guestlist Address that will manage auction approvals
     * @param _tokenInLimit The max tokenIn that the contract can take
     */
    function initialize(
        address _globalAccessControl,
        address _tokenOut,
        address _tokenIn,
        uint256 _saleStart,
        uint256 _saleDuration,
        uint256 _tokenOutPrice,
        address _saleRecipient,
        address _guestlist,
        uint256 _tokenInLimit
    ) external initializer {

Missing: @param _globalAccessControl

3. File: src/StakedCitadel.sol (lines 154-179)

    /// @notice Initializes the Sett. Can only be called once, ideally when the contract is deployed.
    /// @param _token Address of the token that can be deposited into the sett.
    /// @param _governance Address authorized as governance.
    /// @param _keeper Address authorized as keeper.
    /// @param _guardian Address authorized as guardian.
    /// @param _treasury Address to distribute governance fees/rewards to.
    /// @param _strategist Address authorized as strategist.
    /// @param _badgerTree Address of badgerTree used for emissions.
    /// @param _name Specify a custom sett name. Leave empty for default value.
    /// @param _symbol Specify a custom sett symbol. Leave empty for default value.
    /// @param _feeConfig Values for the 4 different types of fees charges by the sett
    ///         [performanceFeeGovernance, performanceFeeStrategist, withdrawToVault, managementFee]
    ///         Each fee should be less than the constant hard-caps defined above.
    function initialize(
        address _token,
        address _governance,
        address _keeper,
        address _guardian,
        address _treasury,
        address _strategist,
        address _badgerTree,
        address _vesting,
        string memory _name,
        string memory _symbol,
        uint256[4] memory _feeConfig
    ) public initializer whenNotPaused {

Missing: @param _vesting

4. File: src/StakedCitadel.sol (lines 357-367)

    /// @notice Deposits `_amount` tokens, issuing shares to `recipient`.
    ///         Checks the guestlist to verify that `recipient` is authorized to make a deposit for the specified `_amount`.
    ///         Note that deposits are not accepted when the Sett is paused or when `pausedDeposit` is true.
    /// @dev See `_depositForWithAuthorization` for details on guestlist authorization.
    /// @param _recipient Address to issue the Sett shares to.
    /// @param _amount Quantity of tokens to deposit.
    function depositFor(
        address _recipient,
        uint256 _amount,
        bytes32[] memory proof
    ) external whenNotPaused {

Missing: @param proof

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

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

    event ProviderAdded(address provider);

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

    event ProviderRemoved(address provider);

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

    event ReportTimestampOutOfRange(address provider);

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

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

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

    event RewardAdded(address indexed _token, uint256 _reward);

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

    event Staked(address indexed _user, uint256 indexed _epoch, uint256 _paidAmount, uint256 _lockedAmount, uint256 _boostedAmount);

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

    event Withdrawn(address indexed _user, uint256 _amount, bool _relocked);

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

    event KickReward(address indexed _user, address indexed _kicked, uint256 _reward);

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

    event RewardPaid(address indexed _user, address indexed _rewardsToken, uint256 _reward);

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

    event Recovered(address _token, uint256 _amount);

11. File: src/CitadelMinter.sol (lines 56-60)

    event FundingPoolWeightSet(
        address pool,
        uint256 weight,
        uint256 totalFundingPoolWeight
    );

12. File: src/CitadelMinter.sol (lines 61-65)

    event CitadelDistributionSplitSet(
        uint256 fundingBps,
        uint256 stakingBps,
        uint256 lockingBps
    );

13. File: src/CitadelMinter.sol (lines 66-70)

    event CitadelDistribution(
        uint256 fundingAmount,
        uint256 stakingAmount,
        uint256 lockingAmount
    );

14. File: src/CitadelMinter.sol (lines 72-76)

    event CitadelDistributionToFunding(
        uint256 startTime,
        uint256 endTime,
        uint256 citadelAmount
    );

15. File: src/CitadelMinter.sol (lines 77-82)

    event CitadelDistributionToFundingPool(
        uint256 startTime,
        uint256 endTime,
        address pool,
        uint256 citadelAmount
    );

16. File: src/CitadelMinter.sol (lines 83-87)

    event CitadelDistributionToStaking(
        uint256 startTime,
        uint256 endTime,
        uint256 citadelAmount
    );

17. File: src/CitadelMinter.sol (lines 88-93)

    event CitadelDistributionToLocking(
        uint256 startTime,
        uint256 endTime,
        uint256 citadelAmount,
        uint256 xCitadelAmount
    );

18. File: src/Funding.sol (lines 62-66)

    event Deposit(
        address indexed buyer,
        uint256 assetIn,
        uint256 citadelOutValue
    );

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

    event CitadelPriceInAssetUpdated(uint256 citadelPrice);

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

    event CitadelPriceBoundsSet(uint256 minPrice, uint256 maxPrice);

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

    event CitadelPriceFlag(uint256 price, uint256 minPrice, uint256 maxPrice);

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

    event AssetCapUpdated(uint256 assetCap);

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

    event Sweep(address indexed token, uint256 amount);

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

    event ClaimToTreasury(address indexed token, uint256 amount);

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

    event DiscountLimitsSet(uint256 minDiscount, uint256 maxDiscount);

26. File: src/Funding.sol (line 88)

    event DiscountSet(uint256 discount);

27. File: src/Funding.sol (line 89)

    event DiscountManagerSet(address discountManager);

28. File: src/interfaces/erc20/IERC20.sol (line 85)

    event Transfer(address indexed from, address indexed to, uint256 value);

29. File: src/interfaces/erc20/IERC20.sol (lines 91-95)

    event Approval(
        address indexed owner,
        address indexed spender,
        uint256 value
    );

30. File: src/KnightingRound.sol (lines 76-81)

    event Sale(
        address indexed buyer,
        uint8 indexed daoId,
        uint256 amountIn,
        uint256 amountOut
    );

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

    event Claim(address indexed claimer, uint256 amount);

32. File: src/KnightingRound.sol (line 85)

    event SaleStartUpdated(uint256 saleStart);

33. File: src/KnightingRound.sol (line 86)

    event SaleDurationUpdated(uint256 saleDuration);

34. File: src/KnightingRound.sol (line 87)

    event TokenOutPriceUpdated(uint256 tokenOutPrice);

35. File: src/KnightingRound.sol (line 90)

    event TokenInLimitUpdated(uint256 tokenInLimit);

36. File: src/KnightingRound.sol (line 92)

    event Sweep(address indexed token, uint256 amount);

37. File: src/StakedCitadel.sol (lines 122-127)

    event TreeDistribution(
        address indexed token,
        uint256 amount,
        uint256 indexed blockNumber,
        uint256 timestamp
    );

38. File: src/StakedCitadel.sol (lines 130-135)

    event Harvested(
        address indexed token,
        uint256 amount,
        uint256 indexed blockNumber,
        uint256 timestamp
    );

39. File: src/StakedCitadel.sol (line 139)

    event SetToEarnBps(uint256 newEarnToBps);

40. File: src/StakedCitadel.sol (line 140)

    event SetMaxWithdrawalFee(uint256 newMaxWithdrawalFee);

41. File: src/StakedCitadel.sol (line 141)

    event SetMaxPerformanceFee(uint256 newMaxPerformanceFee);

42. File: src/StakedCitadel.sol (line 142)

    event SetMaxManagementFee(uint256 newMaxManagementFee);

43. File: src/StakedCitadel.sol (line 146)

    event SetWithdrawalFee(uint256 newWithdrawalFee);

44. File: src/StakedCitadel.sol (line 147)

    event SetPerformanceFeeStrategist(uint256 newPerformanceFeeStrategist);

45. File: src/StakedCitadel.sol (line 148)

    event SetPerformanceFeeGovernance(uint256 newPerformanceFeeGovernance);

46. File: src/StakedCitadel.sol (line 149)

    event SetManagementFee(uint256 newManagementFee);

47. File: src/StakedCitadelVester.sol (lines 41-46)

    event Vest(
        address indexed recipient,
        uint256 _amount,
        uint256 _unlockBegin,
        uint256 _unlockEnd
    );

48. File: src/StakedCitadelVester.sol (lines 47-51)

    event Claimed(
        address indexed owner,
        address indexed recipient,
        uint256 amount
    );

49. File: src/StakedCitadelVester.sol (line 53)

    event SetVestingDuration(uint256 duration);

50. File: src/SupplySchedule.sol (line 36)

    event MintingStartTimeSet(uint256 globalStartTimestamp);

51. File: src/SupplySchedule.sol (line 37)

    event EpochSupplyRateSet(uint256 epoch, uint256 rate);

Non-exploitable reentrancies

Reentrancy in CitadelMinter.mintAndDistribute() (src/CitadelMinter.sol#169-243):
External calls:
- citadelToken.mint(address(this),mintable) (src/CitadelMinter.sol#178-180)
- IVault(cachedXCitadel).deposit(lockingAmount) (src/CitadelMinter.sol#195-197)
- xCitadelLocker.notifyRewardAmount(address(cachedXCitadel),xCitadelToLockers) (src/CitadelMinter.sol#201-205)
- IERC20Upgradeable(address(citadelToken)).safeTransfer(address(cachedXCitadel),stakingAmount) (src/CitadelMinter.sol#217-218)
- _transferToFundingPools(fundingAmount) (src/CitadelMinter.sol#230-231)
- returndata = address(token).functionCall(data,SafeERC20: low-level call failed) (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol#93)
- (success,returndata) = target.call{value: value}(data) (node_modules/@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol#137)
- IERC20Upgradeable(address(citadelToken)).safeTransfer(pool,amount) (src/CitadelMinter.sol#351-353)
External calls sending eth:
- _transferToFundingPools(fundingAmount) (src/CitadelMinter.sol#230-231)
- (success,returndata) = target.call{value: value}(data) (node_modules/@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol#137)
State variables written after the call(s):
- lastMintTimestamp = block.timestamp (src/CitadelMinter.sol#240-241)

Reentrancy in StakedCitadel._withdraw(uint256) (src/StakedCitadel.sol#808-840):
External calls:
- IStrategy(strategy).withdraw(_toWithdraw) (src/StakedCitadel.sol#818-819)
- IVesting(vesting).setupVesting(msg.sender,_amount,block.timestamp) (src/StakedCitadel.sol#830-831)
- token.safeTransfer(vesting,_amount) (src/StakedCitadel.sol#831-833)
State variables written after the call(s):
- _mintSharesFor(treasury,_fee,balance() - _fee) (src/StakedCitadel.sol#836-837)
- _balances[account] += amount (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol#268)
- _mintSharesFor(treasury,_fee,balance() - _fee) (src/StakedCitadel.sol#836-837)
- _totalSupply += amount (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol#267)

Reentrancy in StakedCitadel._depositFor(address,uint256) (src/StakedCitadel.sol#764-779):
External calls:
- token.safeTransferFrom(msg.sender,address(this),_amount) (src/StakedCitadel.sol#774-775)
State variables written after the call(s):
- _mintSharesFor(_recipient,_after - _before,_pool) (src/StakedCitadel.sol#776-777)
- _balances[account] += amount (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol#268)
- _mintSharesFor(_recipient,_after - _before,_pool) (src/StakedCitadel.sol#776-777)
- _totalSupply += amount (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol#267)
Reentrancy in Funding.updateCitadelPriceInAsset() (src/Funding.sol#414-443):
External calls:
- (_citadelPriceInAsset,_valid) = IMedianOracle(citadelPriceInAssetOracle).getData() (src/Funding.sol#422-423)
State variables written after the call(s):
- citadelPriceFlag = true (src/Funding.sol#431-432)
- citadelPriceInAsset = _citadelPriceInAsset (src/Funding.sol#438-439)

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 Gsreset (2900 gas) and variables initialized to non-zero cost Gsset (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 Gsset (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 Gsset (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 Gsset (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 Gcoldsload (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 Gwarmaccess (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 Gwarmaccess (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