Omniscia Gnosis Guild Audit

Integrity Code Style Findings

Integrity Code Style Findings

IYT-01C: Content Validation Grouping

TypeSeverityLocation
Gas OptimizationIntegrity.sol:L153-L169, L177-L189, L205-L233

Description:

The referenced if-else-if clauses can be grouped into a single conditional as they validate multiple operators of the same family (i.e. logical ones, array-based etc.).

Example:

packages/evm/contracts/Integrity.sol
144function _content(
145    ConditionFlat memory condition,
146    uint256 index
147) private pure {
148    Operator operator = condition.operator;
149    ParameterType paramType = condition.paramType;
150    bytes memory compValue = condition.compValue;
151    if (operator == Operator.Pass) {
152        return;
153    } else if (operator == Operator.And) {
154        if (paramType != ParameterType.None) {
155            revert UnsuitableParameterType(index);
156        }
157    } else if (operator == Operator.Or) {
158        if (paramType != ParameterType.None) {
159            revert UnsuitableParameterType(index);
160        }
161    } else if (operator == Operator.Nor) {
162        if (paramType != ParameterType.None) {
163            revert UnsuitableParameterType(index);
164        }
165    } else if (operator == Operator.Xor) {
166        if (paramType != ParameterType.None) {
167            revert UnsuitableParameterType(index);
168        }
169    } else if (operator == Operator.Matches) {
170        if (
171            paramType != ParameterType.Tuple &&
172            paramType != ParameterType.Array &&
173            paramType != ParameterType.AbiEncoded
174        ) {
175            revert UnsuitableParameterType(index);
176        }
177    } else if (operator == Operator.ArraySome) {
178        if (paramType != ParameterType.Array) {
179            revert UnsuitableParameterType(index);
180        }
181    } else if (operator == Operator.ArrayEvery) {
182        if (paramType != ParameterType.Array) {
183            revert UnsuitableParameterType(index);
184        }
185    } else if (operator == Operator.ArraySubset) {
186        if (paramType != ParameterType.Array) {
187            revert UnsuitableParameterType(index);
188        }
189    } else if (operator == Operator.EqualToAvatar) {
190        if (paramType != ParameterType.Static) {
191            revert UnsuitableParameterType(index);
192        }
193    } else if (operator == Operator.EqualTo) {
194        if (
195            paramType != ParameterType.Static &&
196            paramType != ParameterType.Dynamic &&
197            paramType != ParameterType.Tuple &&
198            paramType != ParameterType.Array
199        ) {
200            revert UnsuitableParameterType(index);
201        }
202        if (compValue.length % 32 != 0) {
203            revert UnsuitableCompValue(index);
204        }
205    } else if (operator == Operator.GreaterThan) {
206        if (paramType != ParameterType.Static) {
207            revert UnsuitableParameterType(index);
208        }
209        if (compValue.length != 32) {
210            revert UnsuitableCompValue(index);
211        }
212    } else if (operator == Operator.LessThan) {
213        if (paramType != ParameterType.Static) {
214            revert UnsuitableParameterType(index);
215        }
216        if (compValue.length != 32) {
217            revert UnsuitableCompValue(index);
218        }
219    } else if (operator == Operator.SignedIntGreaterThan) {
220        if (paramType != ParameterType.Static) {
221            revert UnsuitableParameterType(index);
222        }
223        if (compValue.length != 32) {
224            revert UnsuitableCompValue(index);
225        }
226    } else if (operator == Operator.SignedIntLessThan) {
227        if (paramType != ParameterType.Static) {
228            revert UnsuitableParameterType(index);
229        }
230        if (compValue.length != 32) {
231            revert UnsuitableCompValue(index);
232        }
233    } else if (operator == Operator.Bitmask) {
234        if (
235            paramType != ParameterType.Static &&
236            paramType != ParameterType.Dynamic
237        ) {
238            revert UnsuitableParameterType(index);
239        }
240        if (compValue.length != 32) {
241            revert MalformedBitmask(index);
242        }
243    } else if (operator == Operator.Custom) {
244        if (
245            paramType != ParameterType.Static &&
246            paramType != ParameterType.Dynamic &&
247            paramType != ParameterType.Tuple &&
248            paramType != ParameterType.Array
249        ) {
250            revert UnsuitableParameterType(index);
251        }
252    } else if (operator == Operator.WithinAllowance) {
253        if (paramType != ParameterType.Static) {
254            revert UnsuitableParameterType(index);
255        }
256    } else if (operator == Operator.EtherWithinAllowance) {
257        if (paramType != ParameterType.None) {
258            revert UnsuitableParameterType(index);
259        }
260    } else if (operator == Operator.CallWithinAllowance) {
261        if (paramType != ParameterType.None) {
262            revert UnsuitableParameterType(index);
263        }
264    }
265}

Recommendation:

We advise them to be grouped into a single conditional, greatly increasing the legibility of the Integrity::_content function as well as optimizing its execution cost.

Alleviation:

Operator group types are properly grouped in the validation process within Integrity::_node, optimizing the codebase's bytecode significantly.

IYT-02C: Loop Iterator Optimizations

TypeSeverityLocation
Gas OptimizationIntegrity.sol:L35, L62, L68, L84, L102, L119, L278

Description:

The linked for loops increment / decrement their iterator "safely" due to Solidity's built - in safe arithmetics (post-0.8.X).

Example:

packages/evm/contracts/Integrity.sol
35for (uint256 i = 0; i < conditions.length; ++i) {

Recommendation:

We advise the increment / decrement operations to be performed in an unchecked code block as the last statement within each for loop to optimize their execution cost.

Alleviation:

The Gnosis Guild team considered this exhibit but has opted not to apply it to avoid broad usage of the unchecked paradigm. As such, we consider this exhibit acknowledged.

IYT-03C: Redundant Validation of Sub-Type Tree

TypeSeverityLocation
Gas OptimizationIntegrity.sol:L95-L96

Description:

The Integrity::compatibleSubTypeTree is invoked unconditionally for logical expressions (i.e. Operator.And), however, they may contain a single child in certain cases rendering the validation redundant.

Example:

packages/evm/contracts/Integrity.sol
84for (uint256 i = 0; i < conditions.length; i++) {
85    ConditionFlat memory condition = conditions[i];
86    if (
87        (condition.operator >= Operator.And &&
88            condition.operator <= Operator.Xor)
89    ) {
90        // must have no compValue
91        if (condition.compValue.length != 0) {
92            revert UnsuitableCompValue(i);
93        }
94

95        // must have at least one child
96        if (childrenBounds[i].length == 0) {
97            revert UnsuitableChildrenCount(i);
98        }
99    }
100}
101

102for (uint256 i = 0; i < conditions.length; i++) {
103    ConditionFlat memory condition = conditions[i];
104    if (
105        (condition.operator >= Operator.And &&
106            condition.operator <= Operator.Xor)
107    ) {
108        compatibleSubTypeTree(conditions, i, childrenBounds);
109    }
110

111    if (
112        (condition.paramType == ParameterType.Array &&
113            childrenBounds[i].length > 1)
114    ) {
115        compatibleSubTypeTree(conditions, i, childrenBounds);
116    }
117}

Recommendation:

We advise the same childrenBounds conditional of the Integrity::compatibleSubTypeTree invocation for Array parameter types to be assimilated into the referenced if clause, ensuring that the Integrity::compatibleSubTypeTree function is invoked optimally.

Alleviation:

The code of Integrity::_topology (now renamed to Integrity::_tree) has been refactored to no longer contain this inefficiency, validating childrenBounds lengths on a need-to basis and thus optimizing the validation process.

Prev

Consumptions.sol (CSN-C)

Next

MultiSendUnwrapper.sol (MSU-C)

On this page