`OrchestratorFactory_v1.sol`: orchestrator creation can fail due to out of gas

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xc6997765d36ad95d0bb5a97f02b3b02ba3a0326afd65fe67316205da6cdcffa4 Severity: low

Description:

Description

During a creation of an orchestrator in the factory via createOrchestrator(), the supplied moduleConfig array struct parameter is unbounded, which means if high amount of modules are supplied accidentally (or for some reason intentionally) the call can fail due to consuming block.gaslimit. Note that createModule() is a relatively expensive function to execute.

Recommendation

Consider to limit the moduleConfig array of createOrchestrator() to a reasonable upper limit and revert the transaction early. Reverting the transaction at the start of the function will mean that the user doesn't have to pay very high gas costs for the whole failed execution.

    MAX_MODULE_CONFIG = 64;

    function createOrchestrator(...) external returns (IOrchestrator_v1) {
        address clone = Clones.clone(target);
        if (moduleConfigs.length > MAX_MODULE_CONFIG) {
            revert OrchestratorFactory__InvalidModuleLength();
        }
        ...
    }

[PlamenTSV]

There is validation for a maximum module count, which would have the same number of entries as the config:

function __ModuleManager_init(
        address _moduleFactory,
        address[] calldata modules
    ) internal onlyInitializing {
        if (_moduleFactory == address(0)) {
            revert ModuleManagerBase__ModuleFactoryInvalid();
        }
        moduleFactory = _moduleFactory;

        address module;
        uint len = modules.length;

        // Check that the initial list of Modules doesn't exceed the max amount
        // The subtraction by 3 is to ensure enough space for the compulsory modules: fundingManager, authorizer and paymentProcessor
        if (len > (MAX_MODULE_AMOUNT - 3)) {
            revert ModuleManagerBase__ModuleAmountOverLimits();
        }

        for (uint i; i < len; ++i) {
            module = modules[i];

            __ModuleManager_addModule(module);
        }
    }

Apart from that, creator should be aware of the costs of having too many modules.

[0xmahdirostami]

thank you @PlamenTSV