Add support for deprecated interface

Author: kronosapiens

contracts/colony/Colony.sol

@@ -49,10 +49,11 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
   public stoppable auth
   returns (bool)
   {
-    // Prevent transactions to network contracts
+    // Prevent transactions to network contracts or network-managed extensions installed in this colony
     require(_to != address(this), "colony-cannot-target-self");
     require(_to != colonyNetworkAddress, "colony-cannot-target-network");
     require(_to != tokenLockingAddress, "colony-cannot-target-token-locking");
+    require(!isOwnExtension(_to), "colony-cannot-target-own-extensions");
 
     // Prevent transactions to transfer held tokens
     bytes4 sig;
@@ -63,16 +64,6 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
     else if (sig == TRANSFER_SIG) { transferTransactionPreparation(_to, _action); }
     else if (sig == BURN_GUY_SIG || sig == TRANSFER_FROM_SIG) { burnGuyOrTransferFromTransactionPreparation(_action); }
 
-    // Prevent transactions to network-managed extensions installed in this colony
-    require(isContract(_to), "colony-to-must-be-contract");
-    // slither-disable-next-line unused-return
-    try ColonyExtension(_to).identifier() returns (bytes32 extensionId) {
-      require(
-        IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) != _to,
-        "colony-cannot-target-extensions"
-      );
-    } catch {}
-
     bool res = executeCall(_to, 0, _action);
 
     if (sig == APPROVE_SIG) { approveTransactionCleanup(_to, _action); }
@@ -345,23 +336,43 @@ contract Colony is ColonyStorage, PatriciaTreeProofs, MultiChain {
   function installExtension(bytes32 _extensionId, uint256 _version)
   public stoppable auth returns (address)
   {
-    address extension = IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version);
-    return extension;
+    return IColonyNetwork(colonyNetworkAddress).installExtension(_extensionId, _version);
   }
 
-  function upgradeExtension(address payable _extension, uint256 _newVersion)
+  // Deprecated
+  function upgradeExtension(bytes32 _extensionId, uint256 _newVersion)
+  public stoppable auth
+  {
+    IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extensionId, _newVersion);
+  }
+
+  function upgradeExtension(address _extension, uint256 _newVersion)
   public stoppable auth
   {
     IColonyNetwork(colonyNetworkAddress).upgradeExtension(_extension, _newVersion);
   }
 
-  function deprecateExtension(address payable _extension, bool _deprecated)
+  // Deprecated
+  function deprecateExtension(bytes32 _extensionId, bool _deprecated)
+  public stoppable auth
+  {
+    IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extensionId, _deprecated);
+  }
+
+  function deprecateExtension(address _extension, bool _deprecated)
   public stoppable auth
   {
     IColonyNetwork(colonyNetworkAddress).deprecateExtension(_extension, _deprecated);
   }
 
-  function uninstallExtension(address payable _extension)
+  // Deprecated
+  function uninstallExtension(bytes32 _extensionId)
+  public stoppable auth
+  {
+    IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extensionId);
+  }
+
+  function uninstallExtension(address _extension)
   public stoppable auth
   {
     IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extension);

contracts/colony/ColonyStorage.sol

@@ -221,20 +221,7 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
   }
 
   modifier onlyExtension() {
-    // Ensure msg.sender is a contract
-    require(isContract(msg.sender), "colony-sender-must-be-contract");
-
-    // Ensure msg.sender is an extension, must check old & new formats
-    // slither-disable-next-line unused-return
-    try ColonyExtension(msg.sender).identifier() returns (bytes32 extensionId) {
-      require(
-        IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == msg.sender ||
-        IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(msg.sender) == address(this),
-        "colony-must-be-extension"
-      );
-    } catch {
-      require(false, "colony-must-be-extension");
-    }
+    require(isOwnExtension(msg.sender), "colony-must-be-extension");
     _;
   }
 
@@ -283,6 +270,23 @@ contract ColonyStorage is CommonStorage, ColonyDataTypes, ColonyNetworkDataTypes
     return size > 0;
   }
 
+  function isOwnExtension(address addr) internal returns (bool) {
+    // Ensure addr is a contract first, otherwise `try` block will revert
+    if (!isContract(addr)) { return false; }
+
+    // // Ensure addr is an extension installed in the colony, must check old & new formats
+    // // slither-disable-next-line unused-return
+    try ColonyExtension(addr).identifier() returns (bytes32 extensionId) {
+      return (
+        IColonyNetwork(colonyNetworkAddress).getExtensionInstallation(extensionId, address(this)) == addr ||
+        IColonyNetwork(colonyNetworkAddress).getExtensionMultiInstallation(addr) == address(this)
+      );
+    } catch {
+      return false;
+    }
+
+  }
+
   function domainExists(uint256 domainId) internal view returns (bool) {
     return domainId > 0 && domainId <= domainCount;
   }

contracts/colony/IColony.sol

@@ -250,21 +250,40 @@ interface IColony is ColonyDataTypes, IRecovery {
   /// @return extension The address of the extension installation
   function installExtension(bytes32 extensionId, uint256 version) external returns (address extension);
 
+  /// @dev DEPRECATED
+  /// @notice Upgrade an extension in a colony. Secured function to authorised members.
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  /// @param newVersion The version to upgrade to (must be one larger than the current version)
+  function upgradeExtension(bytes32 extensionId, uint256 newVersion) external;
+
   /// @notice Upgrade an extension in a colony. Secured function to authorised members.
   /// @param extension The address of the extension installation
   /// @param newVersion The version to upgrade to (must be one larger than the current version)
-  function upgradeExtension(address payable extension, uint256 newVersion) external;
+  function upgradeExtension(address extension, uint256 newVersion) external;
+
+  /// @dev DEPRECATED
+  /// @notice Set the deprecation of an extension in a colony. Secured function to authorised members.
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  /// @param deprecated Whether to deprecate the extension or not
+  function deprecateExtension(bytes32 extensionId, bool deprecated) external;
 
   /// @notice Set the deprecation of an extension in a colony. Secured function to authorised members.
   /// @param extension The address of the extension installation
   /// @param deprecated Whether to deprecate the extension or not
-  function deprecateExtension(address payable extension, bool deprecated) external;
+  function deprecateExtension(address extension, bool deprecated) external;
+
+  /// @dev DEPRECATED
+  /// @notice Uninstall an extension from a colony. Secured function to authorised members.
+  /// @dev This is a permanent action -- re-installing the extension will deploy a new contract
+  /// @dev It is recommended to deprecate an extension before uninstalling to allow active objects to be resolved
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  function uninstallExtension(bytes32 extensionId) external;
 
   /// @notice Uninstall an extension from a colony. Secured function to authorised members.
   /// @dev This is a permanent action -- re-installing the extension will deploy a new contract
   /// @dev It is recommended to deprecate an extension before uninstalling to allow active objects to be resolved
   /// @param extension The address of the extension installation
-  function uninstallExtension(address payable extension) external;
+  function uninstallExtension(address extension) external;
 
   /// @notice Add a colony domain, and its respective local skill under skill with id `_parentSkillId`.
   /// New funding pot is created and associated with the domain here.

contracts/colonyNetwork/ColonyNetworkDataTypes.sol

@@ -121,18 +121,38 @@ interface ColonyNetworkDataTypes {
   /// @param version The version of the extension
   event ExtensionInstalled(bytes32 indexed extensionId, address indexed extension, address indexed colony, uint256 version);
 
+  /// @dev DEPRECATED
+  /// @notice Event logged when an extension is upgraded in a colony
+  /// @param extensionId The identifier for the extension
+  /// @param colony The address of the colony
+  /// @param version The new version of the extension
+  event ExtensionUpgraded(bytes32 indexed extensionId, address indexed colony, uint256 version);
+
   /// @notice Event logged when an extension is upgraded in a colony
   /// @param extension Address of the extension installation
   /// @param colony The address of the colony
   /// @param version The new version of the extension
   event ExtensionUpgraded(address indexed extension, address indexed colony, uint256 version);
 
+  /// @dev DEPRECATED
+  /// @notice Event logged when an extension is (un)deprecated in a colony
+  /// @param extensionId The identifier for the extension
+  /// @param colony The address of the colony
+  /// @param deprecated Whether the extension is deprecated or not
+  event ExtensionDeprecated(bytes32 indexed extensionId, address indexed colony, bool deprecated);
+
   /// @notice Event logged when an extension is (un)deprecated in a colony
   /// @param extension Address of the extension installation
   /// @param colony The address of the colony
   /// @param deprecated Whether the extension is deprecated or not
   event ExtensionDeprecated(address indexed extension, address indexed colony, bool deprecated);
 
+  /// @dev DEPRECATED
+  /// @notice Event logged when an extension is uninstalled from a colony
+  /// @param extensionId The identifier for the extension
+  /// @param colony The address of the colony
+  event ExtensionUninstalled(bytes32 indexed extensionId, address indexed colony);
+
   /// @notice Event logged when an extension is uninstalled from a colony
   /// @param extension Address of the extension installation
   /// @param colony The address of the colony

contracts/colonyNetwork/ColonyNetworkExtensions.sol

@@ -65,7 +65,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     return address(extension);
   }
 
-  function upgradeExtension(address payable _extension, uint256 _newVersion)
+  // Deprecated
+  function upgradeExtension(bytes32 _extensionId, uint256 _newVersion)
+    public
+    stoppable
+  {
+    address extension = migrateToMultiExtension(_extensionId);
+    upgradeExtension(extension, _newVersion);
+
+    emit ExtensionUpgraded(_extensionId, msg.sender, _newVersion);
+  }
+
+  function upgradeExtension(address _extension, uint256 _newVersion)
     public
     stoppable
     calledByColony
@@ -77,15 +88,26 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     require(_newVersion == ColonyExtension(_extension).version() + 1, "colony-network-extension-bad-increment");
     require(resolvers[extensionId][_newVersion] != address(0x0), "colony-network-extension-bad-version");
 
-    EtherRouter(_extension).setResolver(resolvers[extensionId][_newVersion]);
+    EtherRouter(payable(_extension)).setResolver(resolvers[extensionId][_newVersion]);
     ColonyExtension(_extension).finishUpgrade();
 
     assert(ColonyExtension(_extension).version() == _newVersion);
 
     emit ExtensionUpgraded(_extension, msg.sender, _newVersion);
   }
 
-  function deprecateExtension(address payable _extension, bool _deprecated)
+  // Deprecated
+  function deprecateExtension(bytes32 _extensionId, bool _deprecated)
+    public
+    stoppable
+  {
+    address extension = migrateToMultiExtension(_extensionId);
+    deprecateExtension(extension, _deprecated);
+
+    emit ExtensionDeprecated(_extensionId, msg.sender, _deprecated);
+  }
+
+  function deprecateExtension(address _extension, bool _deprecated)
     public
     stoppable
     calledByColony
@@ -95,7 +117,18 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     emit ExtensionDeprecated(_extension, msg.sender, _deprecated);
   }
 
-  function uninstallExtension(address payable _extension)
+  // Deprecated
+  function uninstallExtension(bytes32 _extensionId)
+    public
+    stoppable
+  {
+    address extension = migrateToMultiExtension(_extensionId);
+    uninstallExtension(extension);
+
+    emit ExtensionUninstalled(_extensionId, msg.sender);
+  }
+
+  function uninstallExtension(address _extension)
     public
     stoppable
     calledByColony
@@ -150,4 +183,12 @@ contract ColonyNetworkExtensions is ColonyNetworkStorage {
     address extension = Resolver(_resolver).lookup(VERSION_SIG);
     return ColonyExtension(extension).version();
   }
+
+  function migrateToMultiExtension(bytes32 _extensionId) internal returns (address) {
+    address extension = installations[_extensionId][msg.sender];
+    require(extension != address(0x0), "colony-network-extension-not-installed");
+
+    multiInstallations[extension] = msg.sender;
+    return extension;
+  }
 }

contracts/colonyNetwork/IColonyNetwork.sol

@@ -319,19 +319,36 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery {
   /// @return extension The address of the extension installation
   function installExtension(bytes32 extensionId, uint256 version) external returns (address extension);
 
+  /// @dev DEPRECATED
+  /// @notice Upgrade an extension in a colony. Can only be called by a Colony.
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  /// @param newVersion Version of the extension to upgrade to (must be one greater than current)
+  function upgradeExtension(bytes32 extensionId, uint256 newVersion) external;
+
   /// @notice Upgrade an extension in a colony. Can only be called by a Colony.
   /// @param extension Address of the extension installation
   /// @param newVersion Version of the extension to upgrade to (must be one greater than current)
-  function upgradeExtension(address payable extension, uint256 newVersion) external;
+  function upgradeExtension(address extension, uint256 newVersion) external;
+
+  /// @dev DEPRECATED
+  /// @notice Set the deprecation of an extension in a colony. Can only be called by a Colony.
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  /// @param deprecated Whether to deprecate the extension or not
+  function deprecateExtension(bytes32 extensionId, bool deprecated) external;
 
   /// @notice Set the deprecation of an extension in a colony. Can only be called by a Colony.
   /// @param extension Address of the extension installation
   /// @param deprecated Whether to deprecate the extension or not
-  function deprecateExtension(address payable extension, bool deprecated) external;
+  function deprecateExtension(address extension, bool deprecated) external;
+
+  /// @dev DEPRECATED
+  /// @notice Uninstall an extension in a colony. Can only be called by a Colony.
+  /// @param extensionId keccak256 hash of the extension name, used as an indentifier
+  function uninstallExtension(bytes32 extensionId) external;
 
   /// @notice Uninstall an extension in a colony. Can only be called by a Colony.
   /// @param extension Address of the extension installation
-  function uninstallExtension(address payable extension) external;
+  function uninstallExtension(address extension) external;
 
   /// @notice Get an extension's resolver.
   /// @param extensionId keccak256 hash of the extension name, used as an indentifier

docs/_Interface_IColony.md

@@ -261,6 +261,19 @@ Set the deprecation of an extension in a colony. Secured function to authorised
 |deprecated|bool|Whether to deprecate the extension or not
 
 
+### `deprecateExtension`
+
+Set the deprecation of an extension in a colony. Secured function to authorised members.
+
+
+**Parameters**
+
+|Name|Type|Description|
+|---|---|---|
+|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
+|deprecated|bool|Whether to deprecate the extension or not
+
+
 ### `editColony`
 
 Called to change the metadata associated with a colony. Expected to be a IPFS hash of a JSON blob, but not enforced to any degree by the contracts
@@ -1761,6 +1774,19 @@ Uninstall an extension from a colony. Secured function to authorised members.
 |extension|address|The address of the extension installation
 
 
+### `uninstallExtension`
+
+Uninstall an extension from a colony. Secured function to authorised members.
+
+*Note: This is a permanent action -- re-installing the extension will deploy a new contract*
+
+**Parameters**
+
+|Name|Type|Description|
+|---|---|---|
+|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
+
+
 ### `unlockToken`
 
 unlock the native colony token, if possible
@@ -1832,6 +1858,19 @@ Upgrade an extension in a colony. Secured function to authorised members.
 |newVersion|uint256|The version to upgrade to (must be one larger than the current version)
 
 
+### `upgradeExtension`
+
+Upgrade an extension in a colony. Secured function to authorised members.
+
+
+**Parameters**
+
+|Name|Type|Description|
+|---|---|---|
+|extensionId|bytes32|keccak256 hash of the extension name, used as an indentifier
+|newVersion|uint256|The version to upgrade to (must be one larger than the current version)
+
+
 ### `userCanSetRoles`
 
 Check whether a given user can modify roles in the target domain `_childDomainId`. Mostly a convenience function to provide a uniform interface for extension contracts validating permissions