feat: refactor whitelist.sol to use RBAC (#893)

* feat: refactor whitelist.sol to use RBAC

* fix: remove poor backwards compat attempt

contracts/ownership/Whitelist.sol

@@ -2,6 +2,7 @@ pragma solidity ^0.4.21;
 
 
 import "./Ownable.sol";
+import "./rbac/RBAC.sol";
 
 
 /**
@@ -9,17 +10,17 @@ import "./Ownable.sol";
  * @dev The Whitelist contract has a whitelist of addresses, and provides basic authorization control functions.
  * @dev This simplifies the implementation of "user permissions".
  */
-contract Whitelist is Ownable {
-  mapping(address => bool) public whitelist;
-
+contract Whitelist is Ownable, RBAC {
   event WhitelistedAddressAdded(address addr);
   event WhitelistedAddressRemoved(address addr);
 
+  string public constant ROLE_WHITELISTED = "whitelist";
+
   /**
    * @dev Throws if called by any account that's not whitelisted.
    */
   modifier onlyWhitelisted() {
-    require(whitelist[msg.sender]);
+    checkRole(msg.sender, ROLE_WHITELISTED);
     _;
   }
 
@@ -28,12 +29,23 @@ contract Whitelist is Ownable {
    * @param addr address
    * @return true if the address was added to the whitelist, false if the address was already in the whitelist
    */
-  function addAddressToWhitelist(address addr) onlyOwner public returns(bool success) {
-    if (!whitelist[addr]) {
-      whitelist[addr] = true;
-      emit WhitelistedAddressAdded(addr);
-      success = true;
-    }
+  function addAddressToWhitelist(address addr)
+    onlyOwner
+    public
+  {
+    addRole(addr, ROLE_WHITELISTED);
+    emit WhitelistedAddressAdded(addr);
+  }
+
+  /**
+   * @dev getter to determine if address is in whitelist
+   */
+  function whitelist(address addr)
+    public
+    view
+    returns (bool)
+  {
+    return hasRole(addr, ROLE_WHITELISTED);
   }
 
   /**
@@ -42,11 +54,12 @@ contract Whitelist is Ownable {
    * @return true if at least one address was added to the whitelist,
    * false if all addresses were already in the whitelist
    */
-  function addAddressesToWhitelist(address[] addrs) onlyOwner public returns(bool success) {
+  function addAddressesToWhitelist(address[] addrs)
+    onlyOwner
+    public
+  {
     for (uint256 i = 0; i < addrs.length; i++) {
-      if (addAddressToWhitelist(addrs[i])) {
-        success = true;
-      }
+      addAddressToWhitelist(addrs[i]);
     }
   }
 
@@ -56,12 +69,12 @@ contract Whitelist is Ownable {
    * @return true if the address was removed from the whitelist,
    * false if the address wasn't in the whitelist in the first place
    */
-  function removeAddressFromWhitelist(address addr) onlyOwner public returns(bool success) {
-    if (whitelist[addr]) {
-      whitelist[addr] = false;
-      emit WhitelistedAddressRemoved(addr);
-      success = true;
-    }
+  function removeAddressFromWhitelist(address addr)
+    onlyOwner
+    public
+  {
+    removeRole(addr, ROLE_WHITELISTED);
+    emit WhitelistedAddressRemoved(addr);
   }
 
   /**
@@ -70,11 +83,12 @@ contract Whitelist is Ownable {
    * @return true if at least one address was removed from the whitelist,
    * false if all addresses weren't in the whitelist in the first place
    */
-  function removeAddressesFromWhitelist(address[] addrs) onlyOwner public returns(bool success) {
+  function removeAddressesFromWhitelist(address[] addrs)
+    onlyOwner
+    public
+  {
     for (uint256 i = 0; i < addrs.length; i++) {
-      if (removeAddressFromWhitelist(addrs[i])) {
-        success = true;
-      }
+      removeAddressFromWhitelist(addrs[i]);
     }
   }
 

test/ownership/Whitelist.test.js

@@ -44,11 +44,6 @@ contract('Whitelist', function (accounts) {
       }
     });
 
-    it('should not announce WhitelistedAddressAdded event if address is already in the whitelist', async function () {
-      const { logs } = await mock.addAddressToWhitelist(whitelistedAddress1, { from: owner });
-      logs.should.be.empty;
-    });
-
     it('should remove address from the whitelist', async function () {
       await expectEvent.inTransaction(
         mock.removeAddressFromWhitelist(whitelistedAddress1, { from: owner }),
@@ -69,11 +64,6 @@ contract('Whitelist', function (accounts) {
       }
     });
 
-    it('should not announce WhitelistedAddressRemoved event if address is not in the whitelist', async function () {
-      const { logs } = await mock.removeAddressFromWhitelist(whitelistedAddress1, { from: owner });
-      logs.should.be.empty;
-    });
-    
     it('should allow whitelisted address to call #onlyWhitelistedCanDoThis', async () => {
       await mock.addAddressToWhitelist(whitelistedAddress1, { from: owner });
       await mock.onlyWhitelistedCanDoThis({ from: whitelistedAddress1 })
@@ -87,13 +77,13 @@ contract('Whitelist', function (accounts) {
         mock.addAddressToWhitelist(whitelistedAddress1, { from: anyone })
       );
     });
-    
+
     it('should not allow "anyone" to remove from the whitelist', async () => {
       await expectThrow(
         mock.removeAddressFromWhitelist(whitelistedAddress1, { from: anyone })
       );
     });
-    
+
     it('should not allow "anyone" to call #onlyWhitelistedCanDoThis', async () => {
       await expectThrow(
         mock.onlyWhitelistedCanDoThis({ from: anyone })