permissions: documentation changes based on review feedback

permission/contract/PermissionsInterface.sol

@@ -3,156 +3,218 @@ pragma solidity ^0.5.3;
 import "./PermissionsImplementation.sol";
 import "./PermissionsUpgradable.sol";
 
-
+/// @title Permissions Interface Contract
+/// @notice This contract is the interface for permissions implementation
+/// @notice contract. for any call, it forwards the call to the implementation
+/// @notice contract
 contract PermissionsInterface {
     PermissionsImplementation private permImplementation;
     PermissionsUpgradable private permUpgradable;
     address private permImplUpgradeable;
 
+    /// @notice constructor
+    /// @param _permImplUpgradeable permissions upgradable contract address
     constructor(address _permImplUpgradeable) public {
         permImplUpgradeable = _permImplUpgradeable;
     }
 
+    /// @notice modifier to verify that caller is permissions upgradable contract
+    /// @notice address
     modifier onlyUpgradeable {
         require(msg.sender == permImplUpgradeable);
         _;
     }
 
-    function setPermImplementation(address _permImplementation) public
-    onlyUpgradeable
-    {
-        permImplementation = PermissionsImplementation(_permImplementation);
-    }
-
-    function getPermissionsImpl() public view returns (address)
-    {
-        return address(permImplementation);
-    }
-
-    function setPolicy(string calldata _nwAdminOrg, string calldata _nwAdminRole, string calldata _oAdminRole) external
-    {
+    /// @notice interface for setting the permissions policy in implementation
+    /// @param _nwAdminOrg network admin organization id
+    /// @param _nwAdminRole default network admin role id
+    /// @param _oAdminRole default organization admin role id
+    function setPolicy(string calldata _nwAdminOrg, string calldata _nwAdminRole, string calldata _oAdminRole) external {
         permImplementation.setPolicy(_nwAdminOrg, _nwAdminRole, _oAdminRole);
     }
 
-    function init(uint _breadth, uint _depth) external
-    {
+    /// @notice interface to initializes the breadth and depth values for
+    /// @notice sub organization management
+    /// @param _breadth controls the number of sub org a parent org can have
+    /// @param _depth controls the depth of nesting allowed for sub orgs
+    function init(uint _breadth, uint _depth) external {
         permImplementation.init(_breadth, _depth);
     }
-
-    function addAdminNodes(string calldata _enodeId) external
-    {
+    /// @notice interface to add new node to an admin organization
+    /// @param _enodeId full enode id of the node to be added
+    function addAdminNodes(string calldata _enodeId) external {
         permImplementation.addAdminNodes(_enodeId);
     }
 
-    function addAdminAccounts(address _acct) external
-    {
+    /// @notice interface to add accounts to an admin organization
+    /// @param _acct account address to be added
+    function addAdminAccounts(address _acct) external {
         permImplementation.addAdminAccounts(_acct);
     }
 
-    // update the network boot status as true
+    /// @notice interface to update network boot up status
+    /// @return bool true or false
     function updateNetworkBootStatus() external
     returns (bool)
     {
         permImplementation.updateNetworkBootStatus();
     }
 
-    //    // Get network boot status
-    function getNetworkBootStatus() external view returns (bool)
-    {
+    /// @notice interface to fetch network boot status
+    /// @return bool network boot status
+    function getNetworkBootStatus() external view returns (bool){
         return permImplementation.getNetworkBootStatus();
     }
 
-    // function for adding a new master org
-    function addOrg(string calldata _orgId, string calldata _enodeId, address _account) external
-    {
+    /// @notice interface to add a new organization to the network
+    /// @param _orgId unique organization id
+    /// @param _enodeId full enode id linked to the organization
+    /// @param _account account id. this will have the org admin privileges
+    function addOrg(string calldata _orgId, string calldata _enodeId, address _account) external {
         permImplementation.addOrg(_orgId, _enodeId, _account, msg.sender);
     }
 
-    function approveOrg(string calldata _orgId, string calldata _enodeId, address _account) external
-    {
+    /// @notice interface to approve a newly added organization
+    /// @param _orgId unique organization id
+    /// @param _enodeId full enode id linked to the organization
+    /// @param _account account id this will have the org admin privileges
+    function approveOrg(string calldata _orgId, string calldata _enodeId, address _account) external {
         permImplementation.approveOrg(_orgId, _enodeId, _account, msg.sender);
     }
 
-    // function for adding a new master org
-    function addSubOrg(string calldata _pOrg, string calldata _orgId, string calldata _enodeId) external
-    {
-        permImplementation.addSubOrg(_pOrg, _orgId, _enodeId, msg.sender);
+    /// @notice interface to add sub org under an org
+    /// @param _pOrgId parent org id under which the sub org is being added
+    /// @param _orgId unique id for the sub organization
+    /// @param _enodeId full enode id linked to the sjb organization
+    function addSubOrg(string calldata _pOrgId, string calldata _orgId, string calldata _enodeId) external {
+        permImplementation.addSubOrg(_pOrgId, _orgId, _enodeId, msg.sender);
     }
 
-    function updateOrgStatus(string calldata _orgId, uint _action) external
-    {
+    /// @notice interface to update the org status
+    /// @param _orgId unique id of the organization
+    /// @param _action 1 for suspending an org and 2 for revoke of suspension
+    function updateOrgStatus(string calldata _orgId, uint _action) external {
         permImplementation.updateOrgStatus(_orgId, _action, msg.sender);
     }
 
-    function approveOrgStatus(string calldata _orgId, uint _action) external
-    {
+    /// @notice interface to approve org status change
+    /// @param _orgId unique id for the sub organization
+    /// @param _action 1 for suspending an org and 2 for revoke of suspension
+    function approveOrgStatus(string calldata _orgId, uint _action) external {
         permImplementation.approveOrgStatus(_orgId, _action, msg.sender);
     }
 
-    // Role related functions
-    function addNewRole(string calldata _roleId, string calldata _orgId, uint _access, bool _voter, bool _admin) external
-    {
+    /// @notice interface to new role definition to an organization
+    /// @param _roleId unique id for the role
+    /// @param _orgId unique id of the organization to which the role belongs
+    /// @param _access 0-ReadOnly, 1-Transact, 2-ContractDeploy, 3-FullAccess
+    /// @param _voter bool indicates if the role is voter role or not
+    /// @param _admin bool indicates if the role is an admin role
+    function addNewRole(string calldata _roleId, string calldata _orgId, uint _access, bool _voter, bool _admin) external {
         permImplementation.addNewRole(_roleId, _orgId, _access, _voter, _admin, msg.sender);
     }
 
-    function removeRole(string calldata _roleId, string calldata _orgId) external
-    {
+    /// @notice interface to remove a role definition from an organization
+    /// @param _roleId unique id for the role
+    /// @param _orgId unique id of the organization to which the role belongs
+    function removeRole(string calldata _roleId, string calldata _orgId) external {
         permImplementation.removeRole(_roleId, _orgId, msg.sender);
     }
 
-    function assignAdminRole(string calldata _orgId, address _account, string calldata _roleId) external
-    {
+    /// @notice interface to assign network admin/org admin role to an account
+    /// @notice this can be executed by network admin accounts only
+    /// @param _orgId unique id of the organization to which the account belongs
+    /// @param _account account id
+    /// @param _roleId role id to be assigned to the account
+    function assignAdminRole(string calldata _orgId, address _account, string calldata _roleId) external {
         permImplementation.assignAdminRole(_orgId, _account, _roleId, msg.sender);
 
     }
-
-    function approveAdminRole(string calldata _orgId, address _account) external
-    {
+    /// @notice interface to approve network admin/org admin role assigment
+    /// @notice this can be executed by network admin accounts only
+    /// @param _orgId unique id of the organization to which the account belongs
+    /// @param _account account id
+    function approveAdminRole(string calldata _orgId, address _account) external {
         permImplementation.approveAdminRole(_orgId, _account, msg.sender);
 
     }
 
-    function assignAccountRole(address _acct, string memory _orgId, string memory _roleId) public
-    {
-        permImplementation.assignAccountRole(_acct, _orgId, _roleId, msg.sender);
-
-    }
-
-    function updateAccountStatus(string calldata _orgId, address _account, uint _status) external
-    {
+    /// @notice interface to update account status
+    /// @notice this can be executed by org admin accounts only
+    /// @param _orgId unique id of the organization to which the account belongs
+    /// @param _account account id
+    /// @param _status 1-suspending 2-activating back 3-blacklisting
+    function updateAccountStatus(string calldata _orgId, address _account, uint _status) external {
         permImplementation.updateAccountStatus(_orgId, _account, _status, msg.sender);
     }
 
-    function addNode(string calldata _orgId, string calldata _enodeId) external
-    {
+    /// @notice interface to add a new node to the organization
+    /// @param _orgId unique id of the organization to which the account belongs
+    /// @param _enodeId full enode id being dded to the org
+    function addNode(string calldata _orgId, string calldata _enodeId) external {
         permImplementation.addNode(_orgId, _enodeId, msg.sender);
 
     }
 
-    function updateNodeStatus(string calldata _orgId, string calldata _enodeId, uint _action) external
-    {
+    /// @notice interface to update node status
+    /// @param _orgId unique id of the organization to which the account belongs
+    /// @param _enodeId full enode id being dded to the org
+    /// @param _action 1-deactivate, 2-activate back, 3-blacklist the node
+    function updateNodeStatus(string calldata _orgId, string calldata _enodeId, uint _action) external {
         permImplementation.updateNodeStatus(_orgId, _enodeId, _action, msg.sender);
     }
 
-    function isNetworkAdmin(address _account) public view returns (bool)
-    {
-        return permImplementation.isNetworkAdmin(_account);
-    }
-
-    function isOrgAdmin(address _account, string memory _orgId) public view returns (bool)
-    {
-
-        return permImplementation.isOrgAdmin(_account, _orgId);
-    }
-
-    function validateAccount(address _account, string memory _orgId) public view returns (bool)
-    {
-        return permImplementation.validateAccount(_account, _orgId);
-    }
-
-    function getPendingOp(string calldata _orgId) external view returns (string memory, string memory, address, uint)
-    {
+    /// @notice interface to fetch detail of any pending approval activities
+    /// @notice for network admin organization
+    /// @param _orgId unique id of the organization to which the account belongs
+    function getPendingOp(string calldata _orgId) external view returns (string memory, string memory, address, uint) {
         return permImplementation.getPendingOp(_orgId);
     }
 
+    /// @notice sets the permissions implementation contract address
+    /// @notice can be called from upgradable contract only
+    /// @param _permImplementation permissions implementation contract address
+    function setPermImplementation(address _permImplementation) external
+    onlyUpgradeable {
+        permImplementation = PermissionsImplementation(_permImplementation);
+    }
+
+    /// @notice returns the address of permissions implementation contract
+    /// @return permissions implementation contract address
+    function getPermissionsImpl() external view returns (address) {
+        return address(permImplementation);
+    }
+
+    /// @notice interface to assigns a role id to the account give
+    /// @param _account account id
+    /// @param _orgId organization id to which the account belongs
+    /// @param _roleId role id to be assigned to the account
+    function assignAccountRole(address _account, string calldata _orgId, string calldata _roleId) external {
+        permImplementation.assignAccountRole(_account, _orgId, _roleId, msg.sender);
+
+    }
+
+    /// @notice interface to check if passed account is an network admin account
+    /// @param _account account id
+    /// @return true/false
+    function isNetworkAdmin(address _account) external view returns (bool) {
+        return permImplementation.isNetworkAdmin(_account);
+    }
+
+    /// @notice interface to check if passed account is an org admin account
+    /// @param _account account id
+    /// @param _orgId organization id
+    /// @return true/false
+    function isOrgAdmin(address _account, string calldata _orgId) external view returns (bool) {
+        return permImplementation.isOrgAdmin(_account, _orgId);
+    }
+
+    /// @notice interface to validate the account for access change operation
+    /// @param _account account id
+    /// @param _orgId organization id
+    /// @return true/false
+    function validateAccount(address _account, string calldata _orgId) external view returns (bool) {
+        return permImplementation.validateAccount(_account, _orgId);
+    }
+
 }

permission/contract/PermissionsUpgradable.sol

@@ -2,69 +2,85 @@ pragma solidity ^0.5.3;
 
 import "./PermissionsInterface.sol";
 
+/// @title Permissions Upgradable Contract
+/// @notice This contract holds the address of current permissions implementation
+/// @notice contract. The contract is owned by a guardian account. Only the
+/// @notice guardian account can change the implementation contract address as
+/// @notice business needs.
 contract PermissionsUpgradable {
 
     address private guardian;
     address private permImpl;
     address private permInterface;
 
-    // sets the guardian account as part of constructor
-    // only this account will be able to change the implementation contract address
-    constructor (address _guardian) public
-    {
+    /// @notice constructor
+    /// @param _guardian account address
+    constructor (address _guardian) public{
         guardian = _guardian;
     }
 
-    modifier onlyCustodian {
+    /// @notice modifier to verify that caller is guardian account
+    modifier onlyGuardian {
         require(msg.sender == guardian);
         _;
     }
 
-    // executed by guardian, links interface and implementation contract addresses
+    /// @notice executed by guardian. Links interface and implementation contract
+    /// @notice addresses. Can be executed by guardian account only
+    /// @param _permInterface permissions interface contract address
+    /// @param _permImpl implementation contract address
     function init(address _permInterface, address _permImpl) external
-    onlyCustodian
-    {
+    onlyGuardian {
         permImpl = _permImpl;
         permInterface = _permInterface;
-        setImpl(permImpl);
+        _setImpl(permImpl);
     }
 
-
-    // guardian can potentially become a contract
-    // implementation change and guardian change are sending from guardian
+    /// @notice changes the implementation contract address to the new address
+    /// @notice address passed. Can be executed by guardian account only
+    /// @param _proposedImpl address of the new permissions implementation contract
     function confirmImplChange(address _proposedImpl) public
-    onlyCustodian
-    {
-        // read the details from current implementation
+    onlyGuardian {
+        // The policy details needs to be carried forward from existing
+        // implementation to new. So first these are read from existing
+        // implementation and then updated in new implementation
         (string memory adminOrg, string memory adminRole, string memory orgAdminRole, bool bootStatus) = PermissionsImplementation(permImpl).getPolicyDetails();
-        setPolicy(_proposedImpl, adminOrg, adminRole, orgAdminRole, bootStatus);
-        // set these values in new implementation
+        _setPolicy(_proposedImpl, adminOrg, adminRole, orgAdminRole, bootStatus);
         permImpl = _proposedImpl;
-        setImpl(permImpl);
+        _setImpl(permImpl);
     }
-
-    function getGuardian() public view returns (address)
-    {
+    /// @notice function to fetch the guardian account address
+    /// @return _guardian guardian account address
+    function getGuardian() public view returns (address) {
         return guardian;
     }
 
-    function getPermImpl() public view returns (address)
-    {
+    /// @notice function to fetch the current implementation address
+    /// @return permissions implementation contract address
+    function getPermImpl() public view returns (address) {
         return permImpl;
     }
-
-    function getPermInterface() public view returns (address)
-    {
+    /// @notice function to fetch the interface address
+    /// @return defpermissions interface contract address
+    function getPermInterface() public view returns (address) {
         return permInterface;
     }
 
-    function setPolicy(address _permImpl, string memory _adminOrg, string memory _adminRole, string memory _orgAdminRole, bool _bootStatus) private
-    {
+    /// @notice function to set the permissions policy details in the
+    /// @notice permissions implementation contract
+    /// @param _permImpl permissions implementation contract address
+    /// @param _adminOrg name of admin organization
+    /// @param _adminRole name of the admin role
+    /// @param _orgAdminRole name of default organization admin role
+    /// @param _bootStatus network boot status
+    function _setPolicy(address _permImpl, string memory _adminOrg, string memory _adminRole, string memory _orgAdminRole, bool _bootStatus) private {
         PermissionsImplementation(_permImpl).setMigrationPolicy(_adminOrg, _adminRole, _orgAdminRole, _bootStatus);
     }
 
-    function setImpl(address _permImpl) private
-    {
+    /// @notice function to set the permissions implementation contract address
+    /// @notice in the permissions interface contract
+    /// @param _permImpl permissions implementation contract address
+    function _setImpl(address _permImpl) private {
         PermissionsInterface(permInterface).setPermImplementation(_permImpl);
     }