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

Tooling to verify signatures with support for ERC1271 #2459

Closed
wants to merge 2 commits into from

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Jan 7, 2021

SignatureChecker is a library for seamless verification of signature with support for both EOA (ECDSA) and smart contract wallets (ERC1271).

While still being a draft, this ERC is supported by smart contract wallets such as Argent.

I believe this feature could greatly improve the compatibility of dapps that relies message signatures with such wallets. In particular since Argent is now able to sign message (including ERC721 typedData) through WalletConnect.

@Amxx Amxx force-pushed the feature/SignatureChecker branch from c83d767 to 4d5f696 Compare January 7, 2021 20:55
@Amxx Amxx added work in progress feature New contracts, functions, or helpers. labels Jan 8, 2021
@frangio frangio mentioned this pull request Jan 14, 2021
@frangio
Copy link
Contributor

frangio commented Jan 14, 2021

One thing I would like to see if we ship this, is documentation outlining how EIP-1271 signatures can be different from normal cryptographic signatures. In particular, the property of non-repudiation is compromised on-chain because a signature that was valid at some point can stop being valid at a later point, according to the "signing" contract. Of course off-chain one could look at the history of the chain but other contracts can't do that.

This is a limitation of the EIP and we should simply document it.

SignatureChecker is a library for seamless verification of signature with support for both EOA (ECDSA) and smart contract wallets (ERC1271)
@Amxx Amxx force-pushed the feature/SignatureChecker branch from 4d5f696 to bdedcd9 Compare January 27, 2021 14:25
@thegostep
Copy link

here is my implementation if useful:

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity 0.7.6;

import {ECDSA} from "@openzeppelin/contracts/cryptography/ECDSA.sol";
import {Address} from "@openzeppelin/contracts/utils/Address.sol";

interface IERC1271 {
    function isValidSignature(bytes32 _messageHash, bytes memory _signature)
        external
        view
        returns (bytes4 magicValue);
}

/// @title ERC1271
/// @notice Module for ERC1271 compatibility
/// @dev Security contact: dev-support@ampleforth.org
abstract contract ERC1271 is IERC1271 {
    using ECDSA for bytes32;
    using Address for address;

    // Valid magic value bytes4(keccak256("isValidSignature(bytes32,bytes)")
    bytes4 internal constant VALID_SIG = IERC1271.isValidSignature.selector;
    // Invalid magic value
    bytes4 internal constant INVALID_SIG = bytes4(0);

    modifier onlyValidSignature(bytes32 permissionHash, bytes memory signature) {
        require(
            isValidSignature(permissionHash, signature) == VALID_SIG,
            "ERC1271: Invalid signature"
        );
        _;
    }

    function _getOwner() internal view virtual returns (address owner);

    function isValidSignature(bytes32 permissionHash, bytes memory signature)
        public
        view
        override
        returns (bytes4)
    {
        address owner = _getOwner();
        if (owner.isContract()) {
            try IERC1271(owner).isValidSignature(permissionHash, signature) returns (
                bytes4 returnVal
            ) {
                return returnVal;
            } catch {
                return INVALID_SIG;
            }
        } else {
            address signer = permissionHash.recover(signature);
            return signer == owner ? VALID_SIG : INVALID_SIG;
        }
    }
}

@frangio frangio changed the base branch from master to solc-0.8 January 27, 2021 21:40
@Amxx Amxx changed the title SignatureChecker library Tooling to verify signatures with support for ERC1271 Jan 28, 2021
@Amxx Amxx closed this Feb 2, 2021
@Amxx Amxx deleted the branch OpenZeppelin:solc-0.8 February 2, 2021 22:54
@MatthiasEgli-Audit
Copy link

What happened to this PR @Amxx ?

@Amxx
Copy link
Collaborator Author

Amxx commented Feb 19, 2021

This targeted a branch that got deleted when solc 0.8 support was added to master. Will reopen a new one after 4.0 preview is published.

Is it something you'd like to see in the repo / you'd use ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New contracts, functions, or helpers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants