Gas Optimizations

[code423n4]

Pack variables together to save gas

Handle

tchkvsky

Scope

Funding.sol

StakedCitadelLocker.sol

Vulnerability details

uint256, mapping, bytes32, string all take 32 bytes in a slot. uint8 uses 1 byte (StakedCitadelLocker.sol), bool uses 1 byte, whileaddress uses 20 bytes. Consider rearranging and packing values that can fit into a slot (i.e. 32 bytes) together. An example is packing a bool and address, since both will only use 21 bytes out of the 32 bytes in a slot (packing can be done in Funding.sol).

This also applies to the values in a struct. They can be packed together.

Tools Used

Manual review

Recommendation

Declare variables that occupy 32 bytes first, then declare the others according to their sizes, in decreasing order. Alternatively, pack variables with smaller size into others (that can accommodate) till a slot is used up.

---

Prefix increments/decrements are cheaper than the postfix form

Handle

tchkvsky

Vulnerability details

These functions do not use prefix increments/decrements (++i/--i)

Impact

Using prefix increment/decrement is more gas efficient

Proof of Concept

CitadelMinter.sol #L152

StakedCitadelLocker.sol #L243, #L322, #L373, #L412, #L464, #L493, #L516, #L535, #L567, #L806, #L953, #L1035

SupplySchedule.sol #L208

GlobalAccessControlManaged.sol#L48

Tools Used

Manual review

Recommendation

Consider using prefix increments/decrements (++i / --i) to save gas

---

public function should be made external

Handle

tchkvsky

Scope

SettAccessControl.sol#L51-54

Vulnerability details

setGovernance() is a public function and it is never called from the contract. It should be declared external instead. This will save some gas.

Tools Used

Manual review

Recommendation

Use external specifier for functions that are never called from the contract.

---

Variable reinitialized to its default value

Handle

tchkvsky

Scope

GlobalAccessControlManged.sol#L47

Vulnerability details

Variable reinitialized to its default value.

Impact

The default value for a bool is false and the validRoleFound variable is reinitialized to false. This wastes gas.

Proof of Concept

bool validRoleFound = false;

Tools Used

Manual review

Recommendation

Remove explicit initialization for default values

---

Use lower values of typeuint instead of uint256 to store hardcoded values

Handle

tchkvsky

Vulnerability details

The state variable MAX_BPS is declared using uint256 which takes 32 bytes. uint14 can be used to store this as the maximum value (i.e. 10,000) is still within range. Also, fundingBps, stakingBps, lockingBps declared in state but used locally should be checked. Also, the parameter _weight in CitadelMinter.setFundingPoolWeight() is between 0 and 10,000 (inclusive). uint14 should be able to store this value for _weight. uint256 uses 32 bytes for storing the value while uint14 almost uses 2 bytes. This will save gas.

Other contracts to consider:

StakedCitadelLocker.sol, StakedCitadelVester.sol

 Proof of Concept

CitadelMinter.sol #L42, #L244-L284,

Funding.sol #L30

StakedCitadel.sol #L68-L117

• ONE_ETH can be stored in uint64

• MAX_BPS can be stored in uint14

• SECS_PER_YEAR can be stored in uint26

• WITHDRAWAL_FEE_HARD_CAP and MANAGEMENT_FEE_HARD_CAPcan be stored in uint8

• PERFORMANCE_FEE_HARD_CAP can be stored in uint12

Tools Used

Manual review

Recommendation

Use lower values of type uint instead of uint256 when storing hardcoded integers.