Bundle ERC20Detailed (#2161)

* Merge ERC20Detailed into ERC20, make derived contracts abstract

* Fix Create2 tests

* Fix failing test

* Default decimals to 18

* Add tests for setupDecimals

* Add changelog entry

* Update CHANGELOG.md

* Update CHANGELOG.md

* Replace isConstructor for !isContract

* Update CHANGELOG.md

Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>

CHANGELOG.md

@@ -30,6 +30,8 @@
  * `ERC777`: removed `_callsTokensToSend` and `_callTokensReceived`. ([#2134](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2134))
  * `EnumerableSet`: renamed `get` to `at`. ([#2151](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2151))
  * `ERC165Checker`: functions no longer have a leading underscore. ([#2150](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2150))
+ * `ERC20Detailed`: this contract was removed and its functionality merged into `ERC20`. ([#2161](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2161))
+ * `ERC20`: added a constructor for `name` and `symbol`. `decimals` now defaults to 18. ([#2161](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2161))
 
 ## 2.5.0 (2020-02-04)
 

contracts/GSN/GSNRecipientERC20Fee.sol

@@ -5,7 +5,6 @@ import "../math/SafeMath.sol";
 import "../access/Ownable.sol";
 import "../token/ERC20/SafeERC20.sol";
 import "../token/ERC20/ERC20.sol";
-import "../token/ERC20/ERC20Detailed.sol";
 
 /**
  * @dev A xref:ROOT:gsn-strategies.adoc#gsn-strategies[GSN strategy] that charges transaction fees in a special purpose ERC20
@@ -30,7 +29,7 @@ contract GSNRecipientERC20Fee is GSNRecipient {
      * @dev The arguments to the constructor are the details that the gas payment token will have: `name` and `symbol`. `decimals` is hard-coded to 18.
      */
     constructor(string memory name, string memory symbol) public {
-        _token = new __unstable__ERC20Owned(name, symbol, 18);
+        _token = new __unstable__ERC20Owned(name, symbol);
     }
 
     /**
@@ -112,10 +111,10 @@ contract GSNRecipientERC20Fee is GSNRecipient {
  * outside of this context.
  */
 // solhint-disable-next-line contract-name-camelcase
-contract __unstable__ERC20Owned is ERC20, ERC20Detailed, Ownable {
+contract __unstable__ERC20Owned is ERC20, Ownable {
     uint256 private constant _UINT256_MAX = 2**256 - 1;
 
-    constructor(string memory name, string memory symbol, uint8 decimals) public ERC20Detailed(name, symbol, decimals) { }
+    constructor(string memory name, string memory symbol) public ERC20(name, symbol) { }
 
     // The owner (GSNRecipientERC20Fee) can mint tokens
     function mint(address account, uint256 amount) public onlyOwner {
@@ -123,7 +122,7 @@ contract __unstable__ERC20Owned is ERC20, ERC20Detailed, Ownable {
     }
 
     // The owner has 'infinite' allowance for all token holders
-    function allowance(address tokenOwner, address spender) public view override(ERC20, IERC20) returns (uint256) {
+    function allowance(address tokenOwner, address spender) public view override returns (uint256) {
         if (spender == owner()) {
             return _UINT256_MAX;
         } else {
@@ -140,7 +139,7 @@ contract __unstable__ERC20Owned is ERC20, ERC20Detailed, Ownable {
         }
     }
 
-    function transferFrom(address sender, address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
+    function transferFrom(address sender, address recipient, uint256 amount) public override returns (bool) {
         if (recipient == owner()) {
             _transfer(sender, recipient, amount);
             return true;

contracts/mocks/Create2Impl.sol

 pragma solidity ^0.6.0;
 
 import "../utils/Create2.sol";
-import "../token/ERC20/ERC20.sol";
+import "../introspection/ERC1820Implementer.sol";
 
 contract Create2Impl {
     function deploy(uint256 value, bytes32 salt, bytes memory code) public {
         Create2.deploy(value, salt, code);
     }
 
-    function deployERC20(uint256 value, bytes32 salt) public {
+    function deployERC1820Implementer(uint256 value, bytes32 salt) public {
         // solhint-disable-next-line indent
-        Create2.deploy(value, salt, type(ERC20).creationCode);
+        Create2.deploy(value, salt, type(ERC1820Implementer).creationCode);
     }
 
     function computeAddress(bytes32 salt, bytes32 codeHash) public view returns (address) {

contracts/mocks/ERC20BurnableMock.sol

@@ -3,7 +3,12 @@ pragma solidity ^0.6.0;
 import "../token/ERC20/ERC20Burnable.sol";
 
 contract ERC20BurnableMock is ERC20Burnable {
-    constructor (address initialAccount, uint256 initialBalance) public {
+    constructor (
+        string memory name,
+        string memory symbol,
+        address initialAccount,
+        uint256 initialBalance
+    ) public ERC20(name, symbol) {
         _mint(initialAccount, initialBalance);
     }
 }

contracts/mocks/ERC20CappedMock.sol

@@ -3,7 +3,9 @@ pragma solidity ^0.6.0;
 import "../token/ERC20/ERC20Capped.sol";
 
 contract ERC20CappedMock is ERC20Capped {
-    constructor (uint256 cap) public ERC20Capped(cap) { }
+    constructor (string memory name, string memory symbol, uint256 cap)
+        public ERC20(name, symbol) ERC20Capped(cap)
+    { }
 
     function mint(address to, uint256 tokenId) public {
         _mint(to, tokenId);

contracts/mocks/ERC20DecimalsMock.sol

+pragma solidity ^0.6.0;
+
+import "../token/ERC20/ERC20.sol";
+
+contract ERC20DecimalsMock is ERC20 {
+    constructor (string memory name, string memory symbol, uint8 decimals) public ERC20(name, symbol) {
+        _setupDecimals(decimals);
+    }
+
+    function setupDecimals(uint8 decimals) public {
+        _setupDecimals(decimals);
+    }
+}

contracts/mocks/ERC20DetailedMock.sol

-pragma solidity ^0.6.0;
-
-import "../token/ERC20/ERC20.sol";
-import "../token/ERC20/ERC20Detailed.sol";
-
-contract ERC20DetailedMock is ERC20, ERC20Detailed {
-    constructor (string memory name, string memory symbol, uint8 decimals)
-        public
-        ERC20Detailed(name, symbol, decimals)
-    {
-
-    }
-}

contracts/mocks/ERC20Mock.sol

@@ -4,7 +4,12 @@ import "../token/ERC20/ERC20.sol";
 
 // mock class using ERC20
 contract ERC20Mock is ERC20 {
-    constructor (address initialAccount, uint256 initialBalance) public payable {
+    constructor (
+        string memory name,
+        string memory symbol,
+        address initialAccount,
+        uint256 initialBalance
+    ) public payable ERC20(name, symbol) {
         _mint(initialAccount, initialBalance);
     }
 

contracts/mocks/ERC20PausableMock.sol

@@ -4,7 +4,12 @@ import "../token/ERC20/ERC20Pausable.sol";
 
 // mock class using ERC20Pausable
 contract ERC20PausableMock is ERC20Pausable {
-    constructor (address initialAccount, uint256 initialBalance) public {
+    constructor (
+        string memory name,
+        string memory symbol,
+        address initialAccount,
+        uint256 initialBalance
+    ) public ERC20(name, symbol) {
         _mint(initialAccount, initialBalance);
     }
 

contracts/mocks/ERC20SnapshotMock.sol

@@ -4,7 +4,12 @@ import "../token/ERC20/ERC20Snapshot.sol";
 
 
 contract ERC20SnapshotMock is ERC20Snapshot {
-    constructor(address initialAccount, uint256 initialBalance) public {
+    constructor(
+        string memory name,
+        string memory symbol,
+        address initialAccount,
+        uint256 initialBalance
+    ) public ERC20(name, symbol) {
         _mint(initialAccount, initialBalance);
     }
 

contracts/token/ERC20/ERC20.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.6.0;
 import "../../GSN/Context.sol";
 import "./IERC20.sol";
 import "../../math/SafeMath.sol";
+import "../../utils/Address.sol";
 
 /**
  * @dev Implementation of the {IERC20} interface.
@@ -30,6 +31,7 @@ import "../../math/SafeMath.sol";
  */
 contract ERC20 is Context, IERC20 {
     using SafeMath for uint256;
+    using Address for address;
 
     mapping (address => uint256) private _balances;
 
@@ -37,6 +39,57 @@ contract ERC20 is Context, IERC20 {
 
     uint256 private _totalSupply;
 
+    string private _name;
+    string private _symbol;
+    uint8 private _decimals;
+
+    /**
+     * @dev Sets the values for {name} and {symbol}, initializes {decimals} with
+     * a default value of 18.
+     *
+     * To select a different value for {decimals}, use {_setupDecimals}.
+     *
+     * All three of these values are immutable: they can only be set once during
+     * construction.
+     */
+    constructor (string memory name, string memory symbol) public {
+        _name = name;
+        _symbol = symbol;
+        _decimals = 18;
+    }
+
+    /**
+     * @dev Returns the name of the token.
+     */
+    function name() public view returns (string memory) {
+        return _name;
+    }
+
+    /**
+     * @dev Returns the symbol of the token, usually a shorter version of the
+     * name.
+     */
+    function symbol() public view returns (string memory) {
+        return _symbol;
+    }
+
+    /**
+     * @dev Returns the number of decimals used to get its user representation.
+     * For example, if `decimals` equals `2`, a balance of `505` tokens should
+     * be displayed to a user as `5,05` (`505 / 10 ** 2`).
+     *
+     * Tokens usually opt for a value of 18, imitating the relationship between
+     * Ether and Wei. This is the value {ERC20} uses, unless {_setupDecimals} is
+     * called.
+     *
+     * NOTE: This information is only used for _display_ purposes: it in
+     * no way affects any of the arithmetic of the contract, including
+     * {IERC20-balanceOf} and {IERC20-transfer}.
+     */
+    function decimals() public view returns (uint8) {
+        return _decimals;
+    }
+
     /**
      * @dev See {IERC20-totalSupply}.
      */
@@ -224,6 +277,18 @@ contract ERC20 is Context, IERC20 {
     }
 
     /**
+     * @dev Sets {decimals} to a value other than the default one of 18.
+     *
+     * Requirements:
+     *
+     * - this function can only be called from a constructor.
+     */
+    function _setupDecimals(uint8 decimals_) internal {
+        require(!address(this).isContract(), "ERC20: decimals cannot be changed after construction");
+        _decimals = decimals_;
+    }
+
+    /**
      * @dev Hook that is called before any transfer of tokens. This includes
      * minting and burning.
      *

contracts/token/ERC20/ERC20Burnable.sol

@@ -8,7 +8,7 @@ import "./ERC20.sol";
  * tokens and those that they have an allowance for, in a way that can be
  * recognized off-chain (via event analysis).
  */
-contract ERC20Burnable is Context, ERC20 {
+abstract contract ERC20Burnable is Context, ERC20 {
     /**
      * @dev Destroys `amount` tokens from the caller.
      *

contracts/token/ERC20/ERC20Capped.sol

@@ -5,7 +5,7 @@ import "./ERC20.sol";
 /**
  * @dev Extension of {ERC20} that adds a cap to the supply of tokens.
  */
-contract ERC20Capped is ERC20 {
+abstract contract ERC20Capped is ERC20 {
     uint256 private _cap;
 
     /**

contracts/token/ERC20/ERC20Detailed.sol

-pragma solidity ^0.6.0;
-
-import "./IERC20.sol";
-
-/**
- * @dev Optional functions from the ERC20 standard.
- */
-abstract contract ERC20Detailed is IERC20 {
-    string private _name;
-    string private _symbol;
-    uint8 private _decimals;
-
-    /**
-     * @dev Sets the values for `name`, `symbol`, and `decimals`. All three of
-     * these values are immutable: they can only be set once during
-     * construction.
-     */
-    constructor (string memory name, string memory symbol, uint8 decimals) public {
-        _name = name;
-        _symbol = symbol;
-        _decimals = decimals;
-    }
-
-    /**
-     * @dev Returns the name of the token.
-     */
-    function name() public view returns (string memory) {
-        return _name;
-    }
-
-    /**
-     * @dev Returns the symbol of the token, usually a shorter version of the
-     * name.
-     */
-    function symbol() public view returns (string memory) {
-        return _symbol;
-    }
-
-    /**
-     * @dev Returns the number of decimals used to get its user representation.
-     * For example, if `decimals` equals `2`, a balance of `505` tokens should
-     * be displayed to a user as `5,05` (`505 / 10 ** 2`).
-     *
-     * Tokens usually opt for a value of 18, imitating the relationship between
-     * Ether and Wei.
-     *
-     * NOTE: This information is only used for _display_ purposes: it in
-     * no way affects any of the arithmetic of the contract, including
-     * {IERC20-balanceOf} and {IERC20-transfer}.
-     */
-    function decimals() public view returns (uint8) {
-        return _decimals;
-    }
-}

contracts/token/ERC20/ERC20Pausable.sol

@@ -11,7 +11,7 @@ import "../../utils/Pausable.sol";
  * period, or having an emergency switch for freezing all token transfers in the
  * event of a large bug.
  */
-contract ERC20Pausable is ERC20, Pausable {
+abstract contract ERC20Pausable is ERC20, Pausable {
     /**
      * @dev See {ERC20-_beforeTokenTransfer}.
      *

contracts/token/ERC20/ERC20Snapshot.sol

@@ -17,7 +17,7 @@ import "./ERC20.sol";
  * account address.
  * @author Validity Labs AG <info@validitylabs.org>
  */
-contract ERC20Snapshot is ERC20 {
+abstract contract ERC20Snapshot is ERC20 {
     // Inspired by Jordi Baylina's MiniMeToken to record historical balances:
     // https://github.com/Giveth/minimd/blob/ea04d950eea153a04c51fa510b068b9dded390cb/contracts/MiniMeToken.sol
 

test/GSN/GSNRecipientERC20Fee.test.js

@@ -6,7 +6,7 @@ const gsn = require('@openzeppelin/gsn-helpers');
 const { expect } = require('chai');
 
 const GSNRecipientERC20FeeMock = contract.fromArtifact('GSNRecipientERC20FeeMock');
-const ERC20Detailed = contract.fromArtifact('ERC20Detailed');
+const ERC20 = contract.fromArtifact('ERC20');
 const IRelayHub = contract.fromArtifact('IRelayHub');
 
 describe('GSNRecipientERC20Fee', function () {
@@ -17,7 +17,7 @@ describe('GSNRecipientERC20Fee', function () {
 
   beforeEach(async function () {
     this.recipient = await GSNRecipientERC20FeeMock.new(name, symbol);
-    this.token = await ERC20Detailed.at(await this.recipient.token());
+    this.token = await ERC20.at(await this.recipient.token());
   });
 
   describe('token', function () {

test/token/ERC20/ERC20.test.js

@@ -11,14 +11,45 @@ const {
 } = require('./ERC20.behavior');
 
 const ERC20Mock = contract.fromArtifact('ERC20Mock');
+const ERC20DecimalsMock = contract.fromArtifact('ERC20DecimalsMock');
 
 describe('ERC20', function () {
   const [ initialHolder, recipient, anotherAccount ] = accounts;
 
+  const name = 'My Token';
+  const symbol = 'MTKN';
+
   const initialSupply = new BN(100);
 
   beforeEach(async function () {
-    this.token = await ERC20Mock.new(initialHolder, initialSupply);
+    this.token = await ERC20Mock.new(name, symbol, initialHolder, initialSupply);
+  });
+
+  it('has a name', async function () {
+    expect(await this.token.name()).to.equal(name);
+  });
+
+  it('has a symbol', async function () {
+    expect(await this.token.symbol()).to.equal(symbol);
+  });
+
+  it('has 18 decimals', async function () {
+    expect(await this.token.decimals()).to.be.bignumber.equal('18');
+  });
+
+  describe('_setupDecimals', function () {
+    const decimals = new BN(6);
+
+    it('can set decimals during construction', async function () {
+      const token = await ERC20DecimalsMock.new(name, symbol, decimals);
+      expect(await token.decimals()).to.be.bignumber.equal(decimals);
+    });
+
+    it('reverts if setting decimals after construction', async function () {
+      const token = await ERC20DecimalsMock.new(name, symbol, decimals);
+
+      await expectRevert(token.setupDecimals(decimals.addn(1)), 'ERC20: decimals cannot be changed after construction');
+    });
   });
 
   shouldBehaveLikeERC20('ERC20', initialSupply, initialHolder, recipient, anotherAccount);

test/token/ERC20/ERC20Burnable.test.js

@@ -10,8 +10,11 @@ describe('ERC20Burnable', function () {
 
   const initialBalance = new BN(1000);
 
+  const name = 'My Token';
+  const symbol = 'MTKN';
+
   beforeEach(async function () {
-    this.token = await ERC20BurnableMock.new(owner, initialBalance, { from: owner });
+    this.token = await ERC20BurnableMock.new(name, symbol, owner, initialBalance, { from: owner });
   });
 
   shouldBehaveLikeERC20Burnable(owner, initialBalance, otherAccounts);

test/token/ERC20/ERC20Capped.test.js

@@ -10,15 +10,18 @@ describe('ERC20Capped', function () {
 
   const cap = ether('1000');
 
+  const name = 'My Token';
+  const symbol = 'MTKN';
+
   it('requires a non-zero cap', async function () {
     await expectRevert(
-      ERC20Capped.new(new BN(0), { from: minter }), 'ERC20Capped: cap is 0'
+      ERC20Capped.new(name, symbol, new BN(0), { from: minter }), 'ERC20Capped: cap is 0'
     );
   });
 
   context('once deployed', async function () {
     beforeEach(async function () {
-      this.token = await ERC20Capped.new(cap, { from: minter });
+      this.token = await ERC20Capped.new(name, symbol, cap, { from: minter });
     });
 
     shouldBehaveLikeERC20Capped(minter, otherAccounts, cap);

test/token/ERC20/ERC20Detailed.test.js

-const { contract } = require('@openzeppelin/test-environment');
-const { BN } = require('@openzeppelin/test-helpers');
-
-const { expect } = require('chai');
-
-const ERC20DetailedMock = contract.fromArtifact('ERC20DetailedMock');
-
-describe('ERC20Detailed', function () {
-  const _name = 'My Detailed ERC20';
-  const _symbol = 'MDT';
-  const _decimals = new BN(18);
-
-  beforeEach(async function () {
-    this.detailedERC20 = await ERC20DetailedMock.new(_name, _symbol, _decimals);
-  });
-
-  it('has a name', async function () {
-    expect(await this.detailedERC20.name()).to.equal(_name);
-  });
-
-  it('has a symbol', async function () {
-    expect(await this.detailedERC20.symbol()).to.equal(_symbol);
-  });
-
-  it('has an amount of decimals', async function () {
-    expect(await this.detailedERC20.decimals()).to.be.bignumber.equal(_decimals);
-  });
-});

test/token/ERC20/ERC20Pausable.test.js

@@ -11,8 +11,11 @@ describe('ERC20Pausable', function () {
 
   const initialSupply = new BN(100);
 
+  const name = 'My Token';
+  const symbol = 'MTKN';
+
   beforeEach(async function () {
-    this.token = await ERC20PausableMock.new(holder, initialSupply);
+    this.token = await ERC20PausableMock.new(name, symbol, holder, initialSupply);
   });
 
   describe('pausable token', function () {

test/token/ERC20/ERC20Snapshot.test.js

@@ -10,8 +10,11 @@ describe('ERC20Snapshot', function () {
 
   const initialSupply = new BN(100);
 
+  const name = 'My Token';
+  const symbol = 'MTKN';
+
   beforeEach(async function () {
-    this.token = await ERC20SnapshotMock.new(initialHolder, initialSupply);
+    this.token = await ERC20SnapshotMock.new(name, symbol, initialHolder, initialSupply);
   });
 
   describe('snapshot', function () {

test/token/ERC20/TokenTimelock.test.js

@@ -10,11 +10,14 @@ const TokenTimelock = contract.fromArtifact('TokenTimelock');
 describe('TokenTimelock', function () {
   const [ beneficiary ] = accounts;
 
+  const name = 'My Token';
+  const symbol = 'MTKN';
+
   const amount = new BN(100);
 
   context('with token', function () {
     beforeEach(async function () {
-      this.token = await ERC20Mock.new(beneficiary, 0); // We're not using the preminted tokens
+      this.token = await ERC20Mock.new(name, symbol, beneficiary, 0); // We're not using the preminted tokens
     });
 
     it('rejects a release time in the past', async function () {

test/utils/Create2.test.js

@@ -5,16 +5,20 @@ const { expect } = require('chai');
 
 const Create2Impl = contract.fromArtifact('Create2Impl');
 const ERC20Mock = contract.fromArtifact('ERC20Mock');
-const ERC20 = contract.fromArtifact('ERC20');
+const ERC1820Implementer = contract.fromArtifact('ERC1820Implementer');
 
 describe('Create2', function () {
   const [deployerAccount] = accounts;
 
   const salt = 'salt message';
   const saltHex = web3.utils.soliditySha3(salt);
-  const constructorByteCode = `${ERC20Mock.bytecode}${web3.eth.abi
-    .encodeParameters(['address', 'uint256'], [deployerAccount, 100]).slice(2)
-  }`;
+
+  const encodedParams = web3.eth.abi.encodeParameters(
+    ['string', 'string', 'address', 'uint256'],
+    ['MyToken', 'MTKN', deployerAccount, 100]
+  ).slice(2);
+
+  const constructorByteCode = `${ERC20Mock.bytecode}${encodedParams}`;
 
   beforeEach(async function () {
     this.factory = await Create2Impl.new();
@@ -36,19 +40,20 @@ describe('Create2', function () {
     expect(onChainComputed).to.equal(offChainComputed);
   });
 
-  it('should deploy a ERC20 from inline assembly code', async function () {
+  it('should deploy a ERC1820Implementer from inline assembly code', async function () {
     const offChainComputed =
-      computeCreate2Address(saltHex, ERC20.bytecode, this.factory.address);
-    await this.factory
-      .deploy(0, saltHex, ERC20.bytecode, { from: deployerAccount });
-    expect(ERC20.bytecode).to.include((await web3.eth.getCode(offChainComputed)).slice(2));
+      computeCreate2Address(saltHex, ERC1820Implementer.bytecode, this.factory.address);
+
+    await this.factory.deployERC1820Implementer(0, saltHex);
+
+    expect(ERC1820Implementer.bytecode).to.include((await web3.eth.getCode(offChainComputed)).slice(2));
   });
 
   it('should deploy a ERC20Mock with correct balances', async function () {
-    const offChainComputed =
-      computeCreate2Address(saltHex, constructorByteCode, this.factory.address);
-    await this.factory
-      .deploy(0, saltHex, constructorByteCode, { from: deployerAccount });
+    const offChainComputed = computeCreate2Address(saltHex, constructorByteCode, this.factory.address);
+
+    await this.factory.deploy(0, saltHex, constructorByteCode);
+
     const erc20 = await ERC20Mock.at(offChainComputed);
     expect(await erc20.balanceOf(deployerAccount)).to.be.bignumber.equal(new BN(100));
   });