Skip to content

Commit

Permalink
[Certora Audit] G-01. OwnerManager.removeOwner(): 1 SLOAD can be sa…
Browse files Browse the repository at this point in the history
…ved in the normal path (#888)

This pull request includes a small but important change to the
`removeOwner` function in the `OwnerManager` contract. The change
optimizes the decrement operation for the `ownerCount` variable.

The previous code reads from storage twice with the `ownerCount`
variable. By doing pre-decrement (which is cheaper than post-decrement),
we can save on 1 SLOAD.

Optimization:

*
[`contracts/base/OwnerManager.sol`](diffhunk://#diff-795fb06764b4c2d991707584a31509badf0b036c9401bfbcb82d6bc9fdebab82L74-L80):
Modified the `removeOwner` function to use pre-decrement for
`ownerCount` to improve efficiency and ensure the threshold check is
performed correctly.
  • Loading branch information
remedcu authored Jan 9, 2025
1 parent a192574 commit 183a588
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions contracts/base/OwnerManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ abstract contract OwnerManager is SelfAuthorized, IOwnerManager {
*/
function removeOwner(address prevOwner, address owner, uint256 _threshold) public override authorized {
// Only allow to remove an owner, if threshold can still be reached.
if (ownerCount - 1 < _threshold) revertWithError("GS201");
// Here we do pre-decrement as it is cheaper and allows us to check if the threshold is still reachable.
if (--ownerCount < _threshold) revertWithError("GS201");
// Validate owner address and check that it corresponds to owner index.
if (owner == address(0) || owner == SENTINEL_OWNERS) revertWithError("GS203");
if (owners[prevOwner] != owner) revertWithError("GS205");
owners[prevOwner] = owners[owner];
owners[owner] = address(0);
ownerCount--;
emit RemovedOwner(owner);
// Change threshold if threshold was changed.
if (threshold != _threshold) changeThreshold(_threshold);
Expand Down

0 comments on commit 183a588

Please sign in to comment.