safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.84k stars 907 forks source link

Support updated EIP-1271 implementation for signature validation #581

Closed akshay-ap closed 1 year ago

akshay-ap commented 1 year ago

Context:

Safe contract supports contract signature validation using EIP-1271. The Safe contracts defines an interface which has a function declaration isValidSignature(bytes, bytes) which is used to validating the contract signatures. But, EIP-1271 defines isValidSignature(bytes32,bytes) for validating contract signatures. This PR addresses this issue by removing the use of legacy implementation of EIP-1271.

Fixes: https://github.com/safe-global/safe-contracts/issues/391 Forum discussion: https://forum.safe.global/t/safe-contract-v1-5-0/3383

Motivation

Changes proposed in the PR:

github-actions[bot] commented 1 year ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

github-actions[bot] commented 1 year ago

Pull Request Test Coverage Report for Build 5255903321


Totals Coverage Status
Change from base Build 5024015688: -0.04%
Covered Lines: 304
Relevant Lines: 312

💛 - Coveralls
akshay-ap commented 1 year ago

I have read the CLA Document and I hereby sign the CLA

andyrobert3 commented 1 year ago

Is there a timeline when this will be merged and release, for which Safe version?

Because in the new EIP1271 version, we would sign on the dataHash (much simpler)

However, in the current Safe EIP1271 implementation, we must follow & implement the EIP712 hashing scheme off-chain, before signing it. Then we pass the original data message, and the final signature after EIP712 hashing scheme

akshay-ap commented 1 year ago

Is there a timeline when this will be merged and release, for which Safe version?

Because in the new EIP1271 version, we would sign on the dataHash (much simpler)

However, in the current Safe EIP1271 implementation, we must follow & implement the EIP712 hashing scheme off-chain, before signing it. Then we pass the original data message, and the final signature after EIP712 hashing scheme

@andyrobert3 This feature will be merged as a part of release v1.5.0. And regarding timeline, after release v1.4.1 we will work on this release.