Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EIP-1344 chainid opcode to signed messages composition #170

Closed
3esmit opened this issue Jan 14, 2020 · 2 comments · Fixed by #264
Closed

Add EIP-1344 chainid opcode to signed messages composition #170

3esmit opened this issue Jan 14, 2020 · 2 comments · Fixed by #264

Comments

@3esmit
Copy link

3esmit commented Jan 14, 2020

Gnosis Wallet is using EIP-712 to sign messages. This standard uses a chainID hardcoded in the domainSeparator, causing that:

  • If Gnosis Wallet is taken account the chainid, signed messages only work for wallet in chain that dont changed the chainID;
  • GnosisSafe in forked chain don't knows it forked, and so messages signed for the other chain are valid in the fork because domainSeparator is hardcoded.

To fix this issue, one of the following solutions could be taken:

  1. EIP-712 should fix this issue by changing their design to use EIP-1344t and gnosis wallet following it.
  2. EIP-712 domainSeparator could be recalculated at every signature check using the new EIP-1344 EVM opcode.
  3. use EIP-191 with EIP-1344 at application parameters.

The less impacting change, except in gas costs, would be 2. The best would be 1..
Or 3. Gnosis Safe don't needs 712 because user trusts its own contract and wallet, and don't have to validate the structured parameters. This is very useful for DEX and other stuff because its a foreign contract.

For case 2. it seems that gnosis wallet is not even using chainID, which breaks from EIP-712 standard.

See https://eips.ethereum.org/EIPS/eip-712 https://eips.ethereum.org/EIPS/eip-1344

@rmeissner
Copy link
Member

EIP-712 does not require to have chain id in the domain object and we left it out because when the contracts were deployed there was no reliable way to retrieve the chain id.

EIP-1344 was started by me exactly because of that issue and this should be adjusted in the future. I would recommend to either change this issue to "Add chain id to domain object" or close this issue and create a new one for this.

@3esmit 3esmit changed the title Signed messages are replayable in case of chain split Add EIP-1344 chainid opcode to signed messages composition Jan 14, 2020
@3esmit
Copy link
Author

3esmit commented Jan 14, 2020

@rmeissner EIP-1344 is the only way to solve this issue of reply signatures after a chain split, and now that EIP-1344 got Final we should implement it in the contracts ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants