-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Add Authorizable contract #743
Comments
@eordano I've just found contracts/ownership/rbac/RBAC.sol contract that provides similar capabilities to Authorizable. Do you think Authorizable will be useful? |
@medvedev1088 why would you prohibit |
@martriay no reason. Forgot to remove it while copying from stackexchange. |
I like the idea and I definitely would have it separate from RBAC. I'd call it What do you think? |
@martriay Sounds good to me. Although not sure about "check to avoid redundant storage writes". The check itself will add 200 gas right? because we need to read from the storage first. In most cases the owner can make this check off-chain instead to prevent redundant storage writes. What do you think? |
As a rule of thumb, I think every contract's logic should be self-contained, without relying on other contracts/users to do things well. Maliciously or by mistake, multiple calls to whitelist the same address can be made and more than 200gas would be wasted along with redundant events in the blockchain. |
@martriay do you think I should |
@medvedev1088 I would make the function return a boolean value indicating if it was successful. This way the contract can be used programmatically by other contracts and react accordingly. |
@martriay Ok, btw the compiler will auto-generate a public getter for the whitelist |
Yes, it can be left out. |
Already merged. Thank you @medvedev1088 ! |
🎉 Description
Something like this:
Originally discussed here https://ethereum.stackexchange.com/questions/39654/smart-contract-multiple-owners/39657#39657
The text was updated successfully, but these errors were encountered: