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

General purpose NFT contract #47

Closed
5 tasks done
bitzoic opened this issue May 27, 2022 · 10 comments · Fixed by #48
Closed
5 tasks done

General purpose NFT contract #47

bitzoic opened this issue May 27, 2022 · 10 comments · Fixed by #48
Assignees
Labels
App: NFT Label used to filter for the app issue New Application Application specification

Comments

@bitzoic
Copy link
Member

bitzoic commented May 27, 2022

WIP

A general purpose NFT contract that will have the following attribute:

  • the contract should represent a collection of NFTs, not just a single token
  • every token minted should have a unique, immutable token id
  • there should be a way to find the owner of any token given its id.
  • access control for minting - the contract should store an Identity as the minter, which allows for either an address or a minter contract to be set as the entity with minting rights
  • an event should be emitted when approvals are made, an NFT is minted or burned, and when transfers are made

The contract ABI will follow the format below:

fn allowMint(minter: Identity) -> bool;
fn approve(to: Identity, token_id: b256) -> bool;
fn balance_of(owner: Identity) -> u64;
fn burn(token_id: b256) -> bool ;
fn constructor(owner: Identity, access_control: bool, token_count: u64) -> bool;
fn getApproved(token_id: b256) -> Identity;
fn getTotalSupply() -> u64;
fn isApprovedForAll(owner: Identity, operator: Identity) -> bool;
fn mint(to: Identity, amount: u64) -> bool ;
fn owner_of(token_id: b256) -> Option<Identity>;
fn setApprovalForAll(owner: Identity, operator: Idenity) -> bool;
fn transferFrom(from: Identity, to: Identity, token_id: b256) -> bool;
@bitzoic bitzoic self-assigned this May 27, 2022
@bitzoic bitzoic linked a pull request May 27, 2022 that will close this issue
@nfurfaro nfurfaro changed the title Basic NFT General purpose NFT contract May 27, 2022
@nfurfaro
Copy link
Contributor

A few things that I'd like to see in an NFT contract to help keep it useful and flexible:

  • the contract should represent a collection of NFTs, not just a single token
  • every token minted should have a unique, immutable token id
  • there should be a way to find the owner of any token given its id.
  • access control for minting - the contract should store an Identity as the minter, which allows for either an address or a minter contract to be set as the entity with minting rights

@nfurfaro
Copy link
Contributor

A good next step for this would be to pin down what the abi looks like.

The specific implementation can follow, and will probably be able to benefit from some cool new functionality in Sway (ie: vec, improved storage interface).

@nfurfaro
Copy link
Contributor

Something along the lines of this as a start (may not be complete yet):

library nft;

use std::identity::Identity;


abi NFT {
    fn mint(to: Identity, amount: u64) ;
    fn burn(token_id: b256) ;
    fn owner_of(token_id: b256) -> Option<Identity>;
    fn balance_of(owner: Identity) -> u64;
    fn transferFrom(from: Identity, to: Identity, token_id: b256)
}

And then some supporting internal functions, ie:

  • fn generate_token_id()
  • register_new_token()
    etc...

@nfurfaro
Copy link
Contributor

Eventually, we'll want to add event logging to this as well.
This is a good reference to get some ideas: https://eips.ethereum.org/EIPS/eip-721

I don't know about the safeTransfer stuff yet, need to think on that some more.
pinging @adlerjohn @SilentCicero for input.

@adlerjohn
Copy link
Contributor

cc @simonr0204 and @pixelcircuits for input

@nfurfaro
Copy link
Contributor

nfurfaro commented May 28, 2022

A separate (but related) issue is token standards, ie: Following existing standards, or creating a new one.
I'm most familiar with ERC721 and ERC1155 (batch transfers are definitely nice to have) myself.
But probably out of scope for this issue.

@pixelcircuits
Copy link

We need to also make sure it's just as easy for contracts to hold NFTs as it is for EOAs. This will be necessary for cross domain bridging in the future. I also like how ERC165 allows for interface detection and we should definitely mirror that process to make for easy data crawling for marketplaces, etc.

@nfurfaro
Copy link
Contributor

nfurfaro commented May 28, 2022

Plus one on the use of erc165!
I think using the Identity type in storage, ie: as the minter, owner, token owner, etc will help to make things a bit more interoperable between EOAs and Contracts.l, but there's likely more we can do on that front.

@Braqzen
Copy link
Contributor

Braqzen commented May 29, 2022

A few things that I'd like to see in an NFT contract to help keep it useful and flexible:

  • the contract should represent a collection of NFTs, not just a single token
  • every token minted should have a unique, immutable token id
  • there should be a way to find the owner of any token given its id.
  • access control for minting - the contract should store an Identity as the minter, which allows for either an address or a minter contract to be set as the entity with minting rights

@bitzoic can you move this checklist into the description above so that GitHub does some nice tracking similar to #1 ?
Could you also add some labels to the issue?

@bitzoic bitzoic added New Feature New addition that does not currently exist Good First Issue Good for newcomers labels May 30, 2022
@Braqzen Braqzen added New Application Application specification and removed New Feature New addition that does not currently exist Good First Issue Good for newcomers labels Jun 5, 2022
@Braqzen Braqzen mentioned this issue Jun 5, 2022
4 tasks
@bitzoic
Copy link
Member Author

bitzoic commented Jun 6, 2022

This issue's tests are currently blocked by FuelLabs/fuels-rs#326

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App: NFT Label used to filter for the app issue New Application Application specification
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants