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

Non-Fungible Token State Verification (for exchanges) #1180

Closed
mg6maciej opened this issue Jun 24, 2018 · 9 comments
Closed

Non-Fungible Token State Verification (for exchanges) #1180

mg6maciej opened this issue Jun 24, 2018 · 9 comments
Labels

Comments

@mg6maciej
Copy link
Contributor

mg6maciej commented Jun 24, 2018

Motivation

This is to start a discussion on secure exchange of NFTs, as there are potential problems with front running, where seller can change state of token (and lower its value) before buy transaction is added to blockchain.

Details & Implementation

While what we really own is a unique number known as tokenId in a namespace of certain contract, often it's something else that makes us want to own that number. Many tokens have state we look at when buying them, be it a number of won/lost battles by a bot, how many children a token currently has or if a kitty owns some awesome hats or balloons.

Such state can easily change between order fill transaction is sent to miners and mined, so for the benefit of users buying these numbers, it would be good to allow them to specify state they care about and fail order fill tx if their desired state is changed. This is particularly useful for protocols that do not escrow tokens like 0x or wyvern.

As this state can often live outside of the original NFT contract, I propose the following interface:

interface StateHashHolder {

    function getStateHash(address nftContractAddr, uint tokenId) external view returns (bytes32);
}

The composed hash that is checked against would be a XOR of all hashes:

contract StateHashVerifier {

    function calculateStateHash(address nftContractAddr, uint tokenId, StateHashHolder[] holder) public view returns (bytes32 hash) {
        for (uint i = 0; i < holder.length; i++) {
            hash ^= holder[i].getStateHash(nftContractAddr, tokenId);
        }
    }
}

This is compatible with currently deployed NFTs, as they could implement StateHashHolder via separate contract.

Here is how it could be implemented for CryptoKitties "virginity":

interface CryptoKitties {
    
    function getKitty(uint256 _id) external view returns (
        bool isGestating,
        bool isReady,
        uint256 cooldownIndex,
        uint256 nextActionAt,
        uint256 siringWithId,
        uint256 birthTime,
        uint256 matronId,
        uint256 sireId,
        uint256 generation,
        uint256 genes
    );
}

contract CryptoKittiesStateHash is StateHashHolder {
    
    function getStateHash(address nftContractAddr, uint tokenId) external view returns (bytes32) {
        if (nftContractAddr != 0x06012c8cf97BEaD5deAe237070F9587f8E7A266d) {
            return 0;
        }
        CryptoKitties cryptoKitties = CryptoKitties(nftContractAddr);
        (,,, uint nextActionAt,,,,,,) = cryptoKitties.getKitty(tokenId);
        // nextActionAt (aka cooldownEndBlock) changes every time you breed a pair of kitties
        return keccak256(abi.encodePacked(nextActionAt));
    }
}

Do you see any other use-cases apart from when exchanging NFTs?

@OrdonTeam
Copy link

Why do we need array of StateHashHolder? Wouldn't one be enough? Extending hash functionality can be achieved by creating new StateHashHolder which uses previous one.

@mg6maciej
Copy link
Contributor Author

@OrdonTeam There are many unrelated contracts that might keep state of a single token.

Let's take CryptoKitties as an example: as a user when buying a kitty, you may be interested in any combination of:

  1. how many children the one you decided to buy has,
  2. if it has a hat on kittyhats.co,
  3. never took part in a race (or never lost one) on kittyrace.com

Each of these states is kept in separate contracts and hashes of their state will likely be calculated inside such contracts in future (or can be done by deploying separate contract as shown above), so there is a need for exchange platforms to uniformly verify for all contracts users care about if state matches.
This could be done if user provides a list of contracts implementing StateHashHolder and list of expected hashes from each contract, but that would use more gas than if user provided list of StateHashHolder and a single hash, hence a need for StateHashVerifier, which could be deployed once using method described in #820 or every exchange platform could copy the simple loop code that XORs hashes inside their own contracts.

@OrdonTeam
Copy link

OrdonTeam commented Jun 26, 2018

👍 I didn't get that as a buyer I'm choosing in which StateHashHolder I'm interested in. Now it makes
perfect sense.

@shrugs
Copy link

shrugs commented Jun 26, 2018

Neat! Defining state holders & the oracles around them will be an interesting problem, but I like this approach for condensing that information into an on-chain-checkable value.

@mg6maciej
Copy link
Contributor Author

mg6maciej commented Jun 27, 2018

Wyvern already has something similar built-in (search for STATICCALL), also for seller side:

https://github.com/ProjectWyvern/wyvern-protocol/blob/master/build/whitepaper.pdf

https://github.com/ProjectWyvern/wyvern-ethereum/blob/master/contracts/exchange/ExchangeCore.sol

Would like to see examples using this.

@mattlockyer
Copy link

@mg6maciej love this idea. I just had a conversation with @mudgen and this topic came up!

We definitely need exchanges, wallet and dapps to be able to treat common token types as equals, provided their on-chain representations meet a certain level of criteria.

Thanks for posting and starting the conversation! 😃

@mudgen
Copy link
Contributor

mudgen commented Aug 16, 2018

I think this standard is important functionality and I like what @mg6maciej has suggested here.

I implemented the getStateHash(uint256 _tokenId) function directly in the Mokens contract as a way to verify that no child NFTs of a moken have been removed or added when transferring a moken.

@github-actions
Copy link

github-actions bot commented Dec 5, 2021

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Dec 5, 2021
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants