Omniscia Evergon Labs Audit

CampaignCreationSkeleton Code Style Findings

CampaignCreationSkeleton Code Style Findings

CCN-01C: Generic Typographic Mistake

Description:

The referenced line contains a typographical mistake (i.e. private variable without an underscore prefix) or generic documentational error (i.e. copy-paste) that should be corrected.

Example:

packages/contracts/contracts/skeletons/CampaignCreationSkeleton.sol
227address hanlderAddress = address(new CampaignAssetManager());

Recommendation:

We advise this to be done so to enhance the legibility of the codebase.

Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):

The typographic mistake has been corrected addressing this exhibit.

CCN-02C: Inefficient mapping Lookups

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

packages/contracts/contracts/skeletons/CampaignCreationSkeleton.sol
199function createCampaign(bytes calldata campaignCreationData) external onlyExternalDelegateCall {
200    ICreatorEligibilityFacet(address(this)).checkCreatorEligibility(msg.sender);
201

202    (bytes4[] memory optionalSelectorsToExecute, bytes[] memory dataToExecute) = abi.decode(
203        campaignCreationData,
204        (bytes4[], bytes[])
205    );
206

207    GeneralStorage.Layout storage generalStorage = GeneralStorage.layout();
208    uint256 campaignId = ++generalStorage.totalCampaigns;
209

210    // On creation
211    generalStorage.campaignsInfo[campaignId].state++;
212    generalStorage.campaignsInfo[campaignId].creator = msg.sender;
213

214    CampaignCreationStorage.Layout storage campaignCreationStorage = CampaignCreationStorage.layout();
215

216    bytes4[] memory requiredSelectors = campaignCreationStorage.requiredSelectors;
217

218    uint256 totalLength = dataToExecute.length;
219    uint256 requiredLength = requiredSelectors.length;
220

221    if (requiredLength + optionalSelectorsToExecute.length != totalLength || totalLength == 0) {
222        revert InvalidCampaignCreationInput(requiredLength, optionalSelectorsToExecute.length, totalLength);
223    }
224

225    // Deploy InputAssetKeeper & RewardAssetHandler for the campaign
226    address keeperAddress = address(new CampaignAssetManager());
227    address hanlderAddress = address(new CampaignAssetManager());
228    generalStorage.campaignsInfo[campaignId].inputAssetKeeper = keeperAddress;
229    generalStorage.campaignsInfo[campaignId].rewardAssetHandler = hanlderAddress;
230

231    emit CampaignAssetManagersDeployed(campaignId, keeperAddress, hanlderAddress);
232

233    // Execute setters
234    uint256 i;
235

236    for (i; i < requiredLength; i++) {
237        bytes memory input = abi.encodeWithSelector(requiredSelectors[i], campaignId, dataToExecute[i]);
238        (bool success, bytes memory returnData) = address(this).call(input);
239        if (!success) {
240            revert RequiredSetterExecutionFailed(campaignId, requiredSelectors[i], dataToExecute[i], returnData);
241        }
242    }
243

244    for (i; i < totalLength; i++) {
245        bytes4 selectorToExecute = optionalSelectorsToExecute[i - requiredLength];
246

247        if (!campaignCreationStorage.isOptionalSelectorSupported[selectorToExecute]) {
248            revert UnsupportedOptionalSelector(selectorToExecute);
249        }
250

251        bytes memory input = abi.encodeWithSelector(selectorToExecute, campaignId, dataToExecute[i]);
252        (bool success, bytes memory returnData) = address(this).call(input);
253

254        if (!success) {
255            revert OptionalSetterExecutionFailed(campaignId, selectorToExecute, dataToExecute[i], returnData);
256        }
257    }
258

259    // Created
260    generalStorage.campaignsInfo[campaignId].state++;
261

262    emit StakingCampaignCreated(campaignId, msg.sender, optionalSelectorsToExecute);
263}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):

All referenced inefficient mapping lookups have been optimized to the greatest extent possible, significantly reducing the gas cost of the functions the statements were located in.

Prev

AccessControlFacetStorage.sol (ACS-C)

Next

CampaignCreationSkeletonNID.sol (CCI-C)

On this page