erc6551 / reference

ERC-6551 reference implementation
153 stars 50 forks source link

Do we need to make ERC1167 a Must deploy method ? #30

Open kaiadachi opened 9 months ago

kaiadachi commented 9 months ago

Hello, First of all, I would like to express my appreciation and respect for all of ERC6551.

I feel that the NFT handling (Account) and deployment method (Registry) are defined as separate contracts, but in effect the Account is dependent on the Registry (ERC1167), which I feel undermines the availability of the ERC. In short, I would argue that an ERC that defines the handling of NFTs does not need to be dependent on how they are deployed. I believe that separating the responsibilities and guaranteeing the behavior independently will make the ERC more available and maintainable.

ERC-1167 Header (10 bytes)
<implementation (address)> (20 bytes)
ERC-1167 Footer (15 bytes)
<salt (bytes32)> (32 bytes)
<chainId (uint256)> (32 bytes)
<tokenContract (address)> (32 bytes)
<tokenId (uint256)> (32 bytes)

Although the data structure is defined as Must, the only part that is actually referenced is the following.

<salt (bytes32)> (32 bytes)
<chainId (uint256)> (32 bytes)
<tokenContract (address)> (32 bytes)
<tokenId (uint256)> (32 bytes)

If this is the case, then I think we can define ERC without depending on ERC1167 for the deployment method by re-implementing it as follows. In this way, no matter what deployment method is used, as long as the bottom 128 bytes (salt,chainId,tokenContract,tokenId) of the runtimeCode is described, the NFT can be circulated in accordance with the ERC6551 standard.

ERC6551Account#token()

-       assembly {
-            extcodecopy(address(), add(footer, 0x20), 0x4d, 0x60)
-        }
+       assembly {
+            extcodecopy(address(), add(footer, 0x20), sub(extcodesize(address()), 0x60), 0x60)
+        }

Regarding the deployment method, regardless of whether there is a Constructor or not, it is possible to deploy by identifying the point (before CODECOPY) where the runtimeCode area is allocated from the creationCode and generating a bytecode with 128 bytes added. If the Constructor does not exist, it can be deployed in the same way as 1167. I have actually deployed it with ERC7546 and ERC1967 and confirmed that it works.

In other words, it is not necessary to define the IF of the Registry in the ERC, but only if the last 128bytes of the runtimeCode contains the salt,chainId,tokenContract,tokenId. This allows you to deploy Beacon, Diamond, UUPS, UCS, or whatever.

Since the deployment method is irrelevant to the essence of ERC6551, shouldn't the registry just provide a reference implementation without defining IFs?