diff --git a/contracts/ownership/Whitelist.sol b/contracts/ownership/Whitelist.sol index 4d6e2a70a..cba5d61bd 100644 --- a/contracts/ownership/Whitelist.sol +++ b/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]); } } diff --git a/test/ownership/Whitelist.test.js b/test/ownership/Whitelist.test.js index e0ae29f96..81ae6733c 100644 --- a/test/ownership/Whitelist.test.js +++ b/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 })