OpenLeverage contest - 0x1f8b's results

Handle

0x1f8b

Vulnerability details

Impact

Gas saving.

Proof of Concept

Without changing too much it's possible to save gas, in the for loops the i variable is increased using i++, it will use less opcodes if use ++i.

Affected places:

• ControllerV1.sol: 303, 375
• OLETokenLock.sol: 33
• OpenLevV1.sol: 52
• OpenLevV1Lib.sol: 260
• XOLE.sol: 362
• BscDexAggregatorV1.sol: 46
• UniV2ClassDex.sol: 60
• EthDexAggregatorV1.sol: 48
• UniV2Dex.sol: 59
• UniV3Dex.sol: 75
• FarmingPools.sol: 126, 132, 164,
• GovernorAlpha.sol: 202, 228, 280, 307
• DexData.sol: 89, 114, 139,
• QueryHelper.sol: 69, 112, 225

Tools Used

Manual review.

Recommended Mitigation Steps

Change i++ to ++i;

Handle

0x1f8b

Vulnerability details

Impact

Gas saving.

Proof of Concept

• contracts\Types.sol: [TradeVars]

Moving the uint32 dexDetail close to address depositErc20 it's possible to save one slot:

struct TradeVars {
        uint depositValue;  
        IERC20 depositErc20;
        uint32 dexDetail;  // moved
        uint fees;
        uint depositAfterFees;      
        uint tradeSize;
        uint newHeld; 
        uint borrowValue;
        uint token0Price;
        uint totalHeld;
    }

• contracts\Types.sol: [CloseTradeVars]

Moving the uint32 dexDetail; close to bool isPartialClose it's possible to save one slot:

struct CloseTradeVars {
        uint16 marketId;
        bool longToken;
        bool depositToken;
        uint closeRatio;          
        bool isPartialClose;        
        uint32 dexDetail; // moved
        uint closeAmountAfterFees;
        uint borrowed;
        uint repayAmount;     
        uint depositDecrease;
        uint depositReturn;  
        uint sellAmount;
        uint receiveAmount;
        uint token0Price;
        uint fees;
    }

• contracts\periphery\QueryHelper.sol [LiqVars]

Moving marginLimit close to LiqStatus it's possible to save one storage slot.

struct LiqVars {
        LiqStatus status;
        uint32 marginLimit;  // moved
        uint lastUpdateTime;
        uint currentMarginRatio;
        uint cAvgMarginRatio;
        uint hAvgMarginRatio;
    }

• contracts\periphery\QueryHelper.sol [LiquidateVars]

Moving isSellAllHeld and dexDetail it's possible to save 2 slots

struct LiquidateVars {// A variable holder for liquidation process
        uint16 marketId;
        bool longToken;
        bool isSellAllHeld; // moved
        uint32 dexDetail; // moved
        uint borrowed; 
        uint fees;      
        uint penalty; 
        uint remainAmountAfterFees;
        uint depositDecrease;   
        uint depositReturn;       
        uint sellAmount;
        uint receiveAmount;
        uint token0Price;
        uint outstandingAmount;
        uint finalRepayAmount;
    }

• contracts\periphery\QueryHelper.sol [MarginRatioVars]

Moving multiplier and multiplier it's possible to save 2 slots

struct MarginRatioVars {
        address heldToken;
        address sellToken;
        address owner;
        uint16 multiplier; // moved
        uint8 decimals; // moved
        uint held;
        bytes dexData;
        uint price;
        uint cAvgPrice;
        uint hAvgPrice; 
        uint lastUpdateTime;
    }

Tools Used

Manual review.

Recommended Mitigation Steps

Apply the mentioned changes

Handle

0x1f8b

Vulnerability details

Impact

Gas saving.

Proof of Concept

The logic around setImplementation store the previous implementation in order to emit the event, but it's possible to save the variable oldImplementation and the storage read for it if the contract emit the event at the begining of the method

The code looks like:

    function setImplementation(address implementation_) public override onlyAdmin {
        address oldImplementation = implementation;
        implementation = implementation_;
        emit NewImplementation(oldImplementation, implementation);
    }

And it could be like:

    function setImplementation(address implementation_) public override onlyAdmin {
        emit NewImplementation(implementation, implementation_);
        implementation = implementation_;
    }

Affected contracts:

• contracts\ControllerDelegator.sol
• contracts\DelegatorInterface.sol
• contracts\OpenLevDelegator.sol
• contracts\XOLEDelegator.sol
• contracts\dex\DexAggregatorDelegator.sol
• contracts\liquidity\LPoolDelegator.sol

Tools Used

Manual review.

Recommended Mitigation Steps

Apply the mentioned changes.

Handle

0x1f8b

Vulnerability details

Impact

Gas saving.

Proof of Concept

It's possible to avoid storage access a save gas using immutable keyword for the following variables:

Affected variables:

• contracts\gov\OLEToken.sol [name, symbol]
• contracts\Adminable.sol [developer]
• contracts\gov\GovernorAlpha.sol [xole, timelock]

Tools Used

Manual review.

Recommended Mitigation Steps

Use immutable keyword