Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?
rfc(0040): add sudt info #290
Changes from 2 commits
9d83489
17942e0
0b54827
ae787c3
a17a810
0d8fb63
6d89f92
d505f84
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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:
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?
cc @CipherWang @louzhixian @Keith-CY
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.
There are libraries for C, Rust, go and JavaScript: https://github.com/nervosnetwork/molecule.
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:
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.
Ok, I think it's fine.