Merge branch 'master' into access-control

CHANGELOG.md

@@ -7,6 +7,17 @@
 
 ### Breaking changes
  * `Roles` was removed, use `AccessControl` as a replacement. ([#2112](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2112))
+ * `ECDSA`: when receiving an invalid signature, `recover` now reverts instead of returning the zero address. ([#2114](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2114))
+ * `Pausable`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `Strings`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `Counters`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `SignedSafeMath`: moved to the `math` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `ERC20Snapshot`: moved to the `token/ERC20` directory. `snapshot` was changed into an `internal` function. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
+ * `Ownable`: moved to the `access` directory. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))
+ * `Ownable`: removed `isOwner`. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))
+ * `Secondary`: removed from the library, use `Ownable` instead. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))
+ * `Escrow`, `ConditionalEscrow`, `RefundEscrow`: these now use `Ownable` instead of `Secondary`, their external API changed accordingly. ([#2120](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2120))
+ * `ERC20`: removed `_burnFrom`. ([#2119](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2119))
 
 ## 2.5.0 (2020-02-04)
 

contracts/GSN/GSNRecipientERC20Fee.sol

@@ -2,7 +2,7 @@ pragma solidity ^0.6.0;
 
 import "./GSNRecipient.sol";
 import "../math/SafeMath.sol";
-import "../ownership/Secondary.sol";
+import "../access/Ownable.sol";
 import "../token/ERC20/SafeERC20.sol";
 import "../token/ERC20/ERC20.sol";
 import "../token/ERC20/ERC20Detailed.sol";
@@ -17,20 +17,20 @@ import "../token/ERC20/ERC20Detailed.sol";
  * internal {_mint} function.
  */
 contract GSNRecipientERC20Fee is GSNRecipient {
-    using SafeERC20 for __unstable__ERC20PrimaryAdmin;
+    using SafeERC20 for __unstable__ERC20Owned;
     using SafeMath for uint256;
 
     enum GSNRecipientERC20FeeErrorCodes {
         INSUFFICIENT_BALANCE
     }
 
-    __unstable__ERC20PrimaryAdmin private _token;
+    __unstable__ERC20Owned private _token;
 
     /**
      * @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__ERC20PrimaryAdmin(name, symbol, 18);
+        _token = new __unstable__ERC20Owned(name, symbol, 18);
     }
 
     /**
@@ -106,42 +106,42 @@ contract GSNRecipientERC20Fee is GSNRecipient {
 }
 
 /**
- * @title __unstable__ERC20PrimaryAdmin
+ * @title __unstable__ERC20Owned
  * @dev An ERC20 token owned by another contract, which has minting permissions and can use transferFrom to receive
  * anyone's tokens. This contract is an internal helper for GSNRecipientERC20Fee, and should not be used
  * outside of this context.
  */
 // solhint-disable-next-line contract-name-camelcase
-contract __unstable__ERC20PrimaryAdmin is ERC20, ERC20Detailed, Secondary {
+contract __unstable__ERC20Owned is ERC20, ERC20Detailed, Ownable {
     uint256 private constant UINT256_MAX = 2**256 - 1;
 
     constructor(string memory name, string memory symbol, uint8 decimals) public ERC20Detailed(name, symbol, decimals) { }
 
-    // The primary account (GSNRecipientERC20Fee) can mint tokens
-    function mint(address account, uint256 amount) public onlyPrimary {
+    // The owner (GSNRecipientERC20Fee) can mint tokens
+    function mint(address account, uint256 amount) public onlyOwner {
         _mint(account, amount);
     }
 
-    // The primary account has 'infinite' allowance for all token holders
-    function allowance(address owner, address spender) public view override(ERC20, IERC20) returns (uint256) {
-        if (spender == primary()) {
+    // The owner has 'infinite' allowance for all token holders
+    function allowance(address tokenOwner, address spender) public view override(ERC20, IERC20) returns (uint256) {
+        if (spender == owner()) {
             return UINT256_MAX;
         } else {
-            return super.allowance(owner, spender);
+            return super.allowance(tokenOwner, spender);
         }
     }
 
-    // Allowance for the primary account cannot be changed (it is always 'infinite')
-    function _approve(address owner, address spender, uint256 value) internal override {
-        if (spender == primary()) {
+    // Allowance for the owner cannot be changed (it is always 'infinite')
+    function _approve(address tokenOwner, address spender, uint256 value) internal override {
+        if (spender == owner()) {
             return;
         } else {
-            super._approve(owner, spender, value);
+            super._approve(tokenOwner, spender, value);
         }
     }
 
     function transferFrom(address sender, address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
-        if (recipient == primary()) {
+        if (recipient == owner()) {
             _transfer(sender, recipient, amount);
             return true;
         } else {

contracts/ownership/Ownable.sol → contracts/access/Ownable.sol

@@ -6,6 +6,9 @@ import "../GSN/Context.sol";
  * there is an account (an owner) that can be granted exclusive access to
  * specific functions.
  *
+ * By default, the owner account will be the one that deploys the contract. This
+ * can later be changed with {transferOwnership}.
+ *
  * This module is used through inheritance. It will make available the modifier
  * `onlyOwner`, which can be applied to your functions to restrict their use to
  * the owner.
@@ -35,17 +38,10 @@ contract Ownable is Context {
      * @dev Throws if called by any account other than the owner.
      */
     modifier onlyOwner() {
-        require(isOwner(), "Ownable: caller is not the owner");
+        require(_owner == _msgSender(), "Ownable: caller is not the owner");
         _;
     }
 
-    /**
-     * @dev Returns true if the caller is the current owner.
-     */
-    function isOwner() public view returns (bool) {
-        return _msgSender() == _owner;
-    }
-
     /**
      * @dev Leaves the contract without owner. It will not be possible to call
      * `onlyOwner` functions anymore. Can only be called by the current owner.

contracts/access/README.adoc

@@ -1,7 +1,9 @@
 = Access
 
-NOTE: This page is incomplete. We're working to improve it for the next release. Stay tuned!
+Contract modules for authorization and access control mechanisms.
 
-== Library
+== Contracts
+
+{{Ownable}}
 
 {{AccessControl}}

contracts/cryptography/ECDSA.sol

@@ -15,10 +15,6 @@ library ECDSA {
      * this function rejects them by requiring the `s` value to be in the lower
      * half order, and the `v` value to be either 27 or 28.
      *
-     * NOTE: This call _does not revert_ if the signature is invalid, or
-     * if the signer is otherwise unable to be retrieved. In those scenarios,
-     * the zero address is returned.
-     *
      * IMPORTANT: `hash` _must_ be the result of a hash operation for the
      * verification to be secure: it is possible to craft signatures that
      * recover to arbitrary addresses for non-hashed data. A safe way to ensure
@@ -28,7 +24,7 @@ library ECDSA {
     function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
         // Check the signature length
         if (signature.length != 65) {
-            return (address(0));
+            revert("ECDSA: invalid signature length");
         }
 
         // Divide the signature in r, s and v variables
@@ -55,15 +51,18 @@ library ECDSA {
         // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept
         // these malleable signatures as well.
         if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
-            return address(0);
+            revert("ECDSA: invalid signature 's' value");
         }
 
         if (v != 27 && v != 28) {
-            return address(0);
+            revert("ECDSA: invalid signature 'v' value");
         }
 
         // If the signature is valid (and not malleable), return the signer address
-        return ecrecover(hash, v, r, s);
+        address signer = ecrecover(hash, v, r, s);
+        require(signer != address(0), "ECDSA: invalid signature");
+
+        return signer;
     }
 
     /**

contracts/drafts/TokenVesting.sol

@@ -1,7 +1,7 @@
 pragma solidity ^0.6.0;
 
 import "../token/ERC20/SafeERC20.sol";
-import "../ownership/Ownable.sol";
+import "../access/Ownable.sol";
 import "../math/SafeMath.sol";
 
 /**

contracts/math/README.adoc

@@ -6,4 +6,6 @@ These are math-related utilities.
 
 {{SafeMath}}
 
+{{SignedSafeMath}}
+
 {{Math}}

contracts/mocks/CountersImpl.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/Counters.sol";
+import "../utils/Counters.sol";
 
 contract CountersImpl {
     using Counters for Counters.Counter;

contracts/mocks/Create2Impl.sol

@@ -13,15 +13,11 @@ contract Create2Impl {
         Create2.deploy(salt, type(ERC20).creationCode);
     }
 
-    function computeAddress(bytes32 salt, bytes memory code) public view returns (address) {
-        return Create2.computeAddress(salt, code);
+    function computeAddress(bytes32 salt, bytes32 codeHash) public view returns (address) {
+        return Create2.computeAddress(salt, codeHash);
     }
 
-    function computeAddress(bytes32 salt, bytes memory code, address deployer) public pure returns (address) {
-        return Create2.computeAddress(salt, code, deployer);
-    }
-
-    function computeAddress(bytes32 salt, bytes32 codeHash, address deployer) public pure returns (address) {
+    function computeAddressWithDeployer(bytes32 salt, bytes32 codeHash, address deployer) public pure returns (address) {
         return Create2.computeAddress(salt, codeHash, deployer);
     }
 }

contracts/mocks/ERC20Mock.sol

@@ -16,10 +16,6 @@ contract ERC20Mock is ERC20 {
         _burn(account, amount);
     }
 
-    function burnFrom(address account, uint256 amount) public {
-        _burnFrom(account, amount);
-    }
-
     function transferInternal(address from, address to, uint256 value) public {
         _transfer(from, to, value);
     }

contracts/mocks/ERC20SnapshotMock.sol

@@ -1,13 +1,17 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/ERC20Snapshot.sol";
+import "../token/ERC20/ERC20Snapshot.sol";
 
 
 contract ERC20SnapshotMock is ERC20Snapshot {
     constructor(address initialAccount, uint256 initialBalance) public {
         _mint(initialAccount, initialBalance);
     }
 
+    function snapshot() public {
+        _snapshot();
+    }
+
     function mint(address account, uint256 amount) public {
         _mint(account, amount);
     }

contracts/mocks/OwnableMock.sol

@@ -1,5 +1,5 @@
 pragma solidity ^0.6.0;
 
-import "../ownership/Ownable.sol";
+import "../access/Ownable.sol";
 
 contract OwnableMock is Ownable { }

contracts/mocks/PausableMock.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../lifecycle/Pausable.sol";
+import "../utils/Pausable.sol";
 
 contract PausableMock is Pausable {
     bool public drasticMeasureTaken;

contracts/mocks/SignedSafeMathMock.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/SignedSafeMath.sol";
+import "../math/SignedSafeMath.sol";
 
 contract SignedSafeMathMock {
     function mul(int256 a, int256 b) public pure returns (int256) {

contracts/mocks/StringsMock.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.6.0;
 
-import "../drafts/Strings.sol";
+import "../utils/Strings.sol";
 
 contract StringsMock {
     function fromUint256(uint256 value) public pure returns (string memory) {