code-423n4 / 2021-11-streaming-findings

0 stars 0 forks source link

Slot packing increases runtime gas consumption due to masking #235

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

Per the Solidity docs:

https://docs.soliditylang.org/en/v0.8.10/internals/layout_in_storage.html#:~:text=When%20using%20elements%20that%20are%20smaller%20than%2032%20bytes%2C%20your%20contract%E2%80%99s%20gas%20usage%20may%20be%20higher

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

It might be beneficial to use reduced-size types if you are dealing with storage values because the compiler will pack multiple elements into one storage slot, and thus, combine multiple reads or writes into a single operation. If you are not reading or writing all the values in a slot at the same time, this can have the opposite effect, though: When one value is written to a multi-value storage slot, the storage slot has to be read first and then combined with the new value such that other data in the same slot is not destroyed.

Every contract has structs that pack multiple fields into slots by using < 256b types. This saves slots but increases runtime gas consumption due to masking of shared slot variables while reading/writing individual variables. The impact is more significant where the shared slot variables are not typically read/written together in functions which may allow the optimizer to combine their reads/writes in SLOADs/SSTOREs because that will not reduce SLOADs/SSTOREs used and instead add more bytecode/gas overhead for masking.

For example:

https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L156-L161

// == slot b ==
uint112 private rewardTokenFeeAmount;
uint112 private depositTokenFlashloanFeeAmount;
uint8 private unlocked = 1;
bool private claimedDepositTokens;
// ============

https://github.com/code-423n4/2021-11-streaming/blob/56d81204a00fc949d29ddd277169690318b36821/Streaming/src/Locke.sol#L253-L261

function lockInternal() internal {
    require(unlocked == 1, "re");
    unlocked = 2;
}
modifier lock {
    lockInternal();
    _;
    unlocked = 1;
}

unlocked, rewardTokenFeeAmount, depositTokenFlashloanFeeAmount and claimedDepositTokens are packing in the same slot, but they are not typically read/written together.