Merge pull request #64 from valory-xyz/fixes

feat and fix: making manager contracts pausable and fixing code

.github/workflows/workflow.yaml

@@ -19,7 +19,7 @@ jobs:
       - name: Run ESLint
         run: ./node_modules/.bin/eslint . --ext .js,.jsx,.ts,.tsx
       - name: Run solhint
-        run: ./node_modules/.bin/solhint contracts/*.sol contracts/interfaces/*.sol contracts/governance/*.sol contracts/multisigs/*.sol
+        run: ./node_modules/.bin/solhint contracts/*.sol contracts/interfaces/*.sol contracts/governance/*.sol contracts/multisigs/*.sol contracts/test/*.sol
 
       # Compile the code
       - name: Compile, adjust uniswap library hash and recompile again

.solhint.json

@@ -2,7 +2,7 @@
   "extends": "solhint:recommended",
   "plugins": [],
   "rules": {
-    "compiler-version": ["error", "^0.8.4"],
+    "compiler-version": ["warn", "^0.8.4"],
     "func-visibility": ["warn",{"ignoreConstructors":true}],
     "no-empty-blocks": "off",
     "not-rely-on-time": "off",

contracts/OLA.sol

@@ -5,10 +5,11 @@ import "@openzeppelin/contracts/access/Ownable.sol";
 import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
 import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
+import "./interfaces/IErrors.sol";
 
 /// @title OLA - Smart contract for the main OLA token
 /// @author AL
-contract OLA is Context, Ownable, ERC20, ERC20Burnable, ERC20Permit {
+contract OLA is IErrors, Context, Ownable, ERC20, ERC20Burnable, ERC20Permit {
     // TODO Maximum possible number of tokens
     // uint256 public constant maxSupply = 500000000e18; // 500 million OLA
     event TreasuryUpdated(address indexed treasury);
@@ -19,9 +20,10 @@ contract OLA is Context, Ownable, ERC20, ERC20Burnable, ERC20Permit {
         treasury = msg.sender;
     }
 
-    modifier onlyTreasury() {
-        // TODO change to revert
-        require(msg.sender == treasury || msg.sender == owner(), "OLA: Unauthorized access");
+    modifier onlyManager() {
+        if (msg.sender != treasury && msg.sender != owner()) {
+            revert ManagerOnly(msg.sender, treasury);
+        }
         _;
     }
 
@@ -35,7 +37,7 @@ contract OLA is Context, Ownable, ERC20, ERC20Burnable, ERC20Permit {
     /// @dev Mints OLA tokens.
     /// @param account Account address.
     /// @param amount OLA token amount.
-    function mint(address account, uint256 amount) public onlyTreasury {
+    function mint(address account, uint256 amount) public onlyManager {
         super._mint(account, amount);
     }
 

contracts/RegistriesManager.sol

@@ -24,18 +24,21 @@ contract RegistriesManager is IStructs, Ownable, Pausable {
     /// @param description Description of the agent.
     /// @param dependencies Set of component dependencies in a sorted ascending order.
     /// @return The id of a minted agent.
-    function mintAgent(address owner, address developer, Multihash memory agentHash,
-        string memory description, uint256[] memory dependencies)
-        public
-        returns (uint256)
+    function mintAgent(
+        address owner,
+        address developer,
+        Multihash memory agentHash,
+        string memory description,
+        uint256[] memory dependencies
+    ) external whenNotPaused returns (uint256)
     {
         return IRegistry(agentRegistry).create(owner, developer, agentHash, description, dependencies);
     }
 
     /// @dev Updates the agent hash.
     /// @param tokenId Token Id.
     /// @param agentHash New IPFS hash of the agent.
-    function updateAgentHash(uint256 tokenId, Multihash memory agentHash) public {
+    function updateAgentHash(uint256 tokenId, Multihash memory agentHash) external {
         return IRegistry(agentRegistry).updateHash(msg.sender, tokenId, agentHash);
     }
 
@@ -46,18 +49,31 @@ contract RegistriesManager is IStructs, Ownable, Pausable {
     /// @param description Description of the component.
     /// @param dependencies Set of component dependencies in a sorted ascending order.
     /// @return The id of a minted component.
-    function mintComponent(address owner, address developer, Multihash memory componentHash,
-        string memory description, uint256[] memory dependencies)
-        public
-        returns (uint256)
+    function mintComponent(
+        address owner,
+        address developer,
+        Multihash memory componentHash,
+        string memory description,
+        uint256[] memory dependencies
+    ) external whenNotPaused returns (uint256)
     {
         return IRegistry(componentRegistry).create(owner, developer, componentHash, description, dependencies);
     }
 
     /// @dev Updates the component hash.
     /// @param tokenId Token Id.
     /// @param componentHash New IPFS hash of the component.
-    function updateComponentHash(uint256 tokenId, Multihash memory componentHash) public {
+    function updateComponentHash(uint256 tokenId, Multihash memory componentHash) external {
         return IRegistry(componentRegistry).updateHash(msg.sender, tokenId, componentHash);
     }
+
+    /// @dev Pauses the contract.
+    function pause() external onlyOwner {
+        _pause();
+    }
+
+    /// @dev Unpauses the contract.
+    function unpause() external onlyOwner {
+        _unpause();
+    }
 }

contracts/ServiceManager.sol

@@ -2,13 +2,14 @@
 pragma solidity ^0.8.4;
 
 import "@openzeppelin/contracts/access/Ownable.sol";
+import "@openzeppelin/contracts/security/Pausable.sol";
 import "./interfaces/IErrors.sol";
 import "./interfaces/IStructs.sol";
 import "./interfaces/IService.sol";
 
 /// @title Service Manager - Periphery smart contract for managing services
 /// @author Aleksandr Kuperman - <[email protected]>
-contract ServiceManager is IErrors, IStructs, Ownable {
+contract ServiceManager is IErrors, IStructs, Ownable, Pausable {
     event MultisigCreate(address multisig);
 
     address public immutable serviceRegistry;
@@ -43,7 +44,7 @@ contract ServiceManager is IErrors, IStructs, Ownable {
         uint256[] memory agentIds,
         AgentParams[] memory agentParams,
         uint256 threshold
-    ) public returns (uint256)
+    ) external whenNotPaused returns (uint256)
     {
         return IService(serviceRegistry).createService(owner, name, description, configHash, agentIds, agentParams,
             threshold);
@@ -65,7 +66,7 @@ contract ServiceManager is IErrors, IStructs, Ownable {
         AgentParams[] memory agentParams,
         uint256 threshold,
         uint256 serviceId
-    ) public
+    ) external
     {
         IService(serviceRegistry).update(msg.sender, name, description, configHash, agentIds, agentParams,
             threshold, serviceId);
@@ -74,7 +75,7 @@ contract ServiceManager is IErrors, IStructs, Ownable {
     /// @dev Activates the service and its sensitive components.
     /// @param serviceId Correspondent service Id.
     /// @return success True, if function executed successfully.
-    function serviceActivateRegistration(uint256 serviceId) public payable returns (bool success) {
+    function serviceActivateRegistration(uint256 serviceId) external payable returns (bool success) {
         success = IService(serviceRegistry).activateRegistration{value: msg.value}(msg.sender, serviceId);
     }
 
@@ -87,7 +88,7 @@ contract ServiceManager is IErrors, IStructs, Ownable {
         uint256 serviceId,
         address[] memory agentInstances,
         uint256[] memory agentIds
-    ) public payable returns (bool success) {
+    ) external payable returns (bool success) {
         success = IService(serviceRegistry).registerAgents{value: msg.value}(msg.sender, serviceId, agentInstances, agentIds);
     }
 
@@ -110,22 +111,32 @@ contract ServiceManager is IErrors, IStructs, Ownable {
     /// @param serviceId Service Id.
     /// @return success True, if function executed successfully.
     /// @return refund Refund to return to the owner.
-    function serviceTerminate(uint256 serviceId) public returns (bool success, uint256 refund) {
+    function serviceTerminate(uint256 serviceId) external returns (bool success, uint256 refund) {
         (success, refund) = IService(serviceRegistry).terminate(msg.sender, serviceId);
     }
 
     /// @dev Unbonds agent instances of the operator from the service.
     /// @param serviceId Service Id.
     /// @return success True, if function executed successfully.
     /// @return refund The amount of refund returned to the operator.
-    function serviceUnbond(uint256 serviceId) public returns (bool success, uint256 refund) {
+    function serviceUnbond(uint256 serviceId) external returns (bool success, uint256 refund) {
         (success, refund) = IService(serviceRegistry).unbond(msg.sender, serviceId);
     }
 
     /// @dev Destroys the service instance and frees up its storage.
     /// @param serviceId Correspondent service Id.
     /// @return success True, if function executed successfully.
-    function serviceDestroy(uint256 serviceId) public returns (bool success) {
+    function serviceDestroy(uint256 serviceId) external returns (bool success) {
         success = IService(serviceRegistry).destroy(msg.sender, serviceId);
     }
+
+    /// @dev Pauses the contract.
+    function pause() external onlyOwner {
+        _pause();
+    }
+
+    /// @dev Unpauses the contract.
+    function unpause() external onlyOwner {
+        _unpause();
+    }
 }

contracts/ServiceRegistry.sol

@@ -173,7 +173,7 @@ contract ServiceRegistry is IErrors, IStructs, Ownable, ERC721Enumerable, Reentr
 
         // Checking for non-empty arrays and correct number of values in them
         if (agentIds.length == 0 || agentIds.length != agentParams.length) {
-            revert WrongAgentsData(agentIds.length, agentParams.length);
+            revert WrongArrayLength(agentIds.length, agentParams.length);
         }
 
         // Check for canonical agent Ids existence and for duplicate Ids
@@ -390,7 +390,7 @@ contract ServiceRegistry is IErrors, IStructs, Ownable, ERC721Enumerable, Reentr
     {
         // Check if the length of canonical agent instance addresses array and ids array have the same length
         if (agentInstances.length != agentIds.length) {
-            revert WrongAgentsData(agentInstances.length, agentIds.length);
+            revert WrongArrayLength(agentInstances.length, agentIds.length);
         }
 
         Service storage service = _mapServices[serviceId];
@@ -506,7 +506,7 @@ contract ServiceRegistry is IErrors, IStructs, Ownable, ERC721Enumerable, Reentr
     {
         // Check for the array size
         if (agentInstances.length != amounts.length) {
-            revert WrongAgentsData(agentInstances.length, amounts.length);
+            revert WrongArrayLength(agentInstances.length, amounts.length);
         }
 
         Service storage service = _mapServices[serviceId];