ERC-6551: Non-fungible Token Bound Accounts

Tokens
,
196

Would be great to have this as an example in the reference repo! Feel free to open a PR :slight_smile:

This method does give accounts more flexibility when implementing alternative authorization mechanisms such as delegation.

198

I have a few suggestions or questions regarding the current spec.

1. Regarding the functionality of isValidSigner which is currently defined as:

    /**
     * @dev Returns a magic value indicating whether a given signer is authorized to act on behalf of the account
     *
     * MUST return the bytes4 magic value 0x523e3260 if the given signer is valid
     *
     * By default, the holder of the non-fungible token the account is bound to MUST be considered a valid
     * signer
     *
     * Accounts MAY implement additional authorization logic which invalidates the holder as a
     * signer or grants signing permissions to other non-holder accounts
     *
     * @param  signer     The address to check signing authorization for
     * @param  context    Additional data used to determine whether the signer is valid
     * @return magicValue Magic value indicating whether the signer is valid
     */
    function isValidSigner(address signer, bytes calldata context)

I believe it should be amended to specify this method SHOULD NOT revert in case the signer is not valid.

I saw that the reference implementation does not revert in case the signature is invalid, but it is important to specify in this case (as well as other cases) the unhappy path the execution takes.

Consider adding (or I can make a PR) with the following specification:

     * MUST return the bytes4 magic value 0x523e3260 if the given signer is valid
     * SHOULD NOT revert in case the given signer is invalid
     * SHOULD return 0x00000000 in case the given signer is invalid

I want to prevent different types of implementations which would require a smart contract interacting with ERC6551Account implementations where it expects that some calls fail (revert) and some return bytes4(0).

2. Add event to signal correct execution.

The interface IERC6551Executable does not emit any event in case of correct execution.

Consider adding an event which logs the correct execution as well as the returned values.

A possible option could look like:

event Execute(
    address indexed to,
    uint256 value,
    bytes data,
    uint256 operation,
    bytes response
);

Additionally, the reference implementation should be updated to emit this event.

3. Gas optimization in case of reverted execution.

Current reference implementation of ERC6551Account execute is:

    function execute(
        address to,
        uint256 value,
        bytes calldata data,
        uint256 operation
    ) external payable returns (bytes memory result) {
        require(_isValidSigner(msg.sender), "Invalid signer");
        require(operation == 0, "Only call operations are supported");

        ++state;

        bool success;
        (success, result) = to.call{value: value}(data);

        if (!success) {
            assembly {
                revert(add(result, 32), mload(result))
            }
        }
    }

If the execution is reverted, the whole contract state is also reverted, thus the state storage variable remains unchanged. By moving the ++state at the end of the method, you reduce the gas used in case the execution is reverted.

However, it is important to note that if the external call eventually calls back into ERC6551Account, the state will not be changed (yet).

This gas optimization might not be a good idea, considering the state does not change until the full execution finalizes. This is something up for discussion. When should the state be updated?

4. Updating the state in case ether is received

This is something that is also up for discussion. Should the contract state be updated in case ether is received?

Thus, it is worth considering if the execution of receive() should update the state and in what conditions.

Attach the current token bound account state to the marketplace order. If the state of the account has changed since the order was placed, consider the offer void. This functionality would need to be supported at the marketplace level.

Considering the Security Considerations, specifically, if a trade would be made invalid by changing the contract state, a simple call to receive() (with 0 ether or even 1 wei) would update and invalidate the Account state.

I am happy to think about these suggestions further, discuss them or even open PRs if any of them go through.

3 Likes
199

Though I’m not an author of this proposal, I would like to express my opinion regarding the proposed suggestions, as I consider them insightful and on point.

  1. I agree with specifying that isValidSigner SHOULD NOT revert when the signer is invalid. This will make it easier for contracts that interact with ERC6551Account to handle errors gracefully.
  2. I fully support the inclusion of an Execute event. This will allow for better off-chain monitoring and make it easier to trace successful interactions with the contract.
  3. Your point about gas optimization is valid only in cases when the external call reverts, which usually can be predicted by wallets or other providers before the execution. I think reducing gas for reverting execution paths, at the expense of potential re-entrancy is not worth it.
  4. The point about updating the contract state when ether is received is interesting and I believe needs to be considered by the authors.
2 Likes
200

Can the NFT wallet’s private key be used to sign something?

Specifically, could you encrypt a string and only let the owner decrypt it using the NFT’s private key?

201

Smart contracts do not have private keys. A NFT cannot own a private key. ERC6551 uses the owner of the NFT (an EOA) as a signer to allow executing code with/from the TBA.

202

@jay I made a PR on the ERC6551 reference implementation.

The examples in the repo, despite being basic, are correct, audited and safe. They can be used as a solid base to build more advanced contracts. In order to allow so, the functions must be virtual, so that they can be overridden, and public to be called as super (like supportsInterfaceId).

This PR simply add virtual to the functions’ signature.

PS > I also made a version of the reference implementation for the npm registry at

published as erc6551 on npm, which has already virtualized the functions, since we use the upgradeable example in one of our projects and needed it.

1 Like
203

Good call - might even be worth specifying that it must not revert, as try-catching external calls is a pain.

This is also a good suggestion. An earlier version of the proposal included an event, but it was removed in favor of letting accounts define their own events (since some accounts already define an execution event such as those supporting ERC-4337). I think it’s worth adding back in.

In order to prevent footguns in the case of reentrancy, the account should update the state prior to the external call. While there may be some gas savings in the case of a reverting call, I don’t know if it makes sense to optimize for the unhappy path while introducing complexity to the happy path.

This is a good question, I think the behavior of state could be made more explicit in the proposal. Since the goal of state is to enable marketplaces to protect against fraud, the thinking is that state is only updated when assets leave the account or an external call is made from the account. In that case, state would not be updated when ether is received.

Thanks for the PR! Merged :slight_smile:

2 Likes
204

Overall I’m supportive of this EIP becoming standard and commend the authors on their work. I have concerns that I don’t believe to be insurmountable but should at least be addressed before acceptance.

Primarily, I think that Fraud Prevention should be fleshed out before acceptance. Although I agree that a full treatment is beyond the scope of the EIP, at a minimum it’s important to establish a thorough argument for it being possible—I think exploration of asset commitments is the best option for this.

My rationale is that if prevention was found to be more difficult than anticipated, the viability of mass adoption of this ERC would be jeopardised. Brief outlines as currently exist are insufficient as they can provide a false sense of surety— for example, the first recommendation (to commit to account state) is trivially bypassed as transfer of token-owned ERC20/721/1155 assets only change the state of different contracts.

Consequently, the recommendation to use account state to prevent fraud should, in my opinion, be updated to explain why it MUST NOT be used.

A minor point, probably for a follow-up EIP, is consideration of a default, minimally viable implementation for air drops. Counterfactual Accounts and Account Ambiguity can combine for undesirable effects if token owners don’t know the implementation address to access one of their transitively owned assets (or even that such ownership exists). A default implementation would ease discovery of transitive ownership at scale.

1 Like
205

When integrating the ERC6551Registry contract into other smart contracts, there is currently no way to securely validate that a passed address parameter is in fact the official ERC6551Registry address.

This is because ERC6551Registry in the reference implementation does not implement ERC165, the standard interface detection pattern. As a result, even though ERC6551Registry extends IERC6551Registry, a consuming contract cannot actually detect and prove the interface is properly implemented.

I can make the change and raise a PR for it, if you like.

206

Thanks for the comments @arrans!

I agree that this should be fleshed out more. There has been a lot of research done in the area of fraud prevention that should be highlighted in the proposal.

Since state is managed by the account and all operations that interact with tokens must be executed via the account, state commitments can ensure that no tokens have been transferred away from the account via direct transactions. The edge case here is approval-based transfers, which would not change account state.

However, account state can be deterministic based on operations executed from the account. This would allow commitments to future states that are only reachable if certain conditions are met (e.g. all active approvals have been revoked). Using deterministic state mitigates the shortcomings of the currently suggested fraud prevention schemes. We are currently considering how this should be written into the proposal, and would love your feedback.

This is a good point, and should be considered in a future proposal.

Since the Registry is a singleton contract whose address will be well-known and included in the proposal, contracts can trivially validate whether a given address matches the canonical Registry address. This address check also guarantees that the IERC6551Registry interface is properly implemented. As such, unlike other ERCs where multiple instances of a contract implementing a certain interface are expected, I don’t think ERC-165 interface detection on the Registry provides much benefit here.

That said, I’m not opposed to adding interface detection if there is a enough demand or a compelling argument for it.

1 Like
207

I’d like to point out a few considerations with the Create2 approach. Ensuring the same address across different blockchains involves adhering to a meticulous process which may not always be feasible. Imagine a scenario where a novel blockchain emerges in the future, and the nonce on that blockchain isn’t consistent with the wallet’s previous deployment nonce. Or consider wanting to leverage an optimized proxy size for new blockchains. There could be numerous reasons making it challenging to maintain the same address universally.

Now, let’s examine this from the perspective of a developer working with ERC6551. When coding the smart contract, hardcoding the address isn’t viable, as it would only be operational on the mainnet and not on staging or during development. Hence, the registry address would be input as a deployment parameter. But then arises the challenge: how does one ensure minimalistic yet effective validation of the address to prevent unintended inclusions? Currently, a feasible approach is implementing a try-catch mechanism and verifying the output of the ERC6551Registry.account function. However, in my opinion, a standardized support for the interface would streamline this process and enhance its robustness.

208

I don’t think this is an accurate characterization. The proposal specifies that the registry must be deployed using Nick’s factory in order to ensure the widest possible support across EVM chains. This factory exists on 31 chains according to BlockScan, and can be deployed to any EVM compatible chain using a pre-signed transaction. Additionally, because this factory is widely used, several chains that do not support pre-signed transactions have included it as a pre-deployed contract.

This proposal is dependent on the create2 opcode, which allows for deployments that are not tied to a given account nonce. Chains that do not support create2 will not be able to support ERC-6551 as a whole (as it requires EVM compatibility), regardless of the method used to deploy the registry contract.

If there are additional optimizations you would like to see included in the proxy bytecode I would love to discuss them and see if there is a way they could be incorporated into this proposal.

The registry can indeed be deployed to both testnets and local development environments at its canonical address. Instructions for how to do so should be documented, this would be a good addition to the reference implementation repo.

If the registry is deployed to staging / testnet environments, this is no longer an issue for developers.

I don’t think ERC-165 would be any more useful than an address equality check for this use case. ERC-165 makes a lot of sense for use cases where many contracts implement a common interface (such as the ERC-6551 accounts), but not as much sense for singleton contracts.

209

There are blockchains that enforce EIP155-like replay protection for security reasons. If that happens, Nick’s method doesn’t work because the chainId has to be part of the transaction. Did you try to deploy it on CELO or CELO Alfajores?

PS > I use CELO Alfajores as favorite testnet.

210

Nick’s factory already exists on Alfajores despite Celo’s lack of support for pre-signed transactions. As a result, the registry can be deployed without issue. Any other chains that enforce replay protection can include this factory as a pre-deployed contract as Celo has done.

1 Like
211

Then I don’t have other questions. Thanks

1 Like
212

Interesting! Paraphrasing to confirm that I understand: we can prove that it was impossible to perform a chain of actions that result in the assets being transferred therefore it is safe to assume that no fraud can be committed.

I think from a technical perspective this is a really elegant solution however I think that buyers still risk being cheated because it’s so much harder to reason about relative to the alternative of declaring transitively owned assets that the buyer expects. That said, I’m keen to see what you write up so please do share it.

1 Like
214

Just checking in after reviewing the last draft
Really good progress, as I was reading, was skeptical at first of the multi-implementation per NFT but I agree in the end that is it actually a great way to not constraint implementations. Nice!

I got few comments

  1. Feel like the Account standard should be a separate ERC.
  2. on that note, execution standards has been attempted before and I wonder why you chose to use a different spec than ERC725X (ERC-725: General data key/value store and execution) which define 2 functions:

function execute(uint256 operationType, address target, uint256 value, bytes memory data) external payable returns(bytes memory)

function executeBatch(uint256[] memory operationsType, address[] memory targets, uint256[] memory values, bytes[] memory datas) external payable returns(bytes[] memory)

It is really similar than the one specified in 6551 except for

  • it has STATICCALL
  • it uses 256 bit to represent operation type
  • it support batch operation in its interface
  • have associated events
2 Likes
215

@wighawag thanks, that means a lot! I definitely agree that there should be a separate smart account execution interface standard, and that this will likely become standardized in the future. Unfortunately there is no strong standard that this proposal can adopt right now, which is why an optional, minimal execution interface has been included in the spec. In my opinion this strikes a balance between having a standard interface across 6551 accounts right now while also allowing future standardization to be adopted without changes to the proposal.

Unfortunately, because ERC-725 has been in draft stage for a long time and does not have a clear path towards finalization, including it as a dependency would mean that ERC-6551 could not become finalized before ERC-725 does.

There are also several developer experience papercuts that exist in ERC-725X that have been adjusted in the recommended ERC-6551 execution interface:

  • using uint8 to represent operation types allows implementations to represent the value of operation as either a uint8 or an enum in their codebase without affecting the execution function selector
  • ERC-725X’s representation of arguments to executeBatch as arrays rather than as a single array of structs introduces additional calldata overhead (and causes two of the parameter names to be grammatically incorrect)
  • ERC-725X’s interpretation of operationType values is different than the equivalent values in most common smart contract accounts (e.g. Safe, Kernel), where an operation value of 1 usually represents DELEGATECALL

If ERC-725 were to be changed to fix these issues and championed towards finalization, or another proposal to standardize the execution interfaces were to gain steam, I would fully support externalizing the account execution interface.

Would love to hear @frozeman’s thoughts on this.

1 Like
217

@jay, I’ve been implementing the Cruna protocol and have used the ERC6551Registry to deploy an NFT’s manager for each tokenID in a Cruna Vault. This experience led me to realize that the applicability of ERC6551 might extend far beyond merely linking wallets to an NFT. In fact, it seems capable of associating any relevant entity with a tokenId.

However, there’s a challenge I’ve encountered: contracts bound using ERC6551 may not feasibly implement the IERC6551Account interface due to the constraints posed by the receive function. Additionally, such contracts might not require signer validation functionality. Their primary need is access to the token’s data.

To address this, I propose an amendment to the protocol. I suggest introducing a new intermediary interface, IERC6551Bond, which could look something like this:

interface IERC6551Bound {
    function token() external view 
    returns (uint256 chainId, address tokenContract, uint256 tokenId);
}

This interface would specifically define a contract as bonded to an NFT and facilitate access to the NFT’s data.

In an ideal scenario, IERC6551Account would be an extension of IERC6551Bond, perhaps as follows:

interface IERC6551Account is IERC6551Bound {

    receive() external payable;

    function state() external view returns (uint256);

    function isValidSigner(address signer, bytes calldata context)
        external
        view
        returns (bytes4 magicValue);
}

What do you think?

PS > If you like the idea, I can raise a PR in the erc6551/reference implementation and in the ERC itself.

PS2 > I think that IERC6551Account’s interfaceId should remain the same since the selectors ar XOR-ed and the interface contains the same selectors.

PS3 > I just realized that it would be also better if in the case of a simple bond, the registry emits a different event — it looks unlikely to pass. Maybe I have to make a new proposal…

218

Could you expand on the constraints you’re referring to here? Implementing an empty receive function or one that always reverts is fine. Similarly isValidSignature and isValidSigner can always return invalid. It seems like this should cover most use cases.