Add EIP-6105: Marketplace extension for EIP-721 #6105
Conversation
github-actions
bot
added
c-new
|
All reviewers have approved. Auto merging... |
|
|
||
| // VIEW | ||
| function getAllListings() external view returns ( Listing[] memory ) { | ||
| uint256 arraylen = saleItems.length; |
There was a problem hiding this comment.
Wouldn't it be better to have pagination? If the marketplace is very large or if accessing the data from another contract when gas cost would matter, it is very important to be able to limit the scope.
And change the salesItems contract property to be of Listing type so that the code is much simpler.
function getListings(uint startIndex, uint fetchCount) external view returns(Listing[] memory) {
uint itemCount = salesItems.length;
if(itemCount == 0) {
return new Listing[](0);
}
// Error if starting after end
require(startIndex < itemCount);
// Trim if fetchCount goes beyond the upper bound
if(startIndex + fetchCount >= itemCount) {
fetchCount = itemCount - startIndex;
}
Listing[] memory out = new Listing[](fetchCount);
for(uint i; i < fetchCount; i++) {
out[i] = salesItems[startIndex + i];
}
return out;
}
There was a problem hiding this comment.
Except it's not accessing data from "another contract." That would defeat the entire purpose of the code. You might be right that putting some kind of index basis on it could be helpful, as could adding some kind of fetch or get that limits by the cost of the listings or any other criteria you see fit. So perhaps the better solution would be to remove getAllListings from the EIP and leave it up to the developer to create sorting algorithms.
The other way would be to include such searches in here to make it more universal for front-end searches so that aggregators will have a better interfact to gather data from a wide number of compliant collections.
I will look into salesItems again. There was some reason it was set up for tokenId rather than the actual struct and for brevity sake we wanted to provide a solution to customers immediately who did not wish to comply with OpenSeas blacklist functionality but still wanted to have control of their own nfts on their own marketplace. Will look into that more this morning to see about cleaning things up.
There was a problem hiding this comment.
I've updated salesItems array like you mentioned. There were some other conflicts when I initially wrote the code but they have resolved themselves now. There still exists two mappings to track indexes for individual tokens to avoid the need to loop through the whole array, and the return values are now much cleaner for the current two fetch functions.
Your method of fetching several at a time should also be feasible, but I have not implemented it yet.
I've tasked @lambdalf-dev with updating the code here on the EIP, it should be updated shortly. Also includes removal of a bug that I found while retesting the codebase.
|
Just want to add this in here, I know there are links, but the purpose and intent of the EIP: #1. Create and internal mechanism to list, delist, and purchase NFTs on a secondary marketplace that is built into the smart contract such that both the marketplace and royalties are on chain, transparent, and under the complete control of the artist/team releasing the NFT, so that even if every public aggregator or marketplace fails, or their policies are such that the creator wishes to disallow listing on their platforms, there will always be a built-in method for trading tokens built directly into the smart contract. #2. Creators maintain the right (with additional code) to fully restrict access from not only public marketplaces, but potential scammers who are trying to fool unsuspecting holders into signing approveForAll(), because only a limited number of contracts or wallets would in that case have access to approve() and setApprovalForAll() functionality. In that case this EIP provides a standard marketplace that the original creator can maintain and ensure safe transactions for their user. #3. By avoiding centralized marketplaces, there is no need for a separate approval() call before a transaction, therefore saving gas that would normally be associated with "handling" NFTs on behalf of the user. #4. The code should be as open and non-restrictive as possible, allowing any aggregator who wishes to probe the contract for current listings while at the same time allowing the developer to tune the marketplace to whatever specific functionalities they would want to make their front-end code more responsive and in-line with their goals. #5. The methods should be as separate from the traditional market functionality of ERC721 as possible so that the code in it's virgin form is 100% compliant with current ERC721 standard. -- Side note: The question of HOW the methods of this EIP accomplish the given task is not as important as coming up with a standard interface to interact with several different types of code-specific implementations. The question was raised about pulling a portion of the data rather than one item or all items. My reasoning for setting up those two functions was to ensure that we could limit the number of calls required from the front-end to get a comprehensive list of tokenIds and prices, as calls from JavaScript code will likely be made through the injected wallet, and thus the process is slow and cumbersome with multiple calls. |
|
I think the most important point about this implementation is that it is an extension to the ERC721 standard. What is the core goal though? To create an on-chain NFT marketplace. I feel like increasing the size of the already lengthy ERC721 contract is not the best approach. You're also then limited to only new contracts, it doesn't pull in support for legacy ERC721 contracts. Why not create an You don't get the gas savings from not needing an approval but if it's really desired, EIP-4494 could be implemented on the contract. interface ERC721Marketplace {
struct Listing {
address tokenContract;
uint256 tokenId;
uint256 tokenPrice;
address to;
}
// perform transferFrom to this contract while tokens is for sale
function listItem( Listing item ) external;
// transfer token back to its original owner
function delistItem( address tokenContract, uint256 tokenId ) external;
...
}
With this kind of system, any ERC721 token could be traded on the marketplace. Of course, a token contract could deny transfer to marketplaces it doesn't like so that point of control is still there. Correct me if I'm missing something but there's no reason to make a ERC standard for a This is the same architectural distinction I realized with arbitrable-wrappers instead of going with something that updates the ERC20 standard like ERC20R One more thought, another potential feature could be purchasing an NFT with another NFT, a kind of swap. |
The real problem we're trying to solve with this proposal is putting projects at risk of finding themselves completely unable to have trades because of the Open Sea Filter Registry. Whether we accept it or not, this thing exists and needs to be dealt with. The way we view it, Open Sea is officially saying "We believe only us should be able to determine who can trade your tokens and how". It is in our opinion an attack to the concept of liberty. That's why we want to add this functionality to the ERC721 standard, therefore taking back that control from Open Sea and giving it back to the creators. If this functionality is given to a different contract, Open Sea still has the power to shut it down. |
|
The Open Sea filter is your inciting incident and that's totally fine. What I'm trying to say is that that you would have better UX and inclusivity by not making this an extension and instead making a fully on-chain marketplace that can support any ERC721 contract. If the marketplace is an extension, that means it on works with token contracts that are new, aware of this EIP, and have enough space remaining in their 24kb bytecode limit to implement the features. It also means that the marketplace must be fully designed at time of token deployment. Anybody with existing NFTs would not be able to move away from Open Sea. There's no backwards compatibility. This would be cumbersome and ignores the great composability of Ethereum contracts. Not every application has to be an ERC standard and that's fine. |
|
The point of this is not to create ANOTHER marketplace contract. There are several of those already in existence and as far as backwards compatibility, this code block still meets the requirements to satisfy those avenues. The whole concept here is to do something new and different that allows for a simple trading mechanic built onto the contract. You're basically saying, this is okay, but it would be better if it was <completely different thing that requires it's own setup/execution/resolve and associated codebase> This was not designed to be a replacement for OpenSea, LooksRare, X2Y2, or any other existing marketplace. "Fully on-chain marketplaces" already exist and they cater to any ERC721. Why would we need to make a new version of that? What purpose would it serve? These are rhetorical questions btw and beyond the scope of this topic. The purpose of this EIP is an on-contract secondary trade system that will continue to work and if the artist/team/community so chooses, they can build this out where their internal marketplace is the only functional sales avenue short of direct wallet-to-wallet transfers. Or it can play along with regular marketplaces. To answer you about the "market system needing to be set up ahead of time," actually it doesn't. You can list and buy NFTs directly on the smart contract. I understand your point, but you are trying to say that this code could be the seed for a full marketplace contract and that is simply not the case nor what this was designed to facilitate. As far as the bytecode limit, take a look at the code OS is requiring to guarantee royalties for creators and tell me again how we're limited on bytecode with already bloated ERC721 contracts. There are a lot of people that refuse to comply and this gives them an option for secondary trading that cannot be revoked by any third party source, including the idea you are referencing of just building another centralized marketplace. |
Actually, I would argue that it is fully backwards compatible as implementation or not of this standard doesn't prevent normal behavior of other existing contracts that would not be aware of its existence. It is simply providing additional functionalities for contracts that implement it. |
|
Sorry if I'm missing the point. I'm not blocking it, just trying to understand the use case.
By making this a marketplace extension, it would be like having a Uniswap liquidity pool as an ERC 20 extension so if you wanted to trade that token, you would interact with the token contract itself. There's nothing about that goal that necessitates having the internal marketplace being in the same contract as the base token. What are the advantages to this model vs the dexes we see in production? I don't think I explained well enough what I meant by backwards compatible. There are many ERC 721 contracts already deployed on public chains that would not be able to benefit from this extension because it's part of the token contract itself. |
|
Well, of course... how could a contract deployed before it existed benefit from it? The goal is to protect newly deployed contracts from the Open Sea Filter Regisry. Now here's a scenario for you to better understand: |
Thanks first of all for the continued follow-up and dialogue. I realize this is (or seems like) a niche application and it is one that I've already onboarded a few people with who don't care if it's accepted as a standard. They want the plugin, so it's important to be clear in these discussions on what it is and is not. I'm sorry if I haven't been able to fully demonstrate this purpose yet, so I'll attempt to hone in on these questions. The main priority here is that any centralized exchange or NFT DEX is going to suffer from the same inherent problem that there is always some entity that is not the CREATOR (lets define as artist/team/whatever) who maintains direct control over the payment of royalties, and authority to manage those royalties. This proposal outlines an alternative, a truly decentralized methodology that each CREATOR controls 100% through their own smart contract, and an internal mechanism to list/delist/trade NFTs within the scope of the contract itself, instead of relying on a third party that might have values that conflict with the CREATOR. One CREATOR in particular that I've been talking to has decided directly that they don't want their NFT traded on OpenSea's platform at all, and will be using the Registry Filter to block OpenSea from trading their token. These disagreements may be over royalties, company transparency, or any other conflict of interest that might arise between the CREATOR and the Marketplace. In the unfortunate event that the CREATOR cannot find common ground with existing marketplaces, it would fall on them to implement their own marketplace, which would require handling of tokens on the customer's behalf, and short of a community wallet to handle direct transfers, approvals would need to be set and that sets into motion this cat/mouse game of marketplaces blacklisting any handler of tokens when an even occurs through the use of approve() or approveAll() (assuming they utilize the Registry and then change their mind later). I admit that this situation is probably an unlikely fringe case, but from a CREATOR perspective what this code does is give them an easy mechanism to host a marketplace for their own tokens regardless of whether they choose to invoke registry or not. And I've also provided broilerplate code that allows them to easily copy a simple HTML page and brand it to fit their project, which interacts with this smart contract code. Another potential advantage here is the move away from defacto marketplaces and the introduction of aggregator websites which might act as both a marketplace for legacy ERC721 contracts that wish not to implement this while also allowing a simple interface for collections that do implement the code to showcase their work and even trade it without any approvals required, thus the "aggregator/marketplace" eliminates liability and security problems for themselves, though a new business model would obviously needed to round out such a niche. All of this can be done completely trustlessly, without the need to set approvals and put trust in a marketplace that they will not mishandle the NFTs, they will not attempt to skirt the royalty scheme or other requirements imposed by the CREATOR, and they will have no functional control over the movement of NFTs from one wallet to another at all. Every time approve() or setApprovalForAll() are invoked, there exists the potential for foul play. This contract update, especially if paired with another use case listed below, would ramp up security for both customers and CREATORs, as well as ensure that the CREATOR is in charge of their own collection forever, without having to worry about surprise mandates that we've seen over the last several months from companies like LooksRare, X2Y2, OpenSea, and others. This is all part one Part II involves the implementation if an internal registry filter of sorts, which is not part of this particular EIP but it is important to understand, whereby the CREATOR can whitelist on their own contract exactly which marketplaces and other contracts will have access to approval functions, and in such case also act as a security measure to protect against probably the most common scam of the day, which is tricking an end user into signing an ApproveAll function. Were such a whitelist implemented, and internal secondary marketplace might provide the ONLY source of trading the NFT on secondary, which seems against the current web3 ethos, but there are niche cases and we have seen them where this is desired for one reason or another to keep trades "inside the community" so to speak. I seem to remember TopShots or a related project doing something like this at the start of 2021. This added security of allowing only authorized users to issue approvals is an accidental security fix that we found while testing this kind of implementation, and it immediately struck a chord with Chameleon Collective Team and other founders and something worth looking into for future contracts. |
| /// - Caller must own `tokenId` | ||
| /// - Must emit a {Listed} event. | ||
| /// | ||
| function listItem( uint256 tokenId, uint256 price, address to ) external; |
There was a problem hiding this comment.
I would like to see this feature support multiple currencies.
Existing marketplaces support creating a sale in ETH or a stablecoin. It's common for NFT ecosystems to create their own tokens that they would like to use for this purpose.
Adding a paymentTokenAddress would add arbitrary ERC20 support to the marketplace, allowing the seller to peg the price to any ERC20.
This would also support projects that want to allow only a subset of ERC20s for sale as they can whitelist ERC20 addresses in their implementation.
This doesn't preclude selling in ETH as address(0) can be used to indicate a sale price in ETH.
There was a problem hiding this comment.
Curious... I think it's a good idea in theory, but it would add some more to the codebase.
We intentionally left the feature-set a little light to leave expansion decisions on the Creator and they could add whatever they needed. Making it a requirement, if it could be done efficiently would be nice. I mean a simple check on the payment for eth sent with the transfer and if not then try to transferFrom with whatever contract information, sounds simple enough.
In that case, if the creator chose to implement those other tokens they could do so easily and if they decided not to then they could leave the base option out. ...
I see some promise in this idea and have been thinking about it since I saw the post earlier, I was away from my computer and busy with other things. Definitely something worth considering. Thank you for the comment and we'll spend some time discussing this.
My main worry would be small artist collections trying to build out a front end to accomodate, but hopefully we can just adapt the broiler code as well easily enough.
Thanks again, will give this some more thought.
There was a problem hiding this comment.
That's a good point, might be worth implementing...
Our initial thought was that the coin to pay with could be up to the project creator, but it would only allow for one type of payment to be used, whereas your idea allows for unlimited options, with more flexibility.
There was a problem hiding this comment.
Agree. EIP should be as concise and general as possible.
I understand these points completely. It makes good sense. The point I'm trying to express is that these marketplaces have no need to be on the same contract as the token.
That's not true. And I think that assumption is why you're not understanding my point yet. You could write an ownerless, unstoppable marketplace contract that respects EIP-2981 that would allow the creator to define their royalties and, if they trust this immutable contract, they can place it into their allow filter as an approved marketplace. Or you could write a contract that accepts a token contract address in the constructor and use the token contract owner as the owner of the marketplace contract. This way they would be linked and the creator of the token would also have full control over any marketplace administrator functionality. There are many ways to create a marketplace and these would all be useful templates for creators. Also, because there's so many ways to create these marketplaces, it's very helpful to be able to manage them separately from the tokens themselves.
This is really the only point that would necessitate putting the marketplace in the same contract as the token but you could have the token owner transfer the NFT directly to the marketplace themselves and use I'm not trying to convince you to change the goal, I'm trying to increase the code quality. |
Those suggestions both suffer from the same limitations though, they are subject to being blacklisted by OpenSea on a whim, which puts them at risk of being disabled if the contract implements the standard OpenSea filter registry, which by the way is very obscure to begin with. |
|
Amazing!It's so gratifying to have someone who thinks the same as us. We came up with the idea of In the above code, we also consider implementing the I would like to ask if you would like us to be co-authors of EIP-6105, let us promote this eip together. We will be very happy if you agree. |
1.I understand that EIP6105 is to establish an intermediary-free NFT trading protocol to avoid a series of problems caused by possible evil in the intermediary trading market. Establishing another NFT intermediary trading market will not help solve the concerns of NFT creators. This approach only allows users to change from trusting one trading market to trusting another trading market, and does not fundamentally solve the problem. We recommend that you read this article to understand our point of view. |
Find me on twitter ;) @offgridgecko or @spottedgeckgo should work. send me a note about this so I know who you are. I'm also on discord @ OffGridGecko #9999 |
|
responding to @5660-eth : On the point of "make offer," we actually went back and forth on this for a while, then I asked, who does it benefit? The answer is flippers and bots, which is not in the spirit of the original purpose of this EIP. We eventually agreed that offers could be added easily enough by the individual coder-project, but we didn't want to bulk up the code with methods for making standing offers that would be accepted/rejected/etc while the item is listed. I designed this to help artists create their own internal marketplaces for their art that stood in the face of current marketplace options and manipulations, and obviously scammers love the offer option as well. In the end, we dumped the idea and decided to keep things as simple as possible and just let the project decide on any upgrades while still making the options for selling on the contract available to 3rd party aggregator websites. As you stated, the primary goal here is avoiding centralized marketplaces and specifically dodging OpenSea's little consortium from blocking sales on other marketplaces. "Not your marketplace, not your royalties" was pretty much my driving motivation from the start. |
|
@offgridgecko I messaged you on twitter. |
| function listItem( uint256 tokenId_, uint256 price_, address buyer_ ) external { | ||
| address _tokenOwner_ = ownerOf( tokenId_ ); | ||
| require( _tokenOwner_ == ownerOf( tokenId_ ), "Not token owner" ); | ||
|
|
||
| _createListing( tokenId_, price_, _tokenOwner_, buyer_ ); | ||
| } |
There was a problem hiding this comment.
| function listItem( uint256 tokenId_, uint256 price_, address buyer_ ) external { | |
| address _tokenOwner_ = ownerOf( tokenId_ ); | |
| require( _tokenOwner_ == ownerOf( tokenId_ ), "Not token owner" ); | |
| _createListing( tokenId_, price_, _tokenOwner_, buyer_ ); | |
| } | |
| function listItem( uint256 tokenId_, uint256 price_, address buyer_ ) external { | |
| address _tokenOwner_ = ownerOf( tokenId_ ); | |
| require( _tokenOwner_ == ownerOf( tokenId_ ), "Not token owner" ); | |
| _createListing( tokenId_, price_, _tokenOwner_, buyer_ ); | |
| } |
Designated buyer does not seem to be used very often. If a particular buyer and seller privately agree on a trading price, they have multiple ways to complete the trading
| // VIEW | ||
| /** | ||
| * @notice Returns the list of all existing listings. | ||
| * | ||
| * @return the list of all existing listings | ||
| */ | ||
| function getAllListings() external view returns ( Listing[] memory ) { | ||
| return _listings; | ||
| } |
There was a problem hiding this comment.
This function doesn't seem to work. solidity doesn't seem to support returning arrays?
There was a problem hiding this comment.
I'll have to check that, I haven't had the time to do unit tests on the code suggested yet. Thanks for pointing it.
There was a problem hiding this comment.
We updated the code for reference
https://github.com/5660-eth/ERC721YY/blob/main/contract/ERC721YY.sol
Yes that's another reason we didn't pursue it. In order to secure funds, they would need to be stored on the smart contract to ensure payment... I'm not sure anyone is keen on actually "putting their money up" but then again such a mechanic would make it less prone to botspam. Basically, without adding a lot of code, I dunno if it should be included in this EIP, but the option is there for people to extend the EIP with that mechanic. |
github-actions
bot
added
w-ci
Update eip-6105.md
Pandapip1
changed the title
|
The commit dd01c64 (as a parent of ba8c6fa) contains errors. |
Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
The rationale can only be updated once the interface and reference implementation have been identified
Update Specification, Rationale and Reference Implementation.
|
Hi, @Pandapip1, @SamWilsn, @axic, @xinbenlv. Do you have time to review and merge this EIP? Previously reported issues have been resolved.Thanks |
There is a viewpoint that adding |
* Proposal for ERC721 Marketplace extension * Update code examples, Fix some linting issues * Update and rename eip-erc721-marketplace-extension.md to eip-6105.md * Update eip-6105.md * Update eip-6105.md * Commit necessary changes * Update eip-6105.md * Apply suggestions from code review Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com> * Update eip-6105.md * Update interface and reference implementation The rationale can only be updated once the interface and reference implementation have been identified * Update eip-6105.md * Update eip-6105.md * Update eip-6105.md Update Specification, Rationale and Reference Implementation. * Update eip-6105.md --------- Co-authored-by: 5660.eth <76733013+5660-eth@users.noreply.github.com> Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
* Proposal for ERC721 Marketplace extension * Update code examples, Fix some linting issues * Update and rename eip-erc721-marketplace-extension.md to eip-6105.md * Update eip-6105.md * Update eip-6105.md * Commit necessary changes * Update eip-6105.md * Apply suggestions from code review Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com> * Update eip-6105.md * Update interface and reference implementation The rationale can only be updated once the interface and reference implementation have been identified * Update eip-6105.md * Update eip-6105.md * Update eip-6105.md Update Specification, Rationale and Reference Implementation. * Update eip-6105.md --------- Co-authored-by: 5660.eth <76733013+5660-eth@users.noreply.github.com> Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: