Omniscia Boson Protocol Audit
GroupHandlerFacet Code Style Findings
GroupHandlerFacet Code Style Findings
GHF-01C: Inefficient mapping Lookups
| Type | Severity | Location |
|---|---|---|
| Gas Optimization |  | GroupHandlerFacet.sol:L113, L122, L127 |
Description:
The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.
Example:
111uint256 len = group.offerIds.length;112//Get the index in the offerIds array, which is 1 less than the offerIdIndexByGroup index113uint256 index = protocolLookups().offerIdIndexByGroup[groupId][offerId] - 1;114
115if (index != len - 1) {116 // if index == len - 1 then only pop and delete are needed117 // Need to fill gap caused by delete if more than one element in storage array118 uint256 offerIdToMove = group.offerIds[len - 1];119 // Copy the last token in the array to this index to fill the gap120 group.offerIds[index] = offerIdToMove;121 //Reset index mapping. Should be index in offerIds array + 1122 protocolLookups().offerIdIndexByGroup[groupId][offerIdToMove] = index + 1;123}124// Delete last offer id in the array, which was just moved to fill the gap125group.offerIds.pop();126// Delete from index mapping127delete protocolLookups().offerIdIndexByGroup[groupId][offerId];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.
Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):
The referenced mapping lookups have been optimized to the greatest extent possible thus greatly reducing the gas cost of the codebase.
GHF-02C: Repetitive Invocations of Getter Function
| Type | Severity | Location |
|---|---|---|
| Gas Optimization | | GroupHandlerFacet.sol:L109, L113, L122, L127 |
Description:
The linked statements repetitively invoke the same getter function whilst its return value remains unchanged.
Example:
101for (uint256 i = 0; i < _offerIds.length; i++) {102 uint256 offerId = _offerIds[i];103
104 // Offer should belong to the group105 (, uint256 groupId) = getGroupIdByOffer(offerId);106 require(_groupId == groupId, OFFER_NOT_IN_GROUP);107
108 // remove groupIdByOffer mapping109 delete protocolLookups().groupIdByOffer[offerId];110
111 uint256 len = group.offerIds.length;112 //Get the index in the offerIds array, which is 1 less than the offerIdIndexByGroup index113 uint256 index = protocolLookups().offerIdIndexByGroup[groupId][offerId] - 1;114
115 if (index != len - 1) {116 // if index == len - 1 then only pop and delete are needed117 // Need to fill gap caused by delete if more than one element in storage array118 uint256 offerIdToMove = group.offerIds[len - 1];119 // Copy the last token in the array to this index to fill the gap120 group.offerIds[index] = offerIdToMove;121 //Reset index mapping. Should be index in offerIds array + 1122 protocolLookups().offerIdIndexByGroup[groupId][offerIdToMove] = index + 1;123 }124 // Delete last offer id in the array, which was just moved to fill the gap125 group.offerIds.pop();126 // Delete from index mapping127 delete protocolLookups().offerIdIndexByGroup[groupId][offerId];128}Recommendation:
We advise the getter function to be invoked once and its result to be stored to a local variable that is consequently utilized optimizing the gas cost of the statements significantly.
Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):
The protocol lookups are now correctly cached to local variable declarations thus optimizing the codebase's gas cost.