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 non-fungible token program #7007

Merged
merged 8 commits into from Nov 19, 2019
Merged

Add non-fungible token program #7007

merged 8 commits into from Nov 19, 2019

Conversation

@garious
Copy link
Member

garious commented Nov 17, 2019

Problem

No problem really - just didn't see a Solana implementation of a non-fungible token.

Summary of Changes

Add new NFT program. Similar in spirit to ERC-721. Aims to be sufficient for a high-performance CryptoKitties or VoIB vSIMs.

@garious garious requested a review from jstarry Nov 17, 2019
@garious garious force-pushed the garious:add-nft branch from bd3f5b4 to 8a7bd3c Nov 17, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 17, 2019

Codecov Report

Merging #7007 into master will decrease coverage by 3%.
The diff coverage is 95.5%.

@@           Coverage Diff            @@
##           master   #7007     +/-   ##
========================================
- Coverage    78.8%   75.7%   -3.1%     
========================================
  Files         223     225      +2     
  Lines       42945   44791   +1846     
========================================
+ Hits        33849   33921     +72     
- Misses       9096   10870   +1774
@garious

This comment has been minimized.

Copy link
Member Author

garious commented Nov 18, 2019

@jackcmay, @mvines, any objection to me stashing this program here until we find a better home for non-protocol programs?

Copy link
Member

jstarry left a comment

Nice! Looks like we aren't aiming to implement something like ERC 721 here at this point but it could be nice to have more ownership rules and user accounts. There's no way to look up all the tokens I, as a user, own and I can't authorize anyone to transfer tokens on my behalf which I assume is for NFT exchanges (but I'm not too familiar with the applications which use ERC-721)

programs/nft_api/src/nft_processor.rs Outdated Show resolved Hide resolved
programs/nft_api/src/nft_processor.rs Outdated Show resolved Hide resolved
programs/nft_api/src/nft_processor.rs Outdated Show resolved Hide resolved
programs/nft_api/src/nft_processor.rs Outdated Show resolved Hide resolved
programs/nft_api/src/nft_processor.rs Outdated Show resolved Hide resolved
programs/nft_api/src/nft_state.rs Outdated Show resolved Hide resolved
programs/nft_api/src/nft_state.rs Outdated Show resolved Hide resolved
@garious

This comment has been minimized.

Copy link
Member Author

garious commented Nov 18, 2019

@jstarry, deleting issuer and id reduced this contract to Solidity's Ownable.

garious added 3 commits Nov 19, 2019
Maybe this should be "Owned" to avoid confusion with an Ownable trait?
Copy link
Member

jstarry left a comment

Nice! lgtm

programs/ownable_api/src/ownable_instruction.rs Outdated Show resolved Hide resolved
@mergify mergify bot dismissed jstarry’s stale review Nov 19, 2019

Pull request has been modified.

@garious garious merged commit bfa2535 into solana-labs:master Nov 19, 2019
12 checks passed
12 checks passed
Summary 1 rule matches and 7 potential rules
Details
buildkite/solana Build #15285 passed (26 minutes, 8 seconds)
Details
buildkite/solana/bench Passed (10 minutes, 30 seconds)
Details
buildkite/solana/checks Passed (1 minute, 47 seconds)
Details
buildkite/solana/coverage Passed (8 minutes, 40 seconds)
Details
buildkite/solana/local-cluster Passed (13 minutes, 16 seconds)
Details
buildkite/solana/move Passed (6 minutes, 29 seconds)
Details
buildkite/solana/pipeline-upload Passed (3 seconds)
Details
buildkite/solana/shellcheck Passed (31 seconds)
Details
buildkite/solana/stable Passed (23 minutes, 57 seconds)
Details
buildkite/solana/stable-perf Passed (10 minutes, 3 seconds)
Details
ci-gate Pull Request accepted for CI pipeline
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.