From b2c60870a9d95aa77f75a605b63c22798aaf9938 Mon Sep 17 00:00:00 2001
From: Shebin John <admin@remedcu.com>
Date: Thu, 9 Jan 2025 17:48:02 +0100
Subject: [PATCH] [Certora Audit] G-09. Cache array length outside of loop
 (#896)

This pull request includes several optimizations to the `OwnerManager`
and `ERC165Handler` contracts by reducing the number of times array
lengths are accessed within loops. These changes aim to improve the
efficiency of the code by storing the array lengths in variables before
the loops.

If not cached, the solidity compiler will always read the length of the
array during each iteration. That is, if it is a storage array, this is
an extra sload operation (100 additional extra gas for each iteration
except for the first) and if it is a memory array, this is an extra
mload operation (3 additional gas for each iteration except for the
first).

Optimizations in `OwnerManager` contract:

*
[`contracts/base/OwnerManager.sol`](diffhunk://#diff-795fb06764b4c2d991707584a31509badf0b036c9401bfbcb82d6bc9fdebab82L38-R39):
Stored `_owners.length` in a variable `ownersLength` before the loop to
avoid repeatedly accessing the array length.
*
[`contracts/base/OwnerManager.sol`](diffhunk://#diff-795fb06764b4c2d991707584a31509badf0b036c9401bfbcb82d6bc9fdebab82L49-R50):
Updated `ownerCount` to use the `ownersLength` variable instead of
accessing `_owners.length` again.

Optimizations in `ERC165Handler` contract:

*
[`contracts/handler/extensible/ERC165Handler.sol`](diffhunk://#diff-aa0838f20fd3f37b00dc661645b4641500e68762b9b624addb99465fcc65a3e0L56-R57):
Stored `handlerWithSelectors.length` in a variable `len` before the loop
in `addSupportedInterfaceBatch` to avoid repeatedly accessing the array
length.
*
[`contracts/handler/extensible/ERC165Handler.sol`](diffhunk://#diff-aa0838f20fd3f37b00dc661645b4641500e68762b9b624addb99465fcc65a3e0L78-R80):
Stored `selectors.length` in a variable `len` before the loop in
`removeSupportedInterfaceBatch` to avoid repeatedly accessing the array
length.
---
 contracts/base/OwnerManager.sol                | 5 +++--
 contracts/handler/extensible/ERC165Handler.sol | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/contracts/base/OwnerManager.sol b/contracts/base/OwnerManager.sol
index 725ea8d3d..77821523a 100644
--- a/contracts/base/OwnerManager.sol
+++ b/contracts/base/OwnerManager.sol
@@ -35,7 +35,8 @@ abstract contract OwnerManager is SelfAuthorized, IOwnerManager {
         if (_threshold == 0) revertWithError("GS202");
         // Initializing Safe owners.
         address currentOwner = SENTINEL_OWNERS;
-        for (uint256 i = 0; i < _owners.length; i++) {
+        uint256 ownersLength = _owners.length;
+        for (uint256 i = 0; i < ownersLength; i++) {
             // Owner address cannot be null.
             address owner = _owners[i];
             if (owner == address(0) || owner == SENTINEL_OWNERS || owner == address(this) || currentOwner == owner)
@@ -46,7 +47,7 @@ abstract contract OwnerManager is SelfAuthorized, IOwnerManager {
             currentOwner = owner;
         }
         owners[currentOwner] = SENTINEL_OWNERS;
-        ownerCount = _owners.length;
+        ownerCount = ownersLength;
         threshold = _threshold;
     }
 
diff --git a/contracts/handler/extensible/ERC165Handler.sol b/contracts/handler/extensible/ERC165Handler.sol
index cf3ece3ed..bc19fe624 100644
--- a/contracts/handler/extensible/ERC165Handler.sol
+++ b/contracts/handler/extensible/ERC165Handler.sol
@@ -55,7 +55,8 @@ abstract contract ERC165Handler is ExtensibleBase, IERC165Handler {
     function addSupportedInterfaceBatch(bytes4 _interfaceId, bytes32[] calldata handlerWithSelectors) external override onlySelf {
         ISafe safe = ISafe(payable(_msgSender()));
         bytes4 interfaceId;
-        for (uint256 i = 0; i < handlerWithSelectors.length; i++) {
+        uint256 len = handlerWithSelectors.length;
+        for (uint256 i = 0; i < len; i++) {
             (bool isStatic, bytes4 selector, address handlerAddress) = MarshalLib.decodeWithSelector(handlerWithSelectors[i]);
             _setSafeMethod(safe, selector, MarshalLib.encode(isStatic, handlerAddress));
             if (i > 0) {
@@ -77,7 +78,8 @@ abstract contract ERC165Handler is ExtensibleBase, IERC165Handler {
     function removeSupportedInterfaceBatch(bytes4 _interfaceId, bytes4[] calldata selectors) external override onlySelf {
         ISafe safe = ISafe(payable(_msgSender()));
         bytes4 interfaceId;
-        for (uint256 i = 0; i < selectors.length; i++) {
+        uint256 len = selectors.length;
+        for (uint256 i = 0; i < len; i++) {
             _setSafeMethod(safe, selectors[i], bytes32(0));
             if (i > 0) {
                 interfaceId ^= selectors[i];