comments

contracts/access/MintingAccessControl.sol

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache 2.0
 pragma solidity 0.8.19;
 
+// solhint-disable no-unused-import
 import {AccessControlEnumerable, AccessControl, IAccessControl} from "@openzeppelin/contracts/access/AccessControlEnumerable.sol";
 
 abstract contract MintingAccessControl is AccessControlEnumerable {

contracts/token/erc20/README.md

@@ -5,12 +5,12 @@ directly or extend.
 
 | Contract                               | Description                                   |
 |--------------------------------------- |-----------------------------------------------|
-| preset/ImmutableERC20MinterBurnerPermit| Provides basic ERC 20 Permit, Capped token supply and burn capability. Designed to be extended.  | 
+| preset/ImmutableERC20MinterBurnerPermit| Provides basic ERC 20 Permit, Capped token supply and burn capability.  | 
 | preset/ImmutableERC20FixedSupplyNoBurn | ERC 20 contract with a fixed supply defined at deployment. | 
 
 ## ImmutableERC20MinterBurnerPermit
 
-This contract contains Permit methods, allowing the token owner to give a third party operator a `Permit` which is a signed message that can be used by the third party to give approval to themselves to operate on the tokens owned by the original owner. Users of permit should take care of the signed messages, as anyone who has access to this signed message can use it to gain access to the tokens. Read more on the EIP here: [EIP-2612](https://eips.ethereum.org/EIPS/eip-2612).
+This contract contains Permit methods, allowing the token owner to give a third party operator a Permit which is a signed message that can be used by the third party to give approval to themselves to operate on the tokens owned by the original owner. Users take care when signing messages. If they inadvertantly sign a malicious permit, then the attacker could use use it to gain access to the user's tokens. Read more on the EIP here: [EIP-2612](https://eips.ethereum.org/EIPS/eip-2612).
 # Status
 
 Contract threat models and audits:

contracts/token/erc20/preset/ImmutableERC20MinterBurnerPermit.sol

@@ -10,10 +10,10 @@ import {IImmutableERC20Errors} from "./Errors.sol";
 
 /**
  * @notice ERC 20 contract that wraps Open Zeppelin's ERC 20 contract.
- * This contract has the concept of an owner, called _owner in the constructor.
+ * This contract has the concept of a hubOwner, called _hubOwner in the constructor.
  * This account has no rights to execute any administrative actions within the contract,
- *  with the exception of transferOwnership and role grants/revokes. This account is accessed via the owner() 
- *  function. The Immutable Hub uses this function to help associate the ERC 20 contract 
+ *  with the exception of renouncing their ownership. 
+ *  The Immutable Hub uses this function to help associate the ERC 20 contract 
  *  with a specific Immutable Hub account.
  */
 contract ImmutableERC20MinterBurnerPermit is ERC20Capped, ERC20Burnable, ERC20Permit, MintingAccessControl {
@@ -22,17 +22,18 @@ contract ImmutableERC20MinterBurnerPermit is ERC20Capped, ERC20Burnable, ERC20Pe
 
     /**
      * @dev Delegate to Open Zeppelin's contract.
+     * @param _roleAdmin The account that has the DEFAULT_ADMIN_ROLE.
+     * @param _minterAdmin The account that has the MINTER_ROLE.
+     * @param _hubOwner The account that owns the contract and is associated with Immutable Hub. 
      * @param _name  Name of the token.
      * @param _symbol Token symbol.
-     * @param _hubOwner The account that owns the contract and is associated with Immutable Hub. 
-     * @param minterRole The account that has the MINTER_ROLE.
      * @param _maxTokenSupply The maximum supply of the token.
-     * @param _admin The account that has the DEFAULT_ADMIN_ROLE.
+
      */
-    constructor(string memory _name, string memory _symbol, address _hubOwner, address minterRole, uint256 _maxTokenSupply, address _admin) ERC20(_name, _symbol) ERC20Permit(_name) ERC20Capped(_maxTokenSupply) {
-        _grantRole(DEFAULT_ADMIN_ROLE, _admin);
+    constructor(address _roleAdmin, address _minterAdmin, address _hubOwner, string memory _name, string memory _symbol, uint256 _maxTokenSupply) ERC20(_name, _symbol) ERC20Permit(_name) ERC20Capped(_maxTokenSupply) {
+        _grantRole(DEFAULT_ADMIN_ROLE, _roleAdmin);
         _grantRole(HUB_OWNER_ROLE, _hubOwner);
-        _grantRole(MINTER_ROLE, minterRole);
+        _grantRole(MINTER_ROLE, _minterAdmin);
     }
 
 

test/token/erc20/preset/ImmutableERC20MinterBurnerPermit.t.sol

@@ -28,7 +28,7 @@ contract ImmutableERC20MinterBurnerPermitTest is Test {
         symbol = "HPY";
         maxSupply = 1000;
 
-        erc20 = new ImmutableERC20MinterBurnerPermit(name, symbol, hubOwner, minter, maxSupply, admin);
+        erc20 = new ImmutableERC20MinterBurnerPermit(admin, minter, hubOwner, name, symbol, maxSupply);
     }
 
     function testInit() public {
@@ -119,4 +119,51 @@ contract ImmutableERC20MinterBurnerPermitTest is Test {
         vm.stopPrank();
     }
 
+    function testBurnFrom() public {
+        uint256 amount = 100;
+        address operator = makeAddr("operator");
+        vm.prank(minter);
+        erc20.mint(tokenReceiver, amount);
+        assertEq(erc20.balanceOf(tokenReceiver), 100);
+        vm.prank(tokenReceiver);
+        erc20.increaseAllowance(operator, amount);
+        vm.prank(operator);
+        erc20.burnFrom(tokenReceiver, amount);
+        assertEq(erc20.balanceOf(tokenReceiver), 0);
+    }
+
+    function testPermit() public {
+        uint256 ownerPrivateKey = 1;
+        uint256 spenderPrivateKey = 2;
+        address owner = vm.addr(ownerPrivateKey);
+        address spender = vm.addr(spenderPrivateKey);
+
+        bytes32 PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9;
+
+        uint256 value = 1e18;
+
+        uint256 deadline = block.timestamp + 1 days;
+        uint256 nonce = erc20.nonces(owner);
+        bytes32 structHash = keccak256(
+            abi.encode(
+                PERMIT_TYPEHASH,
+                owner,
+                spender,
+                value,
+                nonce,
+                deadline
+            )
+        );
+        bytes32 hash = erc20.DOMAIN_SEPARATOR();
+        hash = keccak256(abi.encodePacked("\x19\x01", hash, structHash));
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerPrivateKey, hash);
+
+        vm.startPrank(owner);
+        erc20.permit(owner, spender, value, deadline, v, r, s);
+        vm.stopPrank();
+
+        assertEq(erc20.allowance(owner, spender), value);
+    }
+
+
 }

test/token/erc20/preset/README.md

@@ -29,3 +29,9 @@ All of the tests defined in the table below are in test/erc20/preset/ImmutableER
 | testOnlyMinterCanMunt           | Ensure Only minter role can mint reverts.         | No         | Yes         |
 | testMint                        | Ensure successful minting by minter               | No         | Yes         |
 | testCanOnlyMintUpToMaxSupply    | Ensure can only mint up to max supply             | No         | Yes         |
+| testRenounceLastHubOwnerBlocked | Ensure the last hub owner cannot be renounced     | No         | Yes         |
+| testRenounceLastAdminBlocked    | Ensure the last default admin cannot be renounced | No         | Yes         |
+| testRenounceAdmin               | Ensure admin role can be renounced                | No         | Yes         |
+| testRenounceHubOwner            | Ensure hub owner role can be renounced            | No         | Yes         |
+| testBurnFrom                    | Ensure allowance is required to burnFrom          | Yes        | Yes         |
+| testPermit                      | Ensure Permit works                               | Yes        | Yes         |