search

snapshot-labs / sx-starknet

Core smart contracts of Snapshot X for Starknet
https://docs.snapshotx.xyz
MIT License
118 stars 71 forks source link

remove `verifyingContract` from stark sig #626

Closed pscott closed 3 months ago

pscott commented 4 months ago

Removes the verifyingContract from the stark-sig authenticator. This is done because SNIP12 does not support the verifyingContract key. We also rename StarkEIP712 to SNIP12 for clarity.

The only important changes in this PR are:

  1. Removing the verifyingContract from and updating the domain hash: https://github.com/snapshot-labs/sx-starknet/blob/4e8ebcec63bb8b6319f927b561c547eacaee6892/starknet/src/utils/constants.cairo#L49-L50 (check git blame)
  2. Removing the contract_address from the get_domain_hash function (was previously here, you can check git blame: https://github.com/snapshot-labs/sx-starknet/blob/4e8ebcec63bb8b6319f927b561c547eacaee6892/starknet/src/utils/snip12.cairo#L157)

The remaining are StarkEIP712 -> SNIP12. I realize now it would've been cleaner with only two distinct commits.

Sekhmet commented 4 months ago

@pscott could you deploy starkSig authenticator on sepolia with this removed so I could test full flow?

I remember that at some point there was some issue with either Metamask or ArgentX not being happy with verifyingContract not being passed.

pscott commented 4 months ago

@pscott could you deploy starkSig authenticator on sepolia with this removed so I could test full flow?

I remember that at some point there was some issue with either Metamask or ArgentX not being happy with verifyingContract not being passed.

I think it was metamask that was causing issues, but here it only concerns sx-sig so shouldn't be an issue.

New contract: 0x04a7805f0d4b34d801f2fcf7aace7bd990f3acc30e542523998f9c0d5f203055 on sepolia