code-423n4 / 2021-12-vader-findings

0 stars 0 forks source link

Storage of previous prices and total liquidity weights is suboptimal for gas costs #73

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

TomFrenchBlockchain

Vulnerability details

Impact

Gas costs

Proof of Concept

We hold the previous price and total liquidity weight of USDV and VADER in two fixed size arrays

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/lbt/LiquidityBasedTWAP.sol#L31-L32

This creates some strange syntax for reading these values as we store an enum of the indices of VADER and USDV in order to read the correct element.

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/lbt/LiquidityBasedTWAP.sol#L52-L54

It would be simpler to just replace L31-L32 with

uint256 public vaderTotalLiquidityWeight;
uint256 public vaderPreviousPrices;
uint256 public usdvTotalLiquidityWeight;
uint256 public usdvPreviousPrices;

to avoid the need for this extra computation. We could even go so far as to pack these variables to avoid an SSTORE/SLOAD, further reducing gas costs.

uint128 public vaderTotalLiquidityWeight;
uint128 public vaderPreviousPrice;
uint128 public usdvTotalLiquidityWeight;
uint128 public usdvPreviousPrice;

uint128 gives enough space for vaderPreviousPrice and vaderTotalLiquidityWeight to be a little over 3.6 million times world GDP before they overflow (assuming 18 digit fixed point values) which seems safe.

Recommended Mitigation Steps

As above.