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

Add NFT templete and UT #268

Closed
wants to merge 26 commits into from
Closed

Conversation

ShawnYun
Copy link
Contributor

@ShawnYun ShawnYun commented May 8, 2020

close #239
and copy the demo to neo-project/examples#30

public static event Action<byte[], byte[], BigInteger, byte[]> Transferred; //(byte[] from , byte[] to, BigInteger amount, byte[] TokenId)

//super admin address
private static readonly byte[] superAdmin = Helper.ToScriptHash("Nj9Epc1x2sDmd6yH5qJPYwXRqSRf5X6KHE");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there possibility to change superAdmin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better not to change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contractOwner?

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good example. I will test it asap.

@ShawnYun
Copy link
Contributor Author

@vncoelho Thanks. and there are some UTs need to modify neo.

@ShawnYun ShawnYun marked this pull request as ready for review May 19, 2020 07:42
Co-authored-by: Shargon <shargon@gmail.com>
templates/Template.NFT.CSharp/NFTContract.cs Outdated Show resolved Hide resolved
templates/Template.NFT.CSharp/NFTContract.cs Outdated Show resolved Hide resolved
private static byte[] StoragePrefixTotalBalance(byte[] owner) => new byte[] { 0x11 }.Concat(owner);
private static byte[] StoragePrefixTokenBalance(byte[] owner) => new byte[] { 0x12 }.Concat(owner);
private static byte[] StoragePrefixProperties(byte[] tokenId) => new byte[] { 0x13 }.Concat(tokenId);
private static byte[] StoragePrefixTokensOf(byte[] owner) => new byte[] { 0x14 }.Concat(owner);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we only need 2 tables to represent the ownership relationship
<tokenId+owner,balance>
<owner+tokenId,balance>
This saves storage space

templates/Template.NFT.CSharp/NFTContract.cs Show resolved Hide resolved
templates/Template.NFT.CSharp/NFTContract.cs Outdated Show resolved Hide resolved
private const byte Prefix_TokenOwner = 11;
private const byte Prefix_TokenBalance = 12;
private const byte Prefix_Properties = 13;
private const byte Prefix_TokensOf = 14;
Copy link
Contributor

@igormcoelho igormcoelho Oct 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's quite dangerous to keep using "byte" as prefix marker... it's much better to use "sbyte" instead. I mean, this code may suddently break if some unexpected user decides to put values over 127... and then this guy just gets multiple bytes (two) instead of one (which is quite unexpected usually), may break some alignment in their backends, or even in the smart contract (which is far worse), I don't know. Better incentive better practices, I think. People will certainly learn from this contract example. Do you agree @erikzhang ?

@erikzhang
Copy link
Member

This should be moved to https://github.com/neo-project/examples.

@erikzhang erikzhang closed this Jan 14, 2021
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

Successfully merging this pull request may close these issues.

Write an NFT contract example
8 participants