Merge pull request #348 from SylTi/refactor/HasNoToken

refactor HasNoTokens.sol to extract reclaimToken

contracts/ownership/CanReclaimToken.sol

@@ -0,0 +1,23 @@
+pragma solidity ^0.4.11;
+
+import "./Ownable.sol";
+import "../token/ERC20Basic.sol";
+
+/**
+ * @title Contracts that should be able to recover tokens
+ * @author SylTi
+ * @dev This allow a contract to recover any ERC20 token received in a contract by transfering the balance to the contract owner.
+ * This will prevent any accidental loss of tokens.
+ */
+contract CanReclaimToken is Ownable {
+
+  /**
+   * @dev Reclaim all ERC20Basic compatible tokens
+   * @param token ERC20Basic The address of the token contract
+   */
+  function reclaimToken(ERC20Basic token) external onlyOwner {
+    uint256 balance = token.balanceOf(this);
+    token.transfer(owner, balance);
+  }
+
+}

contracts/ownership/HasNoTokens.sol

@@ -1,7 +1,6 @@
 pragma solidity ^0.4.11;
 
-import "./Ownable.sol";
-import "../token/ERC20Basic.sol";
+import "./CanReclaimToken.sol";
 
 /**
  * @title Contracts that should not own Tokens
@@ -10,7 +9,7 @@ import "../token/ERC20Basic.sol";
  * Should tokens (any ERC20Basic compatible) end up in the contract, it allows the
  * owner to reclaim the tokens.
  */
-contract HasNoTokens is Ownable {
+contract HasNoTokens is CanReclaimToken {
 
  /**
   * @dev Reject all ERC23 compatible tokens
@@ -22,13 +21,4 @@ contract HasNoTokens is Ownable {
     revert();
   }
 
-  /**
-   * @dev Reclaim all ERC20Basic compatible tokens
-   * @param tokenAddr address The address of the token contract
-   */
-  function reclaimToken(address tokenAddr) external onlyOwner {
-    ERC20Basic tokenInst = ERC20Basic(tokenAddr);
-    uint256 balance = tokenInst.balanceOf(this);
-    tokenInst.transfer(owner, balance);
-  }
 }

test/CanReclaimToken.js

@@ -0,0 +1,35 @@
+'use strict';
+import expectThrow from './helpers/expectThrow';
+import toPromise from './helpers/toPromise';
+const CanReclaimToken = artifacts.require('../contracts/ownership/CanReclaimToken.sol');
+const BasicTokenMock = artifacts.require("./helpers/BasicTokenMock.sol");
+
+contract('CanReclaimToken', function(accounts) {
+  let token = null;
+  let canReclaimToken = null;
+
+  beforeEach(async function() {
+    // Create contract and token
+    token = await BasicTokenMock.new(accounts[0], 100);
+    canReclaimToken = await CanReclaimToken.new();
+    // Force token into contract
+    await token.transfer(canReclaimToken.address, 10);
+    const startBalance = await token.balanceOf(canReclaimToken.address);
+    assert.equal(startBalance, 10);
+  });
+
+  it('should allow owner to reclaim tokens', async function() {
+    const ownerStartBalance = await token.balanceOf(accounts[0]);
+    await canReclaimToken.reclaimToken(token.address);
+    const ownerFinalBalance = await token.balanceOf(accounts[0]);
+    const finalBalance = await token.balanceOf(canReclaimToken.address);
+    assert.equal(finalBalance, 0);
+    assert.equal(ownerFinalBalance - ownerStartBalance, 10);
+  });
+
+  it('should allow only owner to reclaim tokens', async function() {
+    await expectThrow(
+      canReclaimToken.reclaimToken(token.address, {from: accounts[1]}),
+    );
+  });
+});