zksync icon indicating copy to clipboard operation
zksync copied to clipboard

EIP1271 signature recovery support for Gnosis safe

Open junaidev opened this issue 5 years ago • 6 comments

Currently zk sync server is unable to recover EIP 1271 signature via gnosis safe implementation of isValidSignature(...) .

isValidSignature() of Gnosis safe supports following:

  • it hash message using format according to its `getMessageHash()` and then adds message prefix and then hash it again before applying ecrecover.
    
  • it apply ecrecover on message hash ( original message with out prefix is hashed using `getMessageHash()` )
    

in Zk sync the message is passed along with prefix and its length, so it doesn't work.

Purposed Change: For supporting EIP 1271 for gnosis safe, need to remove prefix before message.

junaidev avatar Dec 17 '20 14:12 junaidev

Actually, this is done intentionally, since some signers always add this prefix. In case of the wallet not adding the prefix, it is always possible to add the prefix, but not vise versa.

For example, in zksync.js you can provide an EthSignerType argument in which you can describe whether the signer is adding prefixes to messages or not. It is described in docs.

If this argument is omitted, zksync.js will try to deduce whether the signer adds prefixes automatically, but for EIP1271 it's not as easy as for ECDSA signatures.

Anyway, no matter which kind of signer and/or SDK is used, it is always possible to sign the expected messages.

popzxc avatar Dec 17 '20 15:12 popzxc

Some signers but not all might, depends on requirement.

In case of EIP1271 recovery is_eip1271_signature_correct(...) https://github.com/matter-labs/zksync/blob/47bb16fe233d2695c50ccf291dc77dd9f0036dd0/core/bin/zksync_api/src/eth_checker.rs#L55 you are "always" adding prefix via get_sign_message(...) https://github.com/matter-labs/zksync/blob/47bb16fe233d2695c50ccf291dc77dd9f0036dd0/core/bin/zksync_api/src/eth_checker.rs#L46 before passing to isValidSignature which doent work with gnosis safe https://github.com/gnosis/safe-contracts/blob/development/contracts/GnosisSafe.sol#L325 . Reason explained in above issue opened. so for supporting zksync wallet with L1 n of k style multisig ( Gnosis safe multisig ) this needs to be fixed.

junaidev avatar Jan 04 '21 13:01 junaidev

Is this issue a blocker for deploying Safe on current zksync testnet or should it work despite this?

e00dan avatar Apr 12 '23 06:04 e00dan

any update on Era ?

YoleYu avatar Apr 12 '23 09:04 YoleYu

Is this issue a blocker for deploying Safe on current zksync testnet or should it work despite this?

To my understanding Safe won't be available on zkSync Lite, but will be deployed on zkSync Era.

bxpana avatar Apr 28 '23 17:04 bxpana

any update on Era ?

https://blog.matter-labs.io/gm-zkevm-171b12a26b36

bxpana avatar Apr 28 '23 17:04 bxpana