Update Registry's permissioning model to use MultiRole

v1/contracts/Registry.sol

@@ -1,18 +1,26 @@
 pragma solidity ^0.5.0;
 
+import "./MultiRole.sol";
 import "./RegistryInterface.sol";
-import "./Withdrawable.sol";
 
-import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
 import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 
 pragma experimental ABIEncoderV2;
 
 
-contract Registry is RegistryInterface, Withdrawable {
+contract Registry is RegistryInterface, MultiRole {
 
     using SafeMath for uint;
 
+    enum Roles {
+        // The ultimate owner-type role that can change the writer.
+        Governance,
+        // Can add or remove DerivativeCreators.
+        Writer,
+        // Can register derivatives.
+        DerivativeCreator
+    }
+
     // Array of all registeredDerivatives that are approved to use the UMA Oracle.
     RegisteredDerivative[] private registeredDerivatives;
 
@@ -44,14 +52,16 @@ contract Registry is RegistryInterface, Withdrawable {
     // Maps from derivative creator address to whether that derivative creator has been approved to register contracts.
     mapping(address => bool) private derivativeCreators;
 
-    modifier onlyApprovedDerivativeCreator {
-        require(derivativeCreators[msg.sender]);
-        _;
+    constructor() public {
+        _createExclusiveRole(uint(Roles.Governance), uint(Roles.Governance), msg.sender);
+        _createExclusiveRole(uint(Roles.Writer), uint(Roles.Governance), msg.sender);
+        // Start with no derivative creators registered.
+        _createSharedRole(uint(Roles.DerivativeCreator), uint(Roles.Writer), new address[](0));
     }
 
     function registerDerivative(address[] calldata parties, address derivativeAddress)
         external
-        onlyApprovedDerivativeCreator
+        onlyRoleHolder(uint(Roles.DerivativeCreator))
     {
         // Create derivative pointer.
         Pointer storage pointer = derivativePointers[derivativeAddress];
@@ -76,20 +86,6 @@ contract Registry is RegistryInterface, Withdrawable {
         emit RegisterDerivative(derivativeAddress, partiesForEvent);
     }
 
-    function addDerivativeCreator(address derivativeCreator) external onlyOwner {
-        if (!derivativeCreators[derivativeCreator]) {
-            derivativeCreators[derivativeCreator] = true;
-            emit AddDerivativeCreator(derivativeCreator);
-        }
-    }
-
-    function removeDerivativeCreator(address derivativeCreator) external onlyOwner {
-        if (derivativeCreators[derivativeCreator]) {
-            derivativeCreators[derivativeCreator] = false;
-            emit RemoveDerivativeCreator(derivativeCreator);
-        }
-    }
-
     function isDerivativeRegistered(address derivative) external view returns (bool isRegistered) {
         return derivativePointers[derivative].valid == PointerValidity.Valid;
     }
@@ -122,12 +118,5 @@ contract Registry is RegistryInterface, Withdrawable {
         return registeredDerivatives;
     }
 
-    function isDerivativeCreatorAuthorized(address derivativeCreator) external view returns (bool isAuthorized) {
-        return derivativeCreators[derivativeCreator];
-    }
-
     event RegisterDerivative(address indexed derivativeAddress, address[] parties);
-    event AddDerivativeCreator(address indexed addedDerivativeCreator);
-    event RemoveDerivativeCreator(address indexed removedDerivativeCreator);
-
 }

v1/contracts/RegistryInterface.sol

@@ -15,14 +15,6 @@ interface RegistryInterface {
     // Registers a new derivative. Only authorized derivative creators can call this method.
     function registerDerivative(address[] calldata counterparties, address derivativeAddress) external;
 
-    // Adds a new derivative creator to this list of authorized creators. Only the owner of this contract can call
-    // this method.   
-    function addDerivativeCreator(address derivativeCreator) external;
-
-    // Removes a derivative creator to this list of authorized creators. Only the owner of this contract can call this
-    // method.  
-    function removeDerivativeCreator(address derivativeCreator) external;
-
     // Returns whether the derivative has been registered with the registry (and is therefore an authorized participant
     // in the UMA system).
     function isDerivativeRegistered(address derivative) external view returns (bool isRegistered);
@@ -32,7 +24,4 @@ interface RegistryInterface {
 
     // Returns all registered derivatives.
     function getAllRegisteredDerivatives() external view returns (RegisteredDerivative[] memory derivatives);
-
-    // Returns whether an address is authorized to register new derivatives.
-    function isDerivativeCreatorAuthorized(address derivativeCreator) external view returns (bool isAuthorized);
 }

v1/test/Registry.js

@@ -8,14 +8,21 @@ contract("Registry", function(accounts) {
   // A deployed instance of the Registry contract, ready for testing.
   let registry;
 
-  const owner = accounts[0];
-  const creator1 = accounts[1];
-  const creator2 = accounts[2];
-  const rando1 = accounts[3];
-  const rando2 = accounts[4];
+  const governance = accounts[0];
+  const writer = accounts[1];
+  const creator1 = accounts[2];
+  const creator2 = accounts[3];
+  const rando1 = accounts[4];
+  const rando2 = accounts[5];
+
+  // Corresponds to Registry.Roles.
+  const governanceRole = "0";
+  const writerRole = "1";
+  const derivativeCreatorRole = "2";
 
   beforeEach(async function() {
     registry = await Registry.new();
+    await registry.resetMember(writerRole, writer);
   });
 
   const areAddressesEqual = (address1, address2) => {
@@ -24,24 +31,19 @@ contract("Registry", function(accounts) {
 
   it("Derivative creation", async function() {
     // No creators should be registered initially.
-    assert.isNotTrue(await registry.isDerivativeCreatorAuthorized(creator1));
+    assert.isNotTrue(await registry.holdsRole(derivativeCreatorRole, creator1));
 
-    // Only the owner should be able to add derivative creators.
-    assert(await didContractThrow(registry.addDerivativeCreator(creator1, { from: rando1 })));
+    // Only the writer should be able to add derivative creators.
+    assert(await didContractThrow(registry.addMember(derivativeCreatorRole, creator1, { from: rando1 })));
+    assert(await didContractThrow(registry.addMember(derivativeCreatorRole, creator1, { from: governance })));
 
     // Register creator1, but not creator2.
-    let result = await registry.addDerivativeCreator(creator1, { from: owner });
-    assert.isTrue(await registry.isDerivativeCreatorAuthorized(creator1));
-    assert.isFalse(await registry.isDerivativeCreatorAuthorized(creator2));
+    let result = await registry.addMember(derivativeCreatorRole, creator1, { from: writer });
+    assert.isTrue(await registry.holdsRole(derivativeCreatorRole, creator1));
+    assert.isFalse(await registry.holdsRole(derivativeCreatorRole, creator2));
 
-    // Ensure an AddDerivativeCreator event is logged.
-    truffleAssert.eventEmitted(result, "AddDerivativeCreator", ev => {
-      return web3.utils.toChecksumAddress(ev.addedDerivativeCreator) === web3.utils.toChecksumAddress(creator1);
-    });
-
-    // Add it a second time, but check that an event is not emitted.
-    result = await registry.addDerivativeCreator(creator1, { from: owner });
-    truffleAssert.eventNotEmitted(result, "AddDerivativeCreator");
+    // Add it a second time.
+    result = await registry.addMember(derivativeCreatorRole, creator1, { from: writer });
 
     // Try to register an arbitrary contract.
     const arbitraryContract = web3.utils.randomHex(20);
@@ -60,26 +62,27 @@ contract("Registry", function(accounts) {
     });
 
     // Remove the derivative creator.
-    result = await registry.removeDerivativeCreator(creator1, { from: owner });
-    assert.isFalse(await registry.isDerivativeCreatorAuthorized(creator1));
-
-    // Ensure an event was emitted.
-    truffleAssert.eventEmitted(result, "RemoveDerivativeCreator", ev => {
-      return web3.utils.toChecksumAddress(ev.removedDerivativeCreator) === web3.utils.toChecksumAddress(creator1);
-    });
+    result = await registry.removeMember(derivativeCreatorRole, creator1, { from: writer });
+    assert.isFalse(await registry.holdsRole(derivativeCreatorRole, creator1));
 
     // Creation should fail since creator1 is no longer approved.
     const secondContract = web3.utils.randomHex(20);
     assert(await didContractThrow(registry.registerDerivative(parties, secondContract, { from: creator1 })));
 
-    // A second removal should not trigger another event.
-    result = await registry.removeDerivativeCreator(creator1, { from: owner });
-    truffleAssert.eventNotEmitted(result, "RemoveDerivativeCreator");
+    // A second removal should still work.
+    result = await registry.removeMember(derivativeCreatorRole, creator1, { from: writer });
+
+    // Remove the writer.
+    await registry.resetMember(writerRole, governance, { from: governance });
+
+    // The writer can no longer add or remove derivative creators.
+    assert(await didContractThrow(registry.addMember(derivativeCreatorRole, creator1, { from: writer })));
+    assert(await didContractThrow(registry.removeMember(derivativeCreatorRole, creator1, { from: writer })));
   });
 
   it("Register and query derivatives", async function() {
-    await registry.addDerivativeCreator(creator1, { from: owner });
-    await registry.addDerivativeCreator(creator2, { from: owner });
+    await registry.addMember(derivativeCreatorRole, creator1, { from: writer });
+    await registry.addMember(derivativeCreatorRole, creator2, { from: writer });
 
     // Register an arbitrary derivative.
     const derivative1 = web3.utils.randomHex(20);
@@ -124,7 +127,7 @@ contract("Registry", function(accounts) {
 
   it("Double-register derivative", async function() {
     // Approve creator.
-    await registry.addDerivativeCreator(creator1, { from: owner });
+    await registry.addMember(derivativeCreatorRole, creator1, { from: writer });
 
     // Register derivative.
     const derivative1 = web3.utils.randomHex(20);