contracts-governance: fix the mint manager (#2737)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.changeset/forty-sloths-change.md

+---
+'@eth-optimism/contracts-governance': patch
+---
+
+Fix the mint manager

packages/contracts-governance/contracts/MintManager.sol

@@ -5,43 +5,76 @@ import "@openzeppelin/contracts/access/Ownable.sol";
 import "./GovernanceToken.sol";
 
 /**
- * @dev Set as `owner` of the OP token and responsible for the token inflation schedule.
- * Contract acts as the token "mint manager" with permission to the `mint` function only.
- * Currently permitted to mint once per year of up to 2% of the total token supply.
- * Upgradable to allow changes in the inflation schedule.
+ * @title  MintManager
+ * @notice Set as `owner` of the OP token and responsible for the token inflation schedule.
+ *         Contract acts as the token "mint manager" with permission to the `mint` function only.
+ *         Currently permitted to mint once per year of up to 2% of the total token supply.
+ *         Upgradable to allow changes in the inflation schedule.
  */
 contract MintManager is Ownable {
+    /**
+     * @notice The GovernanceToken that the MintManager can mint tokens
+     */
     GovernanceToken public governanceToken;
 
-    uint256 public constant MINT_CAP = 200; // 2%
+    /**
+     * @notice The amount of tokens that can be minted per year. The value is a fixed
+     *          point number with 4 decimals.
+     */
+    uint256 public constant MINT_CAP = 20; // 2%
+
+    /**
+     * @notice The number of decimals for the MINT_CAP.
+     */
+    uint256 public constant DENOMINATOR = 1000;
+
+    /**
+     * @notice The amount of time that must pass before the MINT_CAP number of tokens can
+     *         be minted again.
+     */
     uint256 public constant MINT_PERIOD = 365 days;
+
+    /**
+     * @notice Tracks the time of last mint
+     */
     uint256 public mintPermittedAfter;
 
+    /**
+     * @param _upgrader        The owner of this contract
+     * @param _governanceToken The governance token this contract can mint
+     *                         tokens of
+     */
     constructor(address _upgrader, address _governanceToken) {
         transferOwnership(_upgrader);
         governanceToken = GovernanceToken(_governanceToken);
     }
 
     /**
+     * @notice Only the token owner is allowed to mint a certain amount of OP per year.
+     *
      * @param _account Address to mint new tokens to.
-     * @param _amount Amount of tokens to be minted.
-     * @notice Only the token owner is allowed to mint.
+     * @param _amount  Amount of tokens to be minted.
      */
     function mint(address _account, uint256 _amount) public onlyOwner {
         if (mintPermittedAfter > 0) {
             require(mintPermittedAfter <= block.timestamp, "OP: minting not permitted yet");
 
             require(
-                _amount <= (governanceToken.totalSupply() * MINT_CAP) / 1000,
+                _amount <= (governanceToken.totalSupply() * MINT_CAP) / DENOMINATOR,
                 "OP: mint amount exceeds cap"
             );
         }
 
-        governanceToken.mint(_account, _amount);
-
         mintPermittedAfter = block.timestamp + MINT_PERIOD;
+
+        governanceToken.mint(_account, _amount);
     }
 
+    /**
+     * @notice Upgrade the owner of the governance token to a new MintManager.
+     *
+     * @param _newMintManager The MintManager to upgrade to
+     */
     function upgrade(address _newMintManager) public onlyOwner {
         require(_newMintManager != address(0), "OP: Mint manager cannot be empty");
 

packages/contracts-governance/test/gov-token.test.ts

@@ -154,7 +154,7 @@ describe('Governance Token Testing', () => {
 
       // Minting the full 2% after the first year
       let totalSupply = await governanceToken.totalSupply()
-      let maxInflationAmount = totalSupply.mul(200).div(1000)
+      let maxInflationAmount = totalSupply.mul(20).div(1000)
 
       await fastForwardDays(365)
       await mintManager
@@ -168,7 +168,7 @@ describe('Governance Token Testing', () => {
       // Minting the full 2% after the second year
       await fastForwardDays(365)
       totalSupply = await governanceToken.totalSupply()
-      maxInflationAmount = totalSupply.mul(200).div(1000)
+      maxInflationAmount = totalSupply.mul(20).div(1000)
 
       await mintManager
         .connect(minter)
@@ -181,7 +181,7 @@ describe('Governance Token Testing', () => {
       // Minting the full 2% after the third year
       await fastForwardDays(365)
       totalSupply = await governanceToken.totalSupply()
-      maxInflationAmount = totalSupply.mul(200).div(1000)
+      maxInflationAmount = totalSupply.mul(20).div(1000)
 
       await mintManager
         .connect(minter)
@@ -198,14 +198,14 @@ describe('Governance Token Testing', () => {
         .mint(optimismMultisig.address, initialSupply)
 
       await fastForwardDays(365)
-      const inflationAmount = initialSupply.mul(200).div(1000).sub(1)
+      const inflationAmount = initialSupply.mul(20).div(1000).sub(1)
 
       await mintManager
         .connect(minter)
         .mint(optimismMultisig.address, inflationAmount)
 
       const updatedTotalSupply = await governanceToken.totalSupply()
-      const newTotalSupply = await initialSupply.add(inflationAmount)
+      const newTotalSupply = initialSupply.add(inflationAmount)
       expect(updatedTotalSupply).to.equal(newTotalSupply)
     })
 
@@ -215,7 +215,7 @@ describe('Governance Token Testing', () => {
         .mint(optimismMultisig.address, initialSupply)
 
       await fastForwardDays(369)
-      const inflationAmount = initialSupply.mul(200).div(1000).add(1)
+      const inflationAmount = initialSupply.mul(20).div(1000).add(1)
 
       await expect(
         mintManager