0xProject / ZEIPs

0x Improvement Proposals
Apache License 2.0
91 stars 15 forks source link

Fix v3 EIP-191 non-compliance when validating EIP-1271 signatures #94

Closed arilotter closed 2 years ago

arilotter commented 2 years ago

Summary

Modify the 0x v3 smart contracts to fix signature validation failure when using EIP-1271 Smart Contract wallets that are spec-compliant with EIP-191.

Type:

CORE

Motivation

The 0x v3 EIP-1271 support, documented here, expects a signature created by a wallet's personal_sign method. However, with EIP-191, wallets will prepend "\x19Ethereum Signed Message:\n" + len(message). before the message to be signed, before they return a signature.

Therefore, when an EIP-1271 smart contract wallet that conforms to EIP-191 signs a 0x order like this:

    const typedData = {
        /* ... typed data of Order.. */
    };

    const orderHash = encodeTypedDataHash(typedData);

    const abi = new ethers.utils.Interface(EIP1271DataAbi);
    const msg = abi.encodeFunctionData(
        'OrderWithHash',
        [typedData.message, orderHash]
    )
    const signature = await signer.signMessage(ethers.utils.arrayify(msg), chainID)

the resulting signature is a signature of the message with the "\x19Ethereum Signed Message:\n" + len(message). prefix. Due to the way 0x v3 works with EIP-1271 signatures, you can't use EIP-712 encoding for the orders, and thus the signature will always fail to validate inside the 0x v3 contract.

This isn't a problem in EOA wallets, since they can use the EIP-712 signature type. This also isn't a problem in v4, since v4 fixed this bug - it expects the prefix.

Specification

If the 0x v3 contracts are amended to consider EIP-191 prefixing and include the prefix when validating signatures, 0x v3 will correctly validate EIP-1271 smart contract wallet signatures.

Rationale

0x v4 has fixed this bug.

If 0x v4 supports the same feature set as v3, (bundles, etc), then most dApps will switch to v4, and this proposal becomes less relevant. However, every existing 0x v3 dApp will not work on any EIP-191 compliant EIP-1271 wallet unless they hardcode a workaround for 0x v3 order signatures.

If we need to detect EIP-191 style signatures, this will increase gas costs for all EIP-1271 order fills.

If we don't detect EIP-191 style signatures, and instead add the prefix bytes, this will break compatibility for EIP-1271 wallets that don't follow the EIP-191 specification.

This will require a re-audit of the contract for the requested changes, but will bring it into spec compliance.

Designated team: UFG

Sponsor (for CORE type only): UFG

Notes

https://github.com/0xProject/0x-protocol-specification/blob/master/v3/v3-specification.md#eip-1271-usage https://eips.ethereum.org/EIPS/eip-191 https://docs.0x.org/market-makers/guides/signing-0x-orders#signature-types There is a 0x forum thread here: https://gov.0x.org/t/zeip-proposal-fix-v3-eip-191-non-compliance-when-validating-eip-1271-signatures/3396

arilotter commented 2 years ago

closing this issue after some discussion on the forum thread linked in Notes - this isn't worth fixing, and i will instead submit a PR to docs explaining this caveat.