Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Certora Audit] G-01. OwnerManager.removeOwner(): 1 SLOAD can be saved in the normal path #888

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

remedcu
Copy link
Member

@remedcu remedcu commented Jan 9, 2025

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: Modified the removeOwner function to use pre-decrement for ownerCount to improve efficiency and ensure the threshold check is performed correctly.

@remedcu remedcu self-assigned this Jan 9, 2025
@remedcu remedcu changed the title G-01. OwnerManager.removeOwner(): 1 SLOAD can be saved in the normal path [Certora Audit] G-01. OwnerManager.removeOwner(): 1 SLOAD can be saved in the normal path Jan 9, 2025
@remedcu remedcu marked this pull request as ready for review January 9, 2025 14:24
@remedcu remedcu requested review from akshay-ap and mmv08 January 9, 2025 14:25
@mmv08 mmv08 merged commit 183a588 into main Jan 9, 2025
25 checks passed
@mmv08 mmv08 deleted the g01 branch January 9, 2025 16:19
@github-actions github-actions bot locked and limited conversation to collaborators Jan 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants