Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254)

* remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner

* remove unused ERC223TokenMock

* remove Contactable

* remove TokenDestructible

* remove DeprecatedERC721

* inline Destructible#destroy in Bounty

* remove Destructible

contracts/bounties/BreakInvariantBounty.sol

@@ -2,14 +2,13 @@ pragma solidity ^0.4.24;
 
 
 import "../payment/PullPayment.sol";
-import "../lifecycle/Destructible.sol";
 
 
 /**
  * @title BreakInvariantBounty
  * @dev This bounty will pay out to a researcher if they break invariant logic of the contract.
  */
-contract BreakInvariantBounty is PullPayment, Destructible {
+contract BreakInvariantBounty is PullPayment, Ownable {
   bool public claimed;
   mapping(address => address) public researchers;
 
@@ -47,6 +46,13 @@ contract BreakInvariantBounty is PullPayment, Destructible {
     claimed = true;
   }
 
+  /**
+   * @dev Transfers the current balance to the owner and terminates the contract.
+   */
+  function destroy() public onlyOwner {
+    selfdestruct(owner);
+  }
+
   /**
    * @dev Internal function to deploy the target contract.
    * @return A target contract address

contracts/lifecycle/Destructible.sol

@@ -1,22 +0,0 @@
-pragma solidity ^0.4.24;
-
-
-import "../ownership/Ownable.sol";
-
-
-/**
- * @title Destructible
- * @dev Base contract that can be destroyed by owner. All funds in contract will be sent to the owner.
- */
-contract Destructible is Ownable {
-  /**
-   * @dev Transfers the current balance to the owner and terminates the contract.
-   */
-  function destroy() public onlyOwner {
-    selfdestruct(owner);
-  }
-
-  function destroyAndSend(address _recipient) public onlyOwner {
-    selfdestruct(_recipient);
-  }
-}

contracts/lifecycle/TokenDestructible.sol

@@ -1,36 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "../ownership/Ownable.sol";
-import "../token/ERC20/IERC20.sol";
-
-
-/**
- * @title TokenDestructible:
- * @author Remco Bloemen <remco@2π.com>
- * @dev Base contract that can be destroyed by owner. All funds in contract including
- * listed tokens will be sent to the owner.
- */
-contract TokenDestructible is Ownable {
-
-  constructor() public payable { }
-
-  /**
-   * @notice Terminate contract and refund to owner
-   * @param _tokens List of addresses of ERC20 token contracts to
-   refund.
-   * @notice The called token contracts could try to re-enter this contract. Only
-   supply token contracts you trust.
-   */
-  function destroy(address[] _tokens) public onlyOwner {
-
-    // Transfer tokens to owner
-    for (uint256 i = 0; i < _tokens.length; i++) {
-      IERC20 token = IERC20(_tokens[i]);
-      uint256 balance = token.balanceOf(this);
-      token.transfer(owner, balance);
-    }
-
-    // Transfer Eth to owner and terminate contract
-    selfdestruct(owner);
-  }
-}

contracts/mocks/DestructibleMock.sol

@@ -1,8 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "../lifecycle/Destructible.sol";
-
-
-contract DestructibleMock is Destructible {
-  function() public payable {}
-}

contracts/mocks/ERC223TokenMock.sol

@@ -1,33 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "../token/ERC20/ERC20.sol";
-
-
-contract ERC223ContractInterface {
-  function tokenFallback(address _from, uint256 _value, bytes _data) external;
-}
-
-
-contract ERC223TokenMock is ERC20 {
-
-  constructor(address _initialAccount, uint256 _initialBalance) public {
-    _mint(_initialAccount, _initialBalance);
-  }
-
-  // ERC223 compatible transfer function (except the name)
-  function transferERC223(address _to, uint256 _value, bytes _data) public
-    returns (bool success)
-  {
-    transfer(_to, _value);
-    bool isContract = false;
-    // solium-disable-next-line security/no-inline-assembly
-    assembly {
-      isContract := not(iszero(extcodesize(_to)))
-    }
-    if (isContract) {
-      ERC223ContractInterface receiver = ERC223ContractInterface(_to);
-      receiver.tokenFallback(msg.sender, _value, _data);
-    }
-    return true;
-  }
-}

contracts/mocks/HasNoEtherTest.sol

@@ -1,12 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "../../contracts/ownership/HasNoEther.sol";
-
-
-contract HasNoEtherTest is HasNoEther {
-
-  // Constructor with explicit payable — should still fail
-  constructor() public payable {
-  }
-
-}

contracts/ownership/Contactable.sol

@@ -1,22 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "./Ownable.sol";
-
-
-/**
- * @title Contactable token
- * @dev Basic version of a contactable contract, allowing the owner to provide a string with their
- * contact information.
- */
-contract Contactable is Ownable {
-
-  string public contactInformation;
-
-  /**
-    * @dev Allows the owner to set a string with their contact information.
-    * @param _info The contact information to attach to the contract.
-    */
-  function setContactInformation(string _info) public onlyOwner {
-    contactInformation = _info;
-  }
-}

contracts/ownership/HasNoContracts.sol

@@ -1,22 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "./Ownable.sol";
-
-
-/**
- * @title Contracts that should not own Contracts
- * @author Remco Bloemen <remco@2π.com>
- * @dev Should contracts (anything Ownable) end up being owned by this contract, it allows the owner
- * of this contract to reclaim ownership of the contracts.
- */
-contract HasNoContracts is Ownable {
-
-  /**
-   * @dev Reclaim ownership of Ownable contracts
-   * @param _contractAddr The address of the Ownable to be reclaimed.
-   */
-  function reclaimContract(address _contractAddr) external onlyOwner {
-    Ownable contractInst = Ownable(_contractAddr);
-    contractInst.transferOwnership(owner);
-  }
-}

contracts/ownership/HasNoEther.sol

@@ -1,41 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "./Ownable.sol";
-
-
-/**
- * @title Contracts that should not own Ether
- * @author Remco Bloemen <remco@2π.com>
- * @dev This tries to block incoming ether to prevent accidental loss of Ether. Should Ether end up
- * in the contract, it will allow the owner to reclaim this Ether.
- * @notice Ether can still be sent to this contract by:
- * calling functions labeled `payable`
- * `selfdestruct(contract_address)`
- * mining directly to the contract address
- */
-contract HasNoEther is Ownable {
-
-  /**
-  * @dev Constructor that rejects incoming Ether
-  * The `payable` flag is added so we can access `msg.value` without compiler warning. If we
-  * leave out payable, then Solidity will allow inheriting contracts to implement a payable
-  * constructor. By doing it this way we prevent a payable constructor from working. Alternatively
-  * we could use assembly to access msg.value.
-  */
-  constructor() public payable {
-    require(msg.value == 0);
-  }
-
-  /**
-   * @dev Disallows direct send by setting a default function without the `payable` flag.
-   */
-  function() external {
-  }
-
-  /**
-   * @dev Transfer all Ether held by the contract to the owner.
-   */
-  function reclaimEther() external onlyOwner {
-    owner.transfer(address(this).balance);
-  }
-}

contracts/ownership/HasNoTokens.sol

@@ -1,35 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "./CanReclaimToken.sol";
-
-
-/**
- * @title Contracts that should not own Tokens
- * @author Remco Bloemen <remco@2π.com>
- * @dev This blocks incoming ERC223 tokens to prevent accidental loss of tokens.
- * Should tokens (any ERC20 compatible) end up in the contract, it allows the
- * owner to reclaim the tokens.
- */
-contract HasNoTokens is CanReclaimToken {
-
- /**
-  * @dev Reject all ERC223 compatible tokens
-  * @param _from address The address that is transferring the tokens
-  * @param _value uint256 the amount of the specified token
-  * @param _data Bytes The data passed from the caller.
-  */
-  function tokenFallback(
-    address _from,
-    uint256 _value,
-    bytes _data
-  )
-    external
-    pure
-  {
-    _from;
-    _value;
-    _data;
-    revert();
-  }
-
-}

contracts/ownership/NoOwner.sol

@@ -1,15 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "./HasNoEther.sol";
-import "./HasNoTokens.sol";
-import "./HasNoContracts.sol";
-
-
-/**
- * @title Base contract for contracts that should not own things.
- * @author Remco Bloemen <remco@2π.com>
- * @dev Solves a class of errors where a contract accidentally becomes owner of Ether, Tokens or
- * Owned contracts. See respective base contracts for details.
- */
-contract NoOwner is HasNoEther, HasNoTokens, HasNoContracts {
-}

contracts/token/ERC721/IDeprecatedERC721.sol

@@ -1,15 +0,0 @@
-pragma solidity ^0.4.24;
-
-import "./IERC721.sol";
-
-
-/**
- * @title ERC-721 methods shipped in OpenZeppelin v1.7.0, removed in the latest version of the standard
- * @dev Only use this interface for compatibility with previously deployed contracts
- * Use ERC721 for interacting with new contracts which are standard-compliant
- */
-contract IDeprecatedERC721 is IERC721 {
-  function takeOwnership(uint256 _tokenId) public;
-  function transfer(address _to, uint256 _tokenId) public;
-  function tokensOf(address _owner) public view returns (uint256[]);
-}

test/lifecycle/Destructible.test.js

@@ -1,33 +0,0 @@
-const DestructibleMock = artifacts.require('DestructibleMock');
-const { ethGetBalance } = require('../helpers/web3');
-
-const BigNumber = web3.BigNumber;
-
-require('chai')
-  .use(require('chai-bignumber')(BigNumber))
-  .should();
-
-contract('Destructible', function ([_, owner, recipient]) {
-  beforeEach(async function () {
-    this.destructible = await DestructibleMock.new({ from: owner });
-    await web3.eth.sendTransaction({
-      from: owner,
-      to: this.destructible.address,
-      value: web3.toWei('10', 'ether'),
-    });
-  });
-
-  it('should send balance to owner after destruction', async function () {
-    const initBalance = await ethGetBalance(owner);
-    await this.destructible.destroy({ from: owner });
-    const newBalance = await ethGetBalance(owner);
-    newBalance.should.be.bignumber.gt(initBalance);
-  });
-
-  it('should send balance to recepient after destruction', async function () {
-    const initBalance = await ethGetBalance(recipient);
-    await this.destructible.destroyAndSend(recipient, { from: owner });
-    const newBalance = await ethGetBalance(recipient);
-    newBalance.should.be.bignumber.gt(initBalance);
-  });
-});

test/lifecycle/TokenDestructible.test.js

@@ -1,39 +0,0 @@
-const { ethGetBalance } = require('../helpers/web3');
-
-const TokenDestructible = artifacts.require('TokenDestructible');
-const ERC20Mock = artifacts.require('ERC20Mock');
-
-const BigNumber = web3.BigNumber;
-
-require('chai')
-  .use(require('chai-bignumber')(BigNumber))
-  .should();
-
-contract('TokenDestructible', function ([_, owner]) {
-  let tokenDestructible;
-
-  beforeEach(async function () {
-    tokenDestructible = await TokenDestructible.new({
-      from: owner,
-      value: web3.toWei('10', 'ether'),
-    });
-  });
-
-  it('should send balance to owner after destruction', async function () {
-    const initBalance = await ethGetBalance(owner);
-    await tokenDestructible.destroy([], { from: owner });
-
-    const newBalance = await ethGetBalance(owner);
-    newBalance.should.be.bignumber.gt(initBalance);
-  });
-
-  it('should send tokens to owner after destruction', async function () {
-    const token = await ERC20Mock.new(tokenDestructible.address, 100);
-    (await token.balanceOf(tokenDestructible.address)).should.be.bignumber.equal(100);
-    (await token.balanceOf(owner)).should.be.bignumber.equal(0);
-
-    await tokenDestructible.destroy([token.address], { from: owner });
-    (await token.balanceOf(tokenDestructible.address)).should.be.bignumber.equal(0);
-    (await token.balanceOf(owner)).should.be.bignumber.equal(100);
-  });
-});

test/ownership/Contactable.test.js

@@ -1,25 +0,0 @@
-const Contactable = artifacts.require('Contactable');
-
-contract('Contactable', function () {
-  let contactable;
-
-  beforeEach(async function () {
-    contactable = await Contactable.new();
-  });
-
-  it('should have an empty contact info', async function () {
-    (await contactable.contactInformation()).should.equal('');
-  });
-
-  describe('after setting the contact information', function () {
-    const contactInfo = 'contact information';
-
-    beforeEach(async function () {
-      await contactable.setContactInformation(contactInfo);
-    });
-
-    it('should return the setted contact information', async function () {
-      (await contactable.contactInformation()).should.equal(contactInfo);
-    });
-  });
-});

test/ownership/HasNoContracts.test.js

@@ -1,29 +0,0 @@
-const { expectThrow } = require('../helpers/expectThrow');
-
-const Ownable = artifacts.require('Ownable');
-const HasNoContracts = artifacts.require('HasNoContracts');
-
-contract('HasNoContracts', function ([_, owner, anyone]) {
-  let hasNoContracts = null;
-  let ownable = null;
-
-  beforeEach(async function () {
-    // Create contract and token
-    hasNoContracts = await HasNoContracts.new({ from: owner });
-    ownable = await Ownable.new({ from: owner });
-
-    // Force ownership into contract
-    await ownable.transferOwnership(hasNoContracts.address, { from: owner });
-  });
-
-  it('should allow owner to reclaim contracts', async function () {
-    await hasNoContracts.reclaimContract(ownable.address, { from: owner });
-    (await ownable.owner()).should.equal(owner);
-  });
-
-  it('should allow only owner to reclaim contracts', async function () {
-    await expectThrow(
-      hasNoContracts.reclaimContract(ownable.address, { from: anyone })
-    );
-  });
-});

test/ownership/HasNoEther.test.js

@@ -1,61 +0,0 @@
-const { expectThrow } = require('../helpers/expectThrow');
-const { ethSendTransaction, ethGetBalance } = require('../helpers/web3');
-
-const HasNoEtherTest = artifacts.require('HasNoEtherTest');
-const ForceEther = artifacts.require('ForceEther');
-
-const BigNumber = web3.BigNumber;
-
-require('chai')
-  .use(require('chai-bignumber')(BigNumber))
-  .should();
-
-contract('HasNoEther', function ([_, owner, anyone]) {
-  const amount = web3.toWei('1', 'ether');
-
-  beforeEach(async function () {
-    this.hasNoEther = await HasNoEtherTest.new({ from: owner });
-  });
-
-  it('should not accept ether in constructor', async function () {
-    await expectThrow(HasNoEtherTest.new({ value: amount }));
-  });
-
-  it('should not accept ether', async function () {
-    await expectThrow(
-      ethSendTransaction({
-        from: owner,
-        to: this.hasNoEther.address,
-        value: amount,
-      }),
-    );
-  });
-
-  it('should allow owner to reclaim ether', async function () {
-    const startBalance = await ethGetBalance(this.hasNoEther.address);
-    startBalance.should.be.bignumber.equal(0);
-
-    // Force ether into it
-    const forceEther = await ForceEther.new({ value: amount });
-    await forceEther.destroyAndSend(this.hasNoEther.address);
-    (await ethGetBalance(this.hasNoEther.address)).should.be.bignumber.equal(amount);
-
-    // Reclaim
-    const ownerStartBalance = await ethGetBalance(owner);
-    await this.hasNoEther.reclaimEther({ from: owner });
-    const ownerFinalBalance = await ethGetBalance(owner);
-    ownerFinalBalance.should.be.bignumber.gt(ownerStartBalance);
-
-    (await ethGetBalance(this.hasNoEther.address)).should.be.bignumber.equal(0);
-  });
-
-  it('should allow only owner to reclaim ether', async function () {
-    // Force ether into it
-    const forceEther = await ForceEther.new({ value: amount });
-    await forceEther.destroyAndSend(this.hasNoEther.address);
-    (await ethGetBalance(this.hasNoEther.address)).should.be.bignumber.equal(amount);
-
-    // Reclaim
-    await expectThrow(this.hasNoEther.reclaimEther({ from: anyone }));
-  });
-});

test/ownership/HasNoTokens.test.js

@@ -1,46 +0,0 @@
-const { expectThrow } = require('../helpers/expectThrow');
-
-const HasNoTokens = artifacts.require('HasNoTokens');
-const ERC223TokenMock = artifacts.require('ERC223TokenMock');
-
-const BigNumber = web3.BigNumber;
-
-require('chai')
-  .use(require('chai-bignumber')(BigNumber))
-  .should();
-
-contract('HasNoTokens', function ([_, owner, initialAccount, anyone]) {
-  let hasNoTokens = null;
-  let token = null;
-
-  beforeEach(async function () {
-    // Create contract and token
-    hasNoTokens = await HasNoTokens.new({ from: owner });
-    token = await ERC223TokenMock.new(initialAccount, 100);
-
-    // Force token into contract
-    await token.transfer(hasNoTokens.address, 10, { from: initialAccount });
-
-    (await token.balanceOf(hasNoTokens.address)).should.be.bignumber.equal(10);
-  });
-
-  it('should not accept ERC223 tokens', async function () {
-    await expectThrow(token.transferERC223(hasNoTokens.address, 10, '', { from: initialAccount }));
-  });
-
-  it('should allow owner to reclaim tokens', async function () {
-    const ownerStartBalance = await token.balanceOf(owner);
-    await hasNoTokens.reclaimToken(token.address, { from: owner });
-
-    const ownerFinalBalance = await token.balanceOf(owner);
-    ownerFinalBalance.sub(ownerStartBalance).should.be.bignumber.equal(10);
-
-    (await token.balanceOf(hasNoTokens.address)).should.be.bignumber.equal(0);
-  });
-
-  it('should allow only owner to reclaim tokens', async function () {
-    await expectThrow(
-      hasNoTokens.reclaimToken(token.address, { from: anyone })
-    );
-  });
-});