feat: update event names for OZ standards and test them

contracts/ownership/rbac/RBAC.sol

@@ -11,14 +11,16 @@ import './Roles.sol';
  *      See //contracts/examples/RBACExample.sol for an example of usage.
  * This RBAC method uses strings to key roles. It may be beneficial
  *  for you to write your own implementation of this interface using Enums or similar.
+ * It's also recommended that you define constants in the contract, like ROLE_ADMIN below,
+ *  to avoid typos.
  */
 contract RBAC {
     using Roles for Roles.Role;
 
     mapping (string => Roles.Role) private roles;
 
-    event LogRoleAdded(address addr, string roleName);
-    event LogRoleRemoved(address addr, string roleName);
+    event RoleAdded(address addr, string roleName);
+    event RoleRemoved(address addr, string roleName);
 
     /**
      * A constant role name for indicating admins.
@@ -43,7 +45,7 @@ contract RBAC {
         internal
     {
         roles[roleName].add(addr);
-        LogRoleAdded(addr, roleName);
+        RoleAdded(addr, roleName);
     }
 
     /**
@@ -55,7 +57,7 @@ contract RBAC {
         internal
     {
         roles[roleName].remove(addr);
-        LogRoleRemoved(addr, roleName);
+        RoleRemoved(addr, roleName);
     }
 
     /**

test/RBAC.js → test/RBAC.test.js

@@ -1,17 +1,21 @@
-const RBACMock = artifacts.require('./helpers/RBACMock.sol')
+const RBACMock = artifacts.require('./mocks/RBACMock.sol')
 
 import expectThrow from './helpers/expectThrow'
+import expectEvent from './helpers/expectEvent'
 
 require('chai')
   .use(require('chai-as-promised'))
   .should()
 
+const ROLE_ADVISOR = 'advisor';
+
 contract('RBAC', function(accounts) {
   let mock
 
   const [
     admin,
     anyone,
+    futureAdvisor,
     ...advisors
   ] = accounts
 
@@ -60,9 +64,23 @@ contract('RBAC', function(accounts) {
         .should.be.fulfilled
     })
     it('allows admins to #adminRemoveRole', async () => {
-      await mock.adminRemoveRole(advisors[3], 'advisor', { from: admin })
+      await mock.adminRemoveRole(advisors[3], ROLE_ADVISOR, { from: admin })
         .should.be.fulfilled
     })
+
+    it('announces a RoleAdded event on addRole', async () => {
+      expectEvent.inTransaction(
+        mock.adminAddRole(futureAdvisor, ROLE_ADVISOR, { from: admin }),
+        'RoleAdded'
+      )
+    })
+
+    it('announces a RoleRemoved event on removeRole', async () => {
+      expectEvent.inTransaction(
+        mock.adminRemoveRole(futureAdvisor, ROLE_ADVISOR, { from: admin }),
+        'RoleRemoved'
+      )
+    })
   })
 
   context('in adversarial conditions', () => {

test/helpers/expectEvent.js

@@ -0,0 +1,16 @@
+const assert = require('chai').assert;
+
+const inLogs = async (logs, eventName) => {
+  const event = logs.find(e => e.event === eventName);
+  assert.exists(event);
+};
+
+const inTransaction = async (tx, eventName) => {
+  const { logs } = await tx;
+  return inLogs(logs, eventName);
+};
+
+module.exports = {
+  inLogs,
+  inTransaction,
+};

test/helpers/RBACMock.sol → test/mocks/RBACMock.sol

@@ -5,11 +5,13 @@ import '../../contracts/ownership/rbac/RBAC.sol';
 
 contract RBACMock is RBAC {
 
+    string constant ROLE_ADVISOR = "advisor";
+
     modifier onlyAdminOrAdvisor()
     {
         require(
-            hasRole(msg.sender, "admin") ||
-            hasRole(msg.sender, "advisor")
+            hasRole(msg.sender, ROLE_ADMIN) ||
+            hasRole(msg.sender, ROLE_ADVISOR)
         );
         _;
     }
@@ -17,23 +19,22 @@ contract RBACMock is RBAC {
     function RBACMock(address[] _advisors)
         public
     {
-        addRole(msg.sender, "admin");
-        addRole(msg.sender, "advisor");
+        addRole(msg.sender, ROLE_ADVISOR);
 
         for (uint256 i = 0; i < _advisors.length; i++) {
-            addRole(_advisors[i], "advisor");
+            addRole(_advisors[i], ROLE_ADVISOR);
         }
     }
 
     function onlyAdminsCanDoThis()
-        onlyRole("admin")
+        onlyAdmin
         view
         external
     {
     }
 
     function onlyAdvisorsCanDoThis()
-        onlyRole("advisor")
+        onlyRole(ROLE_ADVISOR)
         view
         external
     {
@@ -60,9 +61,9 @@ contract RBACMock is RBAC {
     {
         // revert if the user isn't an advisor
         //  (perhaps you want to soft-fail here instead?)
-        checkRole(_addr, "advisor");
+        checkRole(_addr, ROLE_ADVISOR);
 
         // remove the advisor's role
-        removeRole(_addr, "advisor");
+        removeRole(_addr, ROLE_ADVISOR);
     }
 }