-
Notifications
You must be signed in to change notification settings - Fork 345
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
merging main #824
merging main #824
Conversation
/// @notice Accepts transfer of admin rights. Only pending admin can accept the role. | ||
function acceptAdmin() external { | ||
address currentPendingAdmin = pendingAdmin; | ||
require(msg.sender == currentPendingAdmin, "ShB not pending admin"); // Only proposed by current admin address can claim the admin rights |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just in case, do you want to switch to using custom errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably but i wasnt sure how many changes we wanted to make before going to deploy/what the scope is as theres a lot of non custom error reverts
cc: @vladbochok
What ❔
Merge
main
back into protocol defense branchWhy ❔
Checklist