Browse Source

Merge branch 'OpenZeppelin:master' into certora/erc1155ext

teryanarmen 3 years ago
parent
commit
6820ff8b9c

+ 3 - 1
CHANGELOG.md

@@ -2,12 +2,13 @@
 
 ## Unreleased
 
- * `Clones`: optimize clone creation ([#3329](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3329))
  * `TimelockController`: Migrate `_call` to `_execute` and allow inheritance and overriding similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317))
  * `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380))
  * `ERC20FlashMint`: Add customizable flash fee receiver. ([#3327](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3327))
  * `ERC20TokenizedVault`: add an extension of `ERC20` that implements the ERC4626 Tokenized Vault Standard. ([#3171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3171))
+ * `SafeERC20`: add `safePermit` as mitigation against phantom permit functions. ([#3280](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3280))
  * `Math`: add a `mulDiv` function that can round the result either up or down. ([#3171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3171))
+ * `Math`: Add a `sqrt` function to compute square roots of integers, rounding either up or down. ([#3242](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3242))
  * `Strings`: add a new overloaded function `toHexString` that converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal representation. ([#3403](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3403))
  * `EnumerableMap`: add new `UintToUintMap` map type. ([#3338](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3338))
  * `EnumerableMap`: add new `Bytes32ToUintMap` map type. ([#3416](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3416))
@@ -18,6 +19,7 @@
  * `ERC721`: removed redundant require statement. ([#3434](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3434))
  * `PaymentSplitter`: add `releasable` getters. ([#3350](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3350))
  * `Initializable`: refactored implementation of modifiers for easier understanding. ([#3450](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3450))
+ * `Proxies`: remove runtime check of ERC1967 storage slots. ([#3455](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3455))
 
 ### Breaking changes
 

+ 10 - 3
contracts/access/Ownable.sol

@@ -29,6 +29,14 @@ abstract contract Ownable is Context {
         _transferOwnership(_msgSender());
     }
 
+    /**
+     * @dev Throws if called by any account other than the owner.
+     */
+    modifier onlyOwner() {
+        _checkOwner();
+        _;
+    }
+
     /**
      * @dev Returns the address of the current owner.
      */
@@ -37,11 +45,10 @@ abstract contract Ownable is Context {
     }
 
     /**
-     * @dev Throws if called by any account other than the owner.
+     * @dev Throws if the sender is not the owner.
      */
-    modifier onlyOwner() {
+    function _checkOwner() internal view virtual {
         require(owner() == _msgSender(), "Ownable: caller is not the owner");
-        _;
     }
 
     /**

+ 5 - 0
contracts/metatx/MinimalForwarder.sol

@@ -8,6 +8,11 @@ import "../utils/cryptography/draft-EIP712.sol";
 
 /**
  * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
+ *
+ * MinimalForwarder is mainly meant for testing, as it is missing features to be a good production-ready forwarder. This
+ * contract does not intend to have all the properties that are needed for a sound forwarding system. A fully
+ * functioning forwarding system with good properties requires more complexity. We suggest you look at other projects
+ * such as the GSN which do have the goal of building a system like that.
  */
 contract MinimalForwarder is EIP712 {
     using ECDSA for bytes32;

+ 4 - 0
contracts/mocks/MathMock.sol

@@ -29,4 +29,8 @@ contract MathMock {
     ) public pure returns (uint256) {
         return Math.mulDiv(a, b, denominator, direction);
     }
+
+    function sqrt(uint256 a, Math.Rounding direction) public pure returns (uint256) {
+        return Math.sqrt(a, direction);
+    }
 }

+ 50 - 0
contracts/mocks/SafeERC20Helper.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.0;
 
 import "../utils/Context.sol";
 import "../token/ERC20/IERC20.sol";
+import "../token/ERC20/extensions/draft-ERC20Permit.sol";
 import "../token/ERC20/utils/SafeERC20.sol";
 
 contract ERC20ReturnFalseMock is Context {
@@ -105,6 +106,43 @@ contract ERC20NoReturnMock is Context {
     }
 }
 
+contract ERC20PermitNoRevertMock is
+    ERC20("ERC20PermitNoRevertMock", "ERC20PermitNoRevertMock"),
+    ERC20Permit("ERC20PermitNoRevertMock")
+{
+    function getChainId() external view returns (uint256) {
+        return block.chainid;
+    }
+
+    function permitThatMayRevert(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual {
+        super.permit(owner, spender, value, deadline, v, r, s);
+    }
+
+    function permit(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual override {
+        try this.permitThatMayRevert(owner, spender, value, deadline, v, r, s) {
+            // do nothing
+        } catch {
+            // do nothing
+        }
+    }
+}
+
 contract SafeERC20Wrapper is Context {
     using SafeERC20 for IERC20;
 
@@ -134,6 +172,18 @@ contract SafeERC20Wrapper is Context {
         _token.safeDecreaseAllowance(address(0), amount);
     }
 
+    function permit(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public {
+        SafeERC20.safePermit(IERC20Permit(address(_token)), owner, spender, value, deadline, v, r, s);
+    }
+
     function setAllowance(uint256 allowance_) public {
         ERC20ReturnTrueMock(address(_token)).setAllowance(allowance_);
     }

+ 15 - 15
contracts/proxy/Clones.sol

@@ -26,10 +26,10 @@ library Clones {
         /// @solidity memory-safe-assembly
         assembly {
             let ptr := mload(0x40)
-            mstore(ptr, 0x602d8060093d393df3363d3d373d3d3d363d7300000000000000000000000000)
-            mstore(add(ptr, 0x13), shl(0x60, implementation))
-            mstore(add(ptr, 0x27), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
-            instance := create(0, ptr, 0x36)
+            mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
+            mstore(add(ptr, 0x14), shl(0x60, implementation))
+            mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
+            instance := create(0, ptr, 0x37)
         }
         require(instance != address(0), "ERC1167: create failed");
     }
@@ -45,10 +45,10 @@ library Clones {
         /// @solidity memory-safe-assembly
         assembly {
             let ptr := mload(0x40)
-            mstore(ptr, 0x602d8060093d393df3363d3d373d3d3d363d7300000000000000000000000000)
-            mstore(add(ptr, 0x13), shl(0x60, implementation))
-            mstore(add(ptr, 0x27), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
-            instance := create2(0, ptr, 0x36, salt)
+            mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
+            mstore(add(ptr, 0x14), shl(0x60, implementation))
+            mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
+            instance := create2(0, ptr, 0x37, salt)
         }
         require(instance != address(0), "ERC1167: create2 failed");
     }
@@ -64,13 +64,13 @@ library Clones {
         /// @solidity memory-safe-assembly
         assembly {
             let ptr := mload(0x40)
-            mstore(ptr, 0x602d8060093d393df3363d3d373d3d3d363d7300000000000000000000000000)
-            mstore(add(ptr, 0x13), shl(0x60, implementation))
-            mstore(add(ptr, 0x27), 0x5af43d82803e903d91602b57fd5bf3ff00000000000000000000000000000000)
-            mstore(add(ptr, 0x37), shl(0x60, deployer))
-            mstore(add(ptr, 0x4b), salt)
-            mstore(add(ptr, 0x6b), keccak256(ptr, 0x36))
-            predicted := keccak256(add(ptr, 0x36), 0x55)
+            mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
+            mstore(add(ptr, 0x14), shl(0x60, implementation))
+            mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf3ff00000000000000000000000000000000)
+            mstore(add(ptr, 0x38), shl(0x60, deployer))
+            mstore(add(ptr, 0x4c), salt)
+            mstore(add(ptr, 0x6c), keccak256(ptr, 0x37))
+            predicted := keccak256(add(ptr, 0x37), 0x55)
         }
     }
 

+ 0 - 1
contracts/proxy/ERC1967/ERC1967Proxy.sol

@@ -20,7 +20,6 @@ contract ERC1967Proxy is Proxy, ERC1967Upgrade {
      * function call, and allows initializing the storage of the proxy like a Solidity constructor.
      */
     constructor(address _logic, bytes memory _data) payable {
-        assert(_IMPLEMENTATION_SLOT == bytes32(uint256(keccak256("eip1967.proxy.implementation")) - 1));
         _upgradeToAndCall(_logic, _data, false);
     }
 

+ 0 - 1
contracts/proxy/beacon/BeaconProxy.sol

@@ -28,7 +28,6 @@ contract BeaconProxy is Proxy, ERC1967Upgrade {
      * - `beacon` must be a contract with the interface {IBeacon}.
      */
     constructor(address beacon, bytes memory data) payable {
-        assert(_BEACON_SLOT == bytes32(uint256(keccak256("eip1967.proxy.beacon")) - 1));
         _upgradeBeaconToAndCall(beacon, data, false);
     }
 

+ 0 - 1
contracts/proxy/transparent/TransparentUpgradeableProxy.sol

@@ -36,7 +36,6 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
         address admin_,
         bytes memory _data
     ) payable ERC1967Proxy(_logic, _data) {
-        assert(_ADMIN_SLOT == bytes32(uint256(keccak256("eip1967.proxy.admin")) - 1));
         _changeAdmin(admin_);
     }
 

+ 23 - 9
contracts/security/Pausable.sol

@@ -34,13 +34,6 @@ abstract contract Pausable is Context {
         _paused = false;
     }
 
-    /**
-     * @dev Returns true if the contract is paused, and false otherwise.
-     */
-    function paused() public view virtual returns (bool) {
-        return _paused;
-    }
-
     /**
      * @dev Modifier to make a function callable only when the contract is not paused.
      *
@@ -49,7 +42,7 @@ abstract contract Pausable is Context {
      * - The contract must not be paused.
      */
     modifier whenNotPaused() {
-        require(!paused(), "Pausable: paused");
+        _requireNotPaused();
         _;
     }
 
@@ -61,10 +54,31 @@ abstract contract Pausable is Context {
      * - The contract must be paused.
      */
     modifier whenPaused() {
-        require(paused(), "Pausable: not paused");
+        _requirePaused();
         _;
     }
 
+    /**
+     * @dev Returns true if the contract is paused, and false otherwise.
+     */
+    function paused() public view virtual returns (bool) {
+        return _paused;
+    }
+
+    /**
+     * @dev Throws if the contract is paused.
+     */
+    function _requireNotPaused() internal view virtual {
+        require(!paused(), "Pausable: paused");
+    }
+
+    /**
+     * @dev Throws if the contract is not paused.
+     */
+    function _requirePaused() internal view virtual {
+        require(paused(), "Pausable: not paused");
+    }
+
     /**
      * @dev Triggers stopped state.
      *

+ 17 - 0
contracts/token/ERC20/utils/SafeERC20.sol

@@ -4,6 +4,7 @@
 pragma solidity ^0.8.0;
 
 import "../IERC20.sol";
+import "../extensions/draft-IERC20Permit.sol";
 import "../../../utils/Address.sol";
 
 /**
@@ -79,6 +80,22 @@ library SafeERC20 {
         }
     }
 
+    function safePermit(
+        IERC20Permit token,
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) internal {
+        uint256 nonceBefore = token.nonces(owner);
+        token.permit(owner, spender, value, deadline, v, r, s);
+        uint256 nonceAfter = token.nonces(owner);
+        require(nonceAfter == nonceBefore + 1, "SafeERC20: permit did not succeed");
+    }
+
     /**
      * @dev Imitates a Solidity high-level call (i.e. a regular function call to a contract), relaxing the requirement
      * on the return value: the return value is optional (but if data is returned, it must not be false).

+ 74 - 0
contracts/utils/math/Math.sol

@@ -149,4 +149,78 @@ library Math {
         }
         return result;
     }
+
+    /**
+     * @dev Returns the square root of a number. It the number is not a perfect square, the value is rounded down.
+     *
+     * Inspired by Henry S. Warren, Jr.'s "Hacker's Delight" (Chapter 11).
+     */
+    function sqrt(uint256 a) internal pure returns (uint256) {
+        if (a == 0) {
+            return 0;
+        }
+
+        // For our first guess, we get the biggest power of 2 which is smaller than the square root of the target.
+        // We know that the "msb" (most significant bit) of our target number `a` is a power of 2 such that we have
+        // `msb(a) <= a < 2*msb(a)`.
+        // We also know that `k`, the position of the most significant bit, is such that `msb(a) = 2**k`.
+        // This gives `2**k < a <= 2**(k+1)` → `2**(k/2) <= sqrt(a) < 2 ** (k/2+1)`.
+        // Using an algorithm similar to the msb conmputation, we are able to compute `result = 2**(k/2)` which is a
+        // good first aproximation of `sqrt(a)` with at least 1 correct bit.
+        uint256 result = 1;
+        uint256 x = a;
+        if (x >> 128 > 0) {
+            x >>= 128;
+            result <<= 64;
+        }
+        if (x >> 64 > 0) {
+            x >>= 64;
+            result <<= 32;
+        }
+        if (x >> 32 > 0) {
+            x >>= 32;
+            result <<= 16;
+        }
+        if (x >> 16 > 0) {
+            x >>= 16;
+            result <<= 8;
+        }
+        if (x >> 8 > 0) {
+            x >>= 8;
+            result <<= 4;
+        }
+        if (x >> 4 > 0) {
+            x >>= 4;
+            result <<= 2;
+        }
+        if (x >> 2 > 0) {
+            result <<= 1;
+        }
+
+        // At this point `result` is an estimation with one bit of precision. We know the true value is a uint128,
+        // since it is the square root of a uint256. Newton's method converges quadratically (precision doubles at
+        // every iteration). We thus need at most 7 iteration to turn our partial result with one bit of precision
+        // into the expected uint128 result.
+        unchecked {
+            result = (result + a / result) >> 1;
+            result = (result + a / result) >> 1;
+            result = (result + a / result) >> 1;
+            result = (result + a / result) >> 1;
+            result = (result + a / result) >> 1;
+            result = (result + a / result) >> 1;
+            result = (result + a / result) >> 1;
+            return min(result, a / result);
+        }
+    }
+
+    /**
+     * @notice Calculates sqrt(a), following the selected rounding direction.
+     */
+    function sqrt(uint256 a, Rounding rounding) internal pure returns (uint256) {
+        uint256 result = sqrt(a);
+        if (rounding == Rounding.Up && result * result < a) {
+            result += 1;
+        }
+        return result;
+    }
 }

+ 1 - 1
docs/modules/ROOT/pages/crosschain.adoc

@@ -1,6 +1,6 @@
 = Adding cross-chain support to contracts
 
-If your contract is targetting to be used in the context of multichain operations, you may need specific tools to identify and process these cross-chain operations.
+If your contract is targeting to be used in the context of multichain operations, you may need specific tools to identify and process these cross-chain operations.
 
 OpenZeppelin provides the xref:api:crosschain.adoc#CrossChainEnabled[`CrossChainEnabled`] abstract contract, that includes dedicated internal functions.
 

File diff suppressed because it is too large
+ 179 - 149
package-lock.json


+ 12 - 0
test/helpers/create2.js

@@ -0,0 +1,12 @@
+function computeCreate2Address (saltHex, bytecode, deployer) {
+  return web3.utils.toChecksumAddress(`0x${web3.utils.sha3(`0x${[
+    'ff',
+    deployer,
+    saltHex,
+    web3.utils.soliditySha3(bytecode),
+  ].map(x => x.replace(/0x/, '')).join('')}`).slice(-40)}`);
+}
+
+module.exports = {
+  computeCreate2Address,
+};

+ 9 - 0
test/helpers/eip712.js

@@ -7,6 +7,14 @@ const EIP712Domain = [
   { name: 'verifyingContract', type: 'address' },
 ];
 
+const Permit = [
+  { name: 'owner', type: 'address' },
+  { name: 'spender', type: 'address' },
+  { name: 'value', type: 'uint256' },
+  { name: 'nonce', type: 'uint256' },
+  { name: 'deadline', type: 'uint256' },
+];
+
 async function domainSeparator (name, version, chainId, verifyingContract) {
   return '0x' + ethSigUtil.TypedDataUtils.hashStruct(
     'EIP712Domain',
@@ -17,5 +25,6 @@ async function domainSeparator (name, version, chainId, verifyingContract) {
 
 module.exports = {
   EIP712Domain,
+  Permit,
   domainSeparator,
 };

+ 15 - 0
test/proxy/Clones.test.js

@@ -1,4 +1,6 @@
 const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
+const { computeCreate2Address } = require('../helpers/create2');
+const { expect } = require('chai');
 
 const shouldBehaveLikeClone = require('./Clones.behaviour');
 
@@ -44,6 +46,19 @@ contract('Clones', function (accounts) {
       const salt = web3.utils.randomHex(32);
       const factory = await ClonesMock.new();
       const predicted = await factory.predictDeterministicAddress(implementation, salt);
+
+      const creationCode = [
+        '0x3d602d80600a3d3981f3363d3d373d3d3d363d73',
+        implementation.replace(/0x/, '').toLowerCase(),
+        '5af43d82803e903d91602b57fd5bf3',
+      ].join('');
+
+      expect(computeCreate2Address(
+        salt,
+        creationCode,
+        factory.address,
+      )).to.be.equal(predicted);
+
       expectEvent(
         await factory.cloneDeterministic(implementation, salt, '0x'),
         'NewInstance',

+ 1 - 9
test/token/ERC20/extensions/draft-ERC20Permit.test.js

@@ -10,15 +10,7 @@ const Wallet = require('ethereumjs-wallet').default;
 
 const ERC20PermitMock = artifacts.require('ERC20PermitMock');
 
-const { EIP712Domain, domainSeparator } = require('../../../helpers/eip712');
-
-const Permit = [
-  { name: 'owner', type: 'address' },
-  { name: 'spender', type: 'address' },
-  { name: 'value', type: 'uint256' },
-  { name: 'nonce', type: 'uint256' },
-  { name: 'deadline', type: 'uint256' },
-];
+const { EIP712Domain, Permit, domainSeparator } = require('../../../helpers/eip712');
 
 contract('ERC20Permit', function (accounts) {
   const [ initialHolder, spender, recipient, other ] = accounts;

+ 121 - 1
test/token/ERC20/utils/SafeERC20.test.js

@@ -1,10 +1,17 @@
-const { expectRevert } = require('@openzeppelin/test-helpers');
+const { constants, expectRevert } = require('@openzeppelin/test-helpers');
 
 const ERC20ReturnFalseMock = artifacts.require('ERC20ReturnFalseMock');
 const ERC20ReturnTrueMock = artifacts.require('ERC20ReturnTrueMock');
 const ERC20NoReturnMock = artifacts.require('ERC20NoReturnMock');
+const ERC20PermitNoRevertMock = artifacts.require('ERC20PermitNoRevertMock');
 const SafeERC20Wrapper = artifacts.require('SafeERC20Wrapper');
 
+const { EIP712Domain, Permit } = require('../../../helpers/eip712');
+
+const { fromRpcSig } = require('ethereumjs-util');
+const ethSigUtil = require('eth-sig-util');
+const Wallet = require('ethereumjs-wallet').default;
+
 contract('SafeERC20', function (accounts) {
   const [ hasNoCode ] = accounts;
 
@@ -39,6 +46,119 @@ contract('SafeERC20', function (accounts) {
 
     shouldOnlyRevertOnErrors();
   });
+
+  describe('with token that doesn\'t revert on invalid permit', function () {
+    const wallet = Wallet.generate();
+    const owner = wallet.getAddressString();
+    const spender = hasNoCode;
+
+    beforeEach(async function () {
+      this.token = await ERC20PermitNoRevertMock.new();
+      this.wrapper = await SafeERC20Wrapper.new(this.token.address);
+
+      const chainId = await this.token.getChainId();
+
+      this.data = {
+        primaryType: 'Permit',
+        types: { EIP712Domain, Permit },
+        domain: { name: 'ERC20PermitNoRevertMock', version: '1', chainId, verifyingContract: this.token.address },
+        message: { owner, spender, value: '42', nonce: '0', deadline: constants.MAX_UINT256 },
+      };
+      this.signature = fromRpcSig(ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data: this.data }));
+    });
+
+    it('accepts owner signature', async function () {
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('0');
+      expect(await this.token.allowance(owner, spender)).to.be.bignumber.equal('0');
+
+      await this.wrapper.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+      expect(await this.token.allowance(owner, spender)).to.be.bignumber.equal(this.data.message.value);
+    });
+
+    it('revert on reused signature', async function () {
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('0');
+      // use valid signature and consume nounce
+      await this.wrapper.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+      // invalid call does not revert for this token implementation
+      await this.token.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+      // invalid call revert when called through the SafeERC20 library
+      await expectRevert(
+        this.wrapper.permit(
+          this.data.message.owner,
+          this.data.message.spender,
+          this.data.message.value,
+          this.data.message.deadline,
+          this.signature.v,
+          this.signature.r,
+          this.signature.s,
+        ),
+        'SafeERC20: permit did not succeed',
+      );
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+    });
+
+    it('revert on invalid signature', async function () {
+      // signature that is not valid for owner
+      const invalidSignature = {
+        v: 27,
+        r: '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
+        s: '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
+      };
+
+      // invalid call does not revert for this token implementation
+      await this.token.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        invalidSignature.v,
+        invalidSignature.r,
+        invalidSignature.s,
+      );
+
+      // invalid call revert when called through the SafeERC20 library
+      await expectRevert(
+        this.wrapper.permit(
+          this.data.message.owner,
+          this.data.message.spender,
+          this.data.message.value,
+          this.data.message.deadline,
+          invalidSignature.v,
+          invalidSignature.r,
+          invalidSignature.s,
+        ),
+        'SafeERC20: permit did not succeed',
+      );
+    });
+  });
 });
 
 function shouldRevertOnAllCalls (reason) {

+ 1 - 10
test/utils/Create2.test.js

@@ -1,5 +1,5 @@
 const { balance, BN, ether, expectRevert, send } = require('@openzeppelin/test-helpers');
-
+const { computeCreate2Address } = require('../helpers/create2');
 const { expect } = require('chai');
 
 const Create2Impl = artifacts.require('Create2Impl');
@@ -90,12 +90,3 @@ contract('Create2', function (accounts) {
     });
   });
 });
-
-function computeCreate2Address (saltHex, bytecode, deployer) {
-  return web3.utils.toChecksumAddress(`0x${web3.utils.sha3(`0x${[
-    'ff',
-    deployer,
-    saltHex,
-    web3.utils.soliditySha3(bytecode),
-  ].map(x => x.replace(/0x/, '')).join('')}`).slice(-40)}`);
-}

+ 34 - 0
test/utils/math/Math.test.js

@@ -182,4 +182,38 @@ contract('Math', function (accounts) {
       });
     });
   });
+
+  describe('sqrt', function () {
+    it('rounds down', async function () {
+      expect(await this.math.sqrt(new BN('0'), Rounding.Down)).to.be.bignumber.equal('0');
+      expect(await this.math.sqrt(new BN('1'), Rounding.Down)).to.be.bignumber.equal('1');
+      expect(await this.math.sqrt(new BN('2'), Rounding.Down)).to.be.bignumber.equal('1');
+      expect(await this.math.sqrt(new BN('3'), Rounding.Down)).to.be.bignumber.equal('1');
+      expect(await this.math.sqrt(new BN('4'), Rounding.Down)).to.be.bignumber.equal('2');
+      expect(await this.math.sqrt(new BN('144'), Rounding.Down)).to.be.bignumber.equal('12');
+      expect(await this.math.sqrt(new BN('999999'), Rounding.Down)).to.be.bignumber.equal('999');
+      expect(await this.math.sqrt(new BN('1000000'), Rounding.Down)).to.be.bignumber.equal('1000');
+      expect(await this.math.sqrt(new BN('1000001'), Rounding.Down)).to.be.bignumber.equal('1000');
+      expect(await this.math.sqrt(new BN('1002000'), Rounding.Down)).to.be.bignumber.equal('1000');
+      expect(await this.math.sqrt(new BN('1002001'), Rounding.Down)).to.be.bignumber.equal('1001');
+      expect(await this.math.sqrt(MAX_UINT256, Rounding.Down))
+        .to.be.bignumber.equal('340282366920938463463374607431768211455');
+    });
+
+    it('rounds up', async function () {
+      expect(await this.math.sqrt(new BN('0'), Rounding.Up)).to.be.bignumber.equal('0');
+      expect(await this.math.sqrt(new BN('1'), Rounding.Up)).to.be.bignumber.equal('1');
+      expect(await this.math.sqrt(new BN('2'), Rounding.Up)).to.be.bignumber.equal('2');
+      expect(await this.math.sqrt(new BN('3'), Rounding.Up)).to.be.bignumber.equal('2');
+      expect(await this.math.sqrt(new BN('4'), Rounding.Up)).to.be.bignumber.equal('2');
+      expect(await this.math.sqrt(new BN('144'), Rounding.Up)).to.be.bignumber.equal('12');
+      expect(await this.math.sqrt(new BN('999999'), Rounding.Up)).to.be.bignumber.equal('1000');
+      expect(await this.math.sqrt(new BN('1000000'), Rounding.Up)).to.be.bignumber.equal('1000');
+      expect(await this.math.sqrt(new BN('1000001'), Rounding.Up)).to.be.bignumber.equal('1001');
+      expect(await this.math.sqrt(new BN('1002000'), Rounding.Up)).to.be.bignumber.equal('1001');
+      expect(await this.math.sqrt(new BN('1002001'), Rounding.Up)).to.be.bignumber.equal('1001');
+      expect(await this.math.sqrt(MAX_UINT256, Rounding.Up))
+        .to.be.bignumber.equal('340282366920938463463374607431768211456');
+    });
+  });
 });

Some files were not shown because too many files changed in this diff