code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

Gas: `removeToken` iteration over all tokens can be avoided #116

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The Manager.removeToken function iterates over all tokens to check for existence of an element in the tokens[_vault] array.

Recommended Mitigation Steps

A more efficient solution would be to use OpenZeppelin's EnumerableSet. It allows iterating (enumerating) over all entries, as well as constant-time existence checks using contains, as well as a constant time remove function.

The trade-off is that modifying an element requires two sstores due to the additional index it needs to keep track of.

GalloDaSballo commented 2 years ago

Agree, EnumerableSet is cleaner and typically saves gas as well

GalloDaSballo commented 2 years ago

Disagree that it's a duplicate as the finding is specific to a particular functionality different from #117