`NestedFactory#removeOperator()` Avoid empty items can save gas

[code423n4]

Handle

WatchPug

Vulnerability details

The for loop in rebuildCache() will cost more gas if removeOperator() wont decrease operators.length.

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/MixinOperatorResolver.sol#L29-L43

function rebuildCache() public {
    bytes32[] memory requiredAddresses = resolverAddressesRequired();
    // The resolver must call this function whenever it updates its state
    for (uint256 i = 0; i < requiredAddresses.length; i++) {
        bytes32 name = requiredAddresses[i];
        // Note: can only be invoked once the resolver has all the targets needed added
        address destination = resolver.getAddress(name);
        if (destination != address(0)) {
            addressCache[name] = destination;
        } else {
            delete addressCache[name];
        }
        emit CacheUpdated(name, destination);
    }
}

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/NestedFactory.sol#L78-L86

/// @inheritdoc INestedFactory
function removeOperator(bytes32 operator) external override onlyOwner {
    uint256 i = 0;
    while (operators[i] != operator) {
        i++;
    }
    require(i > 0, "NestedFactory::removeOperator: Cant remove non-existent operator");
    delete operators[i];
}

Recommendation

Change to:

/// @inheritdoc INestedFactory
function removeOperator(bytes32 operator) external override onlyOwner {
    uint256 length = operators.length;
    for (uint256 i = 0; i < length; i++) {
        if (operators[i] == operator) {
            operators[i] = operators[length - 1];
            operators.pop();
            return;
        }
    }
    revert("NestedFactory::removeOperator: Cant remove non-existent operator");
}

[maximebrugel]

Duplicated : #58

[alcueca]

Part of #220

[alcueca]

Actually, even if caused by the same code, this is a different issue.