Auction Modules and Base Implementation #4

Vectorized · 2022-07-20T01:08:44Z

Actually, I'd prefer to call it minting, so that it can generalize to all kinds of drops.

Terminology:

Minter (A contract that calls the mint function on the NFT contract)
Edition (The NFT contract called by the minter)

Minter contracts are current non-upgradeable (will change later if needed).

Actually, we don't need to make these Minting contracts upgradeable.

Flow:

Artist call createEdition on a Minter contract.
Buyers will call the mint function on the Minter contract, which will then call the mint function on the edition, passing along the msg.value.

Details:

Each type of auction is in a different Solidity file. This is for modularity and code reuse.
So, instead of having the public auction and the permissioned auction combined, we split them into two different files.
If we want, we can add a combined public and permissioned auction like ArtistV6.
For now, we only have the createEdition and deleteEdition functions on the Minter contract. Do we need functions to edit each of the fields individually? This will add quite a bit of work.

linear · 2022-07-20T01:08:45Z

PRO-220 Auction module interface and base implementation

First goal: should be compatible with our existing formats (see buy edition)

signature gated minting, range editions / open editions
should be able to support out of the box formats with ability to plug in custom ones + rules
- airdrop to a passed in list
- reserve x amount of tokens for manual transfers by the artist
- set price discounts for a given list of users for a given song
- set minting rules e.g. address can only buy a maximum of 3 of this song
- auction: fixed mint, dutch auction, free mint for x period, bonding curves
Nice to haves: compatibility or easy porting of existing Manifold extensions
Freeze function

Compare delegate call vs pure extension options and write out a doc. We will discuss + get some input on the path forwards. Tradeoffs in terms of security, flexibility, provenenance/tx history, etc.

changeset-bot · 2022-07-20T01:08:46Z

⚠️ No Changeset found

Latest commit: 1fa1b97

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

gigamesh · 2022-07-20T21:01:02Z

contracts/SoundNft/Mint/OwnedMintees.sol

+
+pragma solidity ^0.8.15;
+
+contract OwnedMintees {


I renamed SoundNft to SoundEdition in this PR, and I think it might be more clear to use "edition" whenever we're referring to that contract (ie: instead of "mintee").

Also using "owner" makes it sound like SoundEdition.owner(), so maybe MintController instead?

So for this contract, maybe MintControllers or EditionMintControllers instead of OwnedMintees?

Sounds good to me.

I will rename it to EditionMinter.

Vectorized · 2022-07-21T23:16:42Z

For simplicity, I've not added the following features for now
(cuz I'll need to write way more tests for them):

Edit the fields in the EditionMintData.
Restrict a wallet to mint up to a certain number of NFTs.

elenadimitrova · 2022-07-24T13:10:39Z

contracts/SoundEdition/SoundEditionV1.sol

+    // ================================
+    // CONSTANTS
+    // ================================
+    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");


This allows for multiple minters to be set per edition. Is this something we want?

Yes.

An artist may want to run different kinds of sales simultaneously.

elenadimitrova · 2022-07-26T08:37:02Z

contracts/SoundEdition/SoundEditionV1.sol

+    ERC721AQueryableUpgradeable,
+    IERC2981Upgradeable,
+    OwnableUpgradeable,
+    AccessControlUpgradeable


Why build in upgradability if SoundEdition is to be immutable?

This is a good question on the team level we may need to revisit.

@elenadimitrova as the SoundEditions are currently being deployed via Clone Proxy, that's why the base contract is using Upgradeable contracts.

elenadimitrova · 2022-07-26T08:46:10Z

contracts/modules/Minters/EditionMinter.sol

+    function _deleteEditionMint(address edition) internal virtual {
+        address controller = _controllers[edition];
+        if (controller == address(0)) revert MintNotFound();
+        if (msg.sender != controller) revert MintControllerUnauthorized();


We can reuse the existing onlyEditionMintController modifier here instead.

elenadimitrova · 2022-07-26T08:47:27Z

contracts/modules/Minters/EditionMinter.sol

+        virtual
+        onlyEditionMintController(edition)
+    {
+        if (controller == address(0)) revert MintControllerSetToZeroAddress();


Instead of this condition we can remove _deleteEditionMint implementation and simply allow controller address to be set to 0 here.

Sounds ok to me.

elenadimitrova · 2022-07-26T08:48:30Z

contracts/modules/Minters/EditionMinter.sol

+    }
+
+    function editionMintController(address edition) public view returns (address) {
+        return _controllers[edition];


Instead of this explicit function we can simply make _controllers mapping public and expose it in the minter interface.

I want to give it a descriptive public accessor, but still keep the internal name short.

elenadimitrova · 2022-07-26T08:54:08Z

contracts/modules/Minters/EditionMinter.sol

+
+pragma solidity ^0.8.15;
+
+abstract contract EditionMinter {


Can you clarify what purpose does this contract serve? I see that the minters are based off it and they write to the controllers mapping but I don't understand why we need that.

I do think however that we need a specific interface for Minters which external developers can implement in a way of a standard our interface can integrate with.

I see that our own minter classes have the common logic to allow different wallets to control different edition mints. This is simply for code reuse.

elenadimitrova · 2022-07-26T08:56:47Z

remappings.txt

+chiru-labs/ERC721A-Upgradeable/=./lib/ERC721A-Upgradeable/contracts/
+solady/=./lib/solady/src/


Why not use Open Zeppelin ECDSA.sol ?

For performance reasons. This saves 700 gas. It also has a much simpler API: returns a zero-address in any case of errors.

Can we submit that as an improvement proposal to OZ? Or is it not applicable?

Code snippets in Solady are designed as PRs to Solmate. transmissions11/solmate#247

Not applicable to OZ — their maintainers have expressed interest to keep the various errors OpenZeppelin/openzeppelin-contracts#3457 (comment)

Solady also contains the very optimized on-chain sort that may be useful for building the cap tables later on.

There is something on OZ's backlog for the next release related to this though (https://github.com/OpenZeppelin/openzeppelin-contracts/milestone/12) 🤔

elenadimitrova · 2022-07-26T09:16:19Z

spec.md

@@ -30,8 +30,16 @@ Sound Protocol 2.0 enables creators to permissinonlessly deploy gas-efficient NF
    - `tokenURI` uses `baseURI` instead, if `metadataModule` is not present
    - Implements `contractURI` (https://docs.opensea.io/docs/contract-level-metadata)
    - Allows freezing of metadata, beyond which the variables can't be modified by `owner`
+  - Minters


Can we add the detail that we allow multiple minters enabled on a SoundEdition at the same time.

Also I think it's worth describing the model for developing Minter extensions.

Unlike metadata module, minters are free to be in any shape and form — our edition contracts will not call them. Only the minters contracts will call the edition contract. It’s a one-way data flow.

Will add into the comments.

…Vectorized/sound-protocol into vectorized/pro-220-auction-base

elenadimitrova · 2022-07-28T07:28:43Z

Ah but why close it? We'll lose all traceability of reviews

Vectorized · 2022-07-28T08:00:21Z

I mistakenly opened a fork for PR cuz didn’t have permission to create a branch back then.

I’ll keep a link on the new PR’s description to this closed PR.

gigamesh reviewed Jul 20, 2022

View reviewed changes

Vectorized force-pushed the vectorized/pro-220-auction-base branch 3 times, most recently from 062b855 to c9a21d2 Compare July 21, 2022 21:11

Vectorized added 5 commits July 21, 2022 23:11

Add initial code

998f39a

Add FixedPricePublicSaleMinter tests

9b22673

Add more tests

b0d7ec2

Fix tests

a790e16

Add more tests

7c74f00

Vectorized force-pushed the vectorized/pro-220-auction-base branch from 723da8e to 7c74f00 Compare July 21, 2022 23:12

Vectorized marked this pull request as ready for review July 21, 2022 23:12

Vectorized added 2 commits July 21, 2022 23:27

Edit spec.md

855852a

Merge branch 'master' into vectorized/pro-220-auction-base

9dc338d

vigneshka requested review from apoorvlathey, elenadimitrova and vigneshka July 22, 2022 14:44

elenadimitrova reviewed Jul 24, 2022

View reviewed changes

Vectorized changed the title ~~Auctions Proof of Concept 0 [WIP]~~ Auction Modules and Base Implementation Jul 25, 2022

apoorvlathey approved these changes Jul 25, 2022

View reviewed changes

elenadimitrova reviewed Jul 26, 2022

View reviewed changes

Vectorized added 3 commits July 27, 2022 08:54

Add suggested changes

69bf024

Merge branch 'vectorized/pro-220-auction-base' of https://github.com/…

37b2a59

…Vectorized/sound-protocol into vectorized/pro-220-auction-base

Edit mint comments

1fa1b97

Vectorized mentioned this pull request Jul 27, 2022

Auction Modules and Base Implementation #18

Merged

Vectorized closed this Jul 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auction Modules and Base Implementation #4

Auction Modules and Base Implementation #4

Vectorized commented Jul 20, 2022 •

edited

Loading

linear bot commented Jul 20, 2022

changeset-bot bot commented Jul 20, 2022 •

edited

Loading

gigamesh Jul 20, 2022 •

edited

Loading

Vectorized Jul 20, 2022 •

edited

Loading

Vectorized commented Jul 21, 2022

elenadimitrova Jul 24, 2022

Vectorized Jul 24, 2022

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022

apoorvlathey Jul 27, 2022 •

edited

Loading

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022 •

edited

Loading

elenadimitrova Jul 26, 2022

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022 •

edited

Loading

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022 •

edited

Loading

elenadimitrova Jul 27, 2022

Vectorized Jul 27, 2022

elenadimitrova Jul 27, 2022

elenadimitrova Jul 26, 2022

elenadimitrova Jul 26, 2022

Vectorized Jul 26, 2022 •

edited

Loading

elenadimitrova commented Jul 28, 2022

Vectorized commented Jul 28, 2022

		chiru-labs/ERC721A-Upgradeable/=./lib/ERC721A-Upgradeable/contracts/
		solady/=./lib/solady/src/

Auction Modules and Base Implementation #4

Auction Modules and Base Implementation #4

Conversation

Vectorized commented Jul 20, 2022 • edited Loading

linear bot commented Jul 20, 2022

changeset-bot bot commented Jul 20, 2022 • edited Loading

⚠️ No Changeset found

gigamesh Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

Vectorized Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

Vectorized commented Jul 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoorvlathey Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vectorized Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vectorized Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vectorized Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vectorized Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

elenadimitrova commented Jul 28, 2022

Vectorized commented Jul 28, 2022

Vectorized commented Jul 20, 2022 •

edited

Loading

changeset-bot bot commented Jul 20, 2022 •

edited

Loading

gigamesh Jul 20, 2022 •

edited

Loading

Vectorized Jul 20, 2022 •

edited

Loading

apoorvlathey Jul 27, 2022 •

edited

Loading

Vectorized Jul 26, 2022 •

edited

Loading

Vectorized Jul 26, 2022 •

edited

Loading

Vectorized Jul 26, 2022 •

edited

Loading

Vectorized Jul 26, 2022 •

edited

Loading