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

fix(StorageRent): remove noop revokeRole #281

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

horsefacts
Copy link
Collaborator

@horsefacts horsefacts commented Jul 24, 2023

Motivation

This line in the StorageRent constructor is a no-op: msg.sender is not ever granted the DEFAULT_ADMIN_ROLE, so revoking it here does nothing.

Change Summary

Remove the no-op line.

Merge Checklist

Choose all relevant options below by adding an x now or at any time before submitting for review


PR-Codex overview

This PR focuses on granting the TREASURER_ROLE to the _initialTreasurer and revoking the DEFAULT_ADMIN_ROLE from msg.sender.

Detailed summary

  • Grant the TREASURER_ROLE to the _initialTreasurer.
  • Revoke the DEFAULT_ADMIN_ROLE from msg.sender.
  • Refresh the price.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@horsefacts horsefacts added the chore A task that is none of the other types label Jul 24, 2023
@github-actions
Copy link

Coverage after merging horsefacts/revoke-role-noop into main will be

98.13%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   Bundler.sol100%100%100%100%
   FnameResolver.sol96.30%75%100%100%122
   IdRegistry.sol100%100%100%100%
   KeyRegistry.sol100%100%100%100%
   StorageRent.sol98.90%98.48%100%98.96%530, 532
src/lib
   TransferHelper.sol0%0%0%0%15, 15, 15, 25–26, 26, 26

@varunsrin varunsrin merged commit 6d3c2aa into main Jul 25, 2023
2 checks passed
@varunsrin varunsrin deleted the horsefacts/revoke-role-noop branch July 25, 2023 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A task that is none of the other types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants