[gms-1627] erc20 improvements (#204)

* [gms-1627] erc20 improvements

* comments

Author: jasonzwli

README.md

@@ -9,7 +9,7 @@ Immutable Contracts is a library of smart contracts targeted at developers who w
     - [ImmutableERC721](./contracts/token/erc721/preset/ImmutableERC721.sol)
     - [ImmutableERC721MintByID](./contracts/token/erc721/preset/ImmutableERC721MintByID.sol)
     - [ImmutableERC1155](./contracts/token/erc1155/preset/ImmutableERC1155.sol)
-    - [ImmutableERC20](./contracts/token/erc20/preset/ImmutableERC20.sol)
+    - [ImmutableERC20MinterBurnerPermit](./contracts/token/erc20/preset/ImmutableERC20MinterBurnerPermit.sol)
     - [ImmutableERC20FixedSupplyNoBurn](./contracts/token/erc20/preset/ImmutableERC20FixedSupplyNoBurn.sol)
 
 - Bridging contracts

audits/token/202404-threat-model-preset-immutable-erc20-minter-burner-permit/ImmutableERC20MinterBurnerPermit.png

@@ -2,7 +2,7 @@
 This document is a thread model for ImmutableERC20 token contracts built by Immutable.
 
 Contracts covered under this model include:
-[ImmutableERC20](../../contracts/token/erc20/preset/ImmutableERC20.sol)
+[ImmutableERC20MinterBurnerPermit](../../contracts/token/erc20/preset/ImmutableERC20MinterBurnerPermit.sol)
 
 
 ## Context
@@ -21,13 +21,12 @@ The ERC20 token built by Immutable is meant to be used by game studios to releas
 
 ### ImmutableERC20
 
-ImmutableERC20 is a simple wrapper around the [ERC20 implementation from openzeppelin](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol). It also extends [ERC20Permit](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC20Permit.sol) allowing token holders to offload the approval costs to a third party who wish to operate on behalf of the holder. 
+ImmutableERC20 is a simple wrapper around the [ERC20 implementation from openzeppelin](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol). It also extends [ERC20Permit](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC20Permit.sol), [ERC20Capped](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC20Capped.sol) and [ERC20Burnable](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC20Burnable.sol) allowing token holders to offload the approval costs to a third party who wish to operate on behalf of the holder. 
 
 The contract defines an immutable `_maxSupply` value to track the maximum number of tokens allowed to be in circulation. This helps the token to derive value using scarcity.
 
-The `renounceOwnership()` method from the `Ownable` extension has been overridden to prevent the contract from being ownerless. This is to help Immutable Hub to link tokens based of their owners. 
+The `renonceRole()` method from the `AccessControl` extension has been overridden to prevent the contract from being ownerless. This is to help Immutable Hub to link tokens based of their owners. 
 
-`Owner` and `DEFAULT_ADMIN_ROLE` will share the same key at depolyment, but the `Ownership` can be transferred.
 
 
 ## Attack Surfaces
@@ -48,7 +47,6 @@ Functions that *change* state:
 | ------------------------------------------------------------- | ----------------- | --------------------- |
 | mint(address,uint256)                                         | 0x40c10f19        | MINTER_ROLE           |
 | burn(address,uint256)                                         | 0x9dc29fac        | None - permissionless |
-| renounceOwnership()                                           | 0x715018a6        | OnlyOwner             |
 | transfer(address,uint256)                                     | 0xa9059cbb        | None - permissionless |
 | approve(address,uint256)                                      | 0x095ea7b3        | None - permissionless |
 | increaseAllowance(address,uint256)                            | 0x39509351        | None - permissionless |
@@ -60,7 +58,7 @@ Functions that *change* state:
 | renounceRole(bytes32,address)                                 | 0x36568abe        | None - permissionless |
 | revokeMinterRole(address)                                     | 0x69e2f0fb        | DEFAULT_ADMIN_ROLE    |
 | revokeRole(bytes32,address)                                   | 0xd547741f        | DEFAULT_ADMIN_ROLE    |
-| transferOwnership(address)                                    | 0xf2fde38b        | OnlyOwner             |
+| burnFrom(address,uint256)                                     | 0x79cc6790          | None - permissionless |
 
 Functions that *do not change* state:
 | Name                                                          | Function Selector | Access Control        |
@@ -84,9 +82,10 @@ Functions that *do not change* state:
 | supportsInterface(bytes4)                                     | 0x01ffc9a7        | None - permissionless |
 | symbol()                                                      | 0x95d89b41        | None - permissionless |
 | totalSupply()                                                 | 0x18160ddd        | None - permissionless |
+| cap()                                                         | 0x355274ea        | None - permissionless |
 
 ## Tests
 `forge test` will run all the related tests for the above mentioned repos. The test plan and cases are written in the test files describing the scenario is it testing for. The test plan can be seen [here](../../test/token/erc20/preset/README.md)
 
 ## Diagram 
-![](./202404-threat-model-preset-immutable-erc20/ImmutableERC20.png)
+![](./202404-threat-model-preset-immutable-erc20/ImmutableERC20MinterBurnerPermit.png)

audits/token/202404-threat-model-preset-immutable-erc20.md

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

audits/token/202404-threat-model-preset-immutable-erc20/ImmutableERC20.png

@@ -3,11 +3,14 @@
 This directory contains ERC 20 token contracts that game studios could choose to use
 directly or extend. 
 
-| Contract                        | Description                                   |
-|---------------------------------|-----------------------------------------------|
-| ImmutableERC20                  | Provides basic ERC 20 capability. Designed to be extended. | 
+| Contract                               | Description                                   |
+|--------------------------------------- |-----------------------------------------------|
+| 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 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/access/MintingAccessControl.sol

@@ -3,8 +3,4 @@ pragma solidity 0.8.19;
 
 interface IImmutableERC20Errors {
     error RenounceOwnershipNotAllowed();
-
-    error MaxSupplyExceeded(uint256 maxSupply);
-
-    error InvalidMaxSupply();
 }

contracts/token/erc20/README.md

@@ -4,7 +4,7 @@ pragma solidity 0.8.19;
 
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import {IImmutableERC20Errors} from "../../../errors/ERC20Errors.sol";
+import {IImmutableERC20Errors} from "./Errors.sol";
 
 
 /**

contracts/errors/ERC20Errors.sol renamed to contracts/token/erc20/preset/Errors.sol

@@ -0,0 +1,71 @@
+// Copyright (c) Immutable Pty Ltd 2018 - 2024
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.19;
+
+import {ERC20Permit, ERC20} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
+import {ERC20Burnable} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
+import {ERC20Capped} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Capped.sol";
+import {MintingAccessControl, AccessControl, IAccessControl} from "../../../access/MintingAccessControl.sol";
+import {IImmutableERC20Errors} from "./Errors.sol";
+
+/**
+ * @notice ERC 20 contract that wraps Open Zeppelin's ERC 20 contract.
+ * 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 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 {
+    /// @notice Role to mint tokens
+    bytes32 public constant HUB_OWNER_ROLE = bytes32("HUB_OWNER_ROLE");
+
+    /**
+     * @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 _maxTokenSupply The maximum supply of the token.
+
+     */
+    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, _minterAdmin);
+    }
+
+
+    /**
+     * @dev Mints `amount` number of token and transfers them to the `to` address.
+     * @param to the address to mint the tokens to.
+     * @param amount  The amount of tokens to mint.
+     */
+    function mint(address to, uint256 amount) external onlyRole(MINTER_ROLE) {
+        _mint(to, amount);
+    }
+
+
+    /**
+     * @dev Renounces the role `role` from the calling account. Prevents the last hub owner and admin from 
+     * renouncing their role.
+     * @param role The role to renounce.
+     * @param account The account to renounce the role from.
+     */
+    function renounceRole(bytes32 role, address account) public override(AccessControl, IAccessControl) {
+        if (getRoleMemberCount(role) == 1 && (role == HUB_OWNER_ROLE || role == DEFAULT_ADMIN_ROLE)) {
+            revert IImmutableERC20Errors.RenounceOwnershipNotAllowed();
+        }
+        super.renounceRole(role, account);
+    }
+
+
+    /**
+     * @dev Delegate to Open Zeppelin's ERC20Capped contract.
+     */
+    function _mint(address account, uint256 amount) internal override(ERC20, ERC20Capped) {
+        ERC20Capped._mint(account, amount);
+    }
+
+}

contracts/token/erc20/preset/ImmutableERC20.sol

@@ -4,7 +4,7 @@ pragma solidity 0.8.19;
 import "forge-std/Test.sol";
 
 import {ImmutableERC20FixedSupplyNoBurn} from "contracts/token/erc20/preset/ImmutableERC20FixedSupplyNoBurn.sol";
-import {IImmutableERC20Errors} from "contracts/errors/ERC20Errors.sol";
+import {IImmutableERC20Errors} from "contracts/token/erc20/preset/Errors.sol";
 
 
 contract ImmutableERC20FixedSupplyNoBurnTest is Test {