Merge pull request #84 from credbull/audit-2-remidiation

Merge CBL Token Audit remediation changes

.gitmodules

@@ -4,3 +4,4 @@
 [submodule "packages/contracts/lib/openzeppelin-contracts"]
 	path = packages/contracts/lib/openzeppelin-contracts
 	url = https://github.com/OpenZeppelin/openzeppelin-contracts
+	branch = release-v5.0

packages/contracts/CBL_README.md

@@ -0,0 +1,114 @@
+# CBL (Credbull) Token Contract
+
+## Overview
+
+The CBL token is an ERC20-compliant token with additional features such as permit, burnable, capped supply, pausability,
+and access control. 
+
+The CBL token extends OpenZeppelin 5.0 contracts: ERC20, ERC20Permit, ERC20Burnable, ERC20Capped, ERC20Pausable, and AccessControl.
+
+## Features
+
+- **Permit:** Allows approvals to be made via signatures, following the ERC20Permit standard.
+- **Burnable:** Tokens can be burned, reducing the total supply.
+- **Capped Supply:** The total supply of tokens is capped.
+- **Pausability:** Token transfers, minting, and burning can be paused and unpaused.
+- **Access Control:** Roles are used to manage permissions for minting and administrative functions.
+
+## Roles
+
+### Admin
+
+The Admin role is assigned to the owner of the contract and has the highest level of control. Admins can:
+
+- Pause and unpause token operations such as transfers, minting, and burning.
+- Manage role assignments, including granting and revoking roles.
+
+### Minter
+
+The Minter role is responsible for creating new tokens. Minters can:
+
+- Mint new tokens to specified addresses.
+- Only accounts with the Minter role can call the minting function.
+
+## State Variables
+
+### MINTER_ROLE
+
+Role identifier for the minter role.
+
+```solidity
+bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
+```
+
+## Functions
+
+### constructor
+
+Constructor to initialize the token contract.
+
+*Sets the owner and minter roles, and initializes the capped supply.*
+
+**Parameters**
+
+| Name         | Type      | Description                                              |
+|--------------|-----------|----------------------------------------------------------|
+| `_owner`     | `address` | The address of the owner who will have the admin role.   |
+| `_minter`    | `address` | The address of the minter who will have the minter role. |
+| `_maxSupply` | `uint256` | The maximum supply of the token.                         |
+
+### pause
+
+Pauses token transfers, minting and burning.
+
+*Can only be called by an account with the admin role.*
+
+### unpause
+
+Unpauses token transfers, minting and burning.
+
+*Can only be called by an account with the admin role.*
+
+### mint
+
+Mints new tokens.
+
+*Can only be called by an account with the minter role.*
+
+**Parameters**
+
+| Name     | Type      | Description                    |
+|----------|-----------|--------------------------------|
+| `to`     | `address` | The address to mint tokens to. |
+| `amount` | `uint256` | The amount of tokens to mint.  |
+
+### _update
+
+*Overrides required by Solidity for multiple inheritance.*
+
+**Parameters**
+
+| Name    | Type      | Description                                    |
+|---------|-----------|------------------------------------------------|
+| `from`  | `address` | The address from which tokens are transferred. |
+| `to`    | `address` | The address to which tokens are transferred.   |
+| `value` | `uint256` | The amount of tokens transferred.              |
+
+## Errors
+
+### CBL__InvalidOwnerAddress
+
+*Error to indicate that the provided owner address is invalid.*
+
+```solidity
+error CBL__InvalidOwnerAddress();
+```
+
+### CBL__InvalidMinterAddress
+
+*Error to indicate that the provided minter address is invalid.*
+
+```solidity
+error CBL__InvalidMinterAddress();
+```
+

packages/contracts/src/token/CBL.sol

@@ -7,37 +7,79 @@ import { ERC20Burnable } from "@openzeppelin/contracts/token/ERC20/extensions/ER
 import { ERC20Pausable } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Pausable.sol";
 import { AccessControl } from "@openzeppelin/contracts/access/AccessControl.sol";
 import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
+import { ERC20Capped } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Capped.sol";
 
-contract CBL is ERC20, ERC20Permit, ERC20Burnable, ERC20Pausable, AccessControl {
+/**
+ * @title CBL (Credbull) Token Contract
+ * @dev ERC20 token with additional features: permit, burnable, capped supply, pausability, and access control.
+ */
+contract CBL is ERC20, ERC20Permit, ERC20Burnable, ERC20Capped, ERC20Pausable, AccessControl {
+    /// @dev Error to indicate that the provided owner address is invalid.
+    error CBL__InvalidOwnerAddress();
+
+    /// @dev Error to indicate that the provided minter address is invalid.
+    error CBL__InvalidMinterAddress();
+
+    /// @notice Role identifier for the minter role.
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
 
-    error CBL__MaxSupplyExceeded();
+    /**
+     * @notice Constructor to initialize the token contract.
+     * @dev Sets the owner and minter roles, and initializes the capped supply.
+     * @param _owner The address of the owner who will have the admin role.
+     * @param _minter The address of the minter who will have the minter role.
+     * @param _maxSupply The maximum supply of the token.
+     */
+    constructor(address _owner, address _minter, uint256 _maxSupply)
+        ERC20("Credbull", "CBL")
+        ERC20Permit("Credbull")
+        ERC20Capped(_maxSupply)
+    {
+        if (_owner == address(0)) {
+            revert CBL__InvalidOwnerAddress();
+        }
 
-    uint256 public immutable MAX_SUPPLY;
+        if (_minter == address(0)) {
+            revert CBL__InvalidMinterAddress();
+        }
 
-    constructor(address _owner, address _minter, uint256 _maxSupply) ERC20("Credbull", "CBL") ERC20Permit("Credbull") {
-        MAX_SUPPLY = _maxSupply;
         _grantRole(DEFAULT_ADMIN_ROLE, _owner);
         _grantRole(MINTER_ROLE, _minter);
     }
 
-    function pause() public onlyRole(DEFAULT_ADMIN_ROLE) {
+    /**
+     * @notice Pauses token transfers, minting and burning.
+     * @dev Can only be called by an account with the admin role.
+     */
+    function pause() external onlyRole(DEFAULT_ADMIN_ROLE) {
         _pause();
     }
 
-    function unpause() public onlyRole(DEFAULT_ADMIN_ROLE) {
+    /**
+     * @notice Unpauses token transfers, minting and burning.
+     * @dev Can only be called by an account with the admin role.
+     */
+    function unpause() external onlyRole(DEFAULT_ADMIN_ROLE) {
         _unpause();
     }
 
-    function mint(address to, uint256 amount) public onlyRole(MINTER_ROLE) {
-        if (totalSupply() + amount > MAX_SUPPLY) {
-            revert CBL__MaxSupplyExceeded();
-        }
+    /**
+     * @notice Mints new tokens.
+     * @dev Can only be called by an account with the minter role.
+     * @param to The address to mint tokens to.
+     * @param amount The amount of tokens to mint.
+     */
+    function mint(address to, uint256 amount) external onlyRole(MINTER_ROLE) {
         _mint(to, amount);
     }
 
-    // The following functions are overrides required by Solidity.
-    function _update(address from, address to, uint256 value) internal override(ERC20, ERC20Pausable) {
+    /**
+     * @dev Overrides required by Solidity for multiple inheritance.
+     * @param from The address from which tokens are transferred.
+     * @param to The address to which tokens are transferred.
+     * @param value The amount of tokens transferred.
+     */
+    function _update(address from, address to, uint256 value) internal override(ERC20, ERC20Pausable, ERC20Capped) {
         super._update(from, to, value);
     }
 }

packages/contracts/test/src/token/CBLTest.t.sol

@@ -5,6 +5,8 @@ pragma solidity ^0.8.20;
 import { Test } from "forge-std/Test.sol";
 import { CBL } from "@credbull/token/CBL.sol";
 import { DeployCBLToken, CBLTokenParams } from "../../../script/DeployCBLToken.s.sol";
+import { ERC20Capped } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Capped.sol";
+import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol";
 
 contract CBLTest is Test {
     CBL private cbl;
@@ -25,11 +27,19 @@ contract CBLTest is Test {
         cbl = deployCBLToken.runTest();
     }
 
+    function test__CBL__ShouldRevertOnZeroAddress() public {
+        vm.expectRevert(CBL.CBL__InvalidOwnerAddress.selector);
+        new CBL(address(0), makeAddr("minter"), type(uint32).max);
+
+        vm.expectRevert(CBL.CBL__InvalidMinterAddress.selector);
+        new CBL(makeAddr("owner"), address(0), type(uint32).max);
+    }
+
     function test__CBL__SuccessfullyDeployCBLToken() public {
         cbl = new CBL(cblTokenParams.owner, cblTokenParams.minter, cblTokenParams.maxSupply);
         assertTrue(cbl.hasRole(cbl.DEFAULT_ADMIN_ROLE(), cblTokenParams.owner));
         assertTrue(cbl.hasRole(cbl.MINTER_ROLE(), cblTokenParams.minter));
-        assertEq(cbl.MAX_SUPPLY(), cblTokenParams.maxSupply);
+        assertEq(cbl.cap(), cblTokenParams.maxSupply);
     }
 
     function test__CBL__ShouldAllowMinterToMint() public {
@@ -60,9 +70,9 @@ contract CBLTest is Test {
 
     function test__CBL__ShouldRevertIfTotalSupplyExceedsMaxSupply() public {
         vm.startPrank(minter);
-        cbl.mint(minter, cbl.MAX_SUPPLY());
+        cbl.mint(minter, cbl.cap());
 
-        vm.expectRevert(CBL.CBL__MaxSupplyExceeded.selector);
+        vm.expectRevert(abi.encodeWithSelector(ERC20Capped.ERC20ExceededCap.selector, cbl.totalSupply() + 1, cbl.cap()));
         cbl.mint(minter, 1);
         vm.stopPrank();
     }
@@ -72,4 +82,29 @@ contract CBLTest is Test {
         cbl.mint(minter, 100);
         assertEq(cbl.totalSupply(), 100);
     }
+
+    function test__CBL__PauseAndUnPauseMintAndBurn() public {
+        vm.prank(minter);
+        cbl.mint(alice, 100);
+
+        vm.prank(owner);
+        cbl.pause();
+
+        vm.prank(minter);
+        vm.expectRevert(Pausable.EnforcedPause.selector);
+        cbl.mint(alice, 100);
+
+        vm.prank(alice);
+        vm.expectRevert(Pausable.EnforcedPause.selector);
+        cbl.burn(100);
+
+        vm.prank(owner);
+        cbl.unpause();
+
+        vm.prank(minter);
+        cbl.mint(alice, 100);
+
+        vm.prank(alice);
+        cbl.burn(100);
+    }
 }