@sullof I’m not sure that these numbers are correct, since proxies deployed by the ERC-6551 registry are only 141 bytes in length (45 bytes for the ERC-1167 proxy and 96 bytes for the static data appended to the bytecode). You can see an example of the deployed proxy bytecode here.
With that said, @k06a’s contribution to this proposal would be welcomed.
Uhm, the bytecode I get compiling reference/ERC6551AccountProxy.sol at main · erc6551/reference · GitHub is that long. I guess I am making some error. Thanks for pointing out to that. I will try to figure out what is wrong in my calculations.
@sullof ah I see! That contract is part of an example upgradable account implementation. In this case, ERC6551AccountProxy is a singleton - only one copy of the contract needs to be deployed. The proxies created by the registry will point to the ERC6551AccountProxy contract as the implementation address. This means that the cost for creating the ERC6551AccountProxy contract only needs to be paid once instead of every time an account is created via the registry.
In the case of ERC-3652, it seems like the 322 byte proxy is deployed each time an account is created. This means that ERC-6551 account proxies will cost ~1/2 as much gas to deploy.
That sounds great. Thanks for explaining.
Interesting proposal and discussion. Are there any apps out there implementing this standard (sans the Draft status)?
I am implementing it in the Cruna Protocol
@jay In the IERC6551Account interface, we have
function owner() external view returns (address);
In our implementation we allow the user to chose if using an immutable bound account or an upgradeable one that can evolve and in the future accept standard not created yet. In the second scenario, we get a conflict between the owner() function required by OwnableUpgradeable and the owner() function required by IERC6551Account, i.e., the owner of the smart contract and the owner of the account.
While I like the simplicity of using owner(), I think it would make our life easier if the name was different.
accountOwner() or tokenOwner() would be good alternatives. What do you think?
BTW, I know that in your reference implementation is the owner of the NFT that can upgrade the implementation. That is a good solution if the owner of an NFT is a developer and can upgrade his own implementation without damaging the storage. However, it adds so much friction for normal users and introduces many risks at many levels. By changing the function’s name, we skip all the conflicts without forcing a solution that can reveal itself as a problem in the future.
Has thought been given on the ability to ‘chain together’ an ownership path? Namely, if Alice owns Token A, and Token A owns Token B, and Token B owns Token C, can Alice directly control the wallet holding Token C?
The issue arises that if Alice uses Address Z to try and take an action on Address X, Address X will reject it, as it would inspect who owns Token B (which is Address Y), and reject the mis-match.
To make this process nestable/chainable, the logic of the wallet contracts the ERC6551 Registry stamps out would need to have that logic. The EIP itself only enforces those wallet contracts have a specific interface (which includes an owner() function), which could do any additional logic. But I think having that logic in the provided example/reference implementation would be a large benefit, and let this EIP cover more use-cases (allow using ERC6551 instead of ERC6059, for example; the key difference that has been highlighted between those two standards).
The token-owned smart contract wallets already are required to be ERC165-compliant, so the owner() logic could easily inspect to see if the address it thinks is the owner of its key token is also a token-owned contract, it can simply return the result of the owner() function of that other contract.
Using the knowledge that all other token-owned contracts are ERC165-compatible and therefore easily-identifiable as another token-owned contract means adding in a guard against creating trivial loops could be done by having the example token-owned contract implement ERC721-receivable (add an onERC721Received function) and using that hook to check the owner() chain to make sure its own address isn’t encountered. An onERC721Received hook wouldn’t catch all occurrences of cycles created, but seems like a good, lightweight way to catch a lot of accidental loop scenarios.
Hey there. If you want the equipping relationship on chain you should definitely check ERC-6220: Composable NFTs utilizing Equippable Parts
These are great comments, thank you @MidnightLightning!
There’s a good discussion on nested execution above - essentially a wallet can receive a “proof” of root ownership that consists of the account data for each token bound account above the one being used for execution. That chain of ownership can be verified by the account, allowing for direct execution against an account by a root owner.
This is true as long as all accounts in the ownership chain are deployed, but breaks if an account in the chain is not deployed (as you won’t be able to call in and look up the token information). Ownership Cycles also allow for interesting burn mechanics to be created, so I don’t think we’ll prevent them at the proposal level. Implementations are encouraged to implement mitigations against asset loss in this scenario.
Ah, I see that now, here, thanks.
The ERC721 standard defines several “extensions” within the EIP itself. For many of these proposed features, you’ve proposed applications implementing ERC6551 just figure out their own mitigations. Are you opposed to having ERC6551 have identified “extensions” that lay out best practices for these proposed additional, optional features beyond a base example implementation? Or is there any effort already creating a new EIP draft with these extensions on ERC6551?
I think some extensions in the proposal are a great idea! Trying to keep the required interface fairly uncluttered, but I think going into detail on these points in the proposal and specifying some extension interfaces could be very helpful.
Did you see my comment above about the owner() function? Your opinion?
Thanks for the reminder, I missed it. How does requiring the owner() method cause a conflict here? It should be fine fine to have two interfaces with an overlapping method, the logic of the method can be changed when the implementation does.
Let’s say that someone writes a upgradeable ERC6551Account contract using OpenZeppelin UUPS proxy contract. Most likely, they will implement
function _authorizeUpgrade(address newImplementation) internal virtual override onlyOwner {}
using OwnableUpgradeable. But the onlyOwner modifier is calling owner() to get the owner of the smart contract. Instead, it will get the owner of the wallet. Depending on how it is implemented, it may break the owner() function in IERC6551Account, which would be a disaster.
The workaround is to not use Ownable and managing the owner of the smart contract in a non-standard way, which may open to other issues.
IMO, renaming the function is the only solution to avoid problems.
If you’re using UUPS and upgrade to a new implementation with a different owner() implementation, the onlyOwner modifier will work as expected (since the old implementation of owner() that looks at token data is no longer part of the contract). This change would break compatibility with this proposal (as the account is no longer token bound), but shouldn’t cause any logic issues in the contract.
You can check out my latest proposal ERC-8192, it could already do nested mapping and infinite dynamic mapping, we can have token A owning token B while token B owns token C, in which token A is owned by a wallet, token C(contract address, tokenId) → token B(contract address, tokenId) → token A(contract address, tokenId)-> wallet address. the ownerOf(tokenC) will be resolved to the correct wallet address that owns token A.
Curious as to why it was designed as an ERC instead of just a trustless immutable primitive service.
I don’t think so. It breaks the compatibility with this proposal only if implementing Ownable we override IERC6551.owner() with Ownable.owner(), making the owner of the contract the owner of any account. That is exactly the problem I see here and why I suggest that the function that checks who is the owner of the account is different than owner().
Look at it from a different perspective.
I learned how to connect to the registry to create a token bound account to my Bored Ape. I choose a suggested implementation and create an account. Later I discover that my account is unable to receive ERC777, ERC1400, etc. and I look for how to upgrade the implementation. Since I am just a token user the probability that someone phishes me and let me upgrade to a malicious implementation becomes very high. If the contract instead is managed by a service that focuses on security, I can have the advantage of having an upgraded account without risks. But the second, right now, is made hard because of the function overlap.
Another case of fraud can be someone showing the source code of their ERC721Account where the final contract is extending a standard ERC721Account and implements Ownable. How many people would be able to see that even if the contract looks legit, it is a scam because it plays with the owner() function?
I see why you are resistant to this change, since you already are using this proposal in production at https://tokenbound.org/ but this is a big issue and if you do not solve it, sooner or later a new standard will be proposed to overcome it, as it always happened in the past.