From faec973e09af21c57e22efdd0229486118476835 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Jan 2021 22:25:39 +0100 Subject: [PATCH] Make non-view functions virtual (#2468) --- contracts/payment/escrow/Escrow.sol | 2 +- contracts/proxy/BeaconProxy.sol | 4 +-- contracts/proxy/Initializable.sol | 16 +++------ contracts/proxy/Proxy.sol | 18 +++++----- contracts/proxy/ProxyAdmin.sol | 26 +++++++------- .../proxy/TransparentUpgradeableProxy.sol | 8 ++--- contracts/proxy/UpgradeableBeacon.sol | 2 +- contracts/proxy/UpgradeableProxy.sol | 4 +-- contracts/token/ERC1155/ERC1155.sol | 3 +- contracts/token/ERC1155/ERC1155Pausable.sol | 4 ++- contracts/token/ERC20/ERC20.sol | 2 +- contracts/token/ERC777/ERC777.sol | 36 ++++++++++--------- 12 files changed, 62 insertions(+), 63 deletions(-) diff --git a/contracts/payment/escrow/Escrow.sol b/contracts/payment/escrow/Escrow.sol index 4e2ea5cb8..945ce231d 100644 --- a/contracts/payment/escrow/Escrow.sol +++ b/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); diff --git a/contracts/proxy/BeaconProxy.sol b/contracts/proxy/BeaconProxy.sol index 8054c4847..90e7e1258 100644 --- a/contracts/proxy/BeaconProxy.sol +++ b/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" diff --git a/contracts/proxy/Initializable.sol b/contracts/proxy/Initializable.sol index 514fbbb19..b38e8f997 100644 --- a/contracts/proxy/Initializable.sol +++ b/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)); } } diff --git a/contracts/proxy/Proxy.sol b/contracts/proxy/Proxy.sol index a444637ed..440d9f1c1 100644 --- a/contracts/proxy/Proxy.sol +++ b/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 { diff --git a/contracts/proxy/ProxyAdmin.sol b/contracts/proxy/ProxyAdmin.sol index f93c204ce..47eee07c3 100644 --- a/contracts/proxy/ProxyAdmin.sol +++ b/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); } } diff --git a/contracts/proxy/TransparentUpgradeableProxy.sol b/contracts/proxy/TransparentUpgradeableProxy.sol index a387943e3..00c360514 100644 --- a/contracts/proxy/TransparentUpgradeableProxy.sol +++ b/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(); } diff --git a/contracts/proxy/UpgradeableBeacon.sol b/contracts/proxy/UpgradeableBeacon.sol index e1350ce36..37e6e73cb 100644 --- a/contracts/proxy/UpgradeableBeacon.sol +++ b/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); } diff --git a/contracts/proxy/UpgradeableProxy.sol b/contracts/proxy/UpgradeableProxy.sol index e9a58db8c..65146c24c 100644 --- a/contracts/proxy/UpgradeableProxy.sol +++ b/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); } diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 4a98ce915..f8e9d1196 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/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( diff --git a/contracts/token/ERC1155/ERC1155Pausable.sol b/contracts/token/ERC1155/ERC1155Pausable.sol index 82d99161a..cf6c301b9 100644 --- a/contracts/token/ERC1155/ERC1155Pausable.sol +++ b/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); diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index ba62e81de..05af2064b 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/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_; } diff --git a/contracts/token/ERC777/ERC777.sol b/contracts/token/ERC777/ERC777.sol index 2bf757980..248936ac4 100644 --- a/contracts/token/ERC777/ERC777.sol +++ b/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");