diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index c7dae6adb..0baa8bd1f 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.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: diff --git a/.github/workflows/core-contracts-storage-check.yml b/.github/workflows/core-contracts-storage-check.yml index 01db6bfff..7e0854c8e 100644 --- a/.github/workflows/core-contracts-storage-check.yml +++ b/.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: diff --git a/packages/contracts/src/dollar/access/AccessControlInternal.sol b/packages/contracts/src/dollar/access/AccessControlInternal.sol index 3698eddf0..2f8dd1e81 100644 --- a/packages/contracts/src/dollar/access/AccessControlInternal.sol +++ b/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 diff --git a/packages/contracts/src/dollar/core/UbiquityDollarToken.sol b/packages/contracts/src/dollar/core/UbiquityDollarToken.sol index 0c157185d..1354c4b6c 100644 --- a/packages/contracts/src/dollar/core/UbiquityDollarToken.sol +++ b/packages/contracts/src/dollar/core/UbiquityDollarToken.sol @@ -3,7 +3,6 @@ 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"; @@ -11,18 +10,6 @@ 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 diff --git a/packages/contracts/src/dollar/facets/AccessControlFacet.sol b/packages/contracts/src/dollar/facets/AccessControlFacet.sol index cd813cfdb..4fe247194 100644 --- a/packages/contracts/src/dollar/facets/AccessControlFacet.sol +++ b/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, diff --git a/packages/contracts/src/dollar/interfaces/IAccessControl.sol b/packages/contracts/src/dollar/interfaces/IAccessControl.sol index a83822635..1ee285f28 100644 --- a/packages/contracts/src/dollar/interfaces/IAccessControl.sol +++ b/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 diff --git a/packages/contracts/src/dollar/interfaces/IIncentive.sol b/packages/contracts/src/dollar/interfaces/IIncentive.sol deleted file mode 100644 index 8f0469a20..000000000 --- a/packages/contracts/src/dollar/interfaces/IIncentive.sol +++ /dev/null @@ -1,29 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.19; - -/** - * @notice Incentive contract interface - * @notice Called by Ubiquity Dollar token contract when transferring with an incentivized address. - * Dollar admin can set an incentive contract for a partner in order to, for example, mint partner's - * project tokens on Dollars transfers. Incentive contracts can be set for the following transfer operations: - * - EOA => contract - * - contract => EOA - * - contract => contract - * - any transfer incentive contract - * @dev Should be appointed as a Minter or Burner as needed - */ -interface IIncentive { - /** - * @notice Applies incentives on transfer - * @param sender the sender address of Ubiquity Dollar - * @param receiver the receiver address of Ubiquity Dollar - * @param operator the operator (msg.sender) of the transfer - * @param amount the amount of Ubiquity Dollar transferred - */ - function incentivize( - address sender, - address receiver, - address operator, - uint256 amount - ) external; -} diff --git a/packages/contracts/test/diamond/facets/AccessControlFacet.t.sol b/packages/contracts/test/diamond/facets/AccessControlFacet.t.sol index eb2439be1..b7c801b18 100644 --- a/packages/contracts/test/diamond/facets/AccessControlFacet.t.sol +++ b/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); + } } diff --git a/packages/contracts/test/diamond/facets/ManagerFacet.t.sol b/packages/contracts/test/diamond/facets/ManagerFacet.t.sol index 0b517eea0..ebfffbdba 100644 --- a/packages/contracts/test/diamond/facets/ManagerFacet.t.sol +++ b/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) diff --git a/packages/contracts/test/dollar/core/UbiquityDollarToken.t.sol b/packages/contracts/test/dollar/core/UbiquityDollarToken.t.sol index 6772136f0..d3b1fe005 100644 --- a/packages/contracts/test/dollar/core/UbiquityDollarToken.t.sol +++ b/packages/contracts/test/dollar/core/UbiquityDollarToken.t.sol @@ -2,21 +2,10 @@ 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; @@ -24,13 +13,7 @@ contract UbiquityDollarTokenTest is LocalTestHelper { 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();