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

magicdrop v1.0.1 #161

Merged
merged 36 commits into from
Jan 21, 2025
Merged

Conversation

wolfy-nft
Copy link
Contributor

@wolfy-nft wolfy-nft commented Dec 6, 2024

ERC721CM v1.0.1

  • add setTransferable function to block transfers
  • add baseURI and tokenURISuffix to the setup function
  • add setup lock, can only be called once

ERC721M v1.0.1

  • add setTransferable function to block transfers
  • add baseURI and tokenURISuffix to the setup function
  • add setup lock, can only be called once

ERC1155M v1.0.1

  • add setup lock, can only be called once

CLI

  • update cli to set baseURI and tokenURISuffix on setup
  • update cli to freeze/thaw contracts
  • update cli to check if contract was already setup
  • updated default transfer list to 1 (ME transfer list)
Screenshot 2024-12-16 at 11 06 19 AM

Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
cli/cmds/contract Outdated Show resolved Hide resolved
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
@wolfy-nft wolfy-nft changed the title erc721cm v1.0.1 magicdrop v1.0.1 Dec 18, 2024
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this can be ignored, its a direct copy of ERC721A with an initializer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pre-approves our seaport conduit for immediate trading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this can be ignored, its a direct copy of ERC721AQueryable that extends ERC721ACloneable

Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
Signed-off-by: Adam Wolf <wolfynft@gmail.com>
@tenthirtyone
Copy link

  1. I think the architecture is backwards for the factory/registry. If it were building a car:

The factory that makes the car can be upgraded
The catalog that lists car models can be upgraded
But the actual cars can never be upgraded or fixed

Once an NFT contract is deployed, it's stuck with whatever implementation it was cloned from. You can't upgrade it.

  1. The entire MagicDropTokenImplRegistry can be replaced with a simple mapping

  2. The initialize/setup is cargo-cult programming (pattern implemented without understanding it). Because the Upgrade pattern is backwards, initialization is not doing what it is supposed to do -> Setting the proxy contract state. So a setup function is hacked in to do what initialize would normally do.

I believe the factory/registry could be replaced by:

The factory/registry could be replaced by:

contract SimpleFactory Ownable {
    uint256 fee;
    mapping(uint256 => address) public implementations;

    constructor(){...}
    
    function deploy(
        uint256 implId,  // An ID that maps to deployed token standards
        string calldata name,
        string calldata symbol,
        address initialOwner
    ) external payable returns (address) {
        require(msg.value >= fee, “Fee too low.”);

        address impl = implementations[implId];
        require(impl != address(0), "Invalid impl");
        
        address instance = LibClone.clone(impl);
        IERC1155M(instance).initialize(name, symbol, initialOwner);
        return instance;
    }

	… setters for fee and implementations
}

NOW, this would make upgrading for ZKSync a 1 line change to the ZKProxy.sol contract I provided in Slack.

@nothing0012 nothing0012 merged commit 877baff into me-foundation:main Jan 21, 2025
3 checks passed
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.

6 participants