SuperMinterV1_1 et al. #296

Vectorized · 2023-10-23T14:09:40Z

SuperMinterV1_1
- The platform per-mint flat fee may be splitted amongst the following:
  - Cheap mint incentive. Only activated when the unitPrice <= cheapMintIncentiveThreshold.
    This goes to the artist (i.e. SoundEdition).
  - Affiliate incentive. This is separate from the old affiliate BPS fee.
    This will be added to the affiliate fee accrued mapping.
  - Anything remaining will go to the platform, and accrued in the platform fee accrued mapping.
- All these fees are flat fees and will be configured by the platform.
- Thus, there will two kinds of affiliate fees:
  - The old BPS based affiliateFee, which will be deducted from the artist's fee.
  - The new per-mint flat affiliate incentive fee, which will be be deducted from the platform's fee.
- If the affiliate is the collector, reverts. This is for making frontend simpler.
SoundEditionV2_1
- Minted event now emits the fromTierTokenIdIndex.
- The tierTokensIdsIn function won't revert if there are no tokens minted.
Both SuperMinterV1_1 and SoundEditionV2_1
- LibMulticaller.sender() -> LibMulticaller.senderOrSigner().
  The multicaller codebase can be found at: https://github.com/Vectorized/multicaller
  It has been reviewed by Kurt (0xPhaze).
- Solady will be updated to the latest version. This mainly affects the MerkleProofLib and SignatureCheckerLib, which both have been reviewed and re-reviewed by Spearbit Cantina.

Suggested review priority:

Check that the modifications to the fee structure are safe.
Check that the MulticallerWithSigner and the usage of it is safe.

changeset-bot · 2023-10-23T14:09:44Z

🦋 Changeset detected

Latest commit: 30e8987

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@soundxyz/sound-protocol	Minor

Not sure what this means? Click here to learn what changesets are.

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

v1.3.1

vigneshka · 2023-11-23T02:27:49Z

contracts/modules/SuperMinterV1_1.sol

-            platformFeesAccrued[d.platform] += f.platformFee; // Accrue the platform fee.
+            l.finalArtistFee = f.total - f.platformFee; // `platformFee <= total`;
+            l.finalPlatformFee = f.platformFee; // Initialize to the platform fee.
+            l.affiliate = p.to == p.affiliate ? address(0) : p.affiliate; // Yeah, we know it's left curved.


contracts/modules/SuperMinterV1_1.sol

vigneshka · 2023-11-23T02:42:52Z

contracts/modules/SuperMinterV1_1.sol

            }
+
+            /* --------------------- FREE MINT FEES --------------------- */


to be changed to artist price incentives

Comment:

The platform can elect to split some of it's fees with the artist as an incentive for low mint prices.
The finalPlatformFee is reduced and the finalArtistFee is increased by this incentive amount.

vigneshka · 2023-11-23T03:09:26Z

contracts/modules/SuperMinterV1_1.sol

+
+            /* ------------------ FIRST COLLECTOR FEES ------------------ */
+
+            if (firstCollector[p.edition] == address(0)) firstCollector[p.edition] = p.to;


I see this as most valuable for rewarding the first collector (in deployAndMint) bringing the contract on chain since they pay extra fees.

If the first minter isn't bringing the contract on chain and not paying any extra fees for that, it's a random reward for being first on superminter and a target for frontrunning. Lmk if you have other thoughts here

vigneshka · 2023-11-27T19:06:08Z

contracts/modules/interfaces/ISuperMinterV1_1.sol

+        // The total affiliate fee.
+        uint256 finalAffiliateFee;
+        // The final cheap mint fee.
+        uint256 finalCheapMintFee;


curious why this is included in the minted struct when other things like affiliateIncentive is not?

Good call out. I added finalAffiliateIncentive to the minted struct and renamed finalCheapMintFee -> finalCheapMintIncentive in the minted struct.

mattg-a16z

The underflow/overflow tracking is getting more and more complex and difficult to easily track, I think we need to either put comments near the calculation on why it can't underflow/overflow or put checks back in.

Additionally, I had questions on how thoroughly we need to be looking at MulticallerWithSigner. I think it has been reviewed during the audit (just checked to be sure), so it seems like we just want to know the integrations are done correctly rather than that the library itself doesn't have issues. Is this correct?

mattg-a16z · 2023-11-30T19:08:58Z

contracts/modules/SuperMinterV1_1.sol

+                // Deduct the BPS based affiliate fee from the artist's fee.
+                l.finalArtistFee -= f.affiliateFee;
+                // Deduct the affiliate incentive from the platform's fee.
+                l.finalPlatformFee -= f.affiliateIncentive;


Tracking through all the calculations and configuration checks to try to resolve if this is underflowable gets too complex to be realiably accurate. Can we remove unchecked or do explicit checks for values that are especially prone to error (values that can't be easily commented on why it isn't underflow/overflowable).

It is also worth considering that any underflow condition breaks balance tracking and will inevitably cause issues.

Left it as checked math.

I think a few hundred gas more is worth the better assurance.

Most of our mints are on L2 anyways.

mattg-a16z · 2023-11-30T19:11:31Z

contracts/modules/SuperMinterV1_1.sol

+            /* -------------------- CHEAP MINT FEES --------------------- */
+
+            if (f.cheapMintIncentive != 0 && f.unitPrice <= f.cheapMintIncentiveThreshold) {
+                l.finalPlatformFee -= f.cheapMintIncentive;


Similar comment here. This feels really prone to possible underflow.

mattg-a16z · 2023-11-30T19:16:42Z

contracts/core/SoundEditionV2_1.sol

@@ -778,7 +780,7 @@ contract SoundEditionV2_1 is ISoundEditionV2_1, ERC721AQueryableUpgradeable, ERC
     * @param roles A roles bitmap.
     */
    function _requireOnlyRolesOrOwner(uint256 roles) internal view {
-        address sender = LibMulticaller.sender();
+        address sender = LibMulticaller.senderOrSigner();


Concerning the MulticallerWithSigner, how thorough does the review need to be? The library seems covered by the audit, so we're really just considering if we think the function usage here is appropriate. Is this correct?

The priority will be to check if the usage of senderOrSigner() here is safe, assuming that the MulticallerWithSigner is implemented perfectly.

I am 96% confident of MulticallerWithSigner.

…> finalCheapMintIncentive

* Add presave functionality to SuperMinterV1_1 * Rename PRESAVE -> PLATFORM_AIRDROP * Force PLATFORM_AIRDROP to only use platform signer * Remove artist-customizable signer, everyone must use platform signer * Make mintTo and platformAirdrop return fromTokenId, use airdrop in platformAirdrop * Remove signer from MintData * Update create2 deployment script

…ized/affiliate-flat-fee

* Implement fee changes * T * Fix prices and add tests * Add tests * Simplified fees and rewards (#300) * nit suggestion * changes * Fix code and fuzz test * Tidy * Nit comment --------- Co-authored-by: Vignesh Hirudayakanth <[email protected]> --------- Co-authored-by: Vignesh Hirudayakanth <[email protected]>

* Clone files and bump versions * SuperMinterV1_1 et al. (#296) * Prep files * Reinstall multicaller * forge install: multicaller v1.3.1 * Add LibMulticaller.senderOrSigner support * Tidy * freeMintIncentive -> cheapMintIncentive * Add comment on the two types of affiliate fees * Remove first collector incentives * Add more comments and use checked math in mintTo * Add finalAffiliateIncentive to Minted log. Change finalCheapMintFee -> finalCheapMintIncentive * Add platform airdrop functionality to SuperMinterV1_1 (#298) * Add presave functionality to SuperMinterV1_1 * Rename PRESAVE -> PLATFORM_AIRDROP * Force PLATFORM_AIRDROP to only use platform signer * Remove artist-customizable signer, everyone must use platform signer * Make mintTo and platformAirdrop return fromTokenId, use airdrop in platformAirdrop * Remove signer from MintData * Update create2 deployment script * Create modern-shirts-try.md * pin version for typechain * tweak ci * more ci tweaks * add back foundry * bump * bump typechain * Implement fee changes (#299) * Implement fee changes * T * Fix prices and add tests * Add tests * Simplified fees and rewards (#300) * nit suggestion * changes * Fix code and fuzz test * Tidy * Nit comment --------- Co-authored-by: Vignesh Hirudayakanth <[email protected]> --------- Co-authored-by: Vignesh Hirudayakanth <[email protected]> * Update modern-shirts-try.md --------- Co-authored-by: Vignesh Hirudayakanth <[email protected]> * Update .github/workflows/canary.yml --------- Co-authored-by: Vectorized <[email protected]>

Vectorized changed the base branch from main to vectorized/affiliate-flat-fee-base November 1, 2023 18:38

Vectorized force-pushed the vectorized/affiliate-flat-fee branch 3 times, most recently from 0e713fb to c4cfd2b Compare November 6, 2023 20:23

Vectorized changed the title ~~Add affiliate flat fees~~ SuperMinterV1_1 et al. Nov 6, 2023

Vectorized force-pushed the vectorized/affiliate-flat-fee-base branch from 6af726d to 3e3985b Compare November 17, 2023 11:52

Prep files

d0e97dc

Vectorized force-pushed the vectorized/affiliate-flat-fee branch from ac89c60 to d0e97dc Compare November 17, 2023 11:54

Vectorized added 4 commits November 17, 2023 16:16

Reinstall multicaller

814e6f2

forge install: multicaller

56ecca9

v1.3.1

Add LibMulticaller.senderOrSigner support

289f5f5

Tidy

767e950

vigneshka reviewed Nov 23, 2023

View reviewed changes

contracts/modules/SuperMinterV1_1.sol Outdated Show resolved Hide resolved

vigneshka reviewed Nov 23, 2023

View reviewed changes

Vectorized added 3 commits November 24, 2023 21:52

freeMintIncentive -> cheapMintIncentive

51a192b

Add comment on the two types of affiliate fees

83f02ac

Remove first collector incentives

00273e4

vigneshka reviewed Nov 27, 2023

View reviewed changes

mattg-a16z reviewed Nov 30, 2023

View reviewed changes

Vectorized and others added 8 commits December 1, 2023 05:34

Add more comments and use checked math in mintTo

e993498

Add finalAffiliateIncentive to Minted log. Change finalCheapMintFee -…

9df9a7a

…> finalCheapMintIncentive

Create modern-shirts-try.md

ab5d9c3

pin version for typechain

9dc3992

tweak ci

8e48a58

more ci tweaks

3e86b2b

add back foundry

babe37c

vigneshka and others added 7 commits December 15, 2023 10:35

Merge branch 'main' of github.com:soundxyz/sound-protocol into vector…

12c912f

…ized/affiliate-flat-fee

bump

78f8bd9

Merge branch 'main' of github.com:soundxyz/sound-protocol into vector…

4abef49

…ized/affiliate-flat-fee

bump typechain

44373d5

Merge branch 'main' of github.com:soundxyz/sound-protocol into vector…

815ce5b

…ized/affiliate-flat-fee

Update modern-shirts-try.md

30e8987

vigneshka merged commit e11e105 into vectorized/affiliate-flat-fee-base Dec 18, 2023
5 checks passed

vigneshka deleted the vectorized/affiliate-flat-fee branch December 18, 2023 06:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SuperMinterV1_1 et al. #296

SuperMinterV1_1 et al. #296

Vectorized commented Oct 23, 2023 •

edited

Loading

changeset-bot bot commented Oct 23, 2023 •

edited

Loading

vigneshka Nov 23, 2023

vigneshka Nov 23, 2023

vigneshka Nov 23, 2023 •

edited

Loading

vigneshka Nov 27, 2023

Vectorized Dec 1, 2023 •

edited

Loading

mattg-a16z left a comment

mattg-a16z Nov 30, 2023

mattg-a16z Nov 30, 2023

Vectorized Dec 1, 2023

mattg-a16z Nov 30, 2023

mattg-a16z Nov 30, 2023

Vectorized Dec 1, 2023

		}

		/* --------------------- FREE MINT FEES --------------------- */


		/* ------------------ FIRST COLLECTOR FEES ------------------ */

		if (firstCollector[p.edition] == address(0)) firstCollector[p.edition] = p.to;

SuperMinterV1_1 et al. #296

SuperMinterV1_1 et al. #296

Conversation

Vectorized commented Oct 23, 2023 • edited Loading

changeset-bot bot commented Oct 23, 2023 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vigneshka Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vectorized Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

mattg-a16z left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vectorized commented Oct 23, 2023 •

edited

Loading

changeset-bot bot commented Oct 23, 2023 •

edited

Loading

vigneshka Nov 23, 2023 •

edited

Loading

Vectorized Dec 1, 2023 •

edited

Loading