Migrate contracts to Diamond pattern - Part1 (#222)

Author: zguesmi

CHANGELOG.md

@@ -2,19 +2,21 @@
 
 ## vNEXT
 
+- Migrate contracts to Diamond pattern - Part1 (#222)
 - Add Github Action CI in order to publish NPM package
 
 ### Updated contracts
 
 - [x] `IexecPoco2Delegate.sol`
 
 ### Features
+<!-- TODO update this with v5.5.1 release notes -->
 - Add gitub action workflow for deployment (#218)
 - Fix new testnets chain name (#217)
 - Deploy on new testnet chains using CreateX factory (#216)
 - Add CreateX factory for new chain deployment (#215)
 - Support Arbitrum & Avalanche Fuji testnets (#215)
-- Housekeeping (#207, )
+- Housekeeping (#208)
 - Add Halborn "Poco v5.5 & Voucher v1.0" audit report (#205)
 - Refactor Factory deployer (#206)
 - Enable native tests on CI (#204)

contracts/Store.sol

@@ -5,7 +5,6 @@ pragma solidity ^0.6.0;
 
 import "@iexec/interface/contracts/IexecHub.sol";
 import "@iexec/solidity/contracts/Libs/SafeMathExtended.sol";
-import "@iexec/solidity/contracts/ERC1538/ERC1538Store.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 import "./libs/IexecLibCore_v5.sol";
@@ -21,10 +20,12 @@ import "./registries/IRegistry.sol";
  * If you want add new variables for expanded features, add them at the     *
  * end, or (better?) create a Store_v2 that inherits from this Store.       *
  *                                                                          *
- * If in doubt, read about ERC1538 memory store.                            *
+ * If in doubt, read about Diamond proxy storage.                           *
  ****************************************************************************/
 
-abstract contract Store is ERC1538Store
+// TODO replace with diamond AppStorage using namespaced storage.
+// TODO check storage padding.
+abstract contract Store
 {
 	// Registries
 	IRegistry internal m_appregistry;

contracts/Store.v8.sol

@@ -14,43 +14,24 @@ import {IexecLibCore_v5} from "./libs/IexecLibCore_v5.sol";
  * If you want add new variables for expanded features, add them at the     *
  * end, or (better?) create a Store_v2 that inherits from this Store.       *
  *                                                                          *
- * If in doubt, read about ERC1538 memory store.                            *
+ * If in doubt, read about Diamond proxy storage.                           *
  ****************************************************************************/
 
-/// @dev Solstruct translation for v0.8.0
-library LibSet_bytes4 {
-    struct set {
-        bytes4[] values;
-        mapping(bytes4 => uint256) indexes;
-    }
-}
-
-library LibMap2_bytes4_address_bytes {
-    struct map {
-        LibSet_bytes4.set keyset;
-        mapping(bytes4 => address) values1;
-        mapping(bytes4 => bytes) values2;
-    }
-}
-
-/// @dev @iexec/solidity ERC1538
-abstract contract ERC1538Store is Ownable {
-    LibMap2_bytes4_address_bytes.map internal m_funcs;
-}
-
 /// @dev registries
 interface IRegistry is IERC721Enumerable {
     function isRegistered(address _entry) external view returns (bool);
 }
 
 /// @dev Poco store
 /**
- * @title Central storage of all modules contracts. It follows the transparent
- * contract standard aka ERC-1538.
+ * @title Central storage of all modules contracts. It follows the Diamond
+ * pattern aka ERC-2535.
  * @dev note the new added state variable "m_dealsBoost" that holds a new type
  * of deals for the PoCo Boost workflow.
  */
-abstract contract Store is ERC1538Store {
+// TODO replace with diamond AppStorage using namespaced storage.
+// TODO check storage padding.
+abstract contract Store {
     // Registries
     //slither-disable-next-line constable-states
     IRegistry internal m_appregistry;

contracts/modules/DelegateBase.sol

@@ -3,7 +3,27 @@
 
 pragma solidity ^0.6.0;
 
-import "@iexec/solidity/contracts/ERC1538/ERC1538Module.sol";
 import "../Store.sol";
+import "./interfaces/IOwnable.sol";
 
-abstract contract DelegateBase is Store, ERC1538Module {}
+// Functions that were declared in ERC1538Store are re-declared here.
+// TODO clean this (use LibDiamond)
+//      - All calls to `owner()` should use `LibDiamond.contractOwner()`.
+// TODO rename this contract to `FacetBase`.
+
+abstract contract DelegateBase is Store {
+
+    modifier onlyOwner() {
+        require(_msgSender() == owner(), "Ownable: caller is not the owner");
+        _;
+    }
+
+    function owner() public view returns (address) {
+        return IOwnable(address(this)).owner();
+    }
+
+    function _msgSender() internal view returns (address ) {
+        return msg.sender;
+    }
+
+}

contracts/modules/DelegateBase.v8.sol

@@ -3,19 +3,30 @@
 
 pragma solidity ^0.8.0;
 
-import {Ownable} from "@openzeppelin/contracts-v5/access/Ownable.sol";
-
+import {IERC5313} from "@openzeppelin/contracts-v5/interfaces/IERC5313.sol";
 import {Store} from "../Store.v8.sol";
 
+// Functions that were declared in ERC1538Store are re-declared here.
+// TODO clean this (use LibDiamond)
+//      - All calls to `owner()` should use `LibDiamond.contractOwner()`.
+// TODO rename this contract to `FacetBase`.
+
 /**
  * @title Base contract of all Delegate contracts.
  * @dev Every module must inherit from this contract.
  */
 abstract contract DelegateBase is Store {
-    /**
-     * @dev Constructor used by all PoCo modules.
-     */
-    constructor() Ownable(msg.sender) {
-        renounceOwnership();
+    modifier onlyOwner() {
+        require(_msgSender() == owner(), "Ownable: caller is not the owner");
+        _;
+    }
+
+    function owner() public view returns (address) {
+        // Make an external call to delegatecall the OwnershipFacet.
+        return IERC5313(address(this)).owner();
+    }
+
+    function _msgSender() internal view returns (address ) {
+        return msg.sender;
     }
 }

contracts/modules/delegates/ENSIntegrationDelegate.sol

@@ -23,7 +23,8 @@ import "@iexec/solidity/contracts/ENStools/ENSReverseRegistration.sol";
 import "../DelegateBase.sol";
 import "../interfaces/ENSIntegration.sol";
 
-
+// TODO don't deploy this as ENSReverseRegistration.ADDR_REVERSE_NODE is not defined on Arbitrum.
+// TODO update this to use ENS on Arbitrum (update everywhere this is used).
 contract ENSIntegrationDelegate is ENSIntegration, ENSReverseRegistration, DelegateBase
 {
 	function setName(address _ens, string calldata _name)

contracts/modules/delegates/IexecMaintenanceDelegate.sol

@@ -9,12 +9,12 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "../DelegateBase.sol";
 import "../interfaces/IexecMaintenance.sol";
 
-
 contract IexecMaintenanceDelegate is IexecMaintenance, DelegateBase
 {
 	using SafeMathExtended  for uint256;
 	using IexecLibOrders_v5 for IexecLibOrders_v5.EIP712Domain;
 
+	// TODO move this to DiamondInit.init().
 	function configure(
 		address          _token,
 		string  calldata _name,

contracts/modules/delegates/IexecPoco2Delegate.sol

@@ -100,7 +100,6 @@ contract IexecPoco2Delegate is IexecPoco2, DelegateBase, IexecEscrow, SignatureV
         return taskid;
     }
 
-    // TODO / COMPILER ERROR: making it external causes "stack too deep" error
     function contribute(
         bytes32 _taskid,
         bytes32 _resultHash,
@@ -175,7 +174,6 @@ contract IexecPoco2Delegate is IexecPoco2, DelegateBase, IexecEscrow, SignatureV
         checkConsensus(_taskid, _resultHash);
     }
 
-    // TODO / COMPILER ERROR: making it external causes "stack too deep" error
     function contributeAndFinalize(
         bytes32 _taskid,
         bytes32 _resultDigest,

contracts/registries/Registry.sol

@@ -74,6 +74,7 @@ abstract contract Registry is IRegistry, ERC721, ENSReverseRegistration, Ownable
 	function setName(address _ens, string calldata _name)
 	external onlyOwner()
 	{
+		// TODO update this as ENSReverseRegistration.ADDR_REVERSE_NODE is not defined on Arbitrum.
 		_setName(IENS(_ens), _name);
 	}
 

contracts/registries/RegistryEntry.sol

@@ -46,6 +46,7 @@ abstract contract RegistryEntry is ENSReverseRegistration
 	function setName(address _ens, string calldata _name)
 	external onlyOwner()
 	{
+		// TODO update this as ENSReverseRegistration.ADDR_REVERSE_NODE is not defined on Arbitrum.
 		_setName(IENS(_ens), _name);
 	}
 }