MessageHash, StakeIntentHash and RedeemIntentHash should be hashed as per EIP-712.

[deepesh-kn]

User Story

Staker / Redeemer needs to sign the structured data. This data should be hashed as per the standard specified in EIP-712.

Following structure exists in the code.
In EIP20Gateways there is struct Stake.
In EIP20Gateways there is struct Redeem.
In MessageBus there is struct Message.

This means that

• Stake intent hash will include STAKE_TYPEHASH.
• Redeem intent hash will include REDEEM_TYPEHASH.
• Message hash will include MESSAGE_TYPEHASH.
• Remove GATEWAY_LINK_TYPEHASH from the code.

The type hash are as per the EIP-712 standards.

• STAKE_TYPEHASH is type hash of Stake.
• REDEEM_TYPEHASH is type hash of Redeem.
• MESSAGE_TYPEHASH is type hash of Message.

[deepesh-kn]

• Initially, I was in favor of updating the structs storage to hold all the data, but after today's discussion, it clears my understanding that we do not need the actual struct to store all the params.
• Rather the type hash defines the expected structure of the data for hashing.

Now the current challenge is that the data user will sign should be readable. In the current implementation, it is not and the user will see hashed data.

So we can have the following two approaches so that data can be presented to the user.
First
example:

        bytes32 STAKE_INTENT_TYPE_HASH = keccak256(
            abi.encodePacked(
            "StakeIntentHash(address staker,uint256 amount,address beneficiary,uint256 gasPrice,uint256 gasLimit,uint256 bounty,uint256 nonce,address gateway)"
            )
        );
        bytes32 MESSAGE_TYPE_HASH = keccak256(
            abi.encodePacked(
                "Message(bytes32 intentHash,address sender,uint256 nonce,address gateway,bytes32 hashLock)"
            )
        );

        bytes32 stakeIntentHash =  keccak256(
            abi.encodePacked(
                STAKE_INTENT_TYPE_HASH,
                staker,
                amount,
                beneficiary,
                gasPrice,
                gasLimit,
                bounty,
                nonce,
                gateway
            )
        );
        bytes32 messageHash = keccak256(
            MESSAGE_TYPE_HASH, 
            stakeIntentHash,
            sender,
            nonce,
            gateway,
            hashLock
        );

• Message hash is independent of the intent hashes and uses the MESSAGE_TYPE_HASH.
• In this case, the user needs to sign the IntentHash (e.g: StakeIntentHash.)
• Message bus has to verify the signature with IntentHash as data.
• StakeIntentHash and MessageHash data have repeated params.
• In this example, we can remove staker param from the stakeIntentHash, as it will be the signer.

Second
example:

bytes32 STAKE_INTENT_TYPE_HASH = keccak256(
            abi.encodePacked(
            "StakeIntentHash(uint256 amount,address beneficiary,uint256 gasPrice,uint256 gasLimit,uint256 bounty)"
            )
        );

        bytes32 STAKE_MESSAGE_TYPE_HASH = keccak256(
            abi.encodePacked(
                "Message(StakeIntentHash intentHash,address sender,uint256 nonce,address gateway)StakeIntentHash(uint256 amount,address beneficiary,uint256 gasPrice,uint256 gasLimit,uint256 bounty)"
            )
        )

 bytes32 stakeIntentHash =  keccak256(
            abi.encodePacked(
                STAKE_INTENT_TYPE_HASH,
                amount,
                beneficiary,
                gasPrice,
                gasLimit,
                bounty                
            )
        );
        bytes32 messageHash = keccak256(
            STAKE_MESSAGE_TYPE_HASH, // this will change for different message types
            stakeIntentHash,
            sender,
            nonce,
            gateway,
        );

• (stake and redeem)Context-specific message hash.
• Here we have the nested structured data.
• Hashed data do not have repeated params.
• hashLock cannot be part of the message hash in this case.
• User signs the messageHash.
• Message bus has to verify the signature with messageHash as data.

Let me know what is your opinion.
cc: @benjaminbollen , @schemar , @sarvesh-ost , @gulshanvasnani

[schemar]

Why can we not add the hash lock?

[gulshanvasnani]

Second example seems to be aligned with the example at EIP712 below :-
https://github.com/ethereum/EIPs/blob/master/assets/eip-712/Example.sol

To me it make sense to move ahead with second approach(without changing Stake and Message struct) if anyone doesn't have any concern.

cc: @benjaminbollen @schemar @deepesh-kn @sarvesh-ost

[schemar]

Yes, the second approach looks better.

[deepesh-kn]

Why can we not add the hash lock?

In this case, message hash is signed by the staker/redeemer. Hashlock is provided by the facilitator, so the hash lock is not part of the message hash.
I strongly feel that the hash lock somehow should be part of message hash so that it can be cryptographically proved on the other chain. Otherwise, any facilitator can just put any Hashlock on the other chain. 🤔

If we create a new hash using message hash and the unlock key than we are not getting ant major benefit as compared to the first approach.

Does anyone have a 3rd approach? I am feeling more inclined towards the first approach.

[schemar]

OK that makes sense. The hash lock definitely has to be there, then. Another question: if the stake intent is part of the message. And the nonce and gateway are part of the stake intent. Then why do we need to add them again to the message?

Would the following not work? The sender could sign the intent hash, the facilitator could sign the message hash, and still all data could be "proven" with the hashes. Am I missing something? 🤔

bytes32 STAKE_INTENT_TYPE_HASH = keccak256(
    abi.encodePacked(
        "StakeIntentHash(address staker,uint256 amount,address beneficiary,uint256 gasPrice,uint256 gasLimit,uint256 bounty,uint256 nonce,address gateway)"
    )
);
bytes32 MESSAGE_TYPE_HASH = keccak256(
    abi.encodePacked(
        "Message(bytes32 intentHash,address sender,bytes32 hashLock)"
    )
);

bytes32 stakeIntentHash = keccak256(
    abi.encodePacked(
        STAKE_INTENT_TYPE_HASH,
        staker,
        amount,
        beneficiary,
        gasPrice,
        gasLimit,
        bounty,
        nonce,
        gateway
    )
);
bytes32 messageHash = keccak256(
    abi.encodePacked(
        MESSAGE_TYPE_HASH, 
        stakeIntentHash,
        sender,
        hashLock
    )
);

[deepesh-kn]

Message bus does not know how the intentHash is formed. MessageBus is abstracted in such a way that it can work with any intentHash, which may not even include the nonce.

• To stop the replay attacks staker's nonce was part of the messageHash.
• Similarly gateway address was added in the messageHash so as to MessageBus can verify that the message belongs to correct gateway.

[schemar]

In that case I think it makes sense to add the parameters in both places. They are there for "different" reasons.

[schemar]

After discussing the topic one more time, we agreed to:

• remove redundant parameters, as the hashes won't be signed
• have two separate message hash types, based on the intent hash

/cc @deepesh-kn @sarvesh-ost