Improved NFT Standard #171
Replies: 13 comments 7 replies
-
[Resolved] Change Proposal: use
|
Beta Was this translation helpful? Give feedback.
-
|
I agree with this
Updated interface, using TypeScript (I believe this is favored for specs?) would be: function nft_transfer(
receiver_id: string,
token_id: string,
memo: string|null
): void {}
function nft_transfer_call(
receiver_id: string,
token_id: string,
memo: string|null,
msg: string
): Promise {} |
Beta Was this translation helpful? Give feedback.
-
|
Yes, I swapped |
Beta Was this translation helpful? Give feedback.
-
|
There is no other team and chain which mastered NFTs than Flow -> we should have a better look at their NFT standard: https://github.com/onflow/flow-nft |
Beta Was this translation helpful? Give feedback.
-
pub struct NFTMetadata {
spec: String, // required, essentially a version like "nft-1.0.0"
name: String, // required, ex. "Mochi Rising — Digital Edition" or "ML Course Completion, Spring 2021"
media: Option<String>, // preferably decentralized URL (can be centralized, though) to associated media
icon: Option<String>, // Data URL
reference: Option<String>, // URL to a JSON file with more info
reference_hash: Option<String> // Base64-encoded sha256 hash of JSON from reference field
}
pub struct Token {
pub owner_id: AccountId,
pub meta: NFTMetadata, // NEW FIELD
}Suggestion: rename And actually, it seems like maybe the two concepts have been mixed a bit?
|
Beta Was this translation helpful? Give feedback.
-
|
I see that the proposal above includes a |
Beta Was this translation helpful? Give feedback.
-
I agree with this approach |
Beta Was this translation helpful? Give feedback.
-
|
How about extending TokenMetadata with:
Probably iat and upd timestamps can be derived from blocks when respective action happened, I do not know. Anyhow often we might need times, but storing them in off-chain metadata (e.g. in reference JSON that needs to be downloaded, decoded and processed) will be too slow and "expensive". |
Beta Was this translation helpful? Give feedback.
-
Is missing Please take a look at latest on NFT Simple: https://github.com/near/core-contracts/tree/ec632f03de777381710d9a145c01720bd23cdf02/nft-simple Which contains approval methods and storage management logic. |
Beta Was this translation helpful? Give feedback.
-
|
can we level set the discussion here? ... I think it would be useful and productive to prototype the design with real code - nft-simple is a good place to start ... "the proof is in the pudding". To help get folks on the same page, I have extracted the interfaces from the project to re-basline the discussion: pub type TokenId = String;
#[derive(BorshDeserialize, BorshSerialize, Serialize, Deserialize)]
#[serde(crate = "near_sdk::serde")]
pub struct Token {
pub owner_id: AccountId,
pub metadata: String,
pub approved_account_ids: HashSet<AccountId>,
}
pub trait NonFungibleTokenCore {
fn nft_transfer(
&mut self,
receiver_id: ValidAccountId,
token_id: TokenId,
enforce_owner_id: Option<ValidAccountId>,
memo: Option<String>,
);
/// Returns `true` if the token was transferred from the sender's account.
fn nft_transfer_call(
&mut self,
receiver_id: ValidAccountId,
token_id: TokenId,
enforce_owner_id: Option<ValidAccountId>,
memo: Option<String>,
msg: String,
) -> Promise;
fn nft_approve_account_id(&mut self, token_id: TokenId, account_id: ValidAccountId) -> bool;
fn nft_revoke_account_id(&mut self, token_id: TokenId, account_id: ValidAccountId) -> bool;
fn nft_revoke_all(&mut self, token_id: TokenId);
fn nft_total_supply(&self) -> U64;
fn nft_token(&self, token_id: TokenId) -> Option<Token>;
}
#[ext_contract(ext_non_fungible_token_receiver)]
trait NonFungibleTokenReceiver {
/// Returns `true` if the token should be returned back to the sender.
/// TODO: Maybe make it inverse. E.g. true to keep it.
fn nft_on_transfer(
&mut self,
sender_id: AccountId,
previous_owner_id: AccountId,
token_id: TokenId,
msg: String,
) -> Promise;
}
#[ext_contract(ext_self)]
trait NonFungibleTokenResolver {
fn nft_resolve_transfer(
&mut self,
owner_id: AccountId,
receiver_id: AccountId,
approved_account_ids: HashSet<AccountId>,
token_id: TokenId,
) -> bool;
}Food for thought ...We have been using the approach of designing the standards by defining the JSON RPC API interfaces using typescript. That is necessary, but that is only part of the story. We need to also define the Rust smart contact interfaces. Thus, I would propose going forward that we start with the Rust smart contract interfaces and then map that to the JSON RPC API, which should fall out of the Rust smart contract interfaces. |
Beta Was this translation helpful? Give feedback.
-
Questions
|
Beta Was this translation helpful? Give feedback.
-
|
This seems like it would be part of the upcoming Enumeration extension. I believe this discussion's description has been updated with this information. |
Beta Was this translation helpful? Give feedback.
-
Issues with Token data structurepub type TokenId = String;
#[derive(BorshDeserialize, BorshSerialize, Serialize, Deserialize)]
#[serde(crate = "near_sdk::serde")]
pub struct Token {
pub owner_id: AccountId,
pub metadata: String,
pub approved_account_ids: HashSet<AccountId>,
}
pub(crate) fn internal_transfer(
&mut self,
sender_id: &AccountId,
receiver_id: &AccountId,
token_id: &TokenId,
enforce_owner_id: Option<&ValidAccountId>,
memo: Option<String>,
) -> (AccountId, HashSet<AccountId>) {
let Token {
owner_id,
metadata,
approved_account_ids,
} = self.tokens_by_id.get(token_id).expect("Token not found");
|
Beta Was this translation helpful? Give feedback.
-
Can you elaborate on this point?
@evgenykuzyakov would we be able to use |
Beta Was this translation helpful? Give feedback.
-
#[derive(BorshDeserialize, BorshSerialize, Serialize, Deserialize)]
#[serde(crate = "near_sdk::serde")]
pub struct Token {
pub owner_id: AccountId,
pub metadata: String,
pub approved_account_ids: HashSet<AccountId>,
}
fn nft_token(&self, token_id: TokenId) -> Option<Token>;The The same goes for metadata. What are the expectations for metadata? As a general rule, leaving anything unbounded creates vulnerabilities.
|
Beta Was this translation helpful? Give feedback.
-
|
An update on the Royalty field we're using, described in yesterday's meeting. Where |
Beta Was this translation helpful? Give feedback.
-
|
There are numerous problems with Eg. Here's a sample implementation. I would like to know how to check that the promise chain returns true. There's also a bug with the Near sdk, on #[ext_contract(ext_self)]
pub trait NonFungibleResolveTransfer {
/// Finalize an `nft_transfer_call` chain of cross-contract calls.
///
/// The `nft_transfer_call` process:
///
/// 1. Sender calls `nft_transfer_call` on FT contract
/// 2. NFT contract transfers token from sender to receiver
/// 3. NFT contract calls `nft_on_transfer` on receiver contract
/// 4+. [receiver contract may make other cross-contract calls]
/// N. NFT contract resolves promise chain with `nft_resolve_transfer`, and may
/// transfer token back to sender
///
/// Requirements:
/// * Contract MUST forbid calls to this function by any account except self
/// * If promise chain failed, contract MUST revert token transfer
/// * If promise chain resolves with `true`, contract MUST return token to
/// `sender_id`
///
/// Arguments:
/// * `sender_id`: the sender of `ft_transfer_call`
/// * `token_id`: the `token_id` argument given to `ft_transfer_call`
/// * `approved_token_ids`: if using Approval Management, contract MUST provide
/// set of original approved accounts in this argument, and restore these
/// approved accounts in case of revert.
///
/// Returns true if token was successfully transferred to `receiver_id`.
#[private]
fn nft_resolve_transfer(
&mut self,
owner_id: String,
token_id: String,
approvals: HashMap<AccountId, u64>,
);
}
#[near_bindgen]
// near-sdk bug:
// cannot find trait `NonFungibleResolveTransfer` in this scope
// impl NonFungibleResolveTransfer for MintbaseStore {
impl MintbaseStore {
#[private]
pub fn nft_resolve_transfer(
&mut self,
owner_id: String,
token_id: String,
approvals: HashMap<AccountId, u64>,
) {
assert_eq!(env::promise_results_count(), 1);
match env::promise_result(0) {
near_sdk::PromiseResult::Successful(_b) => {
//todo:
// how do I check that the promise chain resolves with "true"?
// let b: bool = _b.into(); // nope
if true {
let token_idu64 = token_id.parse::<u64>().unwrap();
let mut token = self.nft_token_internal(token_idu64);
self.transfer_internal(&mut token, owner_id);
approvals.into_iter().for_each(|(k, v)| {
token.approvals.insert(k, v);
});
}
}
near_sdk::PromiseResult::Failed => {
// revert transfer
let mut token = self.nft_token(token_id).expect("no token");
self.transfer_internal(&mut token, owner_id);
approvals.into_iter().for_each(|(k, v)| {
token.approvals.insert(k, v);
});
}
near_sdk::PromiseResult::NotReady => {}
};
}
} |
Beta Was this translation helpful? Give feedback.
-
You can use // Get whether token should be returned
let must_revert = match env::promise_result(0) {
PromiseResult::NotReady => unreachable!(),
PromiseResult::Successful(value) => {
if let Ok(yes_or_no) = near_sdk::serde_json::from_slice::<bool>(&value) {
yes_or_no
} else {
true
}
}
PromiseResult::Failed => true,
};
What bug are you seeing? Is it that the |
Beta Was this translation helpful? Give feedback.
-
|
Bug is that by using the rust-idiomatic |
Beta Was this translation helpful? Give feedback.
-
|
Sample rust trait declaration and implementation of the Typescript API. /// Impl of NEP-171. Note that the impl makes the assumption that `TokenId` has
/// type `String`, where this contract internally uses `u64`, which is more
/// efficient. ref:
/// https://github.com/near/NEPs/blob/master/specs/Standards/NonFungibleToken/Core.md
pub trait NonFungibleContractCore {
/// Simple transfer. Transfer a given `token_id` from current owner to
/// `receiver_id`.
///
/// Requirements
/// * Caller of the method must attach a deposit of 1 yoctoⓃ for security purposes
/// * Contract MUST panic if called by someone other than token owner or,
/// if using Approval Management, one of the approved accounts
/// * `approval_id` is for use with Approval Management extension, see
/// that document for full explanation.
/// * If using Approval Management, contract MUST nullify approved accounts on
/// successful transfer.
///
/// Arguments:
/// * `receiver_id`: the valid NEAR account receiving the token
/// * `token_id`: the token to transfer
/// * `approval_id`: expected approval ID. A number smaller than
/// 2^53, and therefore representable as JSON. See Approval Management
/// standard for full explanation.
/// * `memo` (optional): for use cases that may benefit from indexing or
/// providing information for a transfer
//#[payable]
fn nft_transfer(
&mut self,
receiver_id: AccountId,
token_id: String,
approval_id: Option<U64>,
memo: Option<String>,
);
/// Returns `true` if the token was transferred from the sender's account.
///
/// Transfer token and call a method on a receiver contract. A successful
/// workflow will end in a success execution outcome to the callback on the NFT
/// contract at the method `nft_resolve_transfer`.
///
/// You can think of this as being similar to attaching native NEAR tokens to a
/// function call. It allows you to attach any Non-Fungible Token in a call to a
/// receiver contract.
///
/// Requirements:
/// * Caller of the method must attach a deposit of 1 yoctoⓃ for security
/// purposes
/// * Contract MUST panic if called by someone other than token owner or,
/// if using Approval Management, one of the approved accounts
/// * The receiving contract must implement `ft_on_transfer` according to the
/// standard. If it does not, FT contract's `ft_resolve_transfer` MUST deal
/// with the resulting failed cross-contract call and roll back the transfer.
/// * Contract MUST implement the behavior described in `ft_resolve_transfer`
/// * `approval_id` is for use with Approval Management extension, see
/// that document for full explanation.
/// * If using Approval Management, contract MUST nullify approved accounts on
/// successful transfer.
///
/// Arguments:
/// * `receiver_id`: the valid NEAR account receiving the token.
/// * `token_id`: the token to send.
/// * `approval_id`: expected approval ID. A number smaller than
/// 2^53, and therefore representable as JSON. See Approval Management
/// standard for full explanation.
/// * `memo` (optional): for use cases that may benefit from indexing or
/// providing information for a transfer.
/// * `msg`: specifies information needed by the receiving contract in
/// order to properly handle the transfer. Can indicate both a function to
/// call and the parameters to pass to that function.
//#[payable]
fn nft_transfer_call(
&mut self,
receiver_id: AccountId,
token_id: String,
approval_id: Option<String>,
memo: Option<String>,
msg: String,
) -> Promise;
/// Returns the token with the given `token_id` or `None` if no such token.
fn nft_token(&self, token_id: String) -> Option<Token>;
}
#[near_bindgen]
impl NonFungibleContractCore for MintbaseStore {
fn nft_transfer(
&mut self,
receiver_id: AccountId,
token_id: String,
approval_id: Option<U64>,
#[allow(unused_variables)] memo: Option<String>,
) {
//assert_one_yocto(); // worsens user experience for security
let token_id = token_id.parse::<u64>().unwrap();
let mut token = self.nft_token_internal(token_id);
let pred = env::predecessor_account_id();
if pred != token.owner_id {
// check if pred has an approval
let approval_id: Option<u64> = approval_id.map(|i| i.into());
assert!(self.nft_is_approved_internal(&token, pred, approval_id));
}
self.transfer_internal(&mut token, receiver_id);
// log (optionally: use memo)
}
fn nft_transfer_call(
&mut self,
receiver_id: AccountId,
token_id: String,
approval_id: Option<String>,
#[allow(unused_variables)] memo: Option<String>,
msg: String,
) -> Promise {
// assert_one_yocto();
let token_idu64 = token_id.parse::<u64>().unwrap();
let mut token = self.nft_token_internal(token_idu64);
let pred = env::predecessor_account_id();
if pred != token.owner_id {
// check if pred has an approval
let approval_id: Option<u64> = approval_id.map(|i| i.parse::<u64>().unwrap());
assert!(self.nft_is_approved_internal(&token, pred.to_string(), approval_id));
}
self.transfer_internal(&mut token, receiver_id.to_string());
// log (optionally: use memo)
ext_on_transfer::nft_on_transfer(
pred,
token.owner_id.to_string(),
token_id.to_string(),
msg,
&receiver_id,
NO_DEPOSIT,
DEFAULT_GAS,
)
.then(ext_self::nft_resolve_transfer(
token.owner_id,
token_id,
token.approvals,
&env::current_account_id(),
NO_DEPOSIT,
DEFAULT_GAS,
))
}
fn nft_token(&self, token_id: String) -> Option<Token> {
self.tokens.get(
&token_id
.parse::<u64>()
.unwrap_or_else(|_| env::panic(b"no such token")),
)
}
} |
Beta Was this translation helpful? Give feedback.
-
This GitHub discussion is a continuation from several conversations that resulted in a short thread on the NEAR Governance Forum here:
https://gov.near.org/t/nft-standard-discussion/853/13
Handy reference links:
Other NEAR-related links:
As we step through the different facets of an NFT standard and extension standards, let's keep in mind two NFT scenarios:
These may help guide what we determine to be necessary in an NFT Core standard or not.
Core (with Storage Management)
Data structures / fields
Methods
The following have the same workflow as the explained in the Fungible Token Core spec, except with
nft_transfer_callthere is no value returned from the receiver contract. Instead, the contract shall panic if it's unable to transfer an NFT to an individual. An execution that doesn't panic indicates a successful transfer.Note on
enforce_owner_idin the methods below: if given, the method MUST panic if current owner does not match the providedenforce_owner_idvalue. This prevents an accidental transfer race condition.Metadata
Note that when this NFT contract is created and initialized, the associated
token_storage_usagewill be higher than the previous Core example because of the added metadata fields. The fieldsnameandiconin particular are indeed variable. The frontend can account for this by adding extra deposit when minting. This could be done by padding with a reasonable amount, or by the frontend using the RPC call detailed here that gets genesis configuration and actually determine precisely how much deposit is needed.Approvals
This is a point for continued work, following some healthy debate on the topic. A critical objective is to have a token able to add multiple approvals. In example form, a piece of art is an individual token on an NFT contract and this piece of art can allow Volcano Art and Petroglyph Art shops to list the piece for purchase.
One approach suggested was that the
Tokenobject has a flag, something likehas_approvals: bool. Then separately, another mapping exists linking owner » approvers. The combination of these two is used to determine if a market can take a particular token. This leaves some wiggle room for trust.Example of this problem: Clark Little is an ocean photographer that embraces selling his work as NFTs. Alice visits a market website selling his work and finds a print she'd like to buy. She waits for her partner to come home before purchasing, leaving the tab open. Simultaneously, Surfrider, a beach cleanup organization asks if Clark would donate a couple prints for a raffle. Clark agrees, delists two prints from all market websites, and adds the
surfrideraccount as an approver.surfriderwill take care of all the details regarding the raffle, transferring it to the winner, etc. Alice finalizes her decision and clicks the (stale) website UI to purchase a print that was delisted, since the market is among the approvers and the two prints have the flag ofhas_approvalsthe transfer goes through.Conclusion: in order to avoid introducing trust and inconsistency, approvals need to be tied per token. This would look like:
Enumeration
Similar to how non-fungible tokens in Ethereum work, enumeration is conceptually an "extension" standard. See ERC-721 spec for reference. (Search for "metadata extension")
Universal token identifiers
Royalties
Given the use case of a property deed NFT where royalties wouldn't apply so much, this makes sense as its own standard. This section needs to be fleshed out more, but below is provided some code from a couple resources to help kickstart the standard discussion.
Taking some ideas from Mintbase and the Staking Pool contract regarding the safe representation of a fraction:
And the the
Fractionstruct can be used as a royalty field on a token, like:Beta Was this translation helpful? Give feedback.
All reactions