refactor: [#13] extract logic into libraries and include base contrac…

…ts to ensure order for upgrades

BREAKING CHANGE: storage locations could have changed for any of these contracts, which means old contracts cannot be safely upgraded.

contracts/consumables/Paypr.sol

@@ -14,8 +14,19 @@ import '../core/consumable/ConsumableExchange.sol';
 import '../core/consumable/ConvertibleConsumable.sol';
 import '../core/access/DelegatingRoles.sol';
 
-contract Paypr is ConvertibleConsumable, ConsumableExchange, DelegatingRoles {
+contract Paypr is
+  Initializable,
+  ContextUpgradeSafe,
+  ERC165UpgradeSafe,
+  BaseContract,
+  Consumable,
+  ConvertibleConsumable,
+  ConsumableExchange,
+  Disableable,
+  DelegatingRoles
+{
   using SafeMath for uint256;
+  using TransferLogic for address;
 
   function initializePaypr(
     IConsumableExchange baseToken,
@@ -77,24 +88,24 @@ contract Paypr is ConvertibleConsumable, ConsumableExchange, DelegatingRoles {
     address sender,
     address recipient,
     uint256 amount
-  ) internal override(ConsumableExchange, ConvertibleConsumable) onlyEnabled {
-    ConsumableExchange._transfer(sender, recipient, amount);
+  ) internal override(Consumable, ConsumableExchange, ConvertibleConsumable) onlyEnabled {
+    ConvertibleConsumable._transfer(sender, recipient, amount);
   }
 
   function transferToken(
     IERC20 token,
     uint256 amount,
     address recipient
   ) external override onlyTransferAgent onlyEnabled {
-    _transferToken(token, amount, recipient);
+    address(this).transferToken(token, amount, recipient);
   }
 
   function transferItem(
     IERC721 artifact,
     uint256 itemId,
     address recipient
   ) external override onlyTransferAgent onlyEnabled {
-    _transferItem(artifact, itemId, recipient);
+    address(this).transferItem(artifact, itemId, recipient);
   }
 
   function disable() external override onlyAdmin {

contracts/consumables/TestStableCoin.sol

@@ -24,7 +24,7 @@ pragma solidity ^0.6.0;
 import '@openzeppelin/contracts-ethereum-package/contracts/token/ERC20/ERC20.sol';
 import '../core/access/Roles.sol';
 
-contract TestStableCoin is ERC20UpgradeSafe, Roles {
+contract TestStableCoin is Initializable, ContextUpgradeSafe, ERC20UpgradeSafe, AccessControlUpgradeSafe, Roles {
   function initialize() external initializer {
     __ERC20_init('MyStableCoin', 'MSC');
 

contracts/core/BaseContract.sol

@@ -26,13 +26,7 @@ import '@openzeppelin/contracts-ethereum-package/contracts/introspection/ERC165.
 import './IBaseContract.sol';
 import './BaseContractInterfaceSupport.sol';
 
-contract BaseContract is IBaseContract, ERC165UpgradeSafe {
-  struct ContractInfo {
-    string name;
-    string description;
-    string uri;
-  }
-
+contract BaseContract is Initializable, IBaseContract, ERC165UpgradeSafe {
   ContractInfo private _info;
 
   function _initializeBaseContract(ContractInfo memory info) internal initializer {

contracts/core/Disableable.sol

@@ -21,33 +21,19 @@
 
 pragma solidity ^0.6.0;
 
-abstract contract Disableable {
+import './IDisableable.sol';
+
+abstract contract Disableable is IDisableable {
   bool private _disabled;
 
-  /**
-   * @dev Returns whether or not the contract is disabled
-   */
-  function disabled() external view returns (bool) {
+  function disabled() external override view returns (bool) {
     return _disabled;
   }
 
-  /**
-   * @dev Returns whether or not the contract is enabled
-   */
-  function enabled() external view returns (bool) {
+  function enabled() external override view returns (bool) {
     return !_disabled;
   }
 
-  modifier onlyEnabled() {
-    require(!_disabled, 'Contract is disabled');
-    _;
-  }
-
-  /**
-   * @dev Disables the contract
-   */
-  function disable() external virtual;
-
   /**
    * @dev Disables the contract
    */
@@ -60,11 +46,6 @@ abstract contract Disableable {
     emit Disabled();
   }
 
-  /**
-   * @dev Enables the contract
-   */
-  function enable() external virtual;
-
   /**
    * @dev Enables the contract
    */
@@ -77,15 +58,5 @@ abstract contract Disableable {
     emit Enabled();
   }
 
-  /**
-   * Emitted when the contract is disabled
-   */
-  event Disabled();
-
-  /**
-   * Emitted when the contract is enabled
-   */
-  event Enabled();
-
   uint256[50] private ______gap;
 }

contracts/core/IBaseContract.sol

@@ -24,6 +24,12 @@ pragma solidity ^0.6.0;
 import '@openzeppelin/contracts-ethereum-package/contracts/introspection/IERC165.sol';
 
 interface IBaseContract is IERC165 {
+  struct ContractInfo {
+    string name;
+    string description;
+    string uri;
+  }
+
   function contractName() external view returns (string memory);
 
   function contractDescription() external view returns (string memory);

contracts/core/IDisableable.sol

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2020 The Paypr Company
+ *
+ * This file is part of Paypr Ethereum Contracts.
+ *
+ * Paypr Ethereum Contracts is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Paypr Ethereum Contracts is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Paypr Ethereum Contracts.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+// SPDX-License-Identifier: GPL-3.0-only
+
+pragma solidity ^0.6.0;
+
+interface IDisableable {
+  /**
+   * Emitted when the contract is disabled
+   */
+  event Disabled();
+
+  /**
+   * Emitted when the contract is enabled
+   */
+  event Enabled();
+
+  /**
+   * @dev Returns whether or not the contract is disabled
+   */
+  function disabled() external view returns (bool);
+
+  /**
+   * @dev Returns whether or not the contract is enabled
+   */
+  function enabled() external view returns (bool);
+
+  modifier onlyEnabled() virtual {
+    require(!this.disabled(), 'Contract is disabled');
+    _;
+  }
+
+  /**
+   * @dev Disables the contract
+   */
+  function disable() external;
+
+  /**
+   * @dev Enables the contract
+   */
+  function enable() external;
+}

contracts/core/access/ConfigurableRoles.sol

@@ -27,7 +27,7 @@ import '@openzeppelin/contracts-ethereum-package/contracts/introspection/ERC165.
 import './Roles.sol';
 import './RoleDelegateInterfaceSupport.sol';
 
-contract ConfigurableRoles is Roles, ERC165UpgradeSafe {
+contract ConfigurableRoles is Initializable, ContextUpgradeSafe, ERC165UpgradeSafe, Roles {
   function initializeRoles(IRoleDelegate roleDelegate) public initializer {
     __ERC165_init();
     _registerInterface(RoleDelegateInterfaceSupport.ROLE_DELEGATE_INTERFACE_ID);

contracts/core/access/DelegatingRoles.sol

@@ -27,7 +27,7 @@ import './IRoleDelegate.sol';
 import './RoleDelegateInterfaceSupport.sol';
 import './RoleSupport.sol';
 
-contract DelegatingRoles is ContextUpgradeSafe {
+contract DelegatingRoles is Initializable, ContextUpgradeSafe {
   using EnumerableSet for EnumerableSet.AddressSet;
   using RoleDelegateInterfaceSupport for IRoleDelegate;
 

contracts/core/access/Roles.sol

@@ -26,7 +26,7 @@ import './DelegatingRoles.sol';
 import './IRoleDelegate.sol';
 import './RoleSupport.sol';
 
-contract Roles is IRoleDelegate, DelegatingRoles, AccessControlUpgradeSafe {
+contract Roles is IRoleDelegate, Initializable, ContextUpgradeSafe, AccessControlUpgradeSafe, DelegatingRoles {
   function _initializeRoles(IRoleDelegate roleDelegate) public initializer {
     if (address(roleDelegate) != address(0)) {
       _addRoleDelegate(roleDelegate);

contracts/core/account/Account.sol

@@ -26,13 +26,16 @@ import '@openzeppelin/contracts-ethereum-package/contracts/access/Ownable.sol';
 import '@openzeppelin/contracts-ethereum-package/contracts/introspection/ERC165.sol';
 import '@openzeppelin/contracts-ethereum-package/contracts/token/ERC20/IERC20.sol';
 import '@openzeppelin/contracts-ethereum-package/contracts/token/ERC721/IERC721.sol';
-import '../transfer/BaseTransferring.sol';
 import '../transfer/TransferringInterfaceSupport.sol';
 import '../Disableable.sol';
 import '../access/Roles.sol';
 import '../access/RoleDelegateInterfaceSupport.sol';
+import '../transfer/ITransferring.sol';
+import '../transfer/TransferLogic.sol';
+
+contract Account is Initializable, ITransferring, ContextUpgradeSafe, ERC165UpgradeSafe, Disableable, Roles {
+  using TransferLogic for address;
 
-contract Account is BaseTransferring, Roles, ERC165UpgradeSafe, Disableable {
   function initializeAccount(IRoleDelegate roleDelegate) public initializer {
     __ERC165_init();
     _registerInterface(RoleDelegateInterfaceSupport.ROLE_DELEGATE_INTERFACE_ID);
@@ -52,15 +55,24 @@ contract Account is BaseTransferring, Roles, ERC165UpgradeSafe, Disableable {
     uint256 amount,
     address recipient
   ) external override onlyTransferAgent onlyEnabled {
-    _transferTokenWithExchange(token, amount, recipient);
+    address(this).transferTokenWithExchange(token, amount, recipient);
   }
 
   function transferItem(
     IERC721 artifact,
     uint256 itemId,
     address recipient
   ) external override onlyTransferAgent onlyEnabled {
-    _transferItem(artifact, itemId, recipient);
+    address(this).transferItem(artifact, itemId, recipient);
+  }
+
+  function onERC721Received(
+    address operator,
+    address from,
+    uint256 tokenId,
+    bytes calldata data
+  ) external virtual override returns (bytes4) {
+    return TransferLogic.onERC721Received(operator, from, tokenId, data);
   }
 
   function disable() external override onlyAdmin {

contracts/core/activity/Activity.sol

@@ -31,22 +31,24 @@ import '../consumable/ConsumableConsumerInterfaceSupport.sol';
 import '../consumable/ConsumableProvider.sol';
 import '../consumable/ConsumableProviderInterfaceSupport.sol';
 import '../BaseContract.sol';
-import '../item/ItemUser.sol';
-import '../Disableable.sol';
-import '../transfer/BaseTransferring.sol';
+import '../IDisableable.sol';
 import '../transfer/TransferringInterfaceSupport.sol';
+import '../transfer/ITransferring.sol';
+import '../transfer/TransferLogic.sol';
 
 abstract contract Activity is
+  IDisableable,
+  Initializable,
+  ITransferring,
   IActivity,
   ContextUpgradeSafe,
+  ERC165UpgradeSafe,
   BaseContract,
-  BaseTransferring,
-  Disableable,
   ConsumableConsumer,
-  ConsumableProvider,
-  ItemUser
+  ConsumableProvider
 {
   using Counters for Counters.Counter;
+  using TransferLogic for address;
 
   mapping(address => Counters.Counter) private _executed;
   Counters.Counter private _totalExecuted;
@@ -101,5 +103,14 @@ abstract contract Activity is
     // does nothing by default
   }
 
+  function onERC721Received(
+    address operator,
+    address from,
+    uint256 tokenId,
+    bytes calldata data
+  ) external virtual override returns (bytes4) {
+    return TransferLogic.onERC721Received(operator, from, tokenId, data);
+  }
+
   uint256[50] private ______gap;
 }

contracts/core/activity/ConfigurableActivity.sol

@@ -24,8 +24,11 @@ pragma experimental ABIEncoderV2;
 
 import './Activity.sol';
 import '../access/DelegatingRoles.sol';
+import '../Disableable.sol';
+
+contract ConfigurableActivity is Initializable, ContextUpgradeSafe, Activity, Disableable, DelegatingRoles {
+  using TransferLogic for address;
 
-contract ConfigurableActivity is Activity, DelegatingRoles {
   function initializeActivity(
     ContractInfo memory info,
     IConsumable.ConsumableAmount[] memory amountsToConsume,
@@ -42,15 +45,15 @@ contract ConfigurableActivity is Activity, DelegatingRoles {
     uint256 amount,
     address recipient
   ) external override onlyTransferAgent onlyEnabled {
-    _transferToken(token, amount, recipient);
+    address(this).transferToken(token, amount, recipient);
   }
 
   function transferItem(
     IERC721 artifact,
     uint256 itemId,
     address recipient
   ) external override onlyTransferAgent onlyEnabled {
-    _transferItem(artifact, itemId, recipient);
+    address(this).transferItem(artifact, itemId, recipient);
   }
 
   function disable() external override onlyAdmin {