Fix set role admin

.github/workflows/codeql-analysis.yml

@@ -1,8 +1,10 @@
 name: CodeQL Analysis
 
-on:
-  pull_request:
-  push:
+on: workflow_dispatch
+  # push:
+  #  branches:
+  #    - development
+  # pull_request:  ## temporarily disable for pull requests until https://github.com/ubiquity/ubiquity-dollar/issues/876 is resolved.
 
 jobs:
   code-ql-analysis:

.github/workflows/core-contracts-storage-check.yml

@@ -4,7 +4,7 @@ on:
   push:
     branches:
       - development
-  pull_request:
+  # pull_request:  ## temporarily disable for pull requests until https://github.com/ubiquity/ubiquity-dollar/issues/876 is resolved.
 
 jobs:
   provide_contracts:

packages/contracts/src/dollar/access/AccessControlInternal.sol

@@ -83,6 +83,18 @@ abstract contract AccessControlInternal {
         return LibAccessControl.accessControlStorage().roles[role].adminRole;
     }
 
+    /**
+     * @notice Set admin role for a given role
+     * @param role Role to set
+     * @param adminRole role for the provided role
+     */
+    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
+        LibAccessControl
+            .accessControlStorage()
+            .roles[role]
+            .adminRole = adminRole;
+    }
+
     /**
      * @notice Assigns role to a given account
      * @param role Role to assign

packages/contracts/src/dollar/core/UbiquityDollarToken.sol

@@ -3,26 +3,13 @@ pragma solidity ^0.8.19;
 
 import {ERC20Ubiquity} from "./ERC20Ubiquity.sol";
 import {IERC20Ubiquity} from "../../dollar/interfaces/IERC20Ubiquity.sol";
-import {IIncentive} from "../../dollar/interfaces/IIncentive.sol";
 
 import "../libraries/Constants.sol";
 
 /**
  * @notice Ubiquity Dollar token contract
  */
 contract UbiquityDollarToken is ERC20Ubiquity {
-    /**
-     * @notice Mapping of account and incentive contract address
-     * @dev Address is 0 if there is no incentive contract for the account
-     */
-    mapping(address => address) public incentiveContract;
-
-    /// @notice Emitted on setting an incentive contract for an account
-    event IncentiveContractUpdate(
-        address indexed _incentivized,
-        address indexed _incentiveContract
-    );
-
     /// @notice Ensures initialize cannot be called on the implementation contract
     constructor() {
         _disableInitializers();
@@ -55,109 +42,6 @@ contract UbiquityDollarToken is ERC20Ubiquity {
         _;
     }
 
-    /**
-     * @notice Sets `incentive` contract for `account`
-     * @notice Incentive contracts are applied on Dollar transfers:
-     * - EOA => contract
-     * - contract => EOA
-     * - contract => contract
-     * - any transfer global incentive
-     * @param account Account to incentivize
-     * @param incentive Incentive contract address
-     */
-    function setIncentiveContract(address account, address incentive) external {
-        require(
-            accessControl.hasRole(GOVERNANCE_TOKEN_MANAGER_ROLE, _msgSender()),
-            "Dollar: must have admin role"
-        );
-
-        incentiveContract[account] = incentive;
-        emit IncentiveContractUpdate(account, incentive);
-    }
-
-    /**
-     * @notice Applies incentives on Dollar transfers
-     * @param sender Sender address
-     * @param recipient Recipient address
-     * @param amount Dollar token transfer amount
-     */
-    function _checkAndApplyIncentives(
-        address sender,
-        address recipient,
-        uint256 amount
-    ) internal {
-        // incentive on sender
-        address senderIncentive = incentiveContract[sender];
-        if (senderIncentive != address(0)) {
-            IIncentive(senderIncentive).incentivize(
-                sender,
-                recipient,
-                _msgSender(),
-                amount
-            );
-        }
-
-        // incentive on recipient
-        address recipientIncentive = incentiveContract[recipient];
-        if (recipientIncentive != address(0)) {
-            IIncentive(recipientIncentive).incentivize(
-                sender,
-                recipient,
-                _msgSender(),
-                amount
-            );
-        }
-
-        // incentive on operator
-        address operatorIncentive = incentiveContract[_msgSender()];
-        if (
-            _msgSender() != sender &&
-            _msgSender() != recipient &&
-            operatorIncentive != address(0)
-        ) {
-            IIncentive(operatorIncentive).incentivize(
-                sender,
-                recipient,
-                _msgSender(),
-                amount
-            );
-        }
-
-        // all incentive, if active applies to every transfer
-        address allIncentive = incentiveContract[address(0)];
-        if (allIncentive != address(0)) {
-            IIncentive(allIncentive).incentivize(
-                sender,
-                recipient,
-                _msgSender(),
-                amount
-            );
-        }
-    }
-
-    /**
-     * @notice Moves `amount` of tokens from `from` to `to` and applies incentives.
-     *
-     * This internal function is equivalent to `transfer`, and can be used to
-     * e.g. implement automatic token fees, slashing mechanisms, etc.
-     *
-     * Emits a `Transfer` event.
-     *
-     * Requirements:
-     *
-     * - `from` cannot be the zero address.
-     * - `to` cannot be the zero address.
-     * - `from` must have a balance of at least `amount`.
-     */
-    function _transfer(
-        address sender,
-        address recipient,
-        uint256 amount
-    ) internal override {
-        super._transfer(sender, recipient, amount);
-        _checkAndApplyIncentives(sender, recipient, amount);
-    }
-
     /**
      * @notice Burns Dollars from the `account` address
      * @param account Address to burn tokens from

packages/contracts/src/dollar/facets/AccessControlFacet.sol

@@ -24,6 +24,11 @@ contract AccessControlFacet is
         return _grantRole(role, account);
     }
 
+    /// @inheritdoc IAccessControl
+    function setRoleAdmin(bytes32 role, bytes32 adminRole) external onlyAdmin {
+        _setRoleAdmin(role, adminRole);
+    }
+
     /// @inheritdoc IAccessControl
     function hasRole(
         bytes32 role,

packages/contracts/src/dollar/interfaces/IAccessControl.sol

@@ -22,6 +22,13 @@ interface IAccessControl {
      */
     function getRoleAdmin(bytes32 role) external view returns (bytes32);
 
+    /**
+     * @notice Sets admin role for a given role
+     * @param role Role to set
+     * @param adminRole Admin role to set for a provided role
+     */
+    function setRoleAdmin(bytes32 role, bytes32 adminRole) external;
+
     /**
      * @notice Assigns role to a given account
      * @param role Role to assign

packages/contracts/test/diamond/facets/AccessControlFacet.t.sol

@@ -135,4 +135,25 @@ contract AccessControlFacetTest is DiamondTestSetup {
         );
         assertEq(adminRole, DEFAULT_ADMIN_ROLE);
     }
+
+    function testSetRoleAdmin_ShouldRevertWhenNotAdmin() public {
+        vm.prank(mock_sender);
+
+        vm.expectRevert("Manager: Caller is not admin");
+        accessControlFacet.setRoleAdmin(
+            DOLLAR_TOKEN_BURNER_ROLE,
+            DEFAULT_ADMIN_ROLE
+        );
+    }
+
+    function testSetRoleAdmin_ShouldSetAdminRoleForGivenRole() public {
+        bytes32 adminRole = accessControlFacet.getRoleAdmin(
+            DOLLAR_TOKEN_MINTER_ROLE
+        );
+        assertEq(adminRole, DEFAULT_ADMIN_ROLE);
+        vm.prank(admin);
+        accessControlFacet.setRoleAdmin(DOLLAR_TOKEN_MINTER_ROLE, PAUSER_ROLE);
+        adminRole = accessControlFacet.getRoleAdmin(DOLLAR_TOKEN_MINTER_ROLE);
+        assertEq(adminRole, PAUSER_ROLE);
+    }
 }

packages/contracts/test/diamond/facets/ManagerFacet.t.sol

@@ -99,21 +99,6 @@ contract ManagerFacetTest is DiamondTestSetup {
         assertEq(managerFacet.treasuryAddress(), contract1);
     }
 
-    function testSetIncentiveToDollar_ShouldSucceed() public prankAs(admin) {
-        assertEq(
-            accessControlFacet.hasRole(GOVERNANCE_TOKEN_MANAGER_ROLE, admin),
-            true
-        );
-        assertEq(
-            accessControlFacet.hasRole(
-                GOVERNANCE_TOKEN_MANAGER_ROLE,
-                address(diamond)
-            ),
-            true
-        );
-        managerFacet.setIncentiveToDollar(user1, contract1);
-    }
-
     function testSetMinterRoleWhenInitializing_ShouldSucceed()
         public
         prankAs(admin)

packages/contracts/test/dollar/core/UbiquityDollarToken.t.sol

@@ -2,35 +2,18 @@
 pragma solidity ^0.8.19;
 
 import {UbiquityDollarToken} from "../../../src/dollar/core/UbiquityDollarToken.sol";
-import {IIncentive} from "../../../src/dollar/interfaces/IIncentive.sol";
 
 import "../../helpers/LocalTestHelper.sol";
 
-contract Incentive is IIncentive {
-    function incentivize(
-        address sender,
-        address recipient,
-        address operator,
-        uint256 amount
-    ) public {}
-}
-
 contract UbiquityDollarTokenTest is LocalTestHelper {
-    address incentive_addr;
     address dollar_addr;
     address dollar_manager_address;
 
     address mock_sender = address(0x111);
     address mock_recipient = address(0x222);
     address mock_operator = address(0x333);
 
-    event IncentiveContractUpdate(
-        address indexed _incentivized,
-        address indexed _incentiveContract
-    );
-
     function setUp() public override {
-        incentive_addr = address(new Incentive());
         super.setUp();
         vm.startPrank(admin);
         dollar_addr = address(dollarToken);
@@ -55,59 +38,6 @@ contract UbiquityDollarTokenTest is LocalTestHelper {
         require(dollarToken.getManager() == newDiamond);
     }
 
-    function testSetIncentiveContract_ShouldRevert_IfNotAdmin() public {
-        vm.prank(mock_sender);
-        vm.expectRevert("Dollar: must have admin role");
-        dollarToken.setIncentiveContract(mock_sender, incentive_addr);
-
-        vm.prank(admin);
-        vm.expectEmit(true, true, true, true);
-        emit IncentiveContractUpdate(mock_sender, incentive_addr);
-        dollarToken.setIncentiveContract(mock_sender, incentive_addr);
-    }
-
-    function testTransfer_ShouldCallIncentivize_IfValidTransfer() public {
-        address userA = address(0x100001);
-        address userB = address(0x100001);
-        vm.startPrank(admin);
-        dollarToken.mint(userA, 100);
-        dollarToken.mint(userB, 100);
-        dollarToken.mint(mock_sender, 100);
-
-        dollarToken.setIncentiveContract(mock_sender, incentive_addr);
-        dollarToken.setIncentiveContract(mock_recipient, incentive_addr);
-        dollarToken.setIncentiveContract(mock_operator, incentive_addr);
-        dollarToken.setIncentiveContract(address(0), incentive_addr);
-        dollarToken.setIncentiveContract(dollar_addr, incentive_addr);
-        vm.stopPrank();
-
-        vm.prank(mock_sender);
-        vm.expectCall(
-            incentive_addr,
-            abi.encodeWithSelector(
-                Incentive.incentivize.selector,
-                mock_sender,
-                userB,
-                mock_sender,
-                1
-            )
-        );
-        dollarToken.transfer(userB, 1);
-
-        vm.prank(userA);
-        vm.expectCall(
-            incentive_addr,
-            abi.encodeWithSelector(
-                Incentive.incentivize.selector,
-                userA,
-                mock_recipient,
-                userA,
-                1
-            )
-        );
-        dollarToken.transfer(mock_recipient, 1);
-    }
-
     function testUUPS_ShouldUpgradeAndCall() external {
         UbiquityDollarTokenUpgraded newImpl = new UbiquityDollarTokenUpgraded();