Refactorring of Rescuable

src/contracts/utils/PermissionlessRescuable.sol

@@ -1,14 +1,23 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.8;
 
-import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/IERC20.sol';
 import {RescuableBase} from './RescuableBase.sol';
 import {IPermissionlessRescuable} from './interfaces/IPermissionlessRescuable.sol';
 
 abstract contract PermissionlessRescuable is RescuableBase, IPermissionlessRescuable {
+  /// @notice modifier that checks that recipient is allowed address
+  modifier onlyWhoShouldReceiveFunds(address user) {
+    require(user == whoShouldReceiveFunds(), OnlyAuthorizedReceiver(user));
+    _;
+  }
+
   /// @inheritdoc IPermissionlessRescuable
   function whoShouldReceiveFunds() public view virtual returns (address);
 
+  function whoCanRescue(address user) public view override returns (bool) {
+    return true;
+  }
+
   /// @inheritdoc IPermissionlessRescuable
   function emergencyTokenTransfer(address erc20Token, uint256 amount) external virtual {
     _emergencyTokenTransfer(erc20Token, whoShouldReceiveFunds(), amount);
@@ -18,4 +27,19 @@ abstract contract PermissionlessRescuable is RescuableBase, IPermissionlessRescu
   function emergencyEtherTransfer(uint256 amount) external virtual {
     _emergencyEtherTransfer(whoShouldReceiveFunds(), amount);
   }
+
+  function _emergencyTokenTransfer(
+    address erc20Token,
+    address to,
+    uint256 amount
+  ) internal override onlyWhoShouldReceiveFunds(to) {
+    super._emergencyTokenTransfer(erc20Token, to, amount);
+  }
+
+  function _emergencyEtherTransfer(
+    address to,
+    uint256 amount
+  ) internal override onlyWhoShouldReceiveFunds(to) {
+    super._emergencyEtherTransfer(to, amount);
+  }
 }

src/contracts/utils/Rescuable721.sol

@@ -2,20 +2,20 @@
 pragma solidity ^0.8.8;
 
 import {IRescuable721, IERC721} from './interfaces/IRescuable721.sol';
-import {Rescuable} from './Rescuable.sol';
+import {RescuableBase} from './RescuableBase.sol';
 
 /**
  * @title Rescuable721
  * @author defijesus.eth
  * @notice abstract contract that extend Rescuable with the methods to rescue ERC721 tokens from a contract
  */
-abstract contract Rescuable721 is Rescuable, IRescuable721 {
+abstract contract Rescuable721 is RescuableBase, IRescuable721 {
   /// @inheritdoc IRescuable721
   function emergency721TokenTransfer(
     address erc721Token,
     address to,
     uint256 tokenId
-  ) external virtual onlyRescueGuardian {
+  ) external virtual onlyWhoCanRescue {
     IERC721(erc721Token).transferFrom(address(this), to, tokenId);
 
     emit ERC721Rescued(msg.sender, erc721Token, to, tokenId);

src/contracts/utils/RescuableBase.sol

@@ -5,21 +5,55 @@ import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/IERC20.sol';
 import {SafeERC20} from 'openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol';
 import {IRescuableBase} from './interfaces/IRescuableBase.sol';
 
+/**
+ * @title RescuableBase
+ * @author BGD Labs
+ * @notice Abstract contract providing emergency rescue functionality for ERC20 tokens and native ETH
+ * accidentally sent to a contract. Implements access control via `whoCanRescue` and
+ * rescue amount limits via `maxRescue`, both of which must be defined by inheriting contracts.
+ */
 abstract contract RescuableBase is IRescuableBase {
   using SafeERC20 for IERC20;
 
+  /// @notice modifier that checks that caller is allowed address
+  modifier onlyWhoCanRescue() {
+    require(whoCanRescue(msg.sender), OnlyAuthorizedUser(msg.sender));
+    _;
+  }
+
   /// @inheritdoc IRescuableBase
   function maxRescue(address erc20Token) public view virtual returns (uint256);
 
-  function _emergencyTokenTransfer(address erc20Token, address to, uint256 amount) internal {
+  /// @inheritdoc IRescuableBase
+  function whoCanRescue(address user) public view virtual returns (bool);
+
+  /// @inheritdoc IRescuableBase
+  function emergencyTokenTransfer(
+    address erc20Token,
+    address to,
+    uint256 amount
+  ) external onlyWhoCanRescue {
+    _emergencyTokenTransfer(erc20Token, to, amount);
+  }
+
+  /// @inheritdoc IRescuableBase
+  function emergencyEtherTransfer(address to, uint256 amount) external onlyWhoCanRescue {
+    _emergencyEtherTransfer(to, amount);
+  }
+
+  function _emergencyTokenTransfer(
+    address erc20Token,
+    address to,
+    uint256 amount
+  ) internal virtual {
     uint256 max = maxRescue(erc20Token);
     amount = max > amount ? amount : max;
     IERC20(erc20Token).safeTransfer(to, amount);
 
     emit ERC20Rescued(msg.sender, erc20Token, to, amount);
   }
 
-  function _emergencyEtherTransfer(address to, uint256 amount) internal {
+  function _emergencyEtherTransfer(address to, uint256 amount) internal virtual {
     (bool success, ) = to.call{value: amount}(new bytes(0));
     if (!success) {
       revert EthTransferFailed();

src/contracts/utils/interfaces/IPermissionlessRescuable.sol

@@ -1,14 +1,13 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.8;
 
-import {IRescuableBase} from './IRescuableBase.sol';
-
 /**
  * @title IRescuable
  * @author BGD Labs
  * @notice interface containing the objects, events and methods definitions of the Rescuable contract
  */
-interface IPermissionlessRescuable is IRescuableBase {
+interface IPermissionlessRescuable {
+  error OnlyAuthorizedReceiver(address user);
   /**
    * @notice method called to rescue tokens sent erroneously to the contract. Only callable by owner
    * @param erc20Token address of the token to rescue

src/contracts/utils/interfaces/IRescuableBase.sol

@@ -8,6 +8,7 @@ pragma solidity ^0.8.8;
  */
 interface IRescuableBase {
   error EthTransferFailed();
+  error OnlyAuthorizedUser(address user);
   /**
    * @notice emitted when erc20 tokens get rescued
    * @param caller address that triggers the rescue
@@ -30,6 +31,28 @@ interface IRescuableBase {
    */
   event NativeTokensRescued(address indexed caller, address indexed to, uint256 amount);
 
+  /**
+   * @notice method called to rescue tokens sent erroneously to the contract. Only callable by owner
+   * @param erc20Token address of the token to rescue
+   * @param to address to send the tokens
+   * @param amount of tokens to rescue
+   */
+  function emergencyTokenTransfer(address erc20Token, address to, uint256 amount) external;
+
+  /**
+   * @notice method called to rescue ether sent erroneously to the contract. Only callable by owner
+   * @param to address to send the eth
+   * @param amount of eth to rescue
+   */
+  function emergencyEtherTransfer(address to, uint256 amount) external;
+
+  /**
+   * @notice Checks whether a given address is authorized to perform rescue operations.
+   * @param user address to check for rescue authorization
+   * @return true if the user is allowed to rescue tokens, false otherwise
+   */
+  function whoCanRescue(address user) external view returns (bool);
+
   /**
    * @notice method that defined the maximum amount rescuable for any given asset.
    * @dev there's currently no way to limit the rescuable "native asset", as we assume erc20s as intended underlying.

test/PermissionlessRescuable.t.sol

@@ -2,11 +2,9 @@
 pragma solidity ^0.8.0;
 
 import 'forge-std/Test.sol';
-import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/IERC20.sol';
-import {Address} from 'openzeppelin-contracts/contracts/utils/Address.sol';
 import {ERC20} from './mocks/ERC20.sol';
-import {PermissionlessRescuable as AbstractPermissionlessRescuable, IPermissionlessRescuable} from '../src/contracts/utils/PermissionlessRescuable.sol';
-import {RescuableBase, IRescuableBase} from '../src/contracts/utils/RescuableBase.sol';
+import {PermissionlessRescuable as AbstractPermissionlessRescuable} from '../src/contracts/utils/PermissionlessRescuable.sol';
+import {IRescuableBase} from '../src/contracts/utils/interfaces/IRescuableBase.sol';
 
 // Concrete implementation of PermissionlessRescuable for testing
 contract PermissionlessRescuable is AbstractPermissionlessRescuable {
@@ -25,9 +23,7 @@ contract PermissionlessRescuable is AbstractPermissionlessRescuable {
   /**
    * Mock implementation forcing 10 wei leftover
    */
-  function maxRescue(
-    address erc20
-  ) public view override(RescuableBase, IRescuableBase) returns (uint256) {
+  function maxRescue(address erc20) public view override returns (uint256) {
     if (erc20 == restrictedErc20) {
       uint256 balance = ERC20(erc20).balanceOf(address(this));
       return balance > 10 ? balance - 10 : 0;

test/Rescuable721.t.sol

@@ -2,10 +2,8 @@
 pragma solidity ^0.8.0;
 
 import 'forge-std/Test.sol';
-import {Address} from 'openzeppelin-contracts/contracts/utils/Address.sol';
-import {MockERC721, ERC721} from './mocks/ERC721.sol';
+import {MockERC721} from './mocks/ERC721.sol';
 import {Rescuable721 as AbstractRescuable721} from '../src/contracts/utils/Rescuable721.sol';
-import {RescuableBase, IRescuableBase} from '../src/contracts/utils/RescuableBase.sol';
 
 contract Rescuable721 is AbstractRescuable721 {
   address public immutable ALLOWED;
@@ -14,13 +12,11 @@ contract Rescuable721 is AbstractRescuable721 {
     ALLOWED = allowedAddress;
   }
 
-  function whoCanRescue() public view override returns (address) {
-    return ALLOWED;
+  function whoCanRescue(address user) public view override returns (bool) {
+    return user == ALLOWED;
   }
 
-  function maxRescue(
-    address
-  ) public pure override(RescuableBase, IRescuableBase) returns (uint256) {
+  function maxRescue(address) public pure override returns (uint256) {
     return type(uint256).max;
   }
 }