Contract kill/terminate with selfdestruct but also for tokens?

Question

selfdestructs(owner) refunds Eth to owner. However, it does not transfer any tokens the contract may own. These would get permanently lost.

Would something like the following work? Is there a better approach?

contract Owned {
  address owner;

  function Owned() {
    owner = msg.sender;
  }

  modifier owner_only() {
    if (msg.sender == owner) {
      _;
    }
  }

  function terminate(address tokens[]) owner_only {
    // Transfer tokens to owner (TODO: error handling)
    for(uint i = 0; i < tokens.length; i++)) {
      address addr = tokens[i];
      ERC20Token token = ERC20Token(addr);
      uint256 balance = token.balanceOf(this);
      token.transfer(owner, balance);
    }

    // Transfer Eth to owner and terminate contract
    selfdestruct(owner);
  }
}

Answer 1

selfdestruct is protocol built-in, so it can only deal with Ethers.

Your approach looks good. However bear in mind that all for loops have a maximum number of iterations, as transactions have maximum unit of work they can perform before hitting a gas limit. Based on the looks like you give the list of tokens explicitly, so this is most likely not an issue in your use case.

See loop length here.

Answer 2

I believe a better approach (in terms of gas and scalability) would be:

1. Create a method for the token owners to request the tokens themselves, getting you rid of the loop.

2. Notify the token owners, from outside the chain, that the contract is about to be killed in x time. Set x in the contract, and make every funds request (except from the owner) invalid after the deadline.

3. Selfdestruct after the deadline.

To the best of my knowledge, looping is something to be avoided as much as possible when coding contracts, as they may run out of gas, thus, harming the scalability.