[SNIP] Off-chain signatures (à la EIP712)

This is to discuss the Off-chain signatures (à la EIP712) SNIP:

I’ve heard poseidon hashes should be preferred. Are there backwards compatibility concerns? Perhaps to support both a "hash": "pedersen" | "poseidon" field could be added to the domain.

Hello,
I think we should also standardize how a browser wallet should display the message when waiting the account owner approval. Today, it’s like this :

The EIP712 provides recommendations how to display the message to improve the user experience :

I think that we should also explain in this SNIP how each message field should be displayed. For example, the StarkNetDomain fields should be all displayed with their titles, at the top of the window.
The standardization and the quality of the display is also important to reach the goal of EIP712.

Yes, the intention of the SNIP is to formalize what the ecosystem is doing today but still introduce improvements if they are backward compatible.

I like the idea of anticipating the transition to poseidon. Adding an optional hash parameter to the domain makes sense provided that we define the default to be pedersen when the parameter is not specified.

Hey @frangio
I just updated the description to mention the use of the hashing function.
Could you have a look to make sure it suits what you were asking? :slight_smile:

A few questions

  1. why does type enum uses starknet keccak? and not Pedersen or Poseidon.
  2. ‘StarknetDomain’ instead of ‘StarkNetDomain’ no ?
  3. Defining a json schema might be better here to specify the constraints a little more. We have done that for the sign-in-with-starknet proposal and probably could be utilized here
  4. Could we use the byte array type coming in the next release to remove the 31 character limit for strings? Probably a major impediment for any adoption for off chain signing.

Hey @bowlofchilli,

  1. It uses the same mechanism as a struct, first encoding its representation using sn_keccak, then encoding his value. All this will be encoded using pedersen or poseidon depending on the hashing_function used.
  2. To be retro compatible unfortunately we need to keep StarkNetDomain.
  3. That could be interesting, will look into it :slight_smile:.
  4. Will look into it, the issue is that there is already a string type used for short strings (<= 31 string length) called string :slightly_frowning_face:.

Hope it helps, will ping you again whenever I have more info, or if we update the SNIP.

Thanks for the reply! why are we using starknet_keccak for structs again? I understand for selectors we have to use them because of cairo implementation but why for structs? having multiple hashing methodologies make it unnecessarily complex I think may be the SNIP should explain why starknet_keccak is being used.

Gm,

Just updated the proposal, pining everyone that left a comment :slight_smile:
@frangio , @Phil26 , @bowlofchilli

Regarding enums, I prefer option #1

Same for me. Option 1

Really Nice SNIP, I’m a huge fan of the large variety of types :nail_care:
What do you think about switching to Starknet instead of StarkNet in revision 1 ?

Glad you liked the types!

Regarding the StarkNet string. It can be done, but, in my opinion is not worth the change. Revision 0 will still have the other string, so we’ll have to support both. Since the string not supposed to be user facing. I’d prefer to keep it the way it is, to keep the spec simple, and make it easier for libraries to implement the standard

We ended up doing that change, as even if it adds a bit of complexity. It guarantees that today wallets won’t be able to process revision 1 requests until they support it. Having the wallet processing a rev1 request as a rev0 request can only lead to a wrong signature and will be harder to debug

Glad to hear it, thx for the follow up !