ERC-6551: Non-fungible Token Bound Accounts

Tokens
,
21

You are missing the point. You claim to have provided a token bound proxy account that enables someone do anything and you limit what it does? It is great to make a test cases, but you just made a token bound proxy account for executing generic transactions with separate contracts. You don’t need to limit what you just make, that is not a standard.

Ok, so to leave a room, I think the proposal has to specify three cases where:

  1. wallet <> token bound contract <> another contract (proxy account and your reference implementation)
  2. wallet <> token bound contract

The question is, how will you securely specify second item as you referred on security concerns in your original proposal? and how will you aggregate all the other cases into one (e.g. FNFT by Revest, etc)?

Obviously the same answer from the previous quote.

ERC721A is subset of ERC721? Is it too hard to understand implementing NFT ownership of a newly created contract from create when you just coded? EIPs/assets/eip-5252/contracts/ABT.sol at 39fe51429c2295692b351e4e89f7c38cc42d8ae9 · hskang9/EIPs · GitHub

Ok now put that into the proposal without saying account ambiguity. You also forgot how your registry actually verifies NFT and connected account.

No, you don’t know how ERC-1155 works. ERC-1155 can have total supply of 1, and this makes the fact that there is only one owner per token ID. Perhaps you could set total supply on 2 and ask why it does not work.

No thank you. I work on this to make sense of a proposal. I am not joining your peripheral attempt on glorifying the proposal when it is just a proxy account implementation with NFT. You have to pay me first on handling your issue. I only work for free on this after being too afraid of the world where @vbuterin says zero knowledge solves everything but cannot code one line on actually shipping the product then cashes out 350 million dollars, Do Kwon saying algo-stablecoin will solve all money out there and make hundreds of people suicide after his scam got revealed. They all boast their knowledge about crypto, but they have zero knowledge in action. Also, I think using emojis doesn’t help on showing that you are seriously thinking to pass this. Are you trying to finalize this proposal or what?

From your answers, it seems you do have knowledge in smart contracts, but it does not look like you have knowledge in action as you haven’t known the EIP-1155 can limit the supply. If your proposal’s name is Non-fungible Token Bound Accounts, you must have knowledge in action on all NFTs, including ERC-1155. Hence, you need to implement the case where EIP-1155 is used too. Otherwise, this has to be limited to ERC-721 token bound accounts.

1 Like
22

@hskang9 Thanks for sharing your opinions!

5 Likes
23

Any error in the Registry Implementation. It said with error “Expected even number of hex-nibbles.” in the _creationCode function

24

Thanks for catching that @minhhung123! I’ve updated the reference implementation to fix the issue. Please let me know if you run into any other problems.

1 Like
25

I like this proposal. The functionality it enables of giving 721s predictably-addressed accounts seems genuinely useful. The design seems well thought out — it maintains backwards compatibility and has a small footprint, without unnecessary details.

I think there’s a potential foot-gun, which could also be called a feature, depending on your point of view. Transferring an NFT to the address of an account that is derived from the NFT will make the account inaccessible. This is probably more an implementation consideration than something to spec, but I wonder if implementations should reject a transfer of their own NFT to themselves (in their onERC721Received function), at least by default.

A thought on the security issue you describe for the owner of an NFT pulling assets just before completing a sale/transfer. I think a general method to avoid this (that doesn’t require explicit support from an NFT market or from account implementation contracts) could be to create a wrapper NFT contract whose tokens are minted to anyone transferring an NFT to the contract. Anyone holding a wrapper token can redeem the wrapper for the wrapped token, burning the wrapper in the process. If the tokenIds of the wrapper are not re-used, someone buying/receiving a specific wrapper tokenId can be sure that any accounts linked to the wrapped NFT have not performed any transactions since the wrapper was created, as the wrapper contract is the owner of the NFT while wrapped.

Although this wouldn’t prevent token-approval-based operations from occurring while the NFT is wrapped.

(I’m not suggesting changing the spec, just noting that it seems like this could be mitigated by anyone with an interest in doing so.)

Finally, just a thought on the 1155 support — even though a token can have multiple holders, it’s quite frequently used with a supply of 1 per token. As you describe in the backwards compatibility section, the registry implementation doesn’t enforce 721 interface implementation, so it seems like 1155 support could be an optional choice for an implementation. They could just use balanceOf(...) > 0 to authorise access, and if the 1155 contract allows a supply greater than 1, you just get a kind of 1-of-n multisig account!

26

@h4l this is great feedback, thank you!

This is definitely a potential foot-gun, and I think it’s worth adding a section discussing this to the proposal. Implementations can definitely implement some protection against this by checking that the received NFT is not the NFT which owns the account. However, this issue also arises any time there is an ownership loop involving any number of NFTs. For example, you could have two NFTs and transfer the ownership of each NFT to the account of the other NFT. Since loops could involve a large number of NFTs, iterating through the ownership graph on every NFT transfer to prevent loops would likely exhaust the available gas.

The possibility of asset loss due to ownership loops seems similar to burning an NFT - it’s a destructive action that the holder of an NFT has the right to perform. It also has the potential to be the basis for interesting token mechanics, similar to burning. As a result, I think it should be up to each implementation to decide if they would like to prevent this behavior.

This is a great approach! Another alternative approach inspired by this idea could be to wrap the NFTs stored within the token bound account, allowing additional logic to determine which actions can be taken on these NFTs while they are owned by the token bound account.

This is very true! I wonder if it would be worth modifying the proposal such that it doesn’t assume the existence of a single owner per NFT. This could be accomplished by changing the owner() returns (address) method (which assumes the account has a single owner) to an isOwner(address potentialOwner) returns (bool) method, which returns true if potentialOwner is an owner of the token which controls the account. This may make implementing ERC-1155 support simpler for developers building custom implementations. Would be curious to hear your thoughts on this approach.

27

Yep, I agree. Certainly lots of ways people can find to lock themselves out of their wallets.

That’s an interesting idea, I could imagine NFT projects could provide an official account contract for their NFT which imposes some specific constraints that make sense in the context of the project. Like maybe the project can send some kind of assets/NFTs to the accounts of their NFTs that can only be withdrawn after a delay or after some constraints are met. And then they are naturally taken along with the original NFT if it’s sold/transferred.

That seems like a good approach. And someone wanting to discover the owner for a 721’s account could still do so via the NFT contract, which they can find using token(). I would imagine it shouldn’t be mandatory for an implementation to support 1155s in addition to 721s. Do you think you would specify that implementors just need to support at least one of the two? As mentioned above, I can imagine use cases for projects just creating an implementation that only supports their single specific NFT contract.

28

A few comments after chatting with @jay and team that I think would be big improvements to the EIP:

  1. Having multiple addresses per implementation in a predictable fashion:
    Adding a counter variable per chain/tokenAddress/tokenId/implementation that auto-increments and is used in the nonce for create2
  2. Adding lastTransactionTimestamp to the account interface. The implementation MUST update the lastTransactionTimestamp upon any transaction execution.

The purpose of 2 is to help marketplaces address the issue of front-running removal of assets. Adding this variable allows marketplaces that support this spec to reject order fulfillment automatically if the order contains a restriction on lastTransactionTimestamp. Practically speaking, a supported marketplace would:

  1. Show NFT’s wallet assets and contents, as well as any token approvals that exist. Buyer/bidder has full transparency on what’s own. Buyer/bidder submits the order with the matching lastTransactionTimestamp
  2. Any attempts to frontrun the order will modify lastTransactionTimestamp and will cause the marketplace fulfillment to fail.
2 Likes
29

One other aspect is adding the ability to init bytecode in the registry create function, as well as take in an index. This allows a really good property of being able to have predictable addresses regardless of a base implementation (which can proxy to an upgradeable implementation) on an indexed basis.

Which means that people can see a ‘default’ wallet for every nft in existance and start interacting with them.

We’ve had an extremely similar concept in development since last September, and would love to unify these concepts into a singular spec:

1 Like
30

@wwhchung these are great suggestions, thanks for highlighting them!

Allowing multiple accounts per NFT/implementation combination seems very useful. Perhaps instead of enforcing auto-incrementation at the registry level, the createAccount and account functions could take an extra uint256 seed parameter which would be used to compute the account address. This would allow account creators wishing to utilize auto-incrementing seeds to do so, while also opening up the possibility of “vanity” account addresses or stealth addresses.

I like the idea of tracking changes in account state over time, as that simplifies marketplace support. Perhaps instead of using a timestamp, a function nonce() public returns (uint256) could be added, allowing account implementations to choose how they would like to implement state tracking.

Will make some updates to the proposal to incorporate these concepts.

31

If you take a look at my recent repo changes that’s what I’m doing as well (index that can be specified as well as nonce rather than timestamp).

32

My reference implementation also has a static cloneable proxy that’s upgradeable on a user basis, which takes in initialization bytecode so that the deploy bytecode can be static yet the implementation can be dynamic which allows for predictable addresses.

33

Awesome to see that you and the team are have been experimenting with this concept as well, thanks for making it open source! Would love to unify these approaches under a single proposed standard.

The main concern I have with index-based addresses is that it introduces a significant vector for spam/targeted attacks. Since token bound accounts are permissionlessly deployable by design, using only an index (and not the implementation address) when computing the account address would make it easy for bad actors to “claim” NFT account addresses with malicious/spammy implementations. For example, if the predetermined address for NFT #1 with index 0 is 0xabc..., a malicious actor could deploy a contract to 0xabc... that contains a backdoor allowing them to extract all assets from the account, making that address functionally unusable. This could also be done for a large number of indexes (e.g. the first 100 wallet indexes are all scam implementations), which would introduce a significant burden for both NFT holders wishing to use their accounts as well as developers wishing to adopt this proposal.

By including the implementation address as an input to the account address computation, malicious contracts deployed for an NFT can be ignored by the owner and any platforms choosing to support this proposal, as there are no “default” addresses that can be claimed. It also has the desirable quality of allowing an account’s implementation to be trusted solely based on the account’s address, which has useful ramifications.

Perhaps most importantly, including the implementation address in the account address computation allows developers wishing to provide index-based “default” wallets to easily do so in their applications, while enforcing index-only account addresses at the registry level removes the possibility of ever supporting non-index-based accounts.

2 Likes
34

Hmmm. That’s a good point.

Note that my proposal also computes based on implementation and index. But the spam attack vector still exists.

Perhaps the reference implementation only allows the current token owner to deploy which would prevent this attack at least for the reference implementation.

Note that I do need to patch up my proxy implementation to defer owner check to the implementation

35

Allowing for optional initializer calls during account creation seems very useful!

I missed that, thanks for clarifying.

I agree that desired protections against this type of attack should be enforced at the account implementation layer. However, I’m hesitant to recommend owner-only deployments as the default configuration for a wallet implementation, since there are many potential positive use cases for permissionless deployments that could be inhibited.

36

The reference implementation wouldn’t be part of the eip. But rather the main website explaining and promoting this eip. Similar to royalty registry or delegate.cash

The eip itself would be implementation agnostic. But the site should have a ref implementation that allows every token in existence to have an address from the get-go (which would be based on the ref implementation), and a one click way for any token owner to get write access to the address.

This ref implementation would be deployed to every chain at the same address. And I am suggesting it to be similar to this one:

Which has the features of upgradeable implementation, prevention of non token owners from initializing the wallet and nonce incrementing built in.

Again, this is outside the scope of the eip but IMO needed for the main site and launch in a way that ppl can start engaging with these wallets (pushing content) without the token owner even needing to deploy the wallet.

37

@wwhchung Agreed that a demo site will be super helpful to help folks understand how this EIP works! In the meantime, this is great as an example in the reference implementation repo. Appreciate the PR :slight_smile:

39

We have similar ideas at our latest version of EIP-4972: Update EIP-4972 Pull Request. Although our proposal is not for ERC-721 but for domain names, the ideas behind are quite similar: to give nft/name a bounded account

Our interface is little bit different from EIP-6551 but I think there is a potential to merge these two EIPs to provide a more universal interface. Here are some of my thoughts to simplify the interface:

  1. The “executeCall” function is actually not related to this EIP. Each implementation should be able to customize the way to call other functions. It could be included in the reference implementation though.

  2. The “owner” function is not necessary since it can be derived from the token contract and token id.

  3. The chainId param is not necessary for functions/events at IERC6551Registry since you can always get it from block.chainId, (except you want to register account at chain A for chain B, which doesn’t make sense to me). You can still use chainId as a field in the byte code spec when registering the account but it should not be included in the function params. The chainId returned by token function can also be emitted.

1 Like
40

Another thoughts is that instead of putting implementation as a param of all functions of IERC6551Registry, is it better to extract it out as a general view function so all accounts registered by this registry are guaranteed to share same implementation? It may not be as flexible as current version but it will prevent potential fragmented implementations, making it more easier to integrate.

The new interface will look like this:

interface IERC6551Registry {
    event AccountCreated(
        address tokenContract,
        uint256 tokenId
    );

    function createAccount(
        address tokenContract,
        uint256 tokenId
    ) external returns (address);

    function getImplementation() external returns (address);

    function account(
        address tokenContract,
        uint256 tokenId
    ) external view returns (address);
}

interface IERC6551Account {
    receive() external payable;

    function token()
        external
        view
        returns (address tokenContract, uint256 tokenId);
}
1 Like
41

@dongshu2013 thanks so much for the comments! Awesome to see other folks exploring the potential impact of smart contract accounts tied to on-chain assets.

I agree that this looks a bit out of place in the proposal. However, including a standard execution function in the account interface enables the very desirable characteristic of having a standard interface for interacting with all token bound accounts regardless of the underlying implementation code. Without a standard execution interface you would need some form of interface detection to determine how to execute calls from different account implementations, which would get messy quickly and make the proposal more difficult to adopt. Definitely not married to the current execution function signature though, so open to suggestions there.

You can definitely query the owner directly from the token contract given the token information. However, this function serves two purposes in the proposal. First, as above, it gives a standard interface for interacting with accounts. Second, it makes token bound accounts somewhat compatible with existing infrastructure built around ownable contracts. This may be useful for supporting token bound accounts in some legacy applications. If this proposal is expanded to support semi-fungible tokens, this interface may change and the current owner function would be removed in favor of something like isOwner(address) returns (bool).

This is true for single-chain applications, but including the chainId as a parameter allows wallet implementations wishing to implement cross-chain compatibility to do so. Hardcoding block.chainid removes that possibility.

Having the ability to support any implementation is actually a very important characteristic of this proposal because it allows for client diversity in wallet implementations. If the registry provided a static wallet implementation, it would either be immutable and thus not able to adapt to improvements in the smart contract account ecosystem, or it would be upgradable and thus pose an unacceptable centralization risk. By allowing for arbitrary smart contract implementations that adhere to this standard to be bound to an NFT, this allows the ecosystem to continuously experiment with different approaches to token bound accounts in a fully decentralized manner.

Additionally, since the holder of an NFT has sovereignty over their asset, it would stand to reason that sovereignty would extend to choosing which wallet implementation they would like to use. By allowing arbitrary implementations, applications wishing to adopt this standard but support only a single implementation are free to do so. However, by restricting to a single implementation at the registry level, the reverse would not be true.

Hope this answers your questions! I think there are some really neat applications to be built around accounts bound to a canonical name, and would love to ensure that there are broadly adopted standards that can support this use case.

2 Likes