-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: add function selectors to psp22 trait #316
feat: add function selectors to psp22 trait #316
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## chungquantin/feat-psp22_example #316 +/- ##
==================================================================
Coverage ? 51.44%
==================================================================
Files ? 48
Lines ? 4893
Branches ? 4893
==================================================================
Hits ? 2517
Misses ? 2327
Partials ? 49 |
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.
Curious to hear why all the docs? Personally I think adding the selector is enough and a reference to the psp22 standard in the readme will be enough.
@Daanvdplas Removed doc but still keep the reference section. What's your thoughts? |
Still think it is too much. The link all reference the same URL. This only increases the maintenance and development work for all future standards that we will add. |
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 mistakes in the selector attributes for mint and burn.
Also, there is no evidence that applying these to the trait will ensure that it will be correctly set when used on a contract. Is it possible to add the relevant traits to a contract, ideally in this PR if possible but only if an existing contract exists within the codebase that is suitable, and then attach the resulting contract metadata/ABI as a comment as proof?
If not, just the ABI from some non-versioned contolled contract is fine. The only justification is confirming the assumption that a contract implementing the trait will have standards compliant messages within its ABI.
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.
Nice job! LGTM!
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.
Excellent, thank you so much!
eee369a
into
chungquantin/feat-psp22_example
This reverts commit eee369a.
Add function selectors to the PSP22 trait.
Mintable
andBurnable
, can be found in AZ PSP22 trait: https://github.com/Cardinal-Cryptography/PSP22/blob/main/traits.rsBelow is a
spec
in the json metadata