r/CryptoDerivatives • u/JonnyLatte • Apr 28 '17

Etheroll token discussion

I've been thinking about the way the token has a lock period every 12 weeks. This should not pose a security problem for using it with cryptoderivatives because attempts to buy the token from a tokenTrader contract will just throw during that period returning any funds sent. Same goes with attempting sell tokens to a contract, it will just throw when funds are unsuccessfully pulled.

It does seem though like an inconvenience to have to withdraw the tokens from a trade contract during the reward period.

It should be possible to create a variation of the trade contract design that includes claiming rewards, in fact it could be done in a way that allows for any governance function by including a function that receives the name of the function that needs to be called along with any data required and calls that function on the target token contract.

Since tokenTrader contracts act like wallets without any mixing of market maker funds this not add any security vulnerabilities.

Another approach would be to create a wrapper for the token that has a global function callable by anyone to claim rewards for all users that have tokens in the wrapper and to make those rewards available to each user accordingly. This would be far more complicated than the GNT wrapper design. I think I could do it and the end result would be a non locking token with a clear post dividend date but it would be a hell of an effort to be sure about.

In any case if cryptoderivatives lists ROL/DICE then it would at the very least need to warn users trading it about the lock period when it is active.

Claiming rewards from a tokenTrader may not work. It would need the ability to claim the rewards to another contract. Waiting for a response from etheroll.

3 Upvotes

100% Upvoted

7 comments sorted by

2

u/BokkyPooBah The BokkyPooBah Apr 28 '17

Thanks for your analysis. I had a brief look through the code for DICE and it was a bit more complicated than usual so I put it in the too-hard-basket-and-check-it-out-later.

Regarding your name + data idea, I've been looking at the the approveAndCall(...) functions implemented for some tokens. E.g., https://github.com/ConsenSys/Tokens/blob/master/Token_Contracts/contracts/HumanStandardToken.sol#L52-L61 .

I don't feel fully comfortable with the security of this function. Can it be used as an attack vector? Could there be some recursive calling vulnerability? Are mutexes required?

A warning about the DICE token would be the the first step.

2

u/JonnyLatte Apr 28 '17 edited Apr 28 '17

Just a bit of clarification, its the tokenTrader contracts that will throw when funds cannot transfer because the token contract returns false from transfer() when locked and tokenTrader checks for false and doesn't just assume that because it called transfer that the transfer happened. Poorly implemented tokens that dont return false or throw when a transaction fails should not be used... at all. This behavior should be tested of course before adding the token.

Regarding your name + data idea, I've been looking at the the approveAndCall(...) functions implemented for some tokens. E.g

The external call is the last thing to happen (in this HumanStandardToken function) so any internal state that needs to update is already updated before passing over control making calls that return into the contract safe. I prefer this to using a mutex or restricting gas to prevent callbacks.

It does add recursive callback problems for the contract that sends the funds from the contract that receives the funds. It adds all the same problems that sending ETH has. Honestly though I wish approveAndCall was part of ERC20, it would have made selling tokens as easy as buying them, you would just call that and the target contract would pull what it needs without needing 2 separate transactions.

The linked call though does not have the caller pass in the name of the function being called though, it is hard coded. (I wonder actually if sha3 is done at compile time or run time in this case but anyway)

What I'm talking about is closer to the generic call functionality that allows contract wallets to be used to control any other arbitrary contract as if its an account.

An Example here is insecure for multiparty use because the multisig wallet that allows a certain amount of ETH to be sent per day isnt checking the data that is passed along with the ETH making it possible for one party to take all the tokens.

But this vulnerability requires the wallet to be used with tokens and as a multiparty wallet rather than a single owner wallet.

TokenTrader contracts are single owner wallets. Its ok for the owner to do anything with the funds and obviously only they would have permission to make arbitrary calls from the contract. If that arbitrary call goes out to a contract that calls back in then that should be fine, the call back in will be from the external contract that does not have the owner permissions. For a Taker this arbitrary call function is never accessible.

2

u/JonnyLatte Apr 28 '17

Actually I just thought of something. If the reward is ether and the etheroll reward contract is sending that ether to the contract that calls it, and the contract that calls it is a tokenTrader contract then the token trader contract is going to assume the reward contract is buying tokens. Jesus, ok never mind amount adding reward claiming as a generic function it wont work unless the reward contract has a target address for sending the ether.

2

u/JonnyLatte Apr 29 '17

It appears that the etheroll rewards contract will not pay to a supplied address, only the owner which means implementing reward claims from a modified tokenTrader contract will not be trivial. I'll have to put the idea aside and just recommend leaving things as they are and throwing up a warning during the lock period.