diff --git a/contracts/access/roles/CapperRole.sol b/contracts/access/roles/CapperRole.sol index a37a119e8..1aa2770d9 100644 --- a/contracts/access/roles/CapperRole.sol +++ b/contracts/access/roles/CapperRole.sol @@ -12,7 +12,7 @@ contract CapperRole { Roles.Role private cappers; constructor() public { - cappers.add(msg.sender); + _addCapper(msg.sender); } modifier onlyCapper() { @@ -25,12 +25,16 @@ contract CapperRole { } function addCapper(address account) public onlyCapper { - cappers.add(account); - emit CapperAdded(account); + _addCapper(account); } function renounceCapper() public { - cappers.remove(msg.sender); + _removeCapper(msg.sender); + } + + function _addCapper(address account) internal { + cappers.add(account); + emit CapperAdded(account); } function _removeCapper(address account) internal { diff --git a/contracts/access/roles/MinterRole.sol b/contracts/access/roles/MinterRole.sol index 74b22ccf3..279f4ce00 100644 --- a/contracts/access/roles/MinterRole.sol +++ b/contracts/access/roles/MinterRole.sol @@ -12,7 +12,7 @@ contract MinterRole { Roles.Role private minters; constructor() public { - minters.add(msg.sender); + _addMinter(msg.sender); } modifier onlyMinter() { @@ -25,12 +25,16 @@ contract MinterRole { } function addMinter(address account) public onlyMinter { - minters.add(account); - emit MinterAdded(account); + _addMinter(account); } function renounceMinter() public { - minters.remove(msg.sender); + _removeMinter(msg.sender); + } + + function _addMinter(address account) internal { + minters.add(account); + emit MinterAdded(account); } function _removeMinter(address account) internal { diff --git a/contracts/access/roles/PauserRole.sol b/contracts/access/roles/PauserRole.sol index e6c4e6da7..2c89cec55 100644 --- a/contracts/access/roles/PauserRole.sol +++ b/contracts/access/roles/PauserRole.sol @@ -12,7 +12,7 @@ contract PauserRole { Roles.Role private pausers; constructor() public { - pausers.add(msg.sender); + _addPauser(msg.sender); } modifier onlyPauser() { @@ -25,12 +25,16 @@ contract PauserRole { } function addPauser(address account) public onlyPauser { - pausers.add(account); - emit PauserAdded(account); + _addPauser(account); } function renouncePauser() public { - pausers.remove(msg.sender); + _removePauser(msg.sender); + } + + function _addPauser(address account) internal { + pausers.add(account); + emit PauserAdded(account); } function _removePauser(address account) internal { diff --git a/contracts/access/roles/SignerRole.sol b/contracts/access/roles/SignerRole.sol index a6ab48305..5563c46f0 100644 --- a/contracts/access/roles/SignerRole.sol +++ b/contracts/access/roles/SignerRole.sol @@ -12,7 +12,7 @@ contract SignerRole { Roles.Role private signers; constructor() public { - signers.add(msg.sender); + _addSigner(msg.sender); } modifier onlySigner() { @@ -25,12 +25,16 @@ contract SignerRole { } function addSigner(address account) public onlySigner { - signers.add(account); - emit SignerAdded(account); + _addSigner(account); } function renounceSigner() public { - signers.remove(msg.sender); + _removeSigner(msg.sender); + } + + function _addSigner(address account) internal { + signers.add(account); + emit SignerAdded(account); } function _removeSigner(address account) internal { diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js index 1e489a468..f1c723b78 100644 --- a/test/access/roles/PublicRole.behavior.js +++ b/test/access/roles/PublicRole.behavior.js @@ -89,6 +89,11 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role (await this.contract[`is${rolename}`](authorized)).should.equal(false); }); + it(`emits a ${rolename}Removed event`, async function () { + const { logs } = await this.contract[`renounce${rolename}`]({ from: authorized }); + expectEvent.inLogs(logs, `${rolename}Removed`, { account: authorized }); + }); + it('doesn\'t revert when renouncing unassigned role', async function () { await this.contract[`renounce${rolename}`]({ from: anyone }); });