From e7c99dd7dd3b4b2c4ac0ddb9a43689c70b825ebd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= <nicolas.venturo@gmail.com>
Date: Wed, 26 Sep 2018 11:36:41 -0300
Subject: [PATCH] Roles now emit events in construction and when renouncing.
 (#1329)

* release candidate v2.0.0-rc.1

* fix linter error

(cherry picked from commit c12a1c6898c5db300bfeceea50438b328601705d)

* Roles now emit events in construction and when renouncing.

(cherry picked from commit 21198bf1c1161fabfd0299ecac491f45b702a056)
---
 contracts/access/roles/CapperRole.sol    | 12 ++++++++----
 contracts/access/roles/MinterRole.sol    | 12 ++++++++----
 contracts/access/roles/PauserRole.sol    | 12 ++++++++----
 contracts/access/roles/SignerRole.sol    | 12 ++++++++----
 test/access/roles/PublicRole.behavior.js |  5 +++++
 5 files changed, 37 insertions(+), 16 deletions(-)

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 });
       });