contracts: created CanReclaimTokens

contracts/DarknodeRegistry/DarknodeRegistry.sol

@@ -6,6 +6,7 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 import "../RenToken/RenToken.sol";
 import "./DarknodeRegistryStore.sol";
 import "../libraries/Claimable.sol";
+import "../libraries/CanReclaimTokens.sol";
 
 interface IDarknodePaymentStore {
 }
@@ -20,7 +21,7 @@ interface IDarknodeSlasher {
 
 /// @notice DarknodeRegistry is responsible for the registration and
 /// deregistration of Darknodes.
-contract DarknodeRegistry is Claimable {
+contract DarknodeRegistry is Claimable, CanReclaimTokens {
     using SafeMath for uint256;
 
     string public VERSION; // Passed in as a constructor parameter.
@@ -169,16 +170,6 @@ contract DarknodeRegistry is Claimable {
         numDarknodesPreviousEpoch = 0;
     }
 
-    /// @notice Allow the owner of the contract to recover funds accidentally
-    /// sent to the contract. To withdraw ETH, the token should be set to `0x0`.
-    function recoverTokens(address _token) external onlyOwner {
-        if (_token == address(0x0)) {
-            msg.sender.transfer(address(this).balance);
-        } else {
-            ERC20(_token).transfer(msg.sender, ERC20(_token).balanceOf(address(this)));
-        }
-    }
-
     /// @notice Register a darknode and transfer the bond to this contract.
     /// Before registering, the bond transfer must be approved in the REN
     /// contract. The caller must provide a public encryption key for the

contracts/DarknodeRegistry/DarknodeRegistryStore.sol

@@ -5,11 +5,12 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 import "../libraries/Claimable.sol";
 import "../libraries/LinkedList.sol";
 import "../RenToken/RenToken.sol";
+import "../libraries/CanReclaimTokens.sol";
 
 /// @notice This contract stores data and funds for the DarknodeRegistry
 /// contract. The data / fund logic and storage have been separated to improve
 /// upgradability.
-contract DarknodeRegistryStore is Claimable {
+contract DarknodeRegistryStore is Claimable, CanReclaimTokens {
     using SafeMath for uint256;
 
     string public VERSION; // Passed in as a constructor parameter.
@@ -61,20 +62,7 @@ contract DarknodeRegistryStore is Claimable {
     ) public {
         VERSION = _VERSION;
         ren = _ren;
-    }
-
-    /// @notice Allow the owner of the contract to recover funds accidentally
-    /// sent to the contract. To withdraw ETH, the token should be set to `0x0`.
-    /// @dev The owner is the Darknode Registry so it would need to be updated
-    /// first before being able to recover funds.
-    function recoverTokens(address _token) external onlyOwner {
-        require(_token != address(ren), "DarknodeRegistryStore: not allowed to recover REN");
-
-        if (_token == address(0x0)) {
-            msg.sender.transfer(address(this).balance);
-        } else {
-            ERC20(_token).transfer(msg.sender, ERC20(_token).balanceOf(address(this)));
-        }
+        blacklistRecoverableToken(address(ren));
     }
 
     /// @notice Instantiates a darknode and appends it to the darknodes

contracts/Shifter/ERC20Shifted.sol

@@ -4,25 +4,16 @@ import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";
 import "openzeppelin-solidity/contracts/token/ERC20/ERC20Detailed.sol";
 
 import "../libraries/Claimable.sol";
+import "../libraries/CanReclaimTokens.sol";
 
 /// @notice ERC20Shifted represents a digital asset that has been bridged on to
 /// the Ethereum ledger. It exposes mint and burn functions that can only be
 /// called by it's associated Shifter.
-contract ERC20Shifted is ERC20, ERC20Detailed, Claimable {
+contract ERC20Shifted is ERC20, ERC20Detailed, Claimable, CanReclaimTokens {
 
     /* solium-disable-next-line no-empty-blocks */
     constructor(string memory _name, string memory _symbol, uint8 _decimals) public ERC20Detailed(_name, _symbol, _decimals) {}
 
-    /// @notice Allow the owner of the contract to recover funds accidentally
-    /// sent to the contract. To withdraw ETH, the token should be set to `0x0`.
-    function recoverTokens(address _token) external onlyOwner {
-        if (_token == address(0x0)) {
-            msg.sender.transfer(address(this).balance);
-        } else {
-            ERC20(_token).transfer(msg.sender, ERC20(_token).balanceOf(address(this)));
-        }
-    }
-
     function burn(address _from, uint256 _amount) public onlyOwner {
         _burn(_from, _amount);
     }

contracts/Shifter/Shifter.sol

@@ -7,11 +7,12 @@ import "openzeppelin-solidity/contracts/cryptography/ECDSA.sol";
 import "../libraries/Claimable.sol";
 import "../libraries/String.sol";
 import "./ERC20Shifted.sol";
+import "../libraries/CanReclaimTokens.sol";
 
 /// @notice Shifter handles verifying mint and burn requests. A mintAuthority
 /// approves new assets to be minted by providing a digital signature. An owner
 /// of an asset can request for it to be burnt.
-contract Shifter is Claimable {
+contract Shifter is Claimable, CanReclaimTokens {
     using SafeMath for uint256;
 
     uint8 public version = 2;
@@ -74,16 +75,6 @@ contract Shifter is Claimable {
         updateFeeRecipient(_feeRecipient);
     }
 
-    /// @notice Allow the owner of the contract to recover funds accidentally
-    /// sent to the contract. To withdraw ETH, the token should be set to `0x0`.
-    function recoverTokens(address _token) external onlyOwner {
-        if (_token == address(0x0)) {
-            msg.sender.transfer(address(this).balance);
-        } else {
-            ERC20(_token).transfer(msg.sender, ERC20(_token).balanceOf(address(this)));
-        }
-    }
-
     // Public functions ////////////////////////////////////////////////////////
 
     /// @notice Claims ownership of the token passed in to the constructor.

contracts/Shifter/ShifterRegistry.sol

@@ -4,10 +4,11 @@ import "../libraries/Claimable.sol";
 import "./ERC20Shifted.sol";
 import "../libraries/LinkedList.sol";
 import "./IShifter.sol";
+import "../libraries/CanReclaimTokens.sol";
 
 /// @notice ShifterRegistry is a mapping from assets to their associated
 /// ERC20Shifted and Shifter contracts.
-contract ShifterRegistry is Claimable {
+contract ShifterRegistry is Claimable, CanReclaimTokens {
 
     /// @dev The symbol is included twice because strings have to be hashed
     /// first in order to be used as a log index/topic.
@@ -30,16 +31,6 @@ contract ShifterRegistry is Claimable {
     /// @notice A map of token symbols to canonical shifter addresses
     mapping (string=>address) private tokenBySymbol;
 
-    /// @notice Allow the owner of the contract to recover funds accidentally
-    /// sent to the contract. To withdraw ETH, the token should be set to `0x0`.
-    function recoverTokens(address _token) external onlyOwner {
-        if (_token == address(0x0)) {
-            msg.sender.transfer(address(this).balance);
-        } else {
-            ERC20(_token).transfer(msg.sender, ERC20(_token).balanceOf(address(this)));
-        }
-    }
-
     /// @notice Allow the owner to set the shifter address for a given
     ///         ERC20Shifted token contract.
     ///

contracts/libraries/CanReclaimTokens.sol

@@ -0,0 +1,25 @@
+pragma solidity ^0.5.12;
+
+import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
+import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";
+
+contract CanReclaimTokens is Ownable {
+
+    mapping(address => bool) private recoverableTokensBlacklist;
+
+    function blacklistRecoverableToken(address _token) public {
+        recoverableTokensBlacklist[_token] = true;
+    }
+
+    /// @notice Allow the owner of the contract to recover funds accidentally
+    /// sent to the contract. To withdraw ETH, the token should be set to `0x0`.
+    function recoverTokens(address _token) external onlyOwner {
+        require(!recoverableTokensBlacklist[_token], "CanReclaimTokens: token is not recoverable");
+
+        if (_token == address(0x0)) {
+            msg.sender.transfer(address(this).balance);
+        } else {
+            ERC20(_token).transfer(msg.sender, ERC20(_token).balanceOf(address(this)));
+        }
+    }
+}

test/DarknodeRegistry.ts

@@ -687,7 +687,7 @@ contract("DarknodeRegistry", (accounts: string[]) => {
 
             // Can't recover REN
             await dnrs.recoverTokens(ren.address, { from: accounts[0] })
-                .should.be.rejectedWith(/DarknodeRegistryStore: not allowed to recover REN/);
+                .should.be.rejectedWith(/CanReclaimTokens: token is not recoverable/);
 
             // Only the owner can recover tokens
             await dnrs.recoverTokens(token.address, { from: accounts[1] })