Merge pull request #344 from vzotova/tg-roles

Update permissions related to TG in subscription and coordinator

Author: cygnusv

contracts/contracts/coordination/Coordinator.sol

@@ -93,6 +93,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable
     }
 
     bytes32 public constant TREASURY_ROLE = keccak256("TREASURY_ROLE");
+    bytes32 public constant FEE_MODEL_MANAGER_ROLE = keccak256("FEE_MODEL_MANAGER_ROLE");
 
     ITACoChildApplication public immutable application;
     uint96 private immutable minAuthorization; // TODO use child app for checking eligibility
@@ -651,7 +652,7 @@ contract Coordinator is Initializable, AccessControlDefaultAdminRulesUpgradeable
         }
     }
 
-    function approveFeeModel(IFeeModel feeModel) external onlyRole(TREASURY_ROLE) {
+    function approveFeeModel(IFeeModel feeModel) external onlyRole(FEE_MODEL_MANAGER_ROLE) {
         require(!feeModelsRegistry[feeModel], "Fee model already approved");
         feeModelsRegistry[feeModel] = true;
         emit FeeModelApproved(feeModel);

contracts/contracts/coordination/subscription/StandardSubscription.sol

@@ -248,12 +248,12 @@ contract StandardSubscription is EncryptorSlotsSubscription, Initializable, Owna
 
     /**
      * @notice Withdraws the contract balance to the treasury
-     * @param amount The amount to withdraw
      */
-    function withdrawToTreasury(uint256 amount) external onlyOwner {
-        require(amount <= feeToken.balanceOf(address(this)), "Insufficient balance available");
-        feeToken.safeTransfer(msg.sender, amount);
-        emit WithdrawalToTreasury(msg.sender, amount);
+    function withdrawToTreasury() external {
+        uint256 amount = feeToken.balanceOf(address(this));
+        require(amount > 0, "Insufficient balance available");
+        feeToken.safeTransfer(owner(), amount);
+        emit WithdrawalToTreasury(owner(), amount);
     }
 
     function processRitualPayment(

tests/test_coordinator.py

@@ -81,8 +81,9 @@ def fee_model(project, deployer, coordinator, erc20, treasury):
     contract = project.FlatRateFeeModel.deploy(
         coordinator.address, erc20.address, FEE_RATE, sender=deployer
     )
-    coordinator.grantRole(coordinator.TREASURY_ROLE(), treasury, sender=deployer)
+    coordinator.grantRole(coordinator.FEE_MODEL_MANAGER_ROLE(), treasury, sender=deployer)
     coordinator.approveFeeModel(contract.address, sender=treasury)
+    coordinator.grantRole(coordinator.TREASURY_ROLE(), treasury, sender=deployer)
     return contract
 
 

tests/test_global_allow_list.py

@@ -1,5 +1,4 @@
 import os
-from enum import IntEnum
 
 import ape
 import pytest
@@ -80,7 +79,7 @@ def fee_model(project, deployer, coordinator, erc20, treasury):
     contract = project.FlatRateFeeModel.deploy(
         coordinator.address, erc20.address, FEE_RATE, sender=deployer
     )
-    coordinator.grantRole(coordinator.TREASURY_ROLE(), treasury, sender=deployer)
+    coordinator.grantRole(coordinator.FEE_MODEL_MANAGER_ROLE(), treasury, sender=deployer)
     coordinator.approveFeeModel(contract.address, sender=treasury)
     return contract
 
@@ -120,12 +119,12 @@ def initiate_ritual(coordinator, fee_model, erc20, authority, nodes, allow_logic
     return authority, tx
 
 
-# Using normal and upgradeable versions of global_allow_list. 
+# Using normal and upgradeable versions of global_allow_list.
 # Since both are fixtures, we need a small workaround to use them as parameters.
-# See https://engineeringfordatascience.com/posts/pytest_fixtures_with_parameterize/#using-requestgetfixturevalue-
+# See https://engineeringfordatascience.com/posts/pytest_fixtures_with_parameterize/#using-requestgetfixturevalue-
 @pytest.mark.parametrize(
-    'allow_list_contract',
-    ('global_allow_list', 'upgradeable_global_allow_list'),
+    "allow_list_contract",
+    ("global_allow_list", "upgradeable_global_allow_list"),
 )
 def test_authorize_using_global_allow_list(
     coordinator, nodes, deployer, initiator, erc20, fee_model, allow_list_contract, request

tests/test_standard_subscription.py

@@ -356,19 +356,13 @@ def test_withdraw(erc20, subscription, adopter, adopter_setter, treasury, global
     erc20.approve(subscription.address, ERC20_SUPPLY, sender=adopter)
     subscription.setAdopter(adopter, sender=adopter_setter)
 
-    with ape.reverts():
-        subscription.withdrawToTreasury(1, sender=adopter)
-
     with ape.reverts("Insufficient balance available"):
-        subscription.withdrawToTreasury(1, sender=treasury)
+        subscription.withdrawToTreasury(sender=adopter)
 
     subscription.payForSubscription(0, sender=adopter)
 
     current_base_fee = base_fee(0)
-    with ape.reverts("Insufficient balance available"):
-        subscription.withdrawToTreasury(current_base_fee + 1, sender=treasury)
-
-    tx = subscription.withdrawToTreasury(current_base_fee, sender=treasury)
+    tx = subscription.withdrawToTreasury(sender=adopter)
     assert erc20.balanceOf(treasury) == current_base_fee
     assert erc20.balanceOf(subscription.address) == 0