PermissionChecker Code Style Findings

PCR-01C: Inefficient Usage of Structure

Type	Severity	Location
Gas Optimization		PermissionChecker.sol:L626, L643, L646, L704, L705, L706

Description:

The Context structure, while significantly standardizing the codebase, incurs a gas overhead throughout all functions it is currently utilized. Namely, it reserves enough memory and copies the to argument to all calls redundantly whilst it is solely required by PermissionChecker::_custom.

Additionally, the PermissionChecker::_withinAllowance function solely utilizes the consumptions meaning that the value argument is redundantly passed in as well.

Example:

Copy
612function _custom(
613    bytes calldata data,
614    Condition memory condition,
615    ParameterPayload memory payload,
616    Context memory context
617) private view returns (Status, Result memory) {
618    // 20 bytes on the left
619    ICustomCondition adapter = ICustomCondition(
620        address(bytes20(condition.compValue))
621    );
622    // 12 bytes on the right
623    bytes12 extra = bytes12(uint96(uint256(condition.compValue)));
624
625    (bool success, bytes32 info) = adapter.check(
626        context.to,
627        context.value,
628        data,
629        payload.location,
630        payload.size,
631        extra
632    );
633    return (
634        success ? Status.Ok : Status.CustomConditionViolation,
635        Result({consumptions: context.consumptions, info: info})
636    );
637}
638
639function _withinAllowance(
640    bytes calldata data,
641    Condition memory condition,
642    ParameterPayload memory payload,
643    Context memory context
644) private pure returns (Status, Result memory) {
645    uint256 value = uint256(Decoder.word(data, payload.location));
646    return __consume(value, condition, context.consumptions);
647}
648
649function _etherWithinAllowance(
650    Condition memory condition,
651    Context memory context
652) private pure returns (Status status, Result memory result) {
653    (status, result) = __consume(
654        context.value,
655        condition,
656        context.consumptions
657    );
658    return (
659        status == Status.Ok ? Status.Ok : Status.EtherAllowanceExceeded,
660        result
661    );
662}

Recommendation:

We advise the Context structure to be solely utilized by the PermissionChecker::_custom function and all other functions to be reverted to their original implementation thus leading to an overall gas reduction in the PermissionChecker contract.

Alleviation (e6d315f9170dcf4c622d504bd2fb6eafbdac9b75):

The Gnosis Guild team evaluated this exhibit but has opted not to apply its optimization in favour of code standardization and clarity.

PCR-02C: Regression of Gas Optimization

Type	Severity	Location
Gas Optimization		PermissionChecker.sol:L126, L128, L131

Description:

The role.targets[to] entry was previously cached to a local storage pointer, however, this optimization has been regressed and the role.targets[to] entry is specified throughout the PermissionChecker::_transaction function.

Example:

Copy
126if (role.targets[to].clearance == Clearance.Target) {
127    return (
128        _executionOptions(value, operation, role.targets[to].options),
129        Result({consumptions: consumptions, info: 0})
130    );
131} else if (role.targets[to].clearance == Clearance.Function) {

Recommendation:

We advise the code to revert its changes and properly cache the storage pointer as the role.targets[to] offset is calculated at minimum twice throughout all possible execution paths of the PermissionChecker::_transaction beyond the first if conditional.

Alleviation (e6d315f9170dcf4c622d504bd2fb6eafbdac9b75):

The Gnosis Guild team evaluated this optimization and instead opted to re-order the if-else chain, setting the most frequent execution path as the first if conditional to be evaluated.

As a result of this change, some of the gas cost from the common-case scenario has been shifted to the worst-case scenarios effectively optimizing the median gas cost of the function.

As such, we consider this exhibit alleviated as the relevant code block has been optimized based on the function's usage.