Make non-view functions virtual (#2468)

contracts/payment/escrow/Escrow.sol

@@ -36,7 +36,7 @@ contract Escrow is Ownable {
      * @dev Stores the sent amount as credit to be withdrawn.
      * @param payee The destination address of the funds.
      */
-    function deposit(address payee) public virtual payable onlyOwner {
+    function deposit(address payee) public payable virtual onlyOwner {
         uint256 amount = msg.value;
         _deposits[payee] = _deposits[payee].add(amount);
 

contracts/proxy/BeaconProxy.sol

@@ -56,14 +56,14 @@ contract BeaconProxy is Proxy {
     /**
      * @dev Changes the proxy to use a new beacon.
      *
-     * If `data` is nonempty, it's used as data in a delegate call to the implementation returned by the beacon. 
+     * If `data` is nonempty, it's used as data in a delegate call to the implementation returned by the beacon.
      *
      * Requirements:
      *
      * - `beacon` must be a contract.
      * - The implementation returned by `beacon` must be a contract.
      */
-    function _setBeacon(address beacon, bytes memory data) internal {
+    function _setBeacon(address beacon, bytes memory data) internal virtual {
         require(
             Address.isContract(beacon),
             "BeaconProxy: beacon is not a contract"

contracts/proxy/Initializable.sol

@@ -3,16 +3,17 @@
 // solhint-disable-next-line compiler-version
 pragma solidity >=0.4.24 <0.8.0;
 
+import "../utils/Address.sol";
 
 /**
  * @dev This is a base contract to aid in writing upgradeable contracts, or any kind of contract that will be deployed
  * behind a proxy. Since a proxied contract can't have a constructor, it's common to move constructor logic to an
  * external initializer function, usually called `initialize`. It then becomes necessary to protect this initializer
  * function so it can only be called once. The {initializer} modifier provided by this contract will have this effect.
- * 
+ *
  * TIP: To avoid leaving the proxy in an uninitialized state, the initializer function should be called as early as
  * possible by providing the encoded function call as the `_data` argument to {UpgradeableProxy-constructor}.
- * 
+ *
  * CAUTION: When used with inheritance, manual care must be taken to not invoke a parent initializer twice, or to ensure
  * that all initializers are idempotent. This is not verified automatically as constructors are by Solidity.
  */
@@ -49,15 +50,6 @@ abstract contract Initializable {
 
     /// @dev Returns true if and only if the function is running in the constructor
     function _isConstructor() private view returns (bool) {
-        // extcodesize checks the size of the code stored in an address, and
-        // address returns the current address. Since the code is still not
-        // deployed when running a constructor, any checks on its code size will
-        // yield zero, making it an effective way to detect if a contract is
-        // under construction or not.
-        address self = address(this);
-        uint256 cs;
-        // solhint-disable-next-line no-inline-assembly
-        assembly { cs := extcodesize(self) }
-        return cs == 0;
+        return !Address.isContract(address(this));
     }
 }

contracts/proxy/Proxy.sol

@@ -6,19 +6,19 @@ pragma solidity >=0.6.0 <0.8.0;
  * @dev This abstract contract provides a fallback function that delegates all calls to another contract using the EVM
  * instruction `delegatecall`. We refer to the second contract as the _implementation_ behind the proxy, and it has to
  * be specified by overriding the virtual {_implementation} function.
- * 
+ *
  * Additionally, delegation to the implementation can be triggered manually through the {_fallback} function, or to a
  * different contract through the {_delegate} function.
- * 
+ *
  * The success and return data of the delegated call will be returned back to the caller of the proxy.
  */
 abstract contract Proxy {
     /**
      * @dev Delegates the current call to `implementation`.
-     * 
+     *
      * This function does not return to its internall call site, it will return directly to the external caller.
      */
-    function _delegate(address implementation) internal {
+    function _delegate(address implementation) internal virtual {
         // solhint-disable-next-line no-inline-assembly
         assembly {
             // Copy msg.data. We take full control of memory in this inline assembly
@@ -48,10 +48,10 @@ abstract contract Proxy {
 
     /**
      * @dev Delegates the current call to the address returned by `_implementation()`.
-     * 
+     *
      * This function does not return to its internall call site, it will return directly to the external caller.
      */
-    function _fallback() internal {
+    function _fallback() internal virtual {
         _beforeFallback();
         _delegate(_implementation());
     }
@@ -60,7 +60,7 @@ abstract contract Proxy {
      * @dev Fallback function that delegates calls to the address returned by `_implementation()`. Will run if no other
      * function in the contract matches the call data.
      */
-    fallback () external payable {
+    fallback () external payable virtual {
         _fallback();
     }
 
@@ -68,14 +68,14 @@ abstract contract Proxy {
      * @dev Fallback function that delegates calls to the address returned by `_implementation()`. Will run if call data
      * is empty.
      */
-    receive () external payable {
+    receive () external payable virtual {
         _fallback();
     }
 
     /**
      * @dev Hook that is called before falling back to the implementation. Can happen as part of a manual `_fallback`
      * call, or as part of the Solidity `fallback` or `receive` functions.
-     * 
+     *
      * If overriden should call `super._beforeFallback()`.
      */
     function _beforeFallback() internal virtual {

contracts/proxy/ProxyAdmin.sol

@@ -13,9 +13,9 @@ contract ProxyAdmin is Ownable {
 
     /**
      * @dev Returns the current implementation of `proxy`.
-     * 
+     *
      * Requirements:
-     * 
+     *
      * - This contract must be the admin of `proxy`.
      */
     function getProxyImplementation(TransparentUpgradeableProxy proxy) public view returns (address) {
@@ -28,9 +28,9 @@ contract ProxyAdmin is Ownable {
 
     /**
      * @dev Returns the current admin of `proxy`.
-     * 
+     *
      * Requirements:
-     * 
+     *
      * - This contract must be the admin of `proxy`.
      */
     function getProxyAdmin(TransparentUpgradeableProxy proxy) public view returns (address) {
@@ -43,35 +43,35 @@ contract ProxyAdmin is Ownable {
 
     /**
      * @dev Changes the admin of `proxy` to `newAdmin`.
-     * 
+     *
      * Requirements:
-     * 
+     *
      * - This contract must be the current admin of `proxy`.
      */
-    function changeProxyAdmin(TransparentUpgradeableProxy proxy, address newAdmin) public onlyOwner {
+    function changeProxyAdmin(TransparentUpgradeableProxy proxy, address newAdmin) public virtual onlyOwner {
         proxy.changeAdmin(newAdmin);
     }
 
     /**
      * @dev Upgrades `proxy` to `implementation`. See {TransparentUpgradeableProxy-upgradeTo}.
-     * 
+     *
      * Requirements:
-     * 
+     *
      * - This contract must be the admin of `proxy`.
      */
-    function upgrade(TransparentUpgradeableProxy proxy, address implementation) public onlyOwner {
+    function upgrade(TransparentUpgradeableProxy proxy, address implementation) public virtual onlyOwner {
         proxy.upgradeTo(implementation);
     }
 
     /**
      * @dev Upgrades `proxy` to `implementation` and calls a function on the new implementation. See
      * {TransparentUpgradeableProxy-upgradeToAndCall}.
-     * 
+     *
      * Requirements:
-     * 
+     *
      * - This contract must be the admin of `proxy`.
      */
-    function upgradeAndCall(TransparentUpgradeableProxy proxy, address implementation, bytes memory data) public payable onlyOwner {
+    function upgradeAndCall(TransparentUpgradeableProxy proxy, address implementation, bytes memory data) public payable virtual onlyOwner {
         proxy.upgradeToAndCall{value: msg.value}(implementation, data);
     }
 }

contracts/proxy/TransparentUpgradeableProxy.sol

@@ -91,7 +91,7 @@ contract TransparentUpgradeableProxy is UpgradeableProxy {
      *
      * NOTE: Only the admin can call this function. See {ProxyAdmin-changeProxyAdmin}.
      */
-    function changeAdmin(address newAdmin) external ifAdmin {
+    function changeAdmin(address newAdmin) external virtual ifAdmin {
         require(newAdmin != address(0), "TransparentUpgradeableProxy: new admin is the zero address");
         emit AdminChanged(_admin(), newAdmin);
         _setAdmin(newAdmin);
@@ -102,7 +102,7 @@ contract TransparentUpgradeableProxy is UpgradeableProxy {
      *
      * NOTE: Only the admin can call this function. See {ProxyAdmin-upgrade}.
      */
-    function upgradeTo(address newImplementation) external ifAdmin {
+    function upgradeTo(address newImplementation) external virtual ifAdmin {
         _upgradeTo(newImplementation);
     }
 
@@ -113,7 +113,7 @@ contract TransparentUpgradeableProxy is UpgradeableProxy {
      *
      * NOTE: Only the admin can call this function. See {ProxyAdmin-upgradeAndCall}.
      */
-    function upgradeToAndCall(address newImplementation, bytes calldata data) external payable ifAdmin {
+    function upgradeToAndCall(address newImplementation, bytes calldata data) external payable virtual ifAdmin {
         _upgradeTo(newImplementation);
         Address.functionDelegateCall(newImplementation, data);
     }
@@ -144,7 +144,7 @@ contract TransparentUpgradeableProxy is UpgradeableProxy {
     /**
      * @dev Makes sure the admin cannot access the fallback function. See {Proxy-_beforeFallback}.
      */
-    function _beforeFallback() internal override virtual {
+    function _beforeFallback() internal virtual override {
         require(msg.sender != _admin(), "TransparentUpgradeableProxy: admin cannot fallback to proxy target");
         super._beforeFallback();
     }

contracts/proxy/UpgradeableBeacon.sol

@@ -45,7 +45,7 @@ contract UpgradeableBeacon is IBeacon, Ownable {
      * - msg.sender must be the owner of the contract.
      * - `newImplementation` must be a contract.
      */
-    function upgradeTo(address newImplementation) public onlyOwner {
+    function upgradeTo(address newImplementation) public virtual onlyOwner {
         _setImplementation(newImplementation);
         emit Upgraded(newImplementation);
     }

contracts/proxy/UpgradeableProxy.sol

@@ -44,7 +44,7 @@ contract UpgradeableProxy is Proxy {
     /**
      * @dev Returns the current implementation address.
      */
-    function _implementation() internal override view returns (address impl) {
+    function _implementation() internal view override returns (address impl) {
         bytes32 slot = _IMPLEMENTATION_SLOT;
         // solhint-disable-next-line no-inline-assembly
         assembly {
@@ -57,7 +57,7 @@ contract UpgradeableProxy is Proxy {
      *
      * Emits an {Upgraded} event.
      */
-    function _upgradeTo(address newImplementation) internal {
+    function _upgradeTo(address newImplementation) internal virtual {
         _setImplementation(newImplementation);
         emit Upgraded(newImplementation);
     }

contracts/token/ERC1155/ERC1155.sol

@@ -355,7 +355,8 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
         uint256[] memory amounts,
         bytes memory data
     )
-        internal virtual
+        internal
+        virtual
     { }
 
     function _doSafeTransferAcceptanceCheck(

contracts/token/ERC1155/ERC1155Pausable.sol

@@ -30,7 +30,9 @@ abstract contract ERC1155Pausable is ERC1155, Pausable {
         uint256[] memory amounts,
         bytes memory data
     )
-        internal virtual override
+        internal
+        virtual
+        override
     {
         super._beforeTokenTransfer(operator, from, to, ids, amounts, data);
 

contracts/token/ERC20/ERC20.sol

@@ -284,7 +284,7 @@ contract ERC20 is Context, IERC20 {
      * applications that interact with token contracts will not expect
      * {decimals} to ever change, and may work incorrectly if it does.
      */
-    function _setupDecimals(uint8 decimals_) internal {
+    function _setupDecimals(uint8 decimals_) internal virtual {
         _decimals = decimals_;
     }
 

contracts/token/ERC777/ERC777.sol

@@ -70,7 +70,9 @@ contract ERC777 is Context, IERC777, IERC20 {
         string memory name_,
         string memory symbol_,
         address[] memory defaultOperators_
-    ) public {
+    )
+        public
+    {
         _name = name_;
         _symbol = symbol_;
 
@@ -136,7 +138,7 @@ contract ERC777 is Context, IERC777, IERC20 {
      *
      * Also emits a {IERC20-Transfer} event for ERC20 compatibility.
      */
-    function send(address recipient, uint256 amount, bytes memory data) public override  {
+    function send(address recipient, uint256 amount, bytes memory data) public virtual override  {
         _send(_msgSender(), recipient, amount, data, "", true);
     }
 
@@ -148,7 +150,7 @@ contract ERC777 is Context, IERC777, IERC20 {
      *
      * Also emits a {Sent} event.
      */
-    function transfer(address recipient, uint256 amount) public override returns (bool) {
+    function transfer(address recipient, uint256 amount) public virtual override returns (bool) {
         require(recipient != address(0), "ERC777: transfer to the zero address");
 
         address from = _msgSender();
@@ -167,17 +169,14 @@ contract ERC777 is Context, IERC777, IERC20 {
      *
      * Also emits a {IERC20-Transfer} event for ERC20 compatibility.
      */
-    function burn(uint256 amount, bytes memory data) public override  {
+    function burn(uint256 amount, bytes memory data) public virtual override  {
         _burn(_msgSender(), amount, data, "");
     }
 
     /**
      * @dev See {IERC777-isOperatorFor}.
      */
-    function isOperatorFor(
-        address operator,
-        address tokenHolder
-    ) public view override returns (bool) {
+    function isOperatorFor(address operator, address tokenHolder) public view override returns (bool) {
         return operator == tokenHolder ||
             (_defaultOperators[operator] && !_revokedDefaultOperators[tokenHolder][operator]) ||
             _operators[tokenHolder][operator];
@@ -186,7 +185,7 @@ contract ERC777 is Context, IERC777, IERC20 {
     /**
      * @dev See {IERC777-authorizeOperator}.
      */
-    function authorizeOperator(address operator) public override  {
+    function authorizeOperator(address operator) public virtual override  {
         require(_msgSender() != operator, "ERC777: authorizing self as operator");
 
         if (_defaultOperators[operator]) {
@@ -201,7 +200,7 @@ contract ERC777 is Context, IERC777, IERC20 {
     /**
      * @dev See {IERC777-revokeOperator}.
      */
-    function revokeOperator(address operator) public override  {
+    function revokeOperator(address operator) public virtual override  {
         require(operator != _msgSender(), "ERC777: revoking self as operator");
 
         if (_defaultOperators[operator]) {
@@ -232,7 +231,9 @@ contract ERC777 is Context, IERC777, IERC20 {
         bytes memory data,
         bytes memory operatorData
     )
-    public override
+        public
+        virtual
+        override
     {
         require(isOperatorFor(_msgSender(), sender), "ERC777: caller is not an operator for holder");
         _send(sender, recipient, amount, data, operatorData, true);
@@ -243,7 +244,7 @@ contract ERC777 is Context, IERC777, IERC20 {
      *
      * Emits {Burned} and {IERC20-Transfer} events.
      */
-    function operatorBurn(address account, uint256 amount, bytes memory data, bytes memory operatorData) public override {
+    function operatorBurn(address account, uint256 amount, bytes memory data, bytes memory operatorData) public virtual override {
         require(isOperatorFor(_msgSender(), account), "ERC777: caller is not an operator for holder");
         _burn(account, amount, data, operatorData);
     }
@@ -264,7 +265,7 @@ contract ERC777 is Context, IERC777, IERC20 {
      *
      * Note that accounts cannot have allowance issued by their operators.
      */
-    function approve(address spender, uint256 value) public override returns (bool) {
+    function approve(address spender, uint256 value) public virtual override returns (bool) {
         address holder = _msgSender();
         _approve(holder, spender, value);
         return true;
@@ -279,7 +280,7 @@ contract ERC777 is Context, IERC777, IERC20 {
     *
     * Emits {Sent}, {IERC20-Transfer} and {IERC20-Approval} events.
     */
-    function transferFrom(address holder, address recipient, uint256 amount) public override returns (bool) {
+    function transferFrom(address holder, address recipient, uint256 amount) public virtual override returns (bool) {
         require(recipient != address(0), "ERC777: transfer to the zero address");
         require(holder != address(0), "ERC777: transfer from the zero address");
 
@@ -318,7 +319,8 @@ contract ERC777 is Context, IERC777, IERC20 {
         bytes memory userData,
         bytes memory operatorData
     )
-    internal virtual
+        internal
+        virtual
     {
         require(account != address(0), "ERC777: mint to the zero address");
 
@@ -354,6 +356,7 @@ contract ERC777 is Context, IERC777, IERC20 {
         bool requireReceptionAck
     )
         internal
+        virtual
     {
         require(from != address(0), "ERC777: send from the zero address");
         require(to != address(0), "ERC777: send to the zero address");
@@ -380,7 +383,8 @@ contract ERC777 is Context, IERC777, IERC20 {
         bytes memory data,
         bytes memory operatorData
     )
-        internal virtual
+        internal
+        virtual
     {
         require(from != address(0), "ERC777: burn from the zero address");