hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

Gas Inefficiency in _commitRemoveModule #35

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd3168b2fc7a1fa43915a865ce2b4420907a2686f7f9d46755800a008bb6c725d Severity: gas saving

Description: Description\ The _commitRemoveModule function currently performs a linear search to find the index of the module to be removed:

function _commitRemoveModule(address module) private {
    address[] memory modulesSearchArray = _modules;
    uint moduleIndex = type(uint).max;
    uint length = modulesSearchArray.length;
    for (uint i; i < length; i++) {
        if (modulesSearchArray[i] == module) {
            moduleIndex = i;
            break;
        }
    }
    _modules[moduleIndex] = _modules[length - 1];
    _modules.pop();
    _isModule[module] = false;
    emit ModuleRemoved(module);
}

Issues

Linear Search:

Gas Consumption:

Optimization: Use Mapping for Index Tracking

To optimize, we can maintain a mapping from module addresses to their indices in the _modules array. This allows for O(1) access time. Optimized Implementation

Add Mapping:

mapping(address => uint) private _moduleIndex;

Update _commitAddModule:

 function _commitAddModule(address module) internal {
        _modules.push(module);
        _isModule[module] = true;
        _moduleIndex[module] = _modules.length - 1;
        emit ModuleAdded(module);
    }

Update _commitRemoveModule:

    function _commitRemoveModule(address module) private {
        uint moduleIndex = _moduleIndex[module];
        uint lastIndex = _modules.length - 1;
        address lastModule = _modules[lastIndex];

        // Swap the module to remove with the last module
        _modules[moduleIndex] = lastModule;
        _moduleIndex[lastModule] = moduleIndex;

        // Remove the last element
        _modules.pop();
        delete _moduleIndex[module];
        _isModule[module] = false;

        emit ModuleRemoved(module);
    }

Benefits

Efficiency:

Simplicity:

Conclusion

By using a mapping to track module indices, we can optimize the _commitRemoveModule function for better gas efficiency and performance. This change ensures that module removal operations remain efficient even as the number of modules grows.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

0xmahdirostami commented 5 months ago

Gas saving is done differently in hats finance, read the scope page, please.