feat: Add a mint amount limit and ownership transfer function to Faucet (#162)

* add packages

* feat: add tx limit on mint

* testing

* chore: add to permissionless mode

* feat: handle decimals

* chore: update const to public and tests

* fix: Simply max limit amount require

* feat: Add function to transfer ownership of child contracts

* fix: Allows minting max amount limit

* fix: Remove unneeded new dep in package

---------

Co-authored-by: miguelmtzinf <[email protected]>

Author: foodaka

contracts/mocks/testnet-helpers/Faucet.sol

@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity ^0.8.0;
 
-import {IFaucet} from './IFaucet.sol';
-import {TestnetERC20} from './TestnetERC20.sol';
 import {Ownable} from '@aave/core-v3/contracts/dependencies/openzeppelin/contracts/Ownable.sol';
+import {TestnetERC20} from './TestnetERC20.sol';
+import {IFaucet} from './IFaucet.sol';
 
 /**
  * @title Faucet
@@ -14,10 +14,12 @@ contract Faucet is IFaucet, Ownable {
   // If disabled, anyone can call mint at the faucet, for PoC environments
   bool internal _permissioned;
 
+  // Maximum amount of tokens per mint allowed
+  uint256 public constant MAX_MINT_AMOUNT = 10000;
+
   constructor(address owner, bool permissioned) {
     require(owner != address(0));
     transferOwnership(owner);
-
     _permissioned = permissioned;
   }
 
@@ -37,6 +39,11 @@ contract Faucet is IFaucet, Ownable {
     address to,
     uint256 amount
   ) external override onlyOwnerIfPermissioned returns (uint256) {
+    require(
+      amount <= MAX_MINT_AMOUNT * (10 ** TestnetERC20(token).decimals()),
+      'Error: Mint limit transaction exceeded'
+    );
+
     TestnetERC20(token).mint(to, amount);
     return amount;
   }
@@ -50,4 +57,14 @@ contract Faucet is IFaucet, Ownable {
   function isPermissioned() external view override returns (bool) {
     return _permissioned;
   }
+
+  /// @inheritdoc IFaucet
+  function transferOwnershipOfChild(
+    address[] calldata childContracts,
+    address newOwner
+  ) external override onlyOwner {
+    for (uint256 i = 0; i < childContracts.length; i++) {
+      Ownable(childContracts[i]).transferOwnership(newOwner);
+    }
+  }
 }

contracts/mocks/testnet-helpers/IFaucet.sol

@@ -9,11 +9,7 @@ interface IFaucet {
    * @param amount The amount of tokens to mint
    * @return The amount minted
    **/
-  function mint(
-    address token,
-    address to,
-    uint256 amount
-  ) external returns (uint256);
+  function mint(address token, address to, uint256 amount) external returns (uint256);
 
   /**
    * @notice Enable or disable the need of authentication to call `mint` function
@@ -26,4 +22,11 @@ interface IFaucet {
    * @return Returns a boolean, if true the mode is enabled, if false is disabled
    */
   function isPermissioned() external view returns (bool);
+
+  /**
+   * @notice Transfer the ownership of child contracts
+   * @param childContracts A list of child contract addresses
+   * @param newOwner The address of the new owner
+   */
+  function transferOwnershipOfChild(address[] calldata childContracts, address newOwner) external;
 }

test/faucet.spec.ts

@@ -1,14 +1,17 @@
-import { getFirstSigner, waitForTx } from '@aave/deploy-v3';
+import { HardhatRuntimeEnvironment } from 'hardhat/types';
 import { expect } from 'chai';
 import { parseEther } from 'ethers/lib/utils';
-import { HardhatRuntimeEnvironment } from 'hardhat/types';
+import { ONE_ADDRESS, evmRevert, evmSnapshot, waitForTx } from '@aave/deploy-v3';
 import { makeSuite, TestEnv } from './helpers/make-suite';
+import { Ownable__factory, TestnetERC20__factory } from '../types';
 
 declare let hre: HardhatRuntimeEnvironment;
 
 makeSuite('Faucet', (testEnv: TestEnv) => {
   const mintAmount = parseEther('100');
+
   let faucetOwnable;
+
   before(async () => {
     // Enforce permissioned mode as disabled for deterministic test suite
 
@@ -19,6 +22,7 @@ makeSuite('Faucet', (testEnv: TestEnv) => {
 
     await waitForTx(await faucetOwnable.setPermissioned(false));
   });
+
   describe('Permissioned mode: disabled', () => {
     before(async () => {
       // Enforce permissioned mode as disabled for deterministic test suite
@@ -40,6 +44,39 @@ makeSuite('Faucet', (testEnv: TestEnv) => {
     it('Getter isPermissioned should return false', async () => {
       await expect(await faucetOwnable.isPermissioned()).is.equal(false);
     });
+
+    it('Mint function should mint tokens within limit', async () => {
+      const {
+        users: [, , , user],
+        dai,
+        deployer,
+      } = testEnv;
+
+      const threshold = await faucetOwnable.connect(deployer.signer).MAX_MINT_AMOUNT();
+      const thresholdValue = threshold.toNumber();
+      const withinLimitThreshold = parseEther((thresholdValue).toString());
+
+      await faucetOwnable
+        .connect(deployer.signer)
+        .mint(dai.address, user.address, withinLimitThreshold);
+      await expect(await dai.balanceOf(user.address)).eq(withinLimitThreshold);
+    });
+
+    it('Mint function should revert with values over the limit', async () => {
+      const {
+        users: [, , , user],
+        dai,
+        deployer,
+      } = testEnv;
+
+      const threshold = await faucetOwnable.connect(deployer.signer).MAX_MINT_AMOUNT();
+      const thresholdValue = threshold.toNumber();
+      const maxLimitThreshold = parseEther((thresholdValue + 1).toString());
+
+      await expect(
+        faucetOwnable.connect(deployer.signer).mint(dai.address, user.address, maxLimitThreshold)
+      ).to.be.revertedWith('Error: Mint limit transaction exceeded');
+    });
   });
 
   describe('Permissioned mode: enabled', () => {
@@ -59,17 +96,20 @@ makeSuite('Faucet', (testEnv: TestEnv) => {
     });
 
     it('Mint function can only be called by owner', async () => {
+      const mintAmount = parseEther('100');
       const {
         users: [, , , user],
         dai,
         deployer,
       } = testEnv;
 
+      const initialBalance = await dai.balanceOf(user.address);
+
       await waitForTx(
         await faucetOwnable.connect(deployer.signer).mint(dai.address, user.address, mintAmount)
       );
 
-      await expect(await dai.balanceOf(user.address)).eq(mintAmount);
+      await expect(await dai.balanceOf(user.address)).eq(initialBalance.add(mintAmount));
     });
 
     it('Getter isPermissioned should return true', async () => {
@@ -102,4 +142,41 @@ makeSuite('Faucet', (testEnv: TestEnv) => {
 
     expect(await faucetOwnable.isPermissioned()).equal(true);
   });
+
+  it('Transfer ownership should revert if not owner', async () => {
+    const {
+      deployer,
+      users: [user1],
+    } = testEnv;
+
+    const childContract = await new TestnetERC20__factory(deployer.signer).deploy(
+      'CHILD',
+      'CHILD',
+      18,
+      faucetOwnable.address
+    );
+    expect(await childContract.owner()).to.be.eq(faucetOwnable.address);
+    await expect(
+      faucetOwnable
+        .connect(user1.signer)
+        .transferOwnershipOfChild([childContract.address], ONE_ADDRESS)
+    ).to.be.revertedWith('Ownable: caller is not the owner');
+    expect(await childContract.owner()).to.be.eq(faucetOwnable.address);
+  });
+
+  it('Transfer ownership of child to another address', async () => {
+    const { deployer } = testEnv;
+
+    const childContract = await new TestnetERC20__factory(deployer.signer).deploy(
+      'CHILD',
+      'CHILD',
+      18,
+      faucetOwnable.address
+    );
+    expect(await childContract.owner()).to.be.eq(faucetOwnable.address);
+    await faucetOwnable
+      .connect(deployer.signer)
+      .transferOwnershipOfChild([childContract.address], ONE_ADDRESS);
+    expect(await childContract.owner()).to.be.eq(ONE_ADDRESS);
+  });
 });