Comments on commit `0a7bc27e541a8ea0a2ad159c61f26eaa9c5e0caa`

[yaronvel]

1. Second player can submit the same token and override player-1 payment address.

2. Second player can mimic player-1 move by choosing the same token (even after the first issue is solved) and revealing it after player-1 reveals his secret. This can be considered as an attack vector, as player-1 would still loose gas in this case. You can fix it by

   function token(uint v) constant returns(bytes32) {
       return sha3(v,msg.sender);
   }

3. There are no timeouts. Need to give player 1 his money back if player 2 doesn't join after T hours. And need to give either player 1 or player 2 their money (+ compensation) if the second player does not reveal his move.

4. The same player can call collect twice and steal the other player deposit.

5. Some general code styling: i. Use event to log moves. ii. token is usually a name for actual tokens (currencies). iii. using int8 will not save you storage (will only save storage if inside a structure) and might be a source for bugs for you (overflow) or for the compiler (they had overflow issue for such types). iv. Calling delete is a good citizenship, but probably a waste of gas.

[elazarg]

Thanks!

1. Is very bad. I think it shouldn't be hard to fix though.
2. Is also bad. As for your fix, I am a bit hesitant in tying things to msg.sender since the secret must come fresh from an external account, and using msg.sender feels like involving contract... It's not a good reason or excuse for anything, just a vague feeling.
3. is intentional; I did not want to mess with timeouts yet. Before the second player joined, that's a serious issue. After he joins, as noted in the comment, I assume the loser will want to get the penalty back. A timeout will have its issues too.
4. Is incorrect, as far as I can see. The token is deleted after use, and the payment is sent to the player that originally issued the token. I don't care who is initiating this transaction.

Style: i. I will learn to use events. ii. Correct. What's a better name? I do not like hash. Perhaps I should call it "commitment". iii. My intention was not saving storage, but rather using a "more appropriate" type, and requiring the explicit cast. The possible values are 0, 1 and 2 - how can that overflow? Anyway, I guess an enum will be more appropriate. iv. I use delete in two different places, for different reasons

1. In collect I prevent the use of the same hash more than once.
2. In cleanup I try to reset the state. I hadn't thought about the gas costs, but someone should pay for this reset, I assume? I have a feeling that this reset is vulnerable too.

[elazarg]

I tried to fix (1) by checking for existence:

    if (players[token] != address(0x0))
        throw;

[elazarg]

What about (2) after the above change? The tokens are distinct.

[yaronvel]

Regarding item (1). Your fix would probably work and would also save item 2. However, I think that a cleaner implementation, which will also save item 5.iv is to have an array of size two and store the data for each player in each cell. This way you will not need delete at all. Your delete are also a bit unfair for player 2 as he needs to pay more gas.

I don't understand your comment on item (2). I think it is also cleaner to identify users according to their address and not according to their secret.

Regarding item (3), there is no reason to assume that players are rational (i.e., would not want to lose money in an attack) unless you have too. And with timeouts you don't have too. Also technically, your approach gives rise to a blackmail attack. I can ask you to pay me 5 ether or otherwise I will not reveal my secret (maybe it is even possible for me to enforce it with a blackmail contract). If you are rational you will agree. Or maybe it will become some sort of chicken game. In any case, it would be bad.

I was wrong about item (4).

You can replace token with commit or commitment.

[elazarg]

It is not possible to guarantee that the secret will be revealed using a contract, since it means that the contract holds this secret and therefore it is not a secret anymore.

[elazarg]

I think timeouts are vulnerable to an attack that involves possible knowledge from the real world - e.g. I will guess that the user is not sophisticated and did not automate revealing the secret at the client side, and I will time my transaction in a way that he will not be available to answer - for example at sunday, or when he is on a trip, or something. This possibility will raise the likelihood that people will use contracts to automate the revealing of the secret, but this is not safe since (again) the secret became public as soon as it was sent to this automation-contract. I don't want to encourage that. A better way would be to use a type that cannot be held in storage for the secret, but there is no such type yet.

The blackmail can be solved with higher penalty, I guess.

[yaronvel]

The real vulnerability with timeouts is that someone could attack one of the players and e.g., disconnect him from the internet. While it is safe to assume that DoS over ethereum blockchain are not possible, targeting a specific client is possible.

But other than that, the protocol can be such that the timeline is known in advance. In day one everyone has to commit their decision, and in day two they have to reveal it. If you plan a vacation at day two, don't join the game. (you can also replace one day with 5 minutes).

======================================

Regarding blackmail. Higher penalty will make you even more vulnerable to blackmailing. I can use the following contract to blackmail you (assuming you joined first):

contract blackmail {
     addressToBlackmail;
     elazarPayedMe = false;
     function deposit( token, _addressToBlackmail ) {
                        elzazarRPS.deposit(token);
                        addressToBlackmail = _addressToBlackmail;
      }

      function payBlackmail() {
             if( msg.sender == addressToBlackmail && msg.value = 5 ether ) elazarPayedMe = true;
      }

      function sendContractBalanceToYaron() {
               if( elazarPayedMe ) yaron.send(this.balance);
      }
}

I will call your contract deposit via my blackmail contract and to reveal and collect I will call from my account. I have no incentives to call reveal unless you pay me, since I will not be able to collect the reward anyway. It will be locked forever in the blackmail contract.

[elazarg]

But your contract will not convince me that you will ever pay me, so I have no incentive to send you the money.

[yaronvel]

I will change it to

      function sendContractBalanceToYaron(invToken) {
               if( elazarPayedMe && sha3(invToken) == token ) yaron.send(this.balance);
      }

Also I will change the contract such that if I don't submit the token in 5 hours you will get your 5 ether back.

[elazarg]

The timer sounds good. Another possible blackmail is using a puzzle: I will demand that you provide a proof that you sent US$ 5000 to my account. This can be completely automated using only a contract.

[yaronvel]

If the $5000 should go to my bank Leumi account then we need some oracle to verify a bank transaction occurred. This paper might be of interest to you, it show how to use smart contracts for crimes.

Note that even in the absence of blackmail option. A competitor game service provider might profit in the long-run from playing with sockpuppet that withhold their moves and making a bad reputation for your contract.

[elazarg]

In other words, we need a guarantee of progress (or abort otherwise). How can we be sure that some sort of timer is the only way to do it?

[elazarg]

But correct me if I'm wrong - oracles are nice, but I can simply check using a previously known signature key.

[yaronvel]

If your bank provide digital signature etc, then yes. (A court of law might force your bank to sign such a transaction and in addition bank transactions are revertible. But that is beside the point).

Whether timeouts are a must? Maybe not. You can probably find some crypto tricks such that with enough computation power anybody can guess your secret after enough time. For example submit (x,y,z) such that:

1. sha3(x || 50 bits) = y` i.e., I only have to guess 50 bits to discover your secret, and
2. z is a zero knowledge proof for item 1 (i.e., that such 50 bits exist).

I don't know if item 2 if possible. But in any case, it is probably an over kill for a PSR game. Also if it take one week on one computer, it might take several years in my computer. So I would not like to join such game. But maybe if miners are rewarded for finding the 50 bits...