RulesEngine Code Style Findings

REE-01C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		RulesEngine.sol: • I-1: L300 • I-2: L315, L316, L317

Description:

The linked mathematical operations are guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

Copy
313uint256 j = i;
314
315while (j > 0 && _hooks[sortedActiveIds[j - 1]].priority > keyPriority) {
316    sortedActiveIds[j] = sortedActiveIds[j - 1];
317    j--;
318}

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statements to be wrapped in unchecked code blocks thereby optimizing their execution cost.

Alleviation (e4d6885477):

An unchecked code block was solely introduced for the latter of the two instances of the exhibit.

Alleviation (08da17ef72):

An unchecked code block has been introduced for the remaining instance, applying the exhibit's optimization in full.

REE-02C: Inefficient Loop Limit Evaluation

Type	Severity	Location
Gas Optimization		RulesEngine.sol:L103

Description:

The linked for loop evaluates its limit inefficiently on each iteration.

Example:

Copy
103for (uint256 i = 0; i < _hookIds.length;) {

Recommendation:

We advise the statements within the for loop limit to be relocated outside to a local variable declaration that is consequently utilized for the evaluation to significantly reduce the codebase's gas cost. We should note the same optimization is applicable for storage reads present in such limits as they are newly read on each iteration (i.e. length members of arrays in storage).

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The hook limit is properly cached outside the for loop logic execution, optimizing the code's gas cost.

REE-03C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		RulesEngine.sol: • I-1: L124, L126 • I-2: L136, L138 • I-3: L148, L150

Description:

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

Example:

Copy
123function changeHookPriority(bytes32 hookId, uint256 newPriority) external onlyRoles(roleManager.RULES_ADMIN()) {
124    if (_hooks[hookId].hookAddress == address(0)) revert HookNotFound(hookId);
125
126    _hooks[hookId].priority = newPriority;
127
128    emit HookPriorityChanged(hookId, newPriority);
129}

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 (e4d6885477438291b0d3ca477fab7e088522967b):

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.

REE-04C: Potentially Inefficient Shift Operation

Type	Severity	Location
Gas Optimization		RulesEngine.sol:L105

Description:

The referenced assignment operation is potentially inefficient if the for loop has iterated until the end of the contract (i.e. i == _hookIds.length - 1).

Example:

Copy
103for (uint256 i = 0; i < _hookIds.length;) {
104    if (_hookIds[i] == hookId) {
105        _hookIds[i] = _hookIds[_hookIds.length - 1];
106        _hookIds.pop();
107        break;
108    }
109
110    unchecked {
111        ++i;
112    }
113}

Recommendation:

We advise the assignment to be performed solely when the i iterator is not equal to the _hookIds.length - 1 value, optimizing the code's gas cost.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The referenced array shift's execution has been optimized by wrapping it in the conditional recommended.