-
Notifications
You must be signed in to change notification settings - Fork 159
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
rfc(0040): add sudt info #290
base: master
Are you sure you want to change the base?
Conversation
extra: {website:"https://nervos.org"} // optional | ||
``` | ||
|
||
The serialization format of sudt info should be json. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 drawbacks to use json:
- waste of space
- the smart contracts will need a json library, which will introduce security issue.
I think molecule is OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if Molecule can be used here as it's more space-efficient and a default on CKB. If I remember correctly, the reason for using JSON than Molecule was the ease of use for application developers? Not sure if Molecule's javascript support renders the reason obsolete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xcshuan any thoughts on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use the simplest format, which is repeated length+content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A simple format is consistent with sUDT indeed. But sUDT info data consists of several key/value pairs rather than a single number in sUDT data. Use length+content format here basically reinvents Molecule, with the drawback of lacking deserialization tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a library for molecule available, I think it should be better to use molecule.
There are libraries for C, Rust, go and JavaScript: https://github.com/nervosnetwork/molecule.
At least we need to verify in the script whether the data of the info cell is a valid SUDT Info. I don't know if using json will make this troublesome.
It will. Use JSON in on-chain contract is hard, expensive and may bring security risks as Jindong mentions. Do we really need to verify sudt info format in scripts? It doesn't make sense in my opinion.
If we skip the verification, we can stick to the original method.
If we do need that, I recommend use the format below:
vector Bytes <byte>;
table SudtInfo {
decimals: byte,
name: Bytes,
symbol: Bytes,
extra: Bytes,
}
The extra
may be a problem in real world cases. Since molecule needs schema to parse data, but different user may use different fields here. The best solution for this field I can find is to use an embedded JSON string. In this case, on-chain contracts will only verify the fixed fields with molecule, and we have to develop some tools for users in the SDK to parse the whole struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer not to verify format in CKB script. It simplifies the script/standard a lot. If someone creates an ill-formated SUDT info cell intentionally or accidentally, applications can simply ignore the bad info cell and the result is the same as no SUDT info cell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary: we're considering JSON vs. Molecule, while JSON is simple and widely used, its drawback is more difficult to parse/validate on chain. However it seems unnecessary to include SUDT info parse/validation in on-chain script, which basically avoids the problem.
In this case I suggest to go with JSON and skip on-chain format validation, to keep things simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xcshuan I suggest adding @huwenchao as 2nd author because it's his analysis and suggestions and having him complete the RFC, if you agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding @huwenchao as 2nd author
Ok, I think it's fine.
@xcshuan thoughts on above feedbacks? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've no further question for this PR.
- **Rule 1:** validate the format of info cell data. | ||
- **Rule 2:** The args conform to the rules of type id. | ||
- **Rule 3:** The lockscript of info_cell should be set to deadlock by default, in some cases, developers can choose other lockscript. | ||
- **Rule 4:** After constructing this Info cell, use the hash of info_cell's type_script as the first 32 bytes of the args of the lockscript of SUDT owner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this rule, there is no way to add sudt info for existing script-drive SUDT with this protocol, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, info is bound to the token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a problem. Since token from NexisDAO and Force Bridge can not use this protocol then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consulted Nexisdao, they have designed this, and udt info typescript can't actually sense whether a udt is controlled by a private key or not, it should first judge according to the script-driven mode, and then judge according to the single-owner mode , so for the info cell of udt originating from forcebridge, it can be generated according to the mode of single owner, as long as it is the earliest one.
May better rename |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reserved RFC Number 0040 for this PR. See details in #246
doc: remove the sudt info verification
Hi!! I just found out about this proposal from the discord Github bot! I'm the guy working on iCKB, so basically I'm the new guy in Nervos, still let me add my opinion. I don't believe adding this information on-chain is the right step, reason being no on-chain logic needs this metadata. Then again there is a very good use-case for this metadata in DApps, so makes sense to have it somewhere in an easily accessible format. So why not standardize a schema (fields names, types, which are required and which are optional), a format (JSON?), a location (IPFS?) and define them off-chain? Some time ago I was reasoning about this very same problem with Frank@Lay2, I suggested using IPFS to store this information. Right now I see there are more alternatives:
Another benefit of adopting an off-chain approach is the compatibility with older SUDT tokens, which is actually a nice side effect!! Feel free to let me know your thoughts on this, |
Agreed with @phroi. It's not necessary to turn to a live cell to store such static information since no other scripts would access that. We can put these data to |
Yeah!! IPFS data (without using gateways) is difficult to access, but I pointed at it because it's addressed by content, so simplifies the process.. still without using IPFS, one could store this metadata anywhere and by just adding a signature, we can make sure it has not been tampered with. So I see this flow for DApp developers:
So basically, the way I see it, the metadata is always statically added to website, not dynamically loaded. This approach avoids Metadata XSS Attacks altogether, because the developer will carefully check the new data! @CipherWang what competitive advantage do you see for having this data on-chain? |
If the basic data is on-chain, like what Ethereum does, everyone who devs on CKB can immediately access the data. We has proposed a metadata schema stored in the |
Hi @CipherWang, thanks for taking your time to reply, very appreciated!!
Yeah, about Ethereum, for example ERC721 have (almost) all of its metadata off-chain: https://docs.opensea.io/docs/metadata-standards#implementing-token-uri Switching back to our case, having metadata off-chain would simplify devs experience by allowing token devs to include more heavy metadata like the token's icon, which is basically a requirement when supporting a token in a DApp.
Ten days ago @janx pointed me to the COTA standard and since it uses Merkle trees of course I love it (still it's to be seen how it will affect developer experience). Switching back to CTMeta, I see the token Metadata is a work in progress, still from the CTMeta convention section I got the gist of it (BTW I made a pull request to fix some formatting issues). Basically since the witness only pays transaction fees, then it's cheaper, so one can carry-around the associated cell metadata in the witness. Yeah, it's a smart idea, still I don't know if replicating the metadata information for each transaction does really make sense. For example following CTMeta approach, since it doesn't really make sense to fit all the metadata (like the token icon) into the witness, then it will always happen that either:
|
No description provided.