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

Nonfungible Token Standard #4

Closed
wants to merge 27 commits into from
Closed

Nonfungible Token Standard #4

wants to merge 27 commits into from

Conversation

behaviary
Copy link

@behaviary behaviary commented Jun 28, 2019

EDIT: Updated 18-May-2020 with simpler specification

Nonfungible Token

Summary

A standard interface for non-fungible tokens allowing for ownership and transfer.

Motivation

Non-fungible tokens (NFTs) have been described in many ways: digital goods, collectible items, unique online assets etc. The current core use case for NFTs is speculating on value and trading unique items. The use case of trading NFTs should be natively supported. This is the most basic set of functions needed to create an interoperable NFT that works in an asynchronous environment.

Prior art:

Guide-level explanation

This token should allow the following:

  • Initialize contract once. The given total supply will be owned by the given account ID.
  • Get the total supply of created tokens per contract.
  • Transfer a token to a new owner.
  • Grant access to all token to a third party.
    • A third party account ID will be able to transfer the tokens that they have access to
  • Get current balance for a given account ID.
  • Transfer tokens from one user to another.

There are a few concepts in the scenarios above:

  • Total supply. The total number of tokens in circulation.
  • Token owner. An account ID that owns one or more tokens.
  • Transfer. Action that moves some amount from one account to another account.
  • Escrow. A different account from the balance owner who has permission to one or more tokens.
  • Access. The specific token ID that another account has access to.

Simple transfer

Assumptions

  • the Corgi nft contract is corgi
  • Alice's account is alice
  • Jeraldo's account is jerry
  • The NFT contract has been initialized with a nonzero token supply
  • There exists a token with the ID of 3

High-level

Alice needs to issue one transaction to the Corgi NFT contract to transfer one corgi token to Jeraldo.

Technical calls

  1. alice calls corgi::transfer({"new_owner_id":"jerry", "token_id":3})

Token swap through a third party escrow

Alice wants to transfer one Corgi NFT through a third party escrow to Jeraldo in exchange for one Sausage NFT.

Assumptions

  • the Corgi nft contract is corgi
  • the Sausage nft contract is sausage
  • Alice's account is alice
  • Jeraldo's account is jerry
  • The Escrow contract is escrow
  • The NFT contract has been initialized with a nonzero token supply
  • There exists a Corgi token with the ID of 3 and a Sausage token with the ID of 5
  • The Escrow contract manages how the transfer is facilitated and guarantees requirements are met for transfer

High-level

Both Alice and Jerry will issue asynchronous transactions to their respective contracts, corgi and sausage to grant access to the escrow to trade tokens on their behalf. escrow will call the sasuage token contract asynchrounously to transfer the Sausage token to escrow. After, escrow will also call the corgi contract to asynchornously transfer the Corgi token to escrow. Then, escrow will conduct a transfer to both parties.

  • If both of the transfer_from calls succeed, then Alice will now own one Sausage token and Jerry will own one Corgi token.
  • If one or both of the final transfer_from calls fail, then nothing will happen and escrow should attempt reissuing the failed transaction.

Technical calls

  1. alice makes an async call to corgi::grant_access({"escrow_account_id":"escrow"})
  2. jerry makes an async call to ``sausage::grant_access({"escrow_account_id":"escrow"})`
  3. escrow calls sausage::transfer_from({"owner_id":"jerry", "new_owner_id:"escrow", "token_id": 5})
    • attaches callback escrow::on_transfer({"owner_id":"jerry", "token_contract":"sausage", "token_id": 5})
  4. escrow calls corgi::transfer_from({"owner_id":"alice", "new_owner_id:"escrow", "token_id": 3})
    • attaches callback escrow::on_transfer({"owner_id":"alice", "token_contract":"corgi", "token_id": 3})
  5. In one Promise:
    1. escrow calls corgi::transfer_from({"owner_id":"escrow", "new_owner_id:"jerry", "token_id": 3})
      • attaches callback escrow::on_transfer({"owner_id":"alice", "token_contract:"corgi", "token_id": 3})
    2. escrow calls sausage::transfer_from({"owner_id":"jerry", "new_owner_id:"escrow", "token_id": 5})
      • attaches callback escrow::on_transfer({"owner_id":"jerry", "token_contract":"corgi", "token_id": 3})

Reference-level explanation

Template for smart contract in AssemblyScript

At time of writing, this standard is established with several constraints found in AssemblyScript. The first is that interfaces are not an implemented feature of AssemblyScript, and the second is that classes are not exported in the conversion from AssemblyScript to WASM. This means that the entire contract could be implemented as a class, which might be better for code organization, but it would be deceiving in function.

  type TokenId = u64;
  /******************/
  /* CHANGE METHODS */
  /******************/
  
  // Grant the access to the given `accountId` for the given `tokenId`.
  // Requirements:
  // * The caller of the function (`predecessor_id`) should have access to the token.
  export function grant_access(escrow_account_id: string): void;
  // Revoke the access to the given `accountId` for the given `tokenId`.
  // Requirements:
  // * The caller of the function (`predecessor_id`) should have access to the token.
  export function revoke_access(escrow_account_id: string): void;
  // Transfer the given `tokenId` from the given `accountId`.  Account `newAccountId` becomes the new owner.
  // Requirements:
  // * The caller of the function (`predecessor_id`) should have access to the token.
  export function transfer_from(, owner_id: string, new_owner_id: string, tokenId: TokenId): void;
  // Transfer the given `tokenId` to the given `accountId`.  Account `accountId` becomes the new owner.
  // Requirements:
  // * The caller of the function (`predecessor_id`) should have access to the token.
  export function transfer(new_owner_id: string, token_id: TokenId): void;
  /****************/
  /* VIEW METHODS */
  /****************/
// Returns `true` or `false` based on caller of the function (`predecessor_id) having access to the token
  export function check_access(token_id: TokenId): boolean;

  // Get an individual owner by given `tokenId`.
  export function get_token_owner(token_id: TokenId): string;

Drawbacks

The major design choice to not use a system of approvals for escrow in favor of performance means that it is up to implementors of markets to decide how they manage escrow themselves. This is a dilemma because it increases freedom, while increasing risk of making a mistake on the market side. Ultimately, it will be up to markets and their users to find the best solution to escrow, and we don't believe that approvals is the way to do it. This allows for that solution to be discovered with trail and error. The standard for the market will change, but not the token itself.
This token standard has been whittled down to the simplest fundamental use cases. It relies on extensions and design decisions to be useable.
There are some things that have been in contention in the design of this standard. Namely, the tokenId system relies on unique indices to function. This might cause a problem with use cases that need the lock and unlock functionality.
In addition, the grant_access and revoke_access functions act similarly to approvals, but must operate asynchronously and in batch transactions where appropriate.

Rationale and alternatives

A multi-token standard was considered, as well a standard that allowed for the transfer of any type of token along with the assets associated with this contract. This was foregone for the sake of decoupling the market contracts from the token contracts. The emphasis of this standard is now on simplicity and flexibility. It allows for any type of token to interface with any type of market that accepts this standard. The explicit goal is to maximize developer freedom with a rigid enough foundation to make a standard useful.

Unresolved questions

Primarily edge cases for various applications should be surfaced. For example, the use case of creating an in-game store is different than creating a token for tracking real-world objects digitally. This token attempts to create a standard for both.
Neither a market standard nor an escrow system is addressed here. These should exists in the future, but are purposefully left separate. An item should not care about the place it is sold or agreed on.
The ability to lock and unlock tokens is a likely requirement for many use cases, but there are many challenges around this. The initial solution to solely rely on callbacks was abandoned in favor of an access system that allows escrow contracts to lock and transfer tokens.
Finally, in the original draft, metadata was included in the model for tokens. It was clear through some basic implementations that this is not ideal since users may want to store metadata elsewhere. This could be entirely offchain, or in a separate contract. This creates an unsolved problem of synchronizing metadata with contracts, and needs more design work.

Future possibilities

The next step in the development of this standard is extending it further in a new standard that addresses spcifically how a generic and safe escrow would function, and how metadata should be handled based on the specific use cases of tokens implemented. In addition, an importable module should be developed, allowing developers to integrate a token system with little overhead. Alternative uses of this token are of high interest. Known uses for nonfungible tokens include collectible items online, and item systems in games as discussed throughout. There are many uses cases yet to be invented. These might include tokens for supply chain or even tokens for shared custody of physical items. The possibilities are ultimately going to be driven by community use.

0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
@behaviary behaviary self-assigned this Jul 23, 2019
@behaviary behaviary marked this pull request as ready for review July 23, 2019 03:08
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

I think in general an example implementation of some NFT would be really helpful for people to understand how each methods should be used in practice.

0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
0004-market-integration-token-standard.md Outdated Show resolved Hide resolved
@evgenykuzyakov
Copy link
Contributor

Here is a comment on approvals from discord chat (to keep a record):

In async setting like NEAR other contracts can't rely on approvals being valid. Cause there are always can be a transaction in between the initial approval and removing the approval. In NEP#4 we use transfer with callback to ensure the transfer can either succeed or reverted in secure way.
One reason to have approvals is to share access to a single token by multiple contracts at the same time, an example for this can be listing an NFT on multiple exchanges. Once the sale is in the process the async transaction from Ex will fly to the NEP#4 contract to call transferFrom with callback. If the approval is still valid, the sale will be completed, if not, the sale will be rejected.
Multiple approvals create O(N) operation when moving a token, to remove all approvals at unlock. It's doable, but we need to build an abstract class for this to make it simple to use by developers.

@evgenykuzyakov
Copy link
Contributor

After more offline discussions and talking to a few folks @potatodepaulo and I decided to update the spec to look similar to Eth spec with a few key differences. We propose the following API:

// Grant the access to the given `account_id` for the given `tokenId` .
// Requirements:
// * The caller of the function (`predecessor_id`) should have access to the token.
// * The token should not be locked.
function grantAccess(tokenId: TokenId, account_id: string): void;

// Revokes the access to the given `account_id` for the given `tokenId` .
// Requirements:
// * The caller of the function (`predecessor_id`) should have access to the token.
// * The token should not be locked.
function revokeAccess(tokenId: TokenId, account_id: string): void;

// Locks the given `tokenId` to the caller of the function (`predecessor_id`).
// The `predecessor_id` becomes the owner of the lock.
// Requirements:
// * The caller of the function (`predecessor_id`) should have access to the token.
// * The token should not be locked.
function lock(tokenId: TokenId): void;

// Unlocks the given `tokenId`. Removes lock owner.
// Requirements:
// * The caller of the function (`predecessor_id`) should be the owner of the lock.
// * The token should be locked.
function unlock(tokenId: TokenId): void;

// Transfers the given `tokenId` to the given `account_id`.  Account `account_id` becomes the new owner.
// The token unlocks (if it was locked) and all access is revoked except for
// the new owner.
// Requirements:
// * The caller of the function (`predecessor_id`) should have access to the token.
// * If the token is locked, the locked owner should be `predecessor_id`.
function transfer(tokenId: TokenId, account_id: string): void;

NOTES:

  • In async setting the predecessor_id is the account ID of the immediate previous caller. If a user initiates a new transaction, then the user's account ID is the predecessor. If a contract_a creates an async call towards the token contract, then the contract_a is the predecessor.
  • Each token contains the following properties:
    • owner_id - account ID of the owner
    • lock_owner_id - optional field. An account ID of the owner of the lock. Or the field is not set, if the token is unlocked.
    • access - a set of account IDs who currently hold access to the token.
  • Token owner always has access to the token. But the owner can't override locks.

Examples on how it work:

  • When DEX tries to exchange tokens between 2 NFT contracts, it calls 2 async functions in parallel to lock exchanged tokens.
    The DEX contract receives 2 callbacks and makes a decision:
    • If both locks succeeded, DEX calls transfer to the new corresponding owners. The trade considered successful.
    • If one of the locks succeeds, but another fails, the DEX issues unlock for the successful lock. And the trade considered failed.
    • The benefit of such design is the token can be simultaneously listed on multiple exchanges.
      When a trade fails by the other side, the token doesn't lose access and doesn't get delisted from other exchanges.
  • Simple transfer. The owner can just call transfer to any other user.
  • Temporary lock. The owner can call lock to temporary put a hold on the token which prevents other accounts with access from acting.

@k06a
Copy link
Contributor

k06a commented Sep 7, 2019

I think the best way for swap NFT is to use escrow smart contract. When escrow will own NFT no one will be able to make permitted changes to it. But if you still wish to use your NFT while it is in the order we can do it in 2 steps. Buyer can call a method on escrow contract and escrow will transferFrom NFT from the seller. Then if escrowed NFT looks good buyer can swap it according to initial sellers order.

Currently, 0x project uses a very strange approach: 0xProject/ZEIPs#39. For me it seems it is not enough, because soon we will see NFTs owning NFTs and much more complex things. We need a better approach to make it work automatic for all NFTs. And since ownership is one of the strongest relationships between smart contracts I'd suggest using escrow with 1 or 2-step orders.

Example escrow implementation pseudocode:

mapping(OrderHash => address) buyers;

function preBuy(order: Order) {
    require(buyers[order.hash] == 0);
    buyers[order.hash] = msg.sender;

    order.asset.transferFrom(msg.sender, this, order.assetAmount);
    order.nft.transferFrom(order.seller, this, order.nft_id);
}

function buy(order: Order) {
    require(msg.sender == buyers[order.hash]);

    order.checkSignatureIsStillValidAndInvalidateIt();
    order.nft.transfer(msg.sender, order.nft_id);
    order.asset.transfer(order.seller, order.assetAmount);
}

function cancel(order: Order) {
    require(msg.sender == buyers[order.hash]);
    buyers[order.hash] = 0;

    order.nft.transfer(order.seller, order.nft_id);
    order.asset.transfer(msg.sender, order.assetAmount);
}

@evgenykuzyakov
Copy link
Contributor

@k06a The following doesn't work in async design unless you own both tokens.

order.asset.transferFrom(msg.sender, this, order.assetAmount);
order.nft.transferFrom(order.seller, this, order.nft_id);

@k06a
Copy link
Contributor

k06a commented Sep 9, 2019

@evgenykuzyakov what do you mean? Approve should be enough to do transferFrom. I expected that async environment not adding any breaking restrictions not to break existing concepts.

@evgenykuzyakov
Copy link
Contributor

approve can be invalidated before transferFrom arrives. This can have one of transferFrom succeeds and another fails. I'm writing a DEX implementation example to demonstrate the concept. I'm not convinced we have to separate the escrow and NFT contracts, because exchanges are either have to be able to handle both direct listing by NFT contracts and through escrow contracts, or only escrow contracts. For the 2nd, people are forced to use escrow for selling which doesn't simplify the wallet designs. For the 1st case, DEX design becomes more complicated, but still require handle coins on escrow and in the wallet.

@k06a
Copy link
Contributor

k06a commented Sep 9, 2019

@evgenykuzyakov we should just revert in case of any transferFrom failed. Will this cancel already transferred one?

@evgenykuzyakov
Copy link
Contributor

evgenykuzyakov commented Sep 9, 2019

we should just revert in case of any transferFrom failed. Will this cancel already transferred one?

No, since it's async. We can only revert the local state in case of failure. Any async operation that was successfully issued will not be canceled. So when you call 2 transferFrom, they are not executed immediately, but just scheduled to execute on corresponding contracts.

Here is the DEX example:

// Arbitrary number, but has to be enough for the worst case.
const GAS_FOR_A_CALL = 1000000;
const GAS_TO_PROCESS_CALLBACKS = 3 * GAS_FOR_A_CALL;
const RESULT_STATUS_SUCCESS = 1;

export function buy(orderId: OrderId) {
  let order = orders.get(tokenId);
  // NFT for sale
  let tokenId = order.tokenId;
  let tokenContract = order.tokenContract;
  // Buying price (assuming some fungible tokens)
  let price = order.price;
  let assetContract = order.priceContract;

  // Should lock the order and save it.
  order.lock();

  // Who's trying to buy (the immediate predecessor account id who called this method, it's can be
  // different from the signer of the transaction).
  let orderBuyer = context.predecessor;

  // Locking NFT token
  let sellSideLockArgs: TokenContractLockArgs = {
    tokenId,
  };
  let promiseSellSideLock = ContractPromise.create(tokenContract, "lock", sellSideLockArgs.encode().serialize(), GAS_FOR_A_CALL);

  // Locking amount token (we don't have a standard for this, so improvising)
  let buySideLockArgs: AssetContractLockArgs = {
    owner: orderBuyer,
    amount: price,
  };
  let promiseBuySideLockArgs = ContractPromise.create(assetContract, "lock", buySideLockArgs.encode().serialize(), GAS_FOR_A_CALL);

  // Joining promises to attach a callback for both of them.
  let promiseJoin = ContractPromise.all([promiseSellSideLock, promiseBuySideLockArgs]);

  // Attaching a callback to process the results.
  let processCallbackArgs: ProcessCallbackArgs = {
    orderId,
    orderBuyer,
  }
  let promiseProcessCallbacks = promiseJoin.then(context.contractName, "processCallbacks", processCallbackArgs.encode().serialize(), GAS_TO_PROCESS_CALLBACKS);

  promiseProcessCallbacks.returnAsResult();
}

export function processCallbacks(orderId: OrderId, orderBuyer: string) {
  // Checking that this function was called by us.
  assert(context.predecessor == context.contractName);

  let order = orders.get(tokenId);

  // NFT for sale
  let tokenId = order.tokenId;
  let tokenContract = order.tokenContract;
  let tokenOwner = order.tokenOwner;
  // Buying price (assuming some fungible tokens)
  let price = order.price;
  let assetContract = order.priceContract;

  // Collecting results
  let results = ContractPromise.getResults();
  if (results[0].status == RESULT_STATUS_SUCCESS && results[1].status == RESULT_STATUS_SUCCESS) {
    // Success, exchanging assets to the new owners and closing the order.

    // Transferring NFT token
    let sellSideTransferArgs: TokenContractTransferArgs = {
      tokenId,
      account_id: orderBuyer,
    };
    // We don't care about result of the execution, because it should succeed after lock.
    ContractPromise.create(tokenContract, "transfer", sellSideLockArgs.encode().serialize(), GAS_FOR_A_CALL);

    // Transferring the ERC20 like asset
    let buySideTransferArgs: AssetContractTransferArgs = {
      owner: orderBuyer,
      amount: price,
      new_owner: tokenOwner,
    };
    ContractPromise.create(assetContract, "transfer", buySideTransferArgs.encode().serialize(), GAS_FOR_A_CALL);

    // Closing the order. The sale was done.
    order.close();
  } else {
    // One or both of the locks failed. Unlocking successful locks and the order
    if (results[0].status == RESULT_STATUS_SUCCESS) {
      // Token lock succeeded, unlocking
      let sellSideUnlockArgs: TokenContractUnlockArgs = {
        tokenId,
      };
      ContractPromise.create(tokenContract, "unlock", sellSideUnlockArgs.encode().serialize(), GAS_FOR_A_CALL);
    }

    if (results[1].status == RESULT_STATUS_SUCCESS) {
      // Asset lock succeeded, unlocking
      let buySideUnlockArgs: AssetContractUnlockArgs = {
        owner: orderBuyer,
        amount: price,
      };
      ContractPromise.create(assetContract, "unlock", buySideUnlockArgs.encode().serialize(), GAS_FOR_A_CALL);
    }

    // Unlocking the order
    order.unlock();
  }
}

@k06a
Copy link
Contributor

k06a commented Sep 9, 2019

@evgenykuzyakov but I think we can easily send fetched token back in case of second failure:

const promise1 = order.asset.transferFrom(msg.sender, this, order.assetAmount);
const promise2 = order.nft.transferFrom(order.seller, this, order.nft_id);
await Promise.all([promise1, promise2]);

if (!promise1 && !promise2) {
    return;
}

if (promise1 && !promise2) {
    order.asset.transfer(msg.sender, order.assetAmount);
    return;
}

if (promise2 && !promise1) {
    order.nft.transfer(order.seller, order.nft_id);
    return;
}

@k06a
Copy link
Contributor

k06a commented Sep 9, 2019

Or better provide something like this shouldSuccessAll:

function shouldSuccessAll(calls: callback[], cancels: callback[]) {
    const results = await calls.map(call => call.execute());
    if (!results.all()) {
        await results.map((v,i) => {
            if (!v) {
                cancels[i].execute();
            }
        });
        return false;
    }

    return true;
}

await shouldSuccessAll(
    calls: [
        () => { order.asset.transferFrom(msg.sender, this, order.assetAmount); },
        () => { order.nft.transferFrom(order.seller, this, order.nft_id); },
    ],
    cancels: [
        () => { order.asset.transfer(msg.sender, order.assetAmount); },
        () => { order.nft.transfer(order.seller, order.nft_id); },
    ]
);

@evgenykuzyakov
Copy link
Contributor

And that's why you need locks to not lose approvals in case you revert the transfer. Otherwise, if the token1 fails to transfer, the token2 reverted and all approvals are removed, which delists token2 from any future transfers until the owner approves it again.

@k06a
Copy link
Contributor

k06a commented Sep 9, 2019

Ok, got it. Let me think of it.

@k06a
Copy link
Contributor

k06a commented Sep 15, 2019

@k06a The following doesn't work in async design unless you own both tokens.

order.asset.transferFrom(msg.sender, this, order.assetAmount);
order.nft.transferFrom(order.seller, this, order.nft_id);

@evgenykuzyakov it seems maker allowance will not be wasted in case we first try to grab funds from taker. Which have single tx with multiple actions: approve and exchange calls.

require(order.asset.transferFrom(msg.sender, this, order.assetAmount));
require(order.nft.transferFrom(order.seller, this, order.nft_id));

Copy link
Contributor

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

Few followups in code related to general discussion.

@robert-zaremba
Copy link
Contributor

If I would like to propose more changes in the code - is there any way how I can edit it and make a commit without doing a separate PR?
The change I would like to propose:

  • clarifying the discussion above
  • better explanation and motivation of the interface
  • updated role names and parameter names.

@robert-zaremba
Copy link
Contributor

There is one more thing I would strongly recommend: adding a ref (bytes / string) parameter to both transfer and transfer_from functions.

This would tremendously help with all compliance integrations!

@evgenykuzyakov
Copy link
Contributor

@robert-zaremba Can you clarify on ref?

As noted in the PR discussion, how let's remove owner_id - in case of NFT it can be simple determined inside the transfer_from function.

We can't remove owner_id from transfer_from, because of the async execution. If the 2 owners authorized access to the same marketplace, the marketplace will be able to transfer tokens from both owners. So any of these 2 owners can call a method on the marketplace that will call the transfer_from assuming the ownership.
The token contract can't verify the who's initiated the transfer because we use immediate predecessor instead of the use signer of the transaction as a proof of ownership.

@evgenykuzyakov
Copy link
Contributor

If I would like to propose more changes in the code - is there any way how I can edit it and make a commit without doing a separate PR?

You can use suggestion feature in the github reviews. https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/reviewing-proposed-changes-in-a-pull-request

@robert-zaremba
Copy link
Contributor

Can you clarify on ref?

ref is a reference string. The goal is to be able to link an on-chain transaction with off-chain data. Data can be in a ref string, an URI, or a simple ID. This helps (a lot!) carrying out extra data when needed and bind it in a trustless way.

@robert-zaremba
Copy link
Contributor

@robert-zaremba Can you clarify on ref?

As noted in the PR discussion, how let's remove owner_id - in case of NFT it can be simple determined inside the transfer_from function.

We can't remove owner_id from transfer_from, because of the async execution. If the 2 owners authorized access to the same marketplace, the marketplace will be able to transfer tokens from both owners. So any of these 2 owners can call a method on the marketplace that will call the transfer_from assuming the ownership.
The token contract can't verify the who's initiated the transfer because we use immediate predecessor instead of the use signer of the transaction as a proof of ownership.

Ah, right. We might transfer a token assuming wrong ownership. But, since this is NFT - each token has only one owner, and to transfer it we always have to have allowance of the current owner. So, security wise, we will not be able to transfer tokens we don't have authorization for if the owner query is happening inside the transfer_from (so both the transfer and ownership check is happening in the same context) - since calling functions within the same contract won't create an async call.

@evgenykuzyakov
Copy link
Contributor

evgenykuzyakov commented Jun 2, 2020

Ah, right. We might transfer a token assuming wrong ownership. But, since this is NFT - each token has only one owner, and to transfer it we always have to have allowance of the current owner. So, security wise, we will not be able to transfer tokens we don't have authorization for if the owner query is happening inside the transfer_from (so both the transfer and ownership check is happening in the same context) - since calling functions within the same contract won't create an async call.

Yes it's true, but for this transfer_from has to know who's the expected owner. Since it's called not by the owner, but from the contract (let's call it marketplace) then the setup will be the following:

  • current_account_id - token - the account ID of the contract
  • predecessor_account_id - marketplace - the account ID of the contract that called token contract (through async call)
  • signer_account_id - owner - the account ID of the user that initiated the transaction.

Now from the token perspective the token can be owned either by owner or by other_owner. Depends on the access, both of them might authorized the marketplace contract to act their behalf because they both use marketplace.

The token contract as for NEAR security model always relies on the predecessor_account_id instead of signer_account_id, because the signer might call one contract which can indirectly call token and steal tokens. To avoid it, the for permissions we always check the immediate predecessor instead of the original signer.

This means the token contract in transfer_from can't check who initiated the transaction for transfer, because it shouldn't rely on the signer_account_id. It guarantees that when you authorize some application on NEAR through restricted access keys, it can't steal your tokens, because the predecessor_account_id will be different.

Co-authored-by: Chad Ostrowski <221614+chadoh@users.noreply.github.com>
@render
Copy link

render bot commented Jun 4, 2020

Your Render PR Server at https://nomicon-pr-4.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-bqvi1j08atnabvm8j0ag.

Co-authored-by: Chad Ostrowski <221614+chadoh@users.noreply.github.com>
@render
Copy link

render bot commented Jun 4, 2020

Your Render PR Server at https://nomicon-pr-4.onrender.com is now live!

View it on your dashboard at https://dashboard.render.com/static/srv-bqvi1j08atnabvm8j0ag.

@robert-zaremba
Copy link
Contributor

Finally some serious work is starting on the enterprise side with defining token standards:
https://cointelegraph.com/news/microsoft-nasdaq-and-others-to-set-global-tokenization-standards

@zahhar
Copy link
Contributor

zahhar commented Feb 21, 2021

Hi all,

Are there any news or updates on potential standard composition? e.g.

  1. mandatory features to be implemented by smartcontract?
  2. how strictly method & their attributes must be named when implementing a feature?
  3. what is the minimum set of metadata (public-assessible attributes of the NFT, if any) that must be stored in the blockchain?

My view on metadata:

  • Token_ID
  • Owner_Account_ID
  • Last_Owner_Account_ID
  • Created_At
  • Last_Changed_At
  • Valid_From
  • Valid_To
  • User_Data (string of serialized into json or similar format custom data that contract designer decided to store on-chain for whatever reason)

Am I missing something?

@chadoh
Copy link
Contributor

chadoh commented Mar 15, 2021

Hey @zahhar, see discussion on improved NFT standard at #171

@chadoh
Copy link
Contributor

chadoh commented Mar 16, 2021

@potatodepaulo @mikedotexe I think it makes sense to close this for now. It was never fully finalized, and we now have a replacement NEP that likewise has not yet been finalized. We may as well signal to people that they should refer to the new discussion instead of this one.

New discussion at #171

Feel free to re-open this one if you think we should keep it around for a bit longer.

@chadoh chadoh closed this Mar 16, 2021
smartgeek1003 added a commit to smartgeek1003/NFT that referenced this pull request May 25, 2022
A recommendation for an improvement to the NEP-4 spec at
near/NEPs#4

The change:

    -// * The caller of the function (`predecessor`) should have access to the token.
    +// * The caller of the function (`predecessor`) should own the token.
     export function transfer(new_owner_id: string, token_id: TokenId): void

BREAKING CHANGE: this intentionally deviates from the current spec
smartgeek1003 added a commit to smartgeek1003/NFT that referenced this pull request May 25, 2022
Rather than store an array of accounts with escrow access, store only
one account. While a less practical implementation, it does conform to
the spec and simplifies the code.

In addition, remove non-sensical token_id existence requirements for
`grant_access` and `remove_access` given by current version of
near/NEPs#4

BREAKING CHANGE: this intentionally deviates from the current spec
Co-Authored-By: @amgando
@frol frol deleted the nep-4 branch December 20, 2022 18:47
TenzingSh added a commit to TenzingSh/nftstest that referenced this pull request Jun 26, 2023
A recommendation for an improvement to the NEP-4 spec at
near/NEPs#4

The change:

    -// * The caller of the function (`predecessor`) should have access to the token.
    +// * The caller of the function (`predecessor`) should own the token.
     export function transfer(new_owner_id: string, token_id: TokenId): void

BREAKING CHANGE: this intentionally deviates from the current spec
TenzingSh added a commit to TenzingSh/nftstest that referenced this pull request Jun 26, 2023
Rather than store an array of accounts with escrow access, store only
one account. While a less practical implementation, it does conform to
the spec and simplifies the code.

In addition, remove non-sensical token_id existence requirements for
`grant_access` and `remove_access` given by current version of
near/NEPs#4

BREAKING CHANGE: this intentionally deviates from the current spec
Co-Authored-By: @amgando
jewel528 pushed a commit to jewel528/NFT that referenced this pull request Feb 15, 2024
A recommendation for an improvement to the NEP-4 spec at
near/NEPs#4

The change:

    -// * The caller of the function (`predecessor`) should have access to the token.
    +// * The caller of the function (`predecessor`) should own the token.
     export function transfer(new_owner_id: string, token_id: TokenId): void

BREAKING CHANGE: this intentionally deviates from the current spec
jewel528 pushed a commit to jewel528/NFT that referenced this pull request Feb 15, 2024
Rather than store an array of accounts with escrow access, store only
one account. While a less practical implementation, it does conform to
the spec and simplifies the code.

In addition, remove non-sensical token_id existence requirements for
`grant_access` and `remove_access` given by current version of
near/NEPs#4

BREAKING CHANGE: this intentionally deviates from the current spec
Co-Authored-By: @amgando
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.

10 participants