PIE-1515: New Admin contract with linked list library (Consensys#35)

contracts/Admin.sol

@@ -0,0 +1,37 @@
+pragma solidity >=0.4.22 <0.6.0;
+
+import "./AdminProxy.sol";
+import "./AdminList.sol";
+
+
+contract Admin is AdminProxy, AdminList {
+    modifier onlyAdmin() {
+        require(isAuthorized(msg.sender), "Sender not authorized");
+        _;
+    }
+
+    modifier notSelf(address _address) {
+        require(msg.sender != _address, "Cannnot invoke method with own account as parameter");
+        _;
+    }
+
+    constructor() public {
+        add(msg.sender);
+    }
+
+    function isAuthorized(address _address) public view returns (bool) {
+        return exists(_address);
+    }
+
+    function addAdmin(address _address) public onlyAdmin notSelf(_address) returns (bool) {
+        return add(_address);
+    }
+
+    function removeAdmin(address _address) public onlyAdmin notSelf(_address) returns (bool) {
+        return remove(_address);
+    }
+
+    function getAdmins() public view returns (address[] memory){
+        return getAll();
+    }
+}

contracts/AdminList.sol

@@ -0,0 +1,93 @@
+pragma solidity >=0.4.22 <0.6.0;
+
+import "solidity-linked-list/contracts/StructuredLinkedList.sol";
+
+
+contract AdminList {
+    using StructuredLinkedList for StructuredLinkedList.List;
+
+    StructuredLinkedList.List private list;
+
+    function size() internal view returns (uint256) {
+        return list.sizeOf();
+    }
+
+    function exists(address _address) internal view returns (bool) {
+        return list.nodeExists(uint256(_address));
+    }
+
+    function add(address _address) internal returns (bool) {
+        return list.push(uint(_address), true);
+    }
+
+    function remove(address _address) internal returns (bool) {
+        uint node = uint(_address);
+        return list.remove(node) != 0 ? true : false;
+    }
+
+    function get(uint index) internal view returns (bool _exists, address _address) {
+        uint listSize = list.sizeOf();
+        if (index >= listSize) {
+            return (false, address(0));
+        }
+
+        uint counter = 0;
+        uint pointer = 0;
+        bool hasFound = false;
+
+        while(counter <= listSize) {
+            (bool nodeExists, uint256 prev, uint256 next) = list.getNode(pointer);
+            if (nodeExists) {
+                if (counter == index + 1) {
+                    hasFound = true;
+                    break;
+                } else {
+                    counter++;
+                    pointer = prev;
+                }
+            } else {
+                break;
+            }
+            //Getting rid of unused variable warning
+            next;
+        }
+
+        if (hasFound) {
+            return (true, address(pointer));
+        } else {
+            return (false, address(0));
+        }
+    }
+
+    function getAll() internal view returns (address[] memory) {
+        uint listSize = list.sizeOf();
+        if (listSize == 0) {
+            return new address[](0);
+        }
+
+        address[] memory allAddresses = new address[](listSize);
+
+        bool hasNext = true;
+        uint counter = 0;
+        uint pointer = 0;
+
+        while(hasNext) {
+            (bool nodeExists, uint256 prev, uint256 next) = list.getNode(pointer);
+            if (nodeExists) {
+                if (pointer > 0) {
+                    allAddresses[counter++] = address(pointer);
+                }
+
+                if (prev != 0) {
+                    pointer = prev;
+                } else {
+                    hasNext = false;
+                }
+            }
+            //Getting rid of unused variable warning
+            next;
+        }
+
+        return allAddresses;
+    }
+}

contracts/ExposedAdminList.sol

@@ -0,0 +1,32 @@
+pragma solidity >=0.4.22 <0.6.0;
+
+import "./AdminList.sol";
+
+
+// This class is used as a proxy to allow us to write unit tests.
+// All methods in the original class are internal.
+contract ExposedAdminList is AdminList {
+    function _size() public view returns (uint256) {
+        return size();
+    }
+
+    function _exists(address _address) public view returns (bool) {
+        return exists(_address);
+    }
+
+    function _add(address _address) public returns (bool) {
+        return add(_address);
+    }
+
+    function _remove(address _address) public returns (bool) {
+        return remove(_address);
+    }
+
+    function _get(uint _index) public view returns (bool __exists, address __address) {
+        return get(_index);
+    }
+
+    function _getAll() public view returns (address[] memory) {
+        return getAll();
+    }
+}

contracts/Ingress.sol

@@ -9,8 +9,8 @@ contract Ingress {
     uint version = 1000000;
 
     // Contract keys
-    bytes32 RULES_CONTRACT = 0x72756c6573000000000000000000000000000000000000000000000000000000; // "rules"
-    bytes32 ADMIN_CONTRACT = 0x61646d696e697374726174696f6e000000000000000000000000000000000000; // "administration"
+    bytes32 public RULES_CONTRACT = 0x72756c6573000000000000000000000000000000000000000000000000000000; // "rules"
+    bytes32 public ADMIN_CONTRACT = 0x61646d696e697374726174696f6e000000000000000000000000000000000000; // "administration"
 
     // Registry mapping indexing
     bytes32[] contractKeys;

contracts/Rules.sol

@@ -1,28 +1,17 @@
 pragma solidity >=0.4.22 <0.6.0;
 
-import "./AdminProxy.sol";
 import "./RulesProxy.sol";
 import "./Ingress.sol";
+import "./Admin.sol";
 
 
-contract Rules is AdminProxy, RulesProxy {
-    // These will be assigned at the construction
-    // phase, where `msg.sender` is the account
-    // creating this contract.
-    address public admin = msg.sender;
+contract Rules is RulesProxy {
+
+    // on read-only mode rules can't be added/removed
     bool readOnlyMode = false;
-    struct Admin {
-        address adminAddress;
-        bool active;
-    }
-    mapping(address => Admin) admins;
-    uint adminCount;
-    address[] adminKeys;
     // version of this contract: semver like 1.2.14 represented like 001002014
     uint version = 1000000;
 
-    address ingressContractAddress;
-
     struct Enode {
         bytes next;
         bytes prev;
@@ -39,61 +28,29 @@ contract Rules is AdminProxy, RulesProxy {
     // head of linked list
     bytes headWhitelist;
 
-    // AUTHORIZATION
-    constructor (address ingressAddress) public {
-        // add the deploying contract address as first admin
-        Admin memory orig = Admin(msg.sender, true);
-        admins[msg.sender] = orig;
-        adminKeys.push(msg.sender);
-        adminCount = adminCount + 1;
-
-        // Record the Ingress contract's address
-        ingressContractAddress = ingressAddress;
-    }
+    address private ingressContractAddress;
 
-    // AUTHORIZATION: LIST OF ADMINS
-    modifier onlyAdmin()
-    {
-        require(
-            admins[msg.sender].adminAddress != address(0),
-            "Sender not authorized."
-        );
-        require(admins[msg.sender].active == true, "Sender not authorized");
+    modifier onlyOnEditMode() {
+        require(!readOnlyMode, "In read only mode: rules cannot be modified");
         _;
     }
 
-    function isAuthorized(address source) public view returns (bool) {
-        return admins[source].active;
-    }
+    modifier onlyAdmin() {
+        Ingress ingressContract = Ingress(ingressContractAddress);
+        address adminContractAddress = ingressContract.getContractAddress(ingressContract.ADMIN_CONTRACT());
 
-    function addAdmin(address _newAdmin) public onlyAdmin returns (bool) {
-        if (admins[_newAdmin].active) {
-            return false;
-        }
-        adminKeys.push(_newAdmin);
-        Admin memory newAdmin = Admin(_newAdmin, true);
-        admins[_newAdmin] = newAdmin;
-        adminCount = adminCount + 1;
-        return true;
+        require(adminContractAddress != address(0), "Ingress contract must have Admin contract registered");
+        require(Admin(adminContractAddress).isAuthorized(msg.sender), "Sender not authorized");
+        _;
     }
 
-    function removeAdmin(address oldAdmin) public onlyAdmin returns (bool) {
-        admins[oldAdmin].active = false;
-        adminCount = adminCount - 1;
-        return true;
+    constructor (address ingressAddress) public {
+        ingressContractAddress = ingressAddress;
     }
 
-    // return list of admins
-    function getAllAdmins() public view returns (address[] memory){
-        address[] memory ret = new address[](adminCount);
-        Admin memory a;
-        for (uint i = 0; i < adminKeys.length; i++) {
-            a = admins[adminKeys[i]];
-            if (a.active) {
-                ret[i] = admins[adminKeys[i]].adminAddress;
-            }
-        }
-        return ret;
+    // VERSION
+    function getContractVersion() public view returns (uint) {
+        return version;
     }
 
     // READ ONLY MODE
@@ -113,11 +70,6 @@ contract Rules is AdminProxy, RulesProxy {
         return true;
     }
 
-    // VERSION
-    function getContractVersion() public view returns (uint) {
-        return version;
-    }
-
     // RULES - IS CONNECTION ALLOWED
     function connectionAllowed(
         bytes32 sourceEnodeHigh,
@@ -173,8 +125,7 @@ contract Rules is AdminProxy, RulesProxy {
         bytes32 enodeLow,
         bytes16 enodeIp,
         uint16 enodePort
-    ) public onlyAdmin returns (bool) {
-        require(readOnlyMode == false, "In read only mode: rules cannot be modified");
+    ) public onlyAdmin onlyOnEditMode returns (bool) {
         bytes memory key = computeKey(
             enodeHigh,
             enodeLow,
@@ -220,8 +171,7 @@ contract Rules is AdminProxy, RulesProxy {
         bytes32 enodeLow,
         bytes16 enodeIp,
         uint16 enodePort
-    ) public onlyAdmin returns (bool) {
-        require(readOnlyMode == false, "In read only mode: rules cannot be modified");
+    ) public onlyAdmin onlyOnEditMode returns (bool) {
         bytes memory key = computeKey(
             enodeHigh,
             enodeLow,
@@ -309,7 +259,6 @@ contract Rules is AdminProxy, RulesProxy {
     }
 
     function triggerRulesChangeEvent(bool addsRestrictions) public {
-        Ingress i = Ingress(ingressContractAddress);
-        i.emitRulesChangeEvent(addsRestrictions);
+        Ingress(ingressContractAddress).emitRulesChangeEvent(addsRestrictions);
     }
 }