raiden-network / spec

Spec of the Raiden Network protocol
8 stars 19 forks source link

Assess adding a message type identifier for each use case #67

Closed loredanacirstea closed 6 years ago

loredanacirstea commented 6 years ago

Assess adding a message type identifier for each use case. This will be helpful when dealing with protocol upgradability.

This would mean that the channel participants would sign on:

signature_data = (*message_data || message_type_identifier)

As @hackaugusto exemplified, message_type_identifier used in the case of balance proofs can be similar to:

balance proofs that we receive: 0xcafe
balance proofs that we sign for delegated calls: 0xdead
cooperative close: 0xbeef
LefterisJP commented 6 years ago

I don't think the term balance proof for all of the type of messages is correct. If anything this should be a message type identifier.

loredanacirstea commented 6 years ago

I changed the title & description. A more detailed description should be added at some point.

hackaugusto commented 6 years ago

To clarify my opnion.

At the moment we rely on the data having different shapes to avoid "replays" on the same channel. A balance proof used for closeChannel has one shape, which is different from the balance proof used for updateNonClosingBalanceProof (the field closing_signature was added), which is different from the data used for cooperativeSettle, so these proofs can not be used for another purpose.

This may become harder as we move the project forward. Replays across different versions of the smart contracts should be forbidden by the token_network_address field, as long as we deploy new smart contracts for upgrades, but if we forget to have enforce different shapes for the signed data bad things may happen.

This is one of the main driving factors to me (which I got from loredana's remark), the ID field (or a magic cookie) "qualifies" the signed data.

hackaugusto commented 6 years ago

Additional note: This is also important to make sure that a Raiden message is not a valid transaction. (This could also be achieved by using EIP 712 or 191)

andrevmatos commented 6 years ago

^-- can also be achieved (also compatible with EIP191) by https://github.com/raiden-network/raiden/issues/1531 , until 712 is fully available

LefterisJP commented 6 years ago

So this will be solved by #241? @loredanacirstea

loredanacirstea commented 6 years ago

@LefterisJP , correct. After it is merged, I will also change the specs.