code423n4 commented 2 years ago

Handle

Dravee

Vulnerability details

Impact

SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.

Proof of Concept

The code is as such:

File: ControllerV1.sol
66:     function createLPoolPair(address token0, address token1, uint16 marginLimit, bytes memory dexData) external override {
67:         require(token0 != token1, 'identical address');
68:         require(lpoolPairs[token0][token1].lpool0 == address(0) || lpoolPairs[token1][token0].lpool0 == address(0), 'pool pair exists');
69:         LPoolPairVar memory pairVar = LPoolPairVar(token0, token1, marginLimit, dexData, "OpenLeverage LToken", "LToken");
70:         LPoolDelegator pool0 = new LPoolDelegator();
71:         pool0.initialize(pairVar.token0, pairVar.token0 == wETH ? true : false, address(this), baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink, 1e18,
72:             pairVar.tokenName, pairVar.tokenSymbol, ERC20(pairVar.token0).decimals(), admin, lpoolImplementation);
73:         LPoolDelegator pool1 = new LPoolDelegator();
74:         pool1.initialize(pairVar.token1, pairVar.token1 == wETH ? true : false, address(this), baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink, 1e18,
75:             pairVar.tokenName, pairVar.tokenSymbol, ERC20(pairVar.token1).decimals(), admin, lpoolImplementation);
76:         lpoolPairs[token0][token1] = LPoolPair(address(pool0), address(pool1));
77:         lpoolPairs[token1][token0] = LPoolPair(address(pool0), address(pool1)); //@audit I feel like it should be the opposite
78:         uint16 marketId = (OpenLevInterface(openLev)).addMarket(LPoolInterface(address(pool0)), LPoolInterface(address(pool1)), pairVar.marginLimit, pairVar.dexData);
79:         emit LPoolPairCreated(pairVar.token0, address(pool0), pairVar.token1, address(pool1), marketId, pairVar.marginLimit, pairVar.dexData);
80:     }

Here, these variables are read multiple times from storage: wETH, baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink, admin, lpoolImplementation

By caching these in memory variable and using the newly created memory variables, it's possible to save 7 SLOADs (~700 gas)

Tools Used

VS Code

Recommended Mitigation Steps

Cache the storage values in a memory variable and use them instead of repeatedly reading them from storage.

ColaM12 commented 2 years ago

Duplicate to #137

0xleastwood commented 2 years ago

This warden has made a large number of submissions pointing to basically the same area in different parts of the code. Because of how similar issues are, I don't think its fair to other wardens to have these treated as separate. While I understand gas reports should fix this, I've decided for this contest I'll mark similar duplicates as invalid.

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