Omniscia Gnosis Guild Audit
PermissionChecker Code Style Findings
PermissionChecker Code Style Findings
PCR-01C: Inefficient Iteration of Child Conditions
| Type | Severity | Location |
|---|---|---|
| Gas Optimization |  | PermissionChecker.sol:L423-L426, L431 |
Description:
The PermissionChecker::_xor function will inefficiently loop through all remaining child conditions if more than one validate as Status.Ok.
Example:
406function _xor(407 uint256 value,408 bytes calldata data,409 Condition memory condition,410 ParameterPayload memory payload,411 Consumption[] memory consumptions412) private pure returns (Status, Result memory result) {413 uint256 okCount;414 unchecked {415 for (uint256 i; i < condition.children.length; ++i) {416 (Status status, Result memory _result) = _walk(417 value,418 data,419 condition.children[i],420 payload,421 consumptions422 );423 if (status == Status.Ok) {424 result = _result;425 okCount = okCount + 1;426 }427 }428 }429
430 return431 okCount == 1432 ? (Status.Ok, result)433 : (434 Status.XorViolation,435 Result({consumptions: consumptions, info: 0})436 );437}Recommendation:
We advise the code to instead break when a second Status.Ok is identified within the referenced if clause.
Alleviation:
The code of the PermissionChecker::_xor function has been omitted as a result of finding PCR-01M. As such, we consider this exhibit no longer applicable.
PCR-02C: Optimization of Data Extraction
| Type | Severity | Location |
|---|---|---|
| Gas Optimization | | PermissionChecker.sol:L597-L602 |
Description:
The PermissionChecker::_bitmask function will inefficiently extract a single word from the data payload by using the Decoder::pluck function instead of the Decoder::word counterpart.
Example:
597bool isInline = condition.paramType == ParameterType.Static;598bytes calldata value = Decoder.pluck(599 data,600 payload.location + (isInline ? 0 : 32),601 payload.size - (isInline ? 0 : 32)602);Recommendation:
We advise the code to be optimized akin to PermissionChecker::_compare, performing a Decoder::word operation when the parameter is in-line (Static) and a Decoder::pluck operation when it is dynamic (Dynamic).
Alleviation:
The Gnosis Guild team evaluated this exhibit but opted to retain the current behaviour of the codebase for the sake of code clarity and evident behaviour.