Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Follow best practices for closing_sig #266

Closed
loredanacirstea opened this issue Dec 11, 2017 · 0 comments · Fixed by #273
Closed

Follow best practices for closing_sig #266

loredanacirstea opened this issue Dec 11, 2017 · 0 comments · Fixed by #273

Comments

@loredanacirstea
Copy link
Contributor

loredanacirstea commented Dec 11, 2017

tl;dr

closing_sig should be the receiver's signature on:

  • sender address
  • channel block number
  • balance
  • uRaiden contract address

First proposed in #52

(note, this is the current EIP712 hash implementation - will change).

bytes32 closing_signature_message_hash = keccak256(
          keccak256('address sender', 'uint32 block_created', 'uint192 balance', 'address contract'),
          keccak256(_sender_address, _open_block_number, _balance, address(this))
        );

At the moment, closing_sig (receiver's closing signature for cooperative close) consists only in the receiver signing the sender's signed balance proof.

Reasons for changing this:

  1. There is no known vulnerability at this point due to Ethereum's ECDSA using v to differentiate between the possible curve points for the same signature. Nonetheless, we should follow best practices and sign actual data instead of a signature.

  2. Use EIP712, as we do in the balance proof message, for consistency. Also mentioned in the external audit of the smart contract (point 10 Documenting the external audit of the smart contract #135)

Previous discussions on the signature logic documented in #193.

To recap, current state is:

  • balance_msg_sig is the sender's signature of:
bytes32 message_hash = keccak256(
          keccak256('address receiver', 'uint32 block_created', 'uint192 balance', 'address contract'),
          keccak256(_receiver_address, _open_block_number, _balance, address(this))
        );
  • closing_sig is the recever's signature on sha3(balance_msg_sig)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants