Omniscia Euler Finance Audit

Set Code Style Findings

Set Code Style Findings

STE-01C: Discrepancy of Proposed Maximum Element Limitation

TypeSeverityLocation
Standard ConformitySet.sol:L31, L42, L85, L87

Description:

The Set::MAX_ELEMENTS constant specifies that the value must not exceed 255, however, a value equal to 255 would permit writes up to index 254 to occur for the elements entry of a SetStorage.

As such, the last element of the SetStorage::elements key would never be written to whose index would be 255.

Impact:

The length of the SetStorage::elements entry is inefficient and will always contain an extraneous slot.

Example:

src/Set.sol
16/// @title SetStorage
17/// @notice This struct is used to store the set data.
18/// @dev To optimize the gas consumption, firstElement is stored in the same storage slot as the numElements
19/// @dev so that for sets with one element, only one storage slot has to be read/written. To keep the elements
20/// @dev array indexing consistent and because the first element is stored outside of the array, the elements[0]
21/// @dev is not utilized. The stamp field is used to keep the storage slot non-zero when the element is removed.
22/// @dev It allows for cheaper SSTORE when an element is inserted.
23struct SetStorage {
24    /// @notice The number of elements in the set.
25    uint8 numElements;
26    /// @notice The first element in the set.
27    address firstElement;
28    /// @notice The stamp of the set.
29    uint88 stamp;
30    /// @notice The array of elements in the set. Stores the elements starting from index 1.
31    ElementStorage[2 ** 8] elements;
32}
33

34/// @title Set
35/// @author Euler Labs (https://www.eulerlabs.com/)
36/// @notice This library provides functions for managing sets of addresses.
37/// @dev The maximum number of elements in the set is defined by the constant MAX_ELEMENTS.
38library Set {
39    error TooManyElements();
40    error InvalidIndex();
41

42    uint8 internal constant MAX_ELEMENTS = 10; // must not exceed 255

Recommendation:

As the SetStorage::numElements entry is constrained to the uint8 data type, we advise the SetStorage::elements array length to be configured as 2 ** 8 - 1, or type(uint8).max, properly illustrating the maximum elements that the SetStorage::elements could practically contain.

Alleviation (577d1991de2aa6c1b0578e9e45d363e8d6799408):

As part of exhibit's STE-04C remediations, a consistent constant is utilized for both the SetStorage::elements array as well as the code's original MAX_ELEMENTS constant.

As such, we consider this exhibit indirectly addressed as extraneous slots are no longer present.

STE-02C: Inexistent Avoidance of Bit Masking (Stamp & First Element Missing)

TypeSeverityLocation
Gas OptimizationSet.sol:L138, L141

Description:

The referenced statements are meant to avoid Solidity's compiler-generated bit-masking, however, this is not presently achieved. The code will partially assign to a 256-bit slot, and thus will result in bit masking to be generated by the compiler.

Example:

src/Set.sol
130// set numElements for every execution path to avoid SSTORE and bit masking when the element removed is
131// firstElement
132if (searchIndex != lastIndex) {
133    if (searchIndex == 0) {
134        setStorage.firstElement = setStorage.elements[lastIndex].value;
135        setStorage.numElements = uint8(lastIndex);
136    } else {
137        setStorage.elements[searchIndex].value = setStorage.elements[lastIndex].value;
138        setStorage.numElements = uint8(lastIndex);
139    }
140} else {
141    setStorage.numElements = uint8(lastIndex);
142}

Recommendation:

We advise the referenced statements to be accompanied by assignments to the setStorage.firstElement and setStorage.stamp as the firstElement value already exists in memory, optimizing the code's compiler-incurred gas cost.

Alleviation (577d1991de2aa6c1b0578e9e45d363e8d6799408):

The relevant statements have been introduced around the numElements assignments, optimizing their compiler-generated gas cost as originally intended.

STE-03C: Inexistent Avoidance of Bit Masking (Stamp Missing)

Description:

The Set::remove function will attempt to minimize gas costs by instructing the compiler to overwrite a storage slot in full instead of overwriting sub-sections of it which entail compiler-generated bit masking.

While the first highlighted instance correctly achieves this by "redundantly" re-assigning the stamp thus instructing the compiler that we need a fresh value for it, the second referenced instance does not do this.

Example:

src/Set.sol
117// write full slot at once to avoid SLOAD and bit masking
118if (numElements == 1) {
119    setStorage.numElements = 0;
120    setStorage.firstElement = address(0);
121    setStorage.stamp = DUMMY_STAMP;
122    return true;
123}
124

125uint256 lastIndex;
126unchecked {
127    lastIndex = numElements - 1;
128}
129

130// set numElements for every execution path to avoid SSTORE and bit masking when the element removed is
131// firstElement
132if (searchIndex != lastIndex) {
133    if (searchIndex == 0) {
134        setStorage.firstElement = setStorage.elements[lastIndex].value;
135        setStorage.numElements = uint8(lastIndex);
136    } else {
137        setStorage.elements[searchIndex].value = setStorage.elements[lastIndex].value;
138        setStorage.numElements = uint8(lastIndex);
139    }
140} else {
141    setStorage.numElements = uint8(lastIndex);
142}

Recommendation:

We advise the stamp to be assigned to in the second referenced instance as well, optimizing the first element's removal gas cost.

Alleviation (577d1991de2aa6c1b0578e9e45d363e8d6799408):

An assignment of the stamp member has been introduced in the highlighted code segment, ensuring that the compiler efficiently generates the setStorage data update.

STE-04C: Inexistent Correlation of Constants

TypeSeverityLocation
Code StyleSet.sol:L31, L42

Description:

The 2 ** 8 constant which is utilized to instantiate the length of the elements array does not depend on the MAX_ELEMENTS member although a relation should be enforced.

Example:

src/Set.sol
16/// @title SetStorage
17/// @notice This struct is used to store the set data.
18/// @dev To optimize the gas consumption, firstElement is stored in the same storage slot as the numElements
19/// @dev so that for sets with one element, only one storage slot has to be read/written. To keep the elements
20/// @dev array indexing consistent and because the first element is stored outside of the array, the elements[0]
21/// @dev is not utilized. The stamp field is used to keep the storage slot non-zero when the element is removed.
22/// @dev It allows for cheaper SSTORE when an element is inserted.
23struct SetStorage {
24    /// @notice The number of elements in the set.
25    uint8 numElements;
26    /// @notice The first element in the set.
27    address firstElement;
28    /// @notice The stamp of the set.
29    uint88 stamp;
30    /// @notice The array of elements in the set. Stores the elements starting from index 1.
31    ElementStorage[2 ** 8] elements;
32}
33

34/// @title Set
35/// @author Euler Labs (https://www.eulerlabs.com/)
36/// @notice This library provides functions for managing sets of addresses.
37/// @dev The maximum number of elements in the set is defined by the constant MAX_ELEMENTS.
38library Set {
39    error TooManyElements();
40    error InvalidIndex();
41

42    uint8 internal constant MAX_ELEMENTS = 10; // must not exceed 255

Recommendation:

We advise the constant declaration to be relocated outside the Set library, and the SetStorage structure to utilize the constant as the length of the ElementStorage member optimizing the maintainability of the codebase.

Alleviation (577d1991de2aa6c1b0578e9e45d363e8d6799408):

The code was updated to ensure a consistent maximum element limit is imposed across the contract via the SET_MAX_ELEMENTS constant which is configured at 10 in accordance to our recommendation.

STE-05C: Optimization of Interim Element Removal

TypeSeverityLocation
Gas OptimizationSet.sol:L137, L144

Description:

An optimization can be achieved by caching the setStorage.elements[lastIndex] storage offset which is calculated twice redundantly whenever an element is removed in between the elements array.

Impact:

To note, the adjustment is applicable to all instances but should result in a gas optimization solely for the interim element case as the first-element removal will re-calculate the setStorage.elements[lastIndex] offset using a keccak256 instruction on the same payload which has reduced gas costs at the EVM level. This is not the case for interim removals, which also calculate the keccak256 offset of the setStorage.elements[searchIndex] entry.

Example:

src/Set.sol
130// set numElements for every execution path to avoid SSTORE and bit masking when the element removed is
131// firstElement
132if (searchIndex != lastIndex) {
133    if (searchIndex == 0) {
134        setStorage.firstElement = setStorage.elements[lastIndex].value;
135        setStorage.numElements = uint8(lastIndex);
136    } else {
137        setStorage.elements[searchIndex].value = setStorage.elements[lastIndex].value;
138        setStorage.numElements = uint8(lastIndex);
139    }
140} else {
141    setStorage.numElements = uint8(lastIndex);
142}
143

144setStorage.elements[lastIndex].value = address(0);

Recommendation:

We applied this optimization and observed a median decrease in gas cost and thus advise it to be applied.

Alleviation (577d1991de2aa6c1b0578e9e45d363e8d6799408):

The setStorage.elements[lastIndex] lookup is properly cached and re-used across its previous instances, optimizing the code's median gas cost as advised.

Prev

ExecutionContext.sol (ECT-C)

Next

Finding Types

On this page