002.md

nhtynhty

medium

[INFORMATIONAL/GAS/UX] Use something like an iterable mapping for constant time removal of array elements

Summary

The current implementation of deleteElement has a linearly increasing gas cost with the number of elements in the array, it is ran by users in the _exitMarket function in IronBank.sol as well as a few other spots which are ran by privileged users

Vulnerability Detail

N/A

Impact

Informational/Gas

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/libraries/Arrays.sol#L5 ib-v2/src/libraries/Array.sol

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

library Arrays {
    /**
     * @dev Delete an element from an array.
     * @param self The array to delete from
     * @param element The element to delete
     */
    function deleteElement(address[] storage self, address element) internal {
        uint256 count = self.length;
        for (uint256 i = 0; i < count;) {
            if (self[i] == element) {
                if (i != count - 1) {
                    self[i] = self[count - 1];
                }
                self.pop();
                break;
            }

            unchecked {
                i++;
            }
        }
    }
}

Tool used

N/a

Manual Review

Recommendation

You can use something similar to an 'IterableMapping' to give the deleteElement operation a constant time runtime cost.

You would use a struct that holds the array as well as a mapping(address element => uint256 index) indexOf, this changes the assumptions to an extra sstore when appending or removing from the array, in order to remove the storage read loop, which can be quite expensive