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

Added Opensea's Operator Filter Registry #423

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

datboi-1337
Copy link

This makes the necessary changes to the contract to enable Opensea's new creator fee filters.

This does, of course, restrict the collections using it to only use marketplaces approved by Opensea, which I personally don't love. If we want to be able to set and collect creator fees on Opensea, however, it looks like this is the route we have to take for now.

Because the contracts provided by Opensea require 0.8.13, the compiler version has been changed here as well, and with that, comes specifying overridden contracts. Apart from that, the filter registry stuff has been imported, inherited and the transfer function overrides have been added.

Added operator-filter-registry from ProjectOpensea and made necessary changes to facilitate that change. Hardhat compiler version updated to 0.8.13.
@AnaTekPrimal
Copy link

This is gold, thank you.

Updating dependencies to support NodeJS 18 out-of-the-box (hashlips-lab#428)
@JohnCarp
Copy link

Hey @liarco I hope all is well!

Is there any chance you can take a look at this PR and make some suggestions? Opensea added more functions to this registry and when you try to implement it in this project there are a lot of compile errors.

I see there was a lot of discussion in the Hashlips Discord and the community really can use your help!

https://github.com/ProjectOpenSea/operator-filter-registry

    function setApprovalForAll(address operator, bool approved) public override onlyAllowedOperatorApproval(operator) {
        super.setApprovalForAll(operator, approved);
    }

    function approve(address operator, uint256 tokenId) public override onlyAllowedOperatorApproval(operator) {
        super.approve(operator, tokenId);
    }

    function transferFrom(address from, address to, uint256 tokenId) public override onlyAllowedOperator(from) {
        super.transferFrom(from, to, tokenId);
    }

    function safeTransferFrom(address from, address to, uint256 tokenId) public override onlyAllowedOperator(from) {
        super.safeTransferFrom(from, to, tokenId);
    }

    function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data)
        public
        override
        onlyAllowedOperator(from)
    {
        super.safeTransferFrom(from, to, tokenId, data);
    }

Thank you

Updated contract be fully compliant with Opensea guidelines for the Operator Filter Registry.

Edited mintForAddress to allow owner to mint more than maxPerTx, but will still restrict minting more than maxSupply.

Added separators and reorganized contract a bit to make it a little easier to navigate.
@datboi-1337
Copy link
Author

I was able to get in here and update everything to be fully in compliance with Opensea's Operator Filter Registry, including the approval functions.

@@ -68,15 +73,21 @@ contract YourNftToken is ERC721AQueryable, Ownable, ReentrancyGuard {
_safeMint(_msgSender(), _mintAmount);
}

function mintForAddress(uint256 _mintAmount, address _receiver) public mintCompliance(_mintAmount) onlyOwner {
function mintForAddress(uint256 _mintAmount, address _receiver) public onlyOwner {

Choose a reason for hiding this comment

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

I'm curious why you removed mintCompliance modifier from this function?

Also, if this is ok shouldn't you have the other require there? i.e:

function mintForAddress(uint256 _mintAmount, address _receiver) public onlyOwner { 
    require(_mintAmount > 0 && _mintAmount <= maxMintAmountPerTx, 'Invalid mint amount!');
    require(totalSupply() + _mintAmount <= maxSupply, 'Max supply exceeded!');

    _safeMint(_receiver, _mintAmount);
}

Copy link
Author

@datboi-1337 datboi-1337 Mar 29, 2023

Choose a reason for hiding this comment

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

This is unrelated to the Opernsea filter, just a general tweak that I have found people have issues with sometimes. I did briefly mention it in the push, but this change simply removes the restriction from the owner to mint more than the current maxMintAmountPerTx. It still ensures that maxSupply will not be exceeded, but allows the contract owner to mint multiple tokens, even if maxMintAmountPerTx is set to 1, for example.

@JohnCarp
Copy link

That's pretty good! Left a comment @datboi-1337

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.

3 participants