Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

Expectations for asset_data call on ERC721 assets #45

Open
pointtoken opened this issue Dec 28, 2018 · 5 comments
Open

Expectations for asset_data call on ERC721 assets #45

pointtoken opened this issue Dec 28, 2018 · 5 comments

Comments

@pointtoken
Copy link

What are the expectations for a relayer broadcasting ERC721 orders in the asset_data call https://github.com/0xProject/standard-relayer-api/blob/master/http/v2.md#get-v2asset_pairs?

Because the NFT id is encoded in the asset data, it behaves a little differently than ERC20 assets. Should all active orders be listed, or should the assetData item be encoded with just the smart contract of the NFT, not the actual NFTs? This makes more sense, although the 0x sdk asset utilities class doesn't have an overload to encode an ERC721 item like that, so we weren't sure if the authors wanted all ERC721 assets available in the asset_pair call.

@dekz
Copy link
Member

dekz commented Dec 28, 2018

Having the AssetProxy,Contract address is the correct way.

import ethAbi = require('ethereumjs-abi');
import ethUtil = require('ethereumjs-util');

ethUtil.bufferToHex(
    ethAbi.simpleEncode(
        'ERC721Token(address,uint256)',
        tokenAddress,
        `0x0`,
    ),
).substr(0,74);

We are in the process of evaluating how this SRA endpoint and parameters in the orderbook endpoint should best support ERC721, ERC1155 and Bundled assets/MultiAssetProxy.

I'll look into the best way to export this helper, assetDataUtils might confuse developers thinking they can create orders with the output of this function.

@pointtoken
Copy link
Author

@dekz ok, thanks for clarifying.You may want to amend the code in the launch kit, as it doesn't follow the pattern you describe.

@dekz
Copy link
Member

dekz commented Jan 8, 2019

@pointtoken on discussion with the team it appears my initial reading of the endpoint was not correct.

This endpoint is to be used to show which Assets are available now, rather than which contracts are supported as a trading pair.

So a query to this endpoint should return similar data to the orderbook in that if there is an order for CryptoKitty 123 it should be returned in this endpoint.

In the next version of SRA we will look to add a "supported" asset data endpoint, so clients can discover which Relayers support the token contract when an orderbook is empty.

@pointtoken pointtoken reopened this Jan 9, 2019
@pointtoken
Copy link
Author

pointtoken commented Jan 9, 2019

@dekz Thanks for clarifying. Can you clarify the "available now" statement. For example, if there are no open orders for NFT #123, does it still get shown in the call because, in the past, there was an order?

I am assuming the answer is no -- only NFT aassets with open orders are shown.

But, if no, does that mean for ERC20 asset pairs, aka ZRX/WETH, if there are no open orders for that trading pair, the pair wouldn't be returned in the call? That isn't how relayers currently handle the call.

As far as design, I understand that the asset_pair call provides a lookup mechanism to query for an orderbook. However, in my opintion, the relayer spec really needs the "supported" endpoint that you referred to, such that a UI can show all orders for a given NFT type. Given the way the spec works now, creating that UI or query would require a lot of string slicing of the assetData.

@dekz
Copy link
Member

dekz commented Jan 10, 2019

Yeah I agree with you 100%. This endpoint is not currently that useful in discovery for ERC721. The behaviour of ERC20 and ERC721 in this endpoint is inconsistent directly due to it's reliance on the raw assetData.

ZRX/WETH, if there are no open orders for that, the pair wouldn't be returned in the call

I believe Launch Kit would not return the asset pair if there are no orders, which is consistent, but also feels like the endpoint is not that useful, may as well just ask for the orders which has better parameters for querying on contract address.

From an order consumer point of view maybe it is useful, which asset pairs are available for me to consume right now.

From an order producer point of view it doesn't help me make a decision of whether you want this order or not.

Given the way the spec works now, creating that UI or query would require a lot of string slicing of the assetData.

At this stage (before we publish a new SRA spec) I would suggest using the orders endpoint where it has defined parameters for makerAssetAddress (i.e CryptoKitties contract address) and takerAssetAddress (i.e WETH).

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

No branches or pull requests

2 participants