Add Authorizable contract

[medvedev1088]

🎉 Description

Something like this:

contract Authorizable is Ownable {

    mapping(address => bool) public authorized;

    modifier onlyAuthorized() {
        require(authorized[msg.sender] || owner == msg.sender);
        _;
    }

    function addAuthorized(address _toAdd) onlyOwner public {
        authorized[_toAdd] = true;
    }

    function removeAuthorized(address _toRemove) onlyOwner public {
        authorized[_toRemove] = false;
    }

}

• [+] 📈 This is a feature request.

Originally discussed here https://ethereum.stackexchange.com/questions/39654/smart-contract-multiple-owners/39657#39657

[medvedev1088]

@eordano I've just found contracts/ownership/rbac/RBAC.sol contract that provides similar capabilities to Authorizable. Do you think Authorizable will be useful?

[martriay]

@medvedev1088 why would you prohibit 0x0 to be authorized?

[medvedev1088]

@martriay no reason. Forgot to remove it while copying from stackexchange.

[martriay]

I like the idea and I definitely would have it separate from RBAC. I'd call it Whitelist instead, and have it emit events on every add/removal, along with a check to avoid redundant storage writes and event firings. Also, I would add an isWhitelisted(address) public function.

What do you think?

[medvedev1088]

@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?

[martriay]

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.

[medvedev1088]

@martriay do you think I should revert() or just ignore on redundant call?

[martriay]

@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.

[medvedev1088]

@martriay Ok, btw the compiler will auto-generate a public getter for the whitelist whitelist(address). Do you think we still need isWhitelisted(address)?

[martriay]

Yes, it can be left out.

[medvedev1088]

@martriay @eordano Please take a look #746. Any comments are welcome.

[come-maiz]

Already merged. Thank you @medvedev1088 !