[P1-M04] Refactor registry to prevent multiple address registra… (#1194)

* implementation

Signed-off-by: Christopher Maree <christopher.maree@gmail.com>

* removed redundant require

Signed-off-by: Christopher Maree <christopher.maree@gmail.com>

Author: chrismaree

core/contracts/oracle/implementation/Registry.sol

@@ -94,9 +94,7 @@ contract Registry is RegistryInterface, MultiRole {
         // For all parties in the array add them to the contract's parties.
         financialContract.valid = Validity.Valid;
         for (uint i = 0; i < parties.length; i = i.add(1)) {
-            partyMap[parties[i]].contracts.push(contractAddress);
-            uint newLength = partyMap[parties[i]].contracts.length;
-            partyMap[parties[i]].contractIndex[contractAddress] = newLength - 1;
+            _addPartyToContract(parties[i], contractAddress);
         }
 
         emit NewContractRegistered(contractAddress, msg.sender, parties);
@@ -113,14 +111,8 @@ contract Registry is RegistryInterface, MultiRole {
         address contractAddress = msg.sender;
 
         require(contractMap[contractAddress].valid == Validity.Valid, "Can only add to valid contract");
-        require(!isPartyMemberOfContract(party, contractAddress), "Can only register a party once");
-
-        // Push the contract address and store the index.
-        uint contractIndex = partyMap[party].contracts.length;
-        partyMap[party].contracts.push(contractAddress);
-        partyMap[party].contractIndex[contractAddress] = contractIndex;
 
-        emit PartyAdded(contractAddress, party);
+        _addPartyToContract(party, contractAddress);
     }
 
     /**
@@ -207,4 +199,17 @@ contract Registry is RegistryInterface, MultiRole {
         uint index = partyMap[party].contractIndex[contractAddress];
         return partyMap[party].contracts.length > index && partyMap[party].contracts[index] == contractAddress;
     }
+
+    /****************************************
+     *           INTERNAL FUNCTIONS         *
+     ****************************************/
+
+    function _addPartyToContract(address party, address contractAddress) internal {
+        require(!isPartyMemberOfContract(party, contractAddress), "Can only register a party once");
+        uint contractIndex = partyMap[party].contracts.length;
+        partyMap[party].contracts.push(contractAddress);
+        partyMap[party].contractIndex[contractAddress] = contractIndex;
+
+        emit PartyAdded(contractAddress, party);
+    }
 }

core/test/oracle/Registry.js

@@ -54,6 +54,15 @@ contract("Registry", function(accounts) {
     result = await registry.registerContract(parties, arbitraryContract, { from: creator1 });
     assert.isTrue(await registry.isContractRegistered(arbitraryContract));
 
+    // Make sure a PartyAdded event is emitted on initial contract registration.
+    truffleAssert.eventEmitted(result, "PartyAdded", ev => {
+      return (
+        web3.utils.toChecksumAddress(ev.contractAddress) === web3.utils.toChecksumAddress(arbitraryContract) &&
+        // Check that the party is a member of the parties array used in registration above
+        parties.map(party => web3.utils.toChecksumAddress(party).indexOf(web3.utils.toChecksumAddress(ev.party)))
+      );
+    });
+
     // Make sure a NewContractRegistered event is emitted.
     truffleAssert.eventEmitted(result, "NewContractRegistered", ev => {
       return (
@@ -77,7 +86,6 @@ contract("Registry", function(accounts) {
     await registry.resetMember(RegistryRolesEnum.OWNER, rando1, { from: owner });
 
     // The owner can no longer add or remove contract creators.
-    assert(await didContractThrow(registry.addMember(RegistryRolesEnum.CONTRACT_CREATOR, creator1, { from: owner })));
     assert(
       await didContractThrow(registry.removeMember(RegistryRolesEnum.CONTRACT_CREATOR, creator1, { from: owner }))
     );
@@ -140,6 +148,15 @@ contract("Registry", function(accounts) {
 
     // Cannot register a contract that is already registered.
     assert(await didContractThrow(registry.registerContract([], fc1, { from: creator1 })));
+
+    // Cannot register the same address to a contract multiple times. In other words one
+    // address cant be multiple party members of one contract at registration.
+    const party = web3.utils.randomHex(20);
+    const party2 = web3.utils.randomHex(20);
+
+    assert(
+      await didContractThrow(registry.registerContract([party, party, party, party2], contract1, { from: creator1 }))
+    );
   });
 
   it("Adding parties to contracts", async function() {