ERC-7015: Creator Attribution for ERC721

[montasaurus]

Thanks for talking through the design with me on this @strollinghome! Here are the main pieces of feedback we had while working on support for the draft version at OpenSea.

EIP 1271 Support

We discussed this one, but wanted to repeat it here. In order to support validating 1271 signatures, we needed the creator address added to the log. Thanks for including that! This will allow support for SCW and multi-sig creators.

Note for anyone else implementing verification that you can’t just ecrecover and use that address directly, since it may be an EOA signing for a different address using 1271.

712 Payload / Phishing

The 712 payload included in the spec confused me & I believe might enable an abuse vector.

From the spec:

Creator consent is given by signing an EIP-712 compatible message; all signatures compliant with this EIP MUST include all fields defined. The struct signed is:

struct TokenCreation {
	bytes32 structHash;
}

Reading that, I was expecting the primary type of the 712 payload to be TokenCreation with an included structHash. I think what is intended (as written) is for the structHash to replace hashStruct(message) from the 712 spec, allowing any arbitrary 712 message payload. From the comments above, it looks like this was changed to allow flexibility in the payload for factory contracts.

The issue I see is that this enables contracts to present any signature on the verifying domain (e.g. sign in messages) as a creatorship attestation. This will make phishing these signatures much easier since the user signing them won’t be able to tell what the signature is being used for.

A specific and explicit 712 signature for a creatorship attestation would make this more robust and less prone to abuse. Factory contracts could still support this by prompting the user to sign the specific creator attestation, and then including that signature in the (potentially also signed) payload for the contract creation.

[oveddan]

Hi Adam,

Definitely understand the concern about phishing.

Regarding singing a message containing:

struct TokenCreation {
	bytes32 structHash;
}

If the user is signing a message this way, this still allows contracts to present anything within the bytes32 structHash, and the user still doesn’t know what they are signing, and this still allows contracts to present any signature on the verifying domain (e.g. sign in messages) as a creatorship attestation.

This has disadvantages in that the user is signing an already hashed struct containing the encoded token creation config, without having any visibility into what’s inside it. This doesn’t prevent against phishing as anything can be put into that structHash

If they are to sign a message this way, their signing prompt would look like:

Additionally, front-end code to create the structHash can not take advantage of libraries utility functions to encode eip-712 typed data. This is what code in viem would look like if it was written in this way:

import { encodeAbiParameters, keccak256, parseEther } from "viem";

// front-end code has to figure out how to encode these abi parameters and hash them.
 const structHash = keccak256(
      encodeAbiParameters(
        [
          {
            name: "tokenPrice",
            type: "uint256",
          },
          {
            name: "metadataUri",
            type: "string",
          },
        ],
        [parseEther("0.1"), "ipfs://1231231231"]
      )
    );

    await walletClient!.signTypedData({
      types: {
        TokenCreation: [{ name: "structHash", type: "bytes32" }],
      },
      primaryType: "TokenCreation",
      message: {
        structHash,
      },
      domain: {
        chainId,
        verifyingContract: contractAddress,
        name: "TokenCreationSig",
        version: "1"
      }
    });

[oveddan]

If we are to go the more native/true eip-712 route, which offers a better devex and user experience, the same code above looks like:

    await walletClient!.signTypedData({
      types: {
        TokenCreation: [
          {
            name: "tokenPrice",
            type: "uint256",
          },
          {
            name: "metadataUri",
            type: "string",
          },
        ],
      },
      primaryType: "TokenCreation",
      message: {
        tokenPrice: parseEther("0.1"),
        metadataUri: "ipfs://1231231231",
      },
      domain: {
        chainId,
        verifyingContract: contractAddress,
        name: "TokenCreationSig",
        version: "1"
      }
    });

And the resulting prompt looks like:

Here, the user can see explicitly the fields they are signing.

[montasaurus]

Yeah, I’m not suggesting we use the existing TokenCreation struct as the signature payload. I’d recommend we just include the essential fields we’d need verify.

struct CreatorAttestation {
    address creator;
    address tokenContract;
}

[oveddan]

def can see the need to get tokenContract and creator from the signature, but including those in the struct to sign is redundant as you can get creator by decoding the signer, and tokenContract by the address the signature was signed against. Additionally, there’s nothing enforcing that the creator account set in that field of that signature has explicitly has granted permission to the signer to sign an attributed creation on behalf of them.

Additionally if the signature is to follow this format, token level settings cannot be encoded in that signature, such as token price or metadata uri. that would need to be encoded in a different signature, requiring the creator to sign two messages

[montasaurus]

Sharing some more from our offline conversation with @oveddan:

There are other ways to get the creator and tokenContract in context, but the payload is very succinct and self contained if we just include those two fields. It would be potentially confusing to include fields that may change or would be factory contract implementation specific (like price and metadataUri).

There is some duplication between tokenContract and the verifying contract address in the 712 domain, but removing tokenContract from the payload makes it much less explicit. It wouldn’t be clear to me as a signer that the verifying contract address is also the token contract I’m attesting to if I hadn’t read the spec. Including creator is necessary because of the 1271 issues discussed above.

[montasaurus]

For us to trust that the signer intends to claim creatorship of the token contract, we’d need to have the signature payload:

• clearly state that it’s a creatorship attestation, so the user understands what the signature is being used for
• specify the creator address, since a signature can be valid for multiple creator addresses with 1271
• specify the token contract that’s being created, so it’s not possible to obscure it

We also need to be able to recreate the payload based on the log emitted from the contract so we can verify it matches the requirements + the signature is valid. Additionally, we have to assume that the token contract emitting the event is malicious (as well as the factory contract creating it, if applicable). It’s very common for bad actors to try and claim a prominent user has created a collection so they can collect proceeds from it & we need to be robust to that.

The simplest solution I see for including additional fields for a factory contract is to have the creator sign two payloads in that case. The first signature is for a simple attestation specified in 7015:

struct CreatorAttestation { address creator; address tokenContract; }

The second signature includes the signature from the first payload, so the factory contract can pass it in to the token contract on creation and emit it. The second payload (and this wrapping scheme) wouldn’t be included in the spec, since it’s just an implementation decision of the factory contract.I know signing twice isn’t super ideal, but that’s the only way I see to do this robustly.

[ryanio]

An update here, OpenSea now supports ERC-7015 based on the specification as of Jan 16 2024

[strollinghome]

I checked in with @oveddan as Zora is the main platform using the standard, and given OS (@ryanio) is already indexing the same version Zora is using, I moved the standard to Review, and hope we can get it moved to Last Call soon.