Scope:

• RaidenMicroTransferChannels contract: https://github.com/raiden-network/microraiden/blob/master/contracts/contracts/RaidenMicroTransferChannels.sol
• MicroRaidenEIP712Helper contract: https://github.com/raiden-network/microraiden/blob/master/contracts/contracts/MicroRaidenEIP712Helper.sol
• ECVerify library: https://github.com/raiden-network/microraiden/blob/master/contracts/contracts/lib/ECVerify.sol
• Token.sol: https://github.com/raiden-network/microraiden/blob/master/contracts/contracts/Token.sol

Outcome:

1. redundant: either token or token_address, isToken, prefix
   fixed in Remove isToken, prefix, token_address #196

Reviewed version for issues below: https://github.com/raiden-network/microraiden/tree/81920e4162da8b00ba54212de619668d42c11120

2. tl;dr In this contract, the sender needs to fully trust the owner.
   Owner can be malicious / his account can be compromised and change MicroRaidenEIP712Helper.getMessageHash by deploying another contract with malicious code and changing the address in RaidenMicroTransferChannels with setEip712HelperContract.

Possible scenarios:

• getMessageHash is changed -> channels are locked, because the sender's signature from _balance_msg_sig cannot be recovered.
• owner colludes with 1 or > receivers to modify the function so that they can get more tokens out of the channels than they are owed (amount of tokens limited by the actual channel deposit)

The receiver can use any hash of a signed message from the sender, to be returned in a hardcoded way in the EIP712 helper.

(e.g. : in RaidenMicroTransferChannels.cooperativeClose -> verifyBalanceProof, getMessageHash can return the hash corresponding to _balance - 100 instead of _balance. This means the receiver can provide an authentic _balance_msg_sig with balance = _balance - 100, but use _balance as an argument for cooperativeClose.)

Possible solution:

Adding a function to change the owner would be important. Specially if the owner address gets compromised.
I would put another role. eip712updater so you can set it to 0x0 when the eip712 standard is stabilized. And still maintain the option to setLatestVersionAddress. Needed methods: changeOwner, changeEip712updater .. In the constructors both vars can be set to msg.sender

Having owner and eip712updater diferently allows to cancel the possibility to change the eip712 helper and still update the version.

IMO, if there is an eip712 update, I would deploy a full new contract.

3. This smart contract has a huge risk that people just do a normal transferto this contract of pure ERC20 tokens thinking they were ERC223 tokens. The consequence of that is that he just burn the tokens.
   So, I would add a protection where an owner could remove any foreign tokens sent erroneously to this contract. and also this extra tokens that are there but should not be there. This last will require to add a global variable with the tokens that is supposed to have the contract. This variable will be updated every time a channel is created, toped, or settled.

4. An event for setLatestVersionAddress and setEip712HelperContract would be good.

5. I agree with Lefteris about using ENS for last version. This is much more easy and you remove the need for the owner here.

6. In

   microraiden/contracts/contracts/RaidenMicroTransferChannels.sol

   Line 64 in 81920e4

   event ChannelCreated(

   I would add _open_block_numberlike other ovents. I know it’s not stricly necessary, but standarizes the format of the events.

7. require(_challenge_period > 500)

8. require(Token(_token).totalSupply() > 0) in RaidenMicroTransferChannels constructor, to check if it is really a token.

9. Side comment: I would expect that for EIP223 the data follows the solidity convention: 4 bytes (method selector) and then the params. In this case you would not have to distinguish the function by the length of the data which is not very elegant. ( but more optimal )
   

   microraiden/contracts/contracts/RaidenMicroTransferChannels.sol

   Line 154 in 9290006

   // topUp - receiver address (20 bytes) + open_block_number (4 bytes) = 24 bytes

10. Why do you don’t use eip712 for _closing_sig ?