BqETHSubscription: fix bugs and add tests

contracts/contracts/coordination/BqETHSubscription.sol

@@ -17,6 +17,8 @@ contract BqETHSubscription is IFeeModel {
     Coordinator public immutable coordinator;
     IERC20 public immutable feeToken;
 
+    uint32 public constant INACTIVE_RITUAL_ID = type(uint32).max;
+
     // TODO: DAO Treasury
     // TODO: Should it be updatable?
     address public immutable beneficiary;
@@ -28,10 +30,10 @@ contract BqETHSubscription is IFeeModel {
     uint32 public immutable yellowPeriodDuration;
     uint32 public immutable redPeriodDuration;
 
-    IEncryptionAuthorizer public immutable accessController;
+    IEncryptionAuthorizer public accessController;
 
     uint32 public endOfSubscription;
-    uint32 public acttiveRitualId;
+    uint32 public activeRitualId;
 
     /**
      * @notice Emitted when a subscription is spent
@@ -59,7 +61,6 @@ contract BqETHSubscription is IFeeModel {
      * @param _maxDuration Maximum duration of ritual
      * @param _yellowPeriodDuration Duration of yellow period
      * @param _redPeriodDuration Duration of red period
-     * @param _accessController Address of allowed access controller
      */
     constructor(
         Coordinator _coordinator,
@@ -70,17 +71,12 @@ contract BqETHSubscription is IFeeModel {
         uint256 _maxNodes,
         uint32 _maxDuration,
         uint32 _yellowPeriodDuration,
-        uint32 _redPeriodDuration,
-        IEncryptionAuthorizer _accessController
+        uint32 _redPeriodDuration
     ) {
         require(address(_coordinator) != address(0), "Coordinator cannot be the zero address");
         require(address(_feeToken) != address(0), "Fee token cannot be the zero address");
         require(_beneficiary != address(0), "Beneficiary cannot be the zero address");
         require(_adopter != address(0), "Adopter cannot be the zero address");
-        require(
-            address(_accessController) != address(0),
-            "Access controller cannot be the zero address"
-        );
         coordinator = _coordinator;
         feeToken = _feeToken;
         beneficiary = _beneficiary;
@@ -90,11 +86,10 @@ contract BqETHSubscription is IFeeModel {
         maxDuration = _maxDuration;
         yellowPeriodDuration = _yellowPeriodDuration;
         redPeriodDuration = _redPeriodDuration;
-        accessController = _accessController;
     }
 
     modifier onlyCoordinator() {
-        require(msg.sender == beneficiary, "Only the Coordinator can call this method");
+        require(msg.sender == address(coordinator), "Only the Coordinator can call this method");
         _;
     }
 
@@ -103,11 +98,31 @@ contract BqETHSubscription is IFeeModel {
         _;
     }
 
-    modifier onlyAdopter() {
-        require(msg.sender == beneficiary, "Only the adopter can call this method");
+    modifier onlyAccessController() {
+        require(
+            msg.sender == address(accessController),
+            "Only Access Controller can call this method"
+        );
         _;
     }
 
+    modifier onlyActiveRitual(uint32 ritualId) {
+        require(
+            activeRitualId != INACTIVE_RITUAL_ID && ritualId == activeRitualId,
+            "Ritual must be active"
+        );
+        _;
+    }
+
+    function initialize(IEncryptionAuthorizer _accessController) external {
+        require(
+            address(accessController) == address(0) && address(_accessController) != address(0),
+            "Access controller cannot be the zero address"
+        );
+        accessController = _accessController;
+        activeRitualId = INACTIVE_RITUAL_ID;
+    }
+
     function packageFees() public view returns (uint256) {
         return feeRate * maxDuration * maxNodes;
     }
@@ -152,28 +167,29 @@ contract BqETHSubscription is IFeeModel {
             "Ritual parameters exceed available in package"
         );
         require(
-            accessController == coordinator.getAccessController(ritualId),
-            "Access controller ofr ritual must be approved"
+            address(accessController) != address(0) &&
+                accessController == coordinator.getAccessController(ritualId),
+            "Access controller for ritual must be approved"
         );
 
-        if (acttiveRitualId != 0) {
-            Coordinator.RitualState state = coordinator.getRitualState(ritualId);
+        if (activeRitualId != INACTIVE_RITUAL_ID) {
+            Coordinator.RitualState state = coordinator.getRitualState(activeRitualId);
             require(
                 state == Coordinator.RitualState.DKG_INVALID ||
                     state == Coordinator.RitualState.DKG_TIMEOUT ||
                     state == Coordinator.RitualState.EXPIRED, // TODO check if it's ok
                 "Only failed rituals allowed to be reinitiate"
             );
         }
-        acttiveRitualId = ritualId;
+        activeRitualId = ritualId;
     }
 
     function processRitualExtending(
         address,
         uint32 ritualId,
         uint256,
         uint32
-    ) external view override onlyCoordinator {
+    ) external view override onlyCoordinator onlyActiveRitual(ritualId) {
         (, uint32 endTimestamp) = coordinator.getTimestamps(ritualId);
         require(
             endOfSubscription + yellowPeriodDuration + redPeriodDuration >= endTimestamp,
@@ -184,10 +200,11 @@ contract BqETHSubscription is IFeeModel {
     /**
      * @dev This function is called before the setAuthorizations function
      */
-    function beforeSetAuthorization(uint32, address[] calldata, bool) external view override {
-        require(
-            block.timestamp <= endOfSubscription + yellowPeriodDuration,
-            "Yellow period of subscription has expired"
-        );
+    function beforeSetAuthorization(
+        uint32 ritualId,
+        address[] calldata,
+        bool
+    ) external view override onlyAccessController onlyActiveRitual(ritualId) {
+        require(block.timestamp <= endOfSubscription, "Subscription has expired");
     }
 }

contracts/test/BetaProgramInitiatorTestSet.sol

@@ -2,15 +2,11 @@
 
 pragma solidity ^0.8.0;
 
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import "../contracts/coordination/IEncryptionAuthorizer.sol";
 import "../contracts/coordination/Coordinator.sol";
 import "../contracts/coordination/IFeeModel.sol";
 
 contract CoordinatorForBetaProgramInitiatorMock {
-    using SafeERC20 for IERC20;
-
     struct Ritual {
         address initiator;
         address[] providers;

contracts/test/BqETHSubscriptionTestSet.sol

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity ^0.8.0;
+
+import "../contracts/coordination/IEncryptionAuthorizer.sol";
+import "../contracts/coordination/Coordinator.sol";
+import "../contracts/coordination/IFeeModel.sol";
+
+contract CoordinatorForBqETHSubscriptionMock {
+    struct Ritual {
+        uint32 endTimestamp;
+        IEncryptionAuthorizer accessController;
+        Coordinator.RitualState state;
+    }
+
+    IFeeModel public feeModel;
+
+    mapping(uint32 => Ritual) public rituals;
+
+    function setFeeModel(IFeeModel _feeModel) external {
+        feeModel = _feeModel;
+    }
+
+    function processRitualExtending(
+        address initiator,
+        uint32 ritualId,
+        uint256 numberOfProviders,
+        uint32 duration
+    ) external {
+        feeModel.processRitualExtending(initiator, ritualId, numberOfProviders, duration);
+    }
+
+    function processRitualPayment(
+        address initiator,
+        uint32 ritualId,
+        uint256 numberOfProviders,
+        uint32 duration
+    ) external {
+        feeModel.processRitualPayment(initiator, ritualId, numberOfProviders, duration);
+    }
+
+    function setRitual(
+        uint32 _ritualId,
+        Coordinator.RitualState _state,
+        uint32 _endTimestamp,
+        IEncryptionAuthorizer _accessController
+    ) external {
+        Ritual storage ritual = rituals[_ritualId];
+        ritual.state = _state;
+        ritual.endTimestamp = _endTimestamp;
+        ritual.accessController = _accessController;
+    }
+
+    function getAccessController(uint32 _ritualId) external view returns (IEncryptionAuthorizer) {
+        return rituals[_ritualId].accessController;
+    }
+
+    function getRitualState(uint32 _ritualId) external view returns (Coordinator.RitualState) {
+        return rituals[_ritualId].state;
+    }
+
+    function getTimestamps(
+        uint32 _ritualId
+    ) external view returns (uint32 initTimestamp, uint32 endTimestamp) {
+        initTimestamp = 0;
+        endTimestamp = rituals[_ritualId].endTimestamp;
+    }
+
+    function numberOfRituals() external pure returns (uint256) {
+        return 1;
+    }
+
+    function getAuthority(uint32) external view returns (address) {
+        // solhint-disable-next-line avoid-tx-origin
+        return tx.origin;
+    }
+
+    function isRitualActive(uint32) external view returns (bool) {
+        return true;
+    }
+}