code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

Gas Optimization: Tight variable packing in `XDEFIDistribution.sol` #54

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

Dravee

Vulnerability details

Impact

Solidity contracts have contiguous 32 bytes (256 bits) slots used in storage. By arranging the variables, it is possible to minimize the number of slots used within a contract's storage and therefore reduce deployment costs.

Proof of Concept

In XDEFIDistribution.sol, the order of variables is this way:

    uint88 internal MAX_TOTAL_XDEFI_SUPPLY = uint88(240_000_000_000_000_000_000_000_000);
    uint256 internal constant _pointsMultiplier = uint256(2**128);
    uint256 internal _pointsPerUnit;
    address public immutable XDEFI;
    uint256 public distributableXDEFI;
    uint256 public totalDepositedXDEFI;
    uint256 public totalUnits;
    mapping(uint256 => Position) public positionOf;
    mapping(uint256 => uint8) public bonusMultiplierOf;  // Scaled by 100 (i.e. 1.1x is 110, 2.55x is 255).
    uint256 internal immutable _zeroDurationPointBase;
    string public baseURI;
    address public owner;
    address public pendingOwner;
    uint256 internal _locked;

As we can see, the first variable is a uint88, which is a 12 bytes size variable (way less than 32 bytes). However, due to it's wrong position, it will take up a whole 32 bytes slot. As an address type variable is of size 20 bytes, it would be better to make them contiguous so that they are packed together in a 32 bytes slot.

As for the string variable, it's a dynamic size variable, so it'll always take up a new slot

Recommended Mitigation Steps

Pack the variables in XDEFIDistribution.sol such that the number of slots used in deployment is minimised. The only optimization I can see here is moving the uint88 variable (12 bytes) next to an address variable (20 bytes).

deluca-mike commented 2 years ago

There are no longer any real packing benefits here since MAX_TOTAL_XDEFI_SUPPLY should be constant (and thus not in storage) and is actually getting removed.