Make view and pure functions virtual

contracts/GSN/GSNRecipient.sol

@@ -35,7 +35,7 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
     /**
      * @dev Returns the address of the {IRelayHub} contract for this recipient.
      */
-    function getHubAddr() public view override returns (address) {
+    function getHubAddr() public view virtual override returns (address) {
         return _relayHub;
     }
 
@@ -62,7 +62,7 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      */
     // This function is view for future-proofing, it may require reading from
     // storage in the future.
-    function relayHubVersion() public view returns (string memory) {
+    function relayHubVersion() public view virtual returns (string memory) {
         this; // silence state mutability warning without generating bytecode - see https://github.com/ethereum/solidity/issues/2691
         return "1.0.0";
     }
@@ -73,7 +73,7 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      * Derived contracts should expose this in an external interface with proper access control.
      */
     function _withdrawDeposits(uint256 amount, address payable payee) internal virtual {
-        IRelayHub(_relayHub).withdraw(amount, payee);
+        IRelayHub(getHubAddr()).withdraw(amount, payee);
     }
 
     // Overrides for Context's functions: when called from RelayHub, sender and
@@ -88,7 +88,7 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      * IMPORTANT: Contracts derived from {GSNRecipient} should never use `msg.sender`, and use {_msgSender} instead.
      */
     function _msgSender() internal view virtual override returns (address payable) {
-        if (msg.sender != _relayHub) {
+        if (msg.sender != getHubAddr()) {
             return msg.sender;
         } else {
             return _getRelayedCallSender();
@@ -102,7 +102,7 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      * IMPORTANT: Contracts derived from {GSNRecipient} should never use `msg.data`, and use {_msgData} instead.
      */
     function _msgData() internal view virtual override returns (bytes memory) {
-        if (msg.sender != _relayHub) {
+        if (msg.sender != getHubAddr()) {
             return msg.data;
         } else {
             return _getRelayedCallData();
@@ -162,7 +162,7 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      * @dev Return this in acceptRelayedCall to proceed with the execution of a relayed call. Note that this contract
      * will be charged a fee by RelayHub
      */
-    function _approveRelayedCall() internal pure returns (uint256, bytes memory) {
+    function _approveRelayedCall() internal pure virtual returns (uint256, bytes memory) {
         return _approveRelayedCall("");
     }
 
@@ -171,22 +171,22 @@ abstract contract GSNRecipient is IRelayRecipient, Context {
      *
      * This overload forwards `context` to _preRelayedCall and _postRelayedCall.
      */
-    function _approveRelayedCall(bytes memory context) internal pure returns (uint256, bytes memory) {
+    function _approveRelayedCall(bytes memory context) internal pure virtual returns (uint256, bytes memory) {
         return (_RELAYED_CALL_ACCEPTED, context);
     }
 
     /**
      * @dev Return this in acceptRelayedCall to impede execution of a relayed call. No fees will be charged.
      */
-    function _rejectRelayedCall(uint256 errorCode) internal pure returns (uint256, bytes memory) {
+    function _rejectRelayedCall(uint256 errorCode) internal pure virtual returns (uint256, bytes memory) {
         return (_RELAYED_CALL_REJECTED + errorCode, "");
     }
 
     /*
      * @dev Calculates how much RelayHub will charge a recipient for using `gas` at a `gasPrice`, given a relayer's
      * `serviceFee`.
      */
-    function _computeCharge(uint256 gas, uint256 gasPrice, uint256 serviceFee) internal pure returns (uint256) {
+    function _computeCharge(uint256 gas, uint256 gasPrice, uint256 serviceFee) internal pure virtual returns (uint256) {
         // The fee is expressed as a percentage. E.g. a value of 40 stands for a 40% fee, so the recipient will be
         // charged for 1.4 times the spent amount.
         return (gas * gasPrice * (100 + serviceFee)) / 100;

contracts/GSN/GSNRecipientERC20Fee.sol

@@ -18,14 +18,13 @@ import "../token/ERC20/ERC20.sol";
  * internal {_mint} function.
  */
 contract GSNRecipientERC20Fee is GSNRecipient {
-    using SafeERC20 for __unstable__ERC20Owned;
     using SafeMath for uint256;
 
     enum GSNRecipientERC20FeeErrorCodes {
         INSUFFICIENT_BALANCE
     }
 
-    __unstable__ERC20Owned private _token;
+    IERC20 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.
@@ -37,15 +36,15 @@ contract GSNRecipientERC20Fee is GSNRecipient {
     /**
      * @dev Returns the gas payment token.
      */
-    function token() public view returns (IERC20) {
-        return IERC20(_token);
+    function token() public view virtual returns (IERC20) {
+        return _token;
     }
 
     /**
      * @dev Internal function that mints the gas payment token. Derived contracts should expose this function in their public API, with proper access control mechanisms.
      */
     function _mint(address account, uint256 amount) internal virtual {
-        _token.mint(account, amount);
+        __unstable__ERC20Owned(address(token())).mint(account, amount);
     }
 
     /**
@@ -68,7 +67,7 @@ contract GSNRecipientERC20Fee is GSNRecipient {
         override
         returns (uint256, bytes memory)
     {
-        if (_token.balanceOf(from) < maxPossibleCharge) {
+        if (token().balanceOf(from) < maxPossibleCharge) {
             return _rejectRelayedCall(uint256(GSNRecipientERC20FeeErrorCodes.INSUFFICIENT_BALANCE));
         }
 
@@ -85,7 +84,7 @@ contract GSNRecipientERC20Fee is GSNRecipient {
         (address from, uint256 maxPossibleCharge) = abi.decode(context, (address, uint256));
 
         // The maximum token charge is pre-charged from the user
-        _token.safeTransferFrom(from, address(this), maxPossibleCharge);
+        SafeERC20.safeTransferFrom(token(), from, address(this), maxPossibleCharge);
 
         return 0;
     }
@@ -104,7 +103,7 @@ contract GSNRecipientERC20Fee is GSNRecipient {
         actualCharge = actualCharge.sub(overestimation);
 
         // After the relayed call has been executed and the actual charge estimated, the excess pre-charge is returned
-        _token.safeTransfer(from, maxPossibleCharge.sub(actualCharge));
+        SafeERC20.safeTransfer(token(), from, maxPossibleCharge.sub(actualCharge));
     }
 }
 
@@ -121,12 +120,12 @@ contract __unstable__ERC20Owned is ERC20, Ownable {
     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 {
+    function mint(address account, uint256 amount) public virtual onlyOwner {
         _mint(account, amount);
     }
 
     // The owner has 'infinite' allowance for all token holders
-    function allowance(address tokenOwner, address spender) public view override returns (uint256) {
+    function allowance(address tokenOwner, address spender) public view virtual override returns (uint256) {
         if (spender == owner()) {
             return _UINT256_MAX;
         } else {
@@ -135,15 +134,15 @@ contract __unstable__ERC20Owned is ERC20, Ownable {
     }
 
     // Allowance for the owner cannot be changed (it is always 'infinite')
-    function _approve(address tokenOwner, address spender, uint256 value) internal override {
+    function _approve(address tokenOwner, address spender, uint256 value) internal virtual override {
         if (spender == owner()) {
             return;
         } else {
             super._approve(tokenOwner, spender, value);
         }
     }
 
-    function transferFrom(address sender, address recipient, uint256 amount) public override returns (bool) {
+    function transferFrom(address sender, address recipient, uint256 amount) public virtual override returns (bool) {
         if (recipient == owner()) {
             _transfer(sender, recipient, amount);
             return true;

contracts/access/AccessControl.sol

@@ -84,15 +84,15 @@ abstract contract AccessControl is Context {
     /**
      * @dev Returns `true` if `account` has been granted `role`.
      */
-    function hasRole(bytes32 role, address account) public view returns (bool) {
+    function hasRole(bytes32 role, address account) public view virtual returns (bool) {
         return _roles[role].members.contains(account);
     }
 
     /**
      * @dev Returns the number of accounts that have `role`. Can be used
      * together with {getRoleMember} to enumerate all bearers of a role.
      */
-    function getRoleMemberCount(bytes32 role) public view returns (uint256) {
+    function getRoleMemberCount(bytes32 role) public view virtual returns (uint256) {
         return _roles[role].members.length();
     }
 
@@ -108,7 +108,7 @@ abstract contract AccessControl is Context {
      * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
      * for more information.
      */
-    function getRoleMember(bytes32 role, uint256 index) public view returns (address) {
+    function getRoleMember(bytes32 role, uint256 index) public view virtual returns (address) {
         return _roles[role].members.at(index);
     }
 
@@ -118,7 +118,7 @@ abstract contract AccessControl is Context {
      *
      * To change a role's admin, use {_setRoleAdmin}.
      */
-    function getRoleAdmin(bytes32 role) public view returns (bytes32) {
+    function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
         return _roles[role].adminRole;
     }
 
@@ -133,7 +133,7 @@ abstract contract AccessControl is Context {
      * - the caller must have ``role``'s admin role.
      */
     function grantRole(bytes32 role, address account) public virtual {
-        require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to grant");
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
 
         _grantRole(role, account);
     }
@@ -148,7 +148,7 @@ abstract contract AccessControl is Context {
      * - the caller must have ``role``'s admin role.
      */
     function revokeRole(bytes32 role, address account) public virtual {
-        require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to revoke");
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
 
         _revokeRole(role, account);
     }

contracts/access/Ownable.sol

@@ -32,15 +32,15 @@ abstract contract Ownable is Context {
     /**
      * @dev Returns the address of the current owner.
      */
-    function owner() public view returns (address) {
+    function owner() public view virtual returns (address) {
         return _owner;
     }
 
     /**
      * @dev Throws if called by any account other than the owner.
      */
     modifier onlyOwner() {
-        require(_owner == _msgSender(), "Ownable: caller is not the owner");
+        require(owner() == _msgSender(), "Ownable: caller is not the owner");
         _;
     }
 

contracts/access/TimelockController.sol

@@ -93,33 +93,42 @@ contract TimelockController is AccessControl {
      */
     receive() external payable {}
 
+    /**
+     * @dev Returns whether an id correspond to a registered operation. This
+     * includes both Pending, Ready and Done operations.
+     */
+    function isOperation(bytes32 id) public view virtual returns (bool pending) {
+        return getTimestamp(id) > 0;
+    }
+
     /**
      * @dev Returns whether an operation is pending or not.
      */
-    function isOperationPending(bytes32 id) public view returns (bool pending) {
-        return _timestamps[id] > _DONE_TIMESTAMP;
+    function isOperationPending(bytes32 id) public view virtual returns (bool pending) {
+        return getTimestamp(id) > _DONE_TIMESTAMP;
     }
 
     /**
      * @dev Returns whether an operation is ready or not.
      */
-    function isOperationReady(bytes32 id) public view returns (bool ready) {
+    function isOperationReady(bytes32 id) public view virtual returns (bool ready) {
+        uint256 timestamp = getTimestamp(id);
         // solhint-disable-next-line not-rely-on-time
-        return _timestamps[id] > _DONE_TIMESTAMP && _timestamps[id] <= block.timestamp;
+        return timestamp > _DONE_TIMESTAMP && timestamp <= block.timestamp;
     }
 
     /**
      * @dev Returns whether an operation is done or not.
      */
-    function isOperationDone(bytes32 id) public view returns (bool done) {
-        return _timestamps[id] == _DONE_TIMESTAMP;
+    function isOperationDone(bytes32 id) public view virtual returns (bool done) {
+        return getTimestamp(id) == _DONE_TIMESTAMP;
     }
 
     /**
      * @dev Returns the timestamp at with an operation becomes ready (0 for
      * unset operations, 1 for done operations).
      */
-    function getTimestamp(bytes32 id) public view returns (uint256 timestamp) {
+    function getTimestamp(bytes32 id) public view virtual returns (uint256 timestamp) {
         return _timestamps[id];
     }
 
@@ -128,23 +137,23 @@ contract TimelockController is AccessControl {
      *
      * This value can be changed by executing an operation that calls `updateDelay`.
      */
-    function getMinDelay() public view returns (uint256 duration) {
+    function getMinDelay() public view virtual returns (uint256 duration) {
         return _minDelay;
     }
 
     /**
      * @dev Returns the identifier of an operation containing a single
      * transaction.
      */
-    function hashOperation(address target, uint256 value, bytes calldata data, bytes32 predecessor, bytes32 salt) public pure returns (bytes32 hash) {
+    function hashOperation(address target, uint256 value, bytes calldata data, bytes32 predecessor, bytes32 salt) public pure virtual returns (bytes32 hash) {
         return keccak256(abi.encode(target, value, data, predecessor, salt));
     }
 
     /**
      * @dev Returns the identifier of an operation containing a batch of
      * transactions.
      */
-    function hashOperationBatch(address[] calldata targets, uint256[] calldata values, bytes[] calldata datas, bytes32 predecessor, bytes32 salt) public pure returns (bytes32 hash) {
+    function hashOperationBatch(address[] calldata targets, uint256[] calldata values, bytes[] calldata datas, bytes32 predecessor, bytes32 salt) public pure virtual returns (bytes32 hash) {
         return keccak256(abi.encode(targets, values, datas, predecessor, salt));
     }
 
@@ -187,8 +196,8 @@ contract TimelockController is AccessControl {
      * @dev Schedule an operation that is to becomes valid after a given delay.
      */
     function _schedule(bytes32 id, uint256 delay) private {
-        require(_timestamps[id] == 0, "TimelockController: operation already scheduled");
-        require(delay >= _minDelay, "TimelockController: insufficient delay");
+        require(!isOperation(id), "TimelockController: operation already scheduled");
+        require(delay >= getMinDelay(), "TimelockController: insufficient delay");
         // solhint-disable-next-line not-rely-on-time
         _timestamps[id] = SafeMath.add(block.timestamp, delay);
     }

contracts/drafts/EIP712.sol

@@ -46,7 +46,7 @@ abstract contract EIP712 {
     constructor(string memory name, string memory version) internal {
         bytes32 hashedName = keccak256(bytes(name));
         bytes32 hashedVersion = keccak256(bytes(version));
-        bytes32 typeHash = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); 
+        bytes32 typeHash = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
         _HASHED_NAME = hashedName;
         _HASHED_VERSION = hashedVersion;
         _CACHED_CHAIN_ID = _getChainId();
@@ -57,7 +57,7 @@ abstract contract EIP712 {
     /**
      * @dev Returns the domain separator for the current chain.
      */
-    function _domainSeparatorV4() internal view returns (bytes32) {
+    function _domainSeparatorV4() internal view virtual returns (bytes32) {
         if (_getChainId() == _CACHED_CHAIN_ID) {
             return _CACHED_DOMAIN_SEPARATOR;
         } else {
@@ -92,7 +92,7 @@ abstract contract EIP712 {
      * address signer = ECDSA.recover(digest, signature);
      * ```
      */
-    function _hashTypedDataV4(bytes32 structHash) internal view returns (bytes32) {
+    function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
         return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), structHash));
     }
 

contracts/drafts/ERC20Permit.sol

@@ -45,7 +45,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
                 owner,
                 spender,
                 value,
-                _nonces[owner].current(),
+                nonces(owner),
                 deadline
             )
         );
@@ -62,15 +62,15 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
     /**
      * @dev See {IERC20Permit-nonces}.
      */
-    function nonces(address owner) public view override returns (uint256) {
+    function nonces(address owner) public view virtual override returns (uint256) {
         return _nonces[owner].current();
     }
 
     /**
      * @dev See {IERC20Permit-DOMAIN_SEPARATOR}.
      */
     // solhint-disable-next-line func-name-mixedcase
-    function DOMAIN_SEPARATOR() external view override returns (bytes32) {
+    function DOMAIN_SEPARATOR() external view virtual override returns (bytes32) {
         return _domainSeparatorV4();
     }
 }

contracts/introspection/ERC165.sol

@@ -32,7 +32,7 @@ abstract contract ERC165 is IERC165 {
      *
      * Time complexity O(1), guaranteed to always use less than 30 000 gas.
      */
-    function supportsInterface(bytes4 interfaceId) public view override returns (bool) {
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
         return _supportedInterfaces[interfaceId];
     }