ZEIP Proposal: Fix v3 EIP-191 non-compliance when validating EIP-1271 signatures

⌨️ Technical Development Core Protocol
1

I humbly submit a proposal for a bugfix for 0x v3 contracts:
ZEIP issue link here: Fix v3 EIP-191 non-compliance when validating EIP-1271 signatures · Issue #94 · 0xProject/ZEIPs · GitHub
I’ve intentionally broken the links, since I’m a “new user” and am not allowed more than 2 links per post. The ZEIP issue link has working links.

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] (https:// github. com/0xProject/0x-protocol-specification/blob/master/v3/v3-specification.md#eip-1271-usage), expects a signature created by a wallet’s personal_sign method.
However, with [EIP-191] (https:// eips.ethereum. org/EIPS/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] (https:// github. com/0xProject/0x-protocol-specification/blob/master/v3/v3-specification.md#eip-1271-usage), 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] (https:// eips.ethereum. org/EIPS/eip-712) signature type.
This also isn’t a problem in v4, since v4 fixed this bug - [it expects the prefix.] (https:// docs.0x. org/market-makers/guides/signing-0x-orders#signature-types)

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

2 Likes
2

Thanks \uarilotter for opening this up. While I don’t have a lot of context on the specifics of the signature type you want to introduce, something caught my attention.

Since 0x V4 is the canonical 0x exchange (0x v3 is de facto legacy exchange), I am wondering why you are not using that?
I can see that you point at a non-feature parity

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.

Wouldn’t it be better to propose a change on 0x v4 to introduce bundles, and the other things that are blocking the development of your app?

Talking to @nikita, it seems that you’ve been already bringing up the absence of bundles of assets in 0xv4 in Discord, and found little engagement around it. I am sorry about that.

There are probably 2 factors contributing to that, that I think it’s important to take into consideration to move this forward:

  1. the 0x Labs team will most likely not have bandwidth to prioritize this, as it doesn’t immediate fit with our own roadmaps, very much centered on 0x API. We didn’t have bandwidth to update it to include NFT swaps!
  2. it’s not an easy feat to develop this independently of 0x Labs. This is something I’d love to change.

Developing on 2), when we zoom out: the protocol is extensible, I am sure there a lot of talented smart contract engineers that could take this one, and…the 0x community treasury has over $4M in budget.
Suggested strawman plan of action:

  • Quantify a rough estimate of fair compensation for working on this update
  • attach to a modified ZEIP (about updating v4) an open RFP/request for proposal for teams individuals to work on it. It can be posted in this forum initially, but also we could recur to other channels to get it more resonance
  • collect names of people and individuals that want to work on it
  • temperature check on snapshot to confirm the community is ok to assign project (or even with multiple choice to select multiple candidates)
  • final treasury vote to send upfront payment (could be split into 2 payments, once work is finished)
  • audits could be orchestrated via Code4Arena
3 Likes
3

  1. I’ve learned that v3 shipped long before 1271 became final, and the standard actually evolved a good amount since then; so V3 expects the smart contract wallet to support a completely different interface than what ended up in the final 1271. That is actually the real issue, EIP-191 wasn’t relevant with v3, which btw isn’t currently being actively maintained, nor is it used widely, so the cost/benefit to update it is questionable. It is still very possible for a smart contract wallet to sign orders using the existing v3 functionality, but they would need to implement custom code (admittedly not ideal).

  2. Given that and keeping in mind the comments from @mintcloud regarding Labs resource availability/prioritization, I personally prefer to focus on updating v4 to include bundles vs retrofitting v3. We recently awarded a grant to a team who is working on developing multiswap capabilities on top of v4, which could theoretically and with the appropriate audits be integrated at the protocol level. So I suggest that Labs coordinate initially with that team to assess whether their work is directionally feasible and responsive to the requirement, rather than starting from scratch with an open RFP.

2 Likes
4

Hey all,
Thanks for the in-depth responses. Honestly, the best course of action here is probably to hardcode a workaround into 1271 wallets, and then submit a PR to the docs explaining exactly how to work around this issue. 1271 + v3 is such a tiny share of the market / userbase, it’s probably not worth doing anything more.

I’ll prepare that & open a PR to the docs in the next week once I implement it in my wallet.

Can’t wait for v4 bundles, I would love to migrate my dapp off v3 :slight_smile:

2 Likes
5

Thanks @arilotter for this proposal.

So as I understand, this proposal will be on hold to prioritize including bundles on 0x V4. Can you elaborate on how much usage this additional feature drive to the 0x v4 protocol? I personally don’t use much this feature, but I am keen to support the development of it.

1 Like