Minting token recipient (#51)

* Owner should not be initial minter * Add minter recipient address restriction * Move mintExplicitSender function and checks to ERC20Mintable contract * Emit event when minting recipient change * Added event map for mintable * Reorder functions * Fix test env setup for restricted minter * Fix tests for restricted minter * Remove TODO * Add test case as requested by @peteremiljensen
etoroxlabs · Jan 11, 2019 · 063917b · 063917b
1 parent d1d3d0f
commit 063917b
Show file tree

Hide file tree

Showing 15 changed files with 194 additions and 58 deletions.
diff --git a/contracts/access/roles/MinterRole.sol b/contracts/access/roles/MinterRole.sol
@@ -11,10 +11,6 @@ contract MinterRole is Ownable {
 
     Roles.Role private minters;
 
-    constructor() internal {
-        _addMinter(msg.sender);
-    }
-
     modifier onlyMinter() {
         require(isMinter(msg.sender), "not minter");
         _;

diff --git a/contracts/mocks/ExternalERC20MintableMock.sol b/contracts/mocks/ExternalERC20MintableMock.sol
@@ -3,10 +3,21 @@ pragma solidity ^0.4.24;
 import "../token/ERC20/ExternalERC20Mintable.sol";
 import "./MinterRoleMock.sol";
 
-contract ExternalERC20MintableMock is ExternalERC20Mintable, MinterRoleMock {
+contract ExternalERC20MintableMock is  ExternalERC20Mintable, MinterRoleMock {
 
-    constructor() ExternalERC20(new ExternalERC20Storage()) public {
+    constructor(address initMintingRecipient)
+        ExternalERC20(new ExternalERC20Storage())
+        ExternalERC20Mintable(initMintingRecipient)
+        public {
         _externalERC20Storage.transferImplementor(this);
         _externalERC20Storage.transferOwnership(msg.sender);
     }
+
+    function mint(address to, uint256 amount) public {
+        _mintExplicitSender(msg.sender, to, amount);
+    }
+
+    function changeMintingRecipient(address to) public {
+        _changeMintingRecipient(msg.sender, to);
+    }
 }
diff --git a/contracts/mocks/TokenXExplicitSenderMock.sol b/contracts/mocks/TokenXExplicitSenderMock.sol
@@ -17,6 +17,6 @@ contract TokenXExplicitSenderMock is TokenXExplicitSender {
         public
         TokenXExplicitSender(
             name, symbol, decimals, accesslist, whitelistEnabled,
-            stor, upgradedFrom, initialDeployment)
+            stor, address(0xf00f), upgradedFrom, initialDeployment)
     {}
 }
diff --git a/contracts/mocks/TokenXMock.sol b/contracts/mocks/TokenXMock.sol
@@ -15,14 +15,15 @@ contract TokenXMock is TokenX, PauserRoleMock {
         Accesslist accesslist,
         bool whitelistEnabled,
         ExternalERC20Storage stor,
+        address mintingRecip,
         IUpgradableTokenX upgradedFrom,
         bool initialDeployment,
         address initialAccount,
         uint256 initialBalance
     )
         TokenX(
             name, symbol, decimals,
-            accesslist, whitelistEnabled, stor, upgradedFrom,
+            accesslist, whitelistEnabled, stor, mintingRecip, upgradedFrom,
             initialDeployment
         )
         public

diff --git a/contracts/token/ERC20/ExternalERC20Mintable.sol b/contracts/token/ERC20/ExternalERC20Mintable.sol
@@ -8,21 +8,49 @@ import "../../access/roles/MinterRole.sol";
  * @dev ERC20 minting logic
  */
 contract ExternalERC20Mintable is ExternalERC20, MinterRole {
+
+    address private mintingRecipientAccount;
+
+    event MintingRecipientAccountChanged(address prev, address next);
+
+    constructor(address _mintingRecipientAccount) internal {
+        _changeMintingRecipient(msg.sender, _mintingRecipientAccount);
+    }
+
+    /**
+     * @dev Allows the owner to change the current minting recipient account
+     * @param _mintingRecipientAccount address of new minting recipient
+     */
+    function _changeMintingRecipient(
+        address sender,
+        address _mintingRecipientAccount
+    )
+        internal
+    {
+        require(owner() == sender, "is not owner");
+        require(_mintingRecipientAccount != address(0),
+                "zero minting recipient");
+        address prev = mintingRecipientAccount;
+        mintingRecipientAccount = _mintingRecipientAccount;
+        emit MintingRecipientAccountChanged(prev, mintingRecipientAccount);
+    }
+
     /**
      * @dev Function to mint tokens
      * @param to The address that will receive the minted tokens.
      * @param value The amount of tokens to mint.
      * @return A boolean that indicates if the operation was successful.
      */
-    function mint(
+    function _mintExplicitSender(
+        address sender,
         address to,
         uint256 value
     )
-        public
-        onlyMinter
-        returns (bool)
+        internal
+        requireMinter(sender)
     {
+        require(to == mintingRecipientAccount,
+                "not minting to mintingRecipientAccount");
         _mint(to, value);
-        return true;
     }
 }
diff --git a/contracts/token/IUpgradableTokenX.sol b/contracts/token/IUpgradableTokenX.sol
@@ -59,6 +59,10 @@ contract IUpgradableTokenX {
         public
         returns (bool);
 
+    function changeMintingRecipientExplicitSender(address sender,
+                                                  address mintingRecip)
+        public;
+
     function burnExplicitSender(address sender, uint256 value) public;
 
     function burnFromExplicitSender(address sender,

diff --git a/contracts/token/TokenX.sol b/contracts/token/TokenX.sol
@@ -18,6 +18,7 @@ contract TokenX is ITokenX, TokenXExplicitSender {
         Accesslist accesslist,
         bool whitelistEnabled,
         ExternalERC20Storage externalERC20Storage,
+        address mintingRecipientAccount,
         address upgradedFrom,
         bool initialDeployment
     )
@@ -29,6 +30,7 @@ contract TokenX is ITokenX, TokenXExplicitSender {
             accesslist,
             whitelistEnabled,
             externalERC20Storage,
+            mintingRecipientAccount,
             upgradedFrom,
             initialDeployment
         ) {
@@ -196,6 +198,14 @@ contract TokenX is ITokenX, TokenXExplicitSender {
         }
     }
 
+    function changeMintingRecipient(address mintingRecip) public {
+        if (isUpgraded()) {
+            upgradedToken.changeMintingRecipientExplicitSender(msg.sender, mintingRecip);
+        } else {
+            super.changeMintingRecipient(mintingRecip);
+        }
+    }
+
     function pause () public {
         if (isUpgraded()) {
             revert("Token is upgraded. Call pause from new token.");
@@ -211,4 +221,5 @@ contract TokenX is ITokenX, TokenXExplicitSender {
             super.unpause();
         }
     }
+
 }
diff --git a/contracts/token/TokenXExplicitSender.sol b/contracts/token/TokenXExplicitSender.sol
@@ -18,9 +18,9 @@ import "./IUpgradableTokenX.sol";
 contract TokenXExplicitSender is IUpgradableTokenX,
     ExternalERC20,
     ExternalERC20Burnable,
+    ExternalERC20Mintable,
     ERC20Detailed,
     AccesslistGuarded,
-    MinterRole,
     BurnerRole,
     Pausable
 {
@@ -54,11 +54,13 @@ contract TokenXExplicitSender is IUpgradableTokenX,
         Accesslist accesslist,
         bool whitelistEnabled,
         ExternalERC20Storage externalERC20Storage,
+        address mintingRecipientAccount,
         address upgradedFrom,
         bool initialDeployment
     )
         internal
         ExternalERC20(externalERC20Storage)
+        ExternalERC20Mintable(mintingRecipientAccount)
         ERC20Detailed(name, symbol, decimals)
         AccesslistGuarded(accesslist, whitelistEnabled)
     {
@@ -282,13 +284,20 @@ contract TokenXExplicitSender is IUpgradableTokenX,
         public
         isEnabled
         senderIsProxy
-        requireMinter(sender)
         returns (bool success)
     {
-        super._mint(to, value);
+        super._mintExplicitSender(sender, to, value);
         return true;
     }
 
+    function changeMintingRecipientExplicitSender(address sender, address mintingRecip)
+        public
+        isEnabled
+        senderIsProxy
+    {
+        super._changeMintingRecipient(sender, mintingRecip);
+    }
+
     function transfer(address to, uint256 value)
         public
         isEnabled
@@ -356,12 +365,19 @@ contract TokenXExplicitSender is IUpgradableTokenX,
     function mint(address to, uint256 value)
         public
         isEnabled
-        onlyMinter
         returns (bool)
     {
-        super._mint(to, value);
+        super._mintExplicitSender(msg.sender, to, value);
         return true;
     }
 
+    function changeMintingRecipient(address _mintingRecipientAddress)
+        public
+        isEnabled
+        onlyOwner
+    {
+        super._changeMintingRecipient(msg.sender, _mintingRecipientAddress);
+    }
+
 
 }
diff --git a/migrations/4_setup_accounts.js b/migrations/4_setup_accounts.js
@@ -12,14 +12,17 @@ module.exports = function (deployer, _network, accounts) {
   }
 };
 
-async function setupAccounts ([owner, whitelistAdmin, whitelisted, ...restAccounts]) {
+async function setupAccounts ([owner, whitelistAdmin, whitelisted, minter, ...restAccounts]) {
   /*
     The purpose of this is to automatically setup the test environment accounts.
     DO NOT use in production yet.
   */
 
   const intialMintValue = 100;
 
+  // FIXME: Use some real address here
+  const mintTargetAccount = 0xd00f;
+
   // Setup whitelists
   const accesslistContract = await Accesslist.deployed();
 
@@ -50,16 +53,16 @@ async function setupAccounts ([owner, whitelistAdmin, whitelisted, ...restAccoun
       const token = await TokenX.new(
         td.name, td.symbol, td.decimals,
         accesslistContract.address, td.whitelistEnabled, externalERC20Storage.address,
-        0, true,
+        mintTargetAccount, 0, true,
         { from: owner });
 
+      token.addMinter(minter, { from: owner });
       await tokenManagerContract.addToken(td.name, token.address);
       return token;
-    })
-  );
+    }));
 
   // Mint tokens
   await Promise.all(
-    tokens.map((t) => t.mint(owner, intialMintValue, { from: owner }))
+    tokens.map((t) => t.mint(mintTargetAccount, intialMintValue, { from: minter }))
   );
 }
diff --git a/test/UpgradeToken.test.js b/test/UpgradeToken.test.js
@@ -32,14 +32,14 @@ contract('Upgrade Token', async function ([_, tokenManagerOwner, oldTokenOwner,
 
        const oldToken = await TokenX.new(
          tokenName, 'e', oldTokenDecimals,
-         accesslist.address, true, storage.address, 0, true,
+         accesslist.address, true, storage.address, 0xf00f, 0, true,
          { from: oldTokenOwner }
        );
 
        const newToken = await TokenX.new(
          tokenName, 'e', newTokenDecimals,
          accesslist.address, true,
-         storage.address, oldToken.address, false,
+         storage.address, 0xf00f, oldToken.address, false,
          { from: newTokenOwner }
        );
 

diff --git a/test/roles/MinterRole.test.js b/test/roles/MinterRole.test.js
@@ -7,7 +7,8 @@ const MinterRoleMock = artifacts.require('MinterRoleMock');
 contract('MinterRole', function ([_, minter, otherMinter, ...otherAccounts]) {
   beforeEach(async function () {
     this.contract = await MinterRoleMock.new({ from: minter });
-    await this.contract.addMinter(otherMinter, { from: minter });
+    [minter, otherMinter].forEach(
+      async (x) => this.contract.addMinter(x, { from: minter }));
   });
 
   shouldBehaveLikePublicRole(minter, otherMinter, otherAccounts, 'minter');

diff --git a/test/token/ERC20/ExternalERC20Mintable.events.js b/test/token/ERC20/ExternalERC20Mintable.events.js
@@ -0,0 +1,8 @@
+const utils = require('./../..//utils.js');
+
+module.exports = utils.makeEventMap({
+  changeMintingRecipient: (prevAddr, nextAddr) => [
+    { eventName: 'MintingRecipientAccountChanged',
+      paramMap: { prev: prevAddr,
+                  next: nextAddr } }]
+});
diff --git a/test/token/ERC20/ExternalERC20Mintable.test.js b/test/token/ERC20/ExternalERC20Mintable.test.js
@@ -9,20 +9,22 @@ const ExternalERC20MintableMock = artifacts.require('ExternalERC20MintableMock')
 const { shouldBehaveLikePublicRole } = require('../../roles/behaviors/PublicRole.behavior');
 
 contract('ExternalERC20Mintable', function ([_, minter, otherMinter, ...otherAccounts]) {
+  const mintingRecipientAccount = '0x000000000000000000000000000000000000d00f';
   beforeEach(async function () {
-    this.token = await ExternalERC20MintableMock.new({ from: minter });
+    this.token = await ExternalERC20MintableMock.new(mintingRecipientAccount, { from: minter });
+    this.token.addMinter(minter, { from: minter });
   });
 
   describe('minter role', function () {
     beforeEach(async function () {
       this.contract = this.token;
-      await this.contract.addMinter(otherMinter, { from: minter });
+      this.token.addMinter(otherMinter, { from: minter });
     });
 
     shouldBehaveLikePublicRole(minter, otherMinter, otherAccounts, 'minter');
   });
 
-  shouldBehaveLikeERC20Mintable(minter, otherAccounts);
+  shouldBehaveLikeERC20Mintable(minter, minter, otherAccounts, mintingRecipientAccount);
 
   describe('When sharing storage', function () {
     beforeEach(async function () {
@@ -37,7 +39,7 @@ contract('ExternalERC20Mintable', function ([_, minter, otherMinter, ...otherAcc
       (await this.token.totalSupply()).should.be.bignumber.equal(0);
       (await this.token2.totalSupply()).should.be.bignumber.equal(0);
 
-      await this.token.mint(minter, 100, { from: minter });
+      await this.token.mint(mintingRecipientAccount, 100, { from: minter });
 
       (await this.token.totalSupply()).should.be.bignumber.equal(100);
       (await this.token2.totalSupply()).should.be.bignumber.equal(100);