Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The limit of key length for storage must be increased. #1152

Open
nujabes403 opened this issue Jun 25, 2020 · 2 comments
Open

The limit of key length for storage must be increased. #1152

nujabes403 opened this issue Jun 25, 2020 · 2 comments

Comments

@nujabes403
Copy link
Contributor

nujabes403 commented Jun 25, 2020

Currently, there is a limit for key of storage which is [1, 64] lengths.

However, it must be increased for transferring NFT token to contract.

I found this issue while I'm sending my NFT token to contract.

cost0, err = h.MapPut(Token721MetadataMapPrefix+tokenSym+Token721MetadataKeySeparator+to, tokenID, metaDataJSON, publisher)

Please see the code below, this code is run while calling 'token721.iost', 'transfer'.

cost0, err = h.MapPut(
  Token721MetadataMapPrefix+tokenSym+Token721MetadataKeySeparator+to, 
  tokenID, 
  metaDataJSON, 
  publisher
)

Token721MetadataMapPrefix+tokenSym+Token721MetadataKeySeparator+to

Let's see this one.

  Token721MetadataMapPrefix    = "T721M"
  Token721MetadataKeySeparator = "#"

My NFT's tokenSym is: uniqueasset
'to' is set to some contractAddress: ContractCCx8LpBwqLdQCsrQ7hEPu8dTnd4zKFTu5jzojvAM75W6

When it is combined,
"T721M" + "uniqueasset" + "#" + "ContractCCx8LpBwqLdQCsrQ7hEPu8dTnd4zKFTu5jzojvAM75W6"

image

The length exceeds [1, 64] boundary which is the reason why the transaction like this (https://www.iostabc.com/tx/7oDnRgb9XEhzsAR9JZAfLma8j89zUBvsAXJNrr7VwrHj) was failed.

To activate NFT ecosystem in IOST, there would be many scenario for transferring NFT to contract, for example NFT market.

So the key length limit must be increased.

@sswsdsn @mlj1991 @flybikeGx @ziranliu @lispc @lileicool1

@lispc
Copy link
Collaborator

lispc commented Jul 6, 2020

After a careful code review, we think it is better not to change the 64 limit, which is a so breaking change.
Instead, we think we can change the data structure. We will change the current tokenSym + owner -> tokenId -> metaData map, to two maps, the first is tokenSym -> owner -> tokenId, the second is tokenSym -> tokenId -> metaData. This will be a less breaking change.

@nujabes403
Copy link
Contributor Author

@lispc Good!! Let me know when the code is updated. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants