-
Notifications
You must be signed in to change notification settings - Fork 101
Extract balance message hash logic for EIP712 in a separate contract #200
Extract balance message hash logic for EIP712 in a separate contract #200
Conversation
@@ -0,0 +1,34 @@ | |||
pragma solidity ^0.4.17; | |||
|
|||
contract URaidenEIP712HelperContract { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
###Naming Suggestion###
We call the microraiden contract RaidenMicroTransferChannels.sol
so the library contract should also use micro
and not u
. So I would suggest something like MicoRaidenEIP712Helper
. or MicroRaidenEIP712Signer
. No need to keep the contract
part in the name. We know it's a contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MicoRaidenEIP712Helper
it is then. I tried a couple of names, but I knew you are better at this and will probably find a good one hahah.
pure | ||
returns (bytes32) | ||
{ | ||
// Used in the RaidenMicroTransferChannels contract |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment. Already written above.
// The hashed strings should be kept in sync with this function's parameters | ||
// (variable names and types) | ||
bytes32 message_hash = keccak256( | ||
keccak256('address receiver', 'uint32 block_created', 'uint192 balance', 'address contract'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test here guarantees that they are kept in sync, right? If not then we need such a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests have been added here https://github.com/raiden-network/microraiden/pull/200/files#diff-bd130ba0c786d648c876d74356582f11R50
EIP712 is not standardized and can have breaking changes. - adds a setter in the uRaiden contract for setting the `URaidenEIP712HelperContract` address, callable only by the `RaidenMicroTransferChannels` contract owner - exposes `getMessageHash` as a `pure` function, used in the uRaiden `verifyBalanceProof` function
6340118
to
2ffcc17
Compare
merge after #198
fixes #181 - you can change the balance proof message hash logic at any point.
EIP712 is not standardized and can have breaking changes.
URaidenEIP712HelperContract
address, callable only by theRaidenMicroTransferChannels
contract ownergetMessageHash
as apure
function, used in the uRaidenverifyBalanceProof
function