-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add platform airdrop functionality to SuperMinterV1_1 #298
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this in a slightly different way this time. I went through all the key changes to make sure I noted having reviewed the change or asked for verification that my understanding is correct. Overall, it seems like the main changes are:
- Removing the mint-specific signers in favor of platform signers
- Addition of another mint type
- A new minting function (airdrop) to support the new mint type that verifies a platform signature before airdropping to n many addresses
- Changes to the editions to allow the minter to call the airdrop function directly
- Changes to configuration functions to keep people from setting price or max mintable for an airdrop mint
If this matches your expectations, then we're probably good. I'll do another once-over tomorrow morning to make sure I'm not missing some strange edge case, but in the mean time, here is the review.
@@ -232,7 +232,7 @@ contract SoundEditionV2_1 is ISoundEditionV2_1, ERC721AQueryableUpgradeable, ERC | |||
uint8 tier, | |||
address[] calldata to, | |||
uint256 quantity | |||
) external payable onlyRolesOrOwner(ADMIN_ROLE) returns (uint256 fromTokenId) { | |||
) external payable onlyRolesOrOwner(ADMIN_ROLE | MINTER_ROLE) returns (uint256 fromTokenId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked this in context. The change is to allow airdrop
to be called by the minter to facilitate the minter's new airdrop functionality. This checks out no response needed
|
||
// Perform the sub workflows depending on the mint mode. | ||
uint8 mode = d.mode; | ||
if (mode == VERIFY_MERKLE) _verifyMerkle(d, p); | ||
else if (mode == VERIFY_SIGNATURE) _verifyAndClaimSignature(d, p); | ||
else if (mode == PLATFORM_AIRDROP) revert InvalidMode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is important, the main risk creating a new state involves running older functions in unexpected states. This keeps mintTo
from being called for platform airdrops. This comment is to note down where the expected mitigations are, no response needed.
function computePlatformAirdropDigest(PlatformAirdrop calldata p) public view returns (bytes32) { | ||
// prettier-ignore | ||
return | ||
_hashTypedData(keccak256(abi.encode( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EIP712 digest, addresses most signature gotchas. Good to see.
revert InvalidSignature(); | ||
if (block.timestamp > p.signedDeadline) revert SignatureExpired(); | ||
uint256 mintId = LibOps.packId(p.edition, p.tier, p.scheduleNum); | ||
if (!_claimsBitmaps[mintId].toggle(p.signedClaimTicket)) revert SignatureAlreadyUsed(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we have the EIP712 signature verification that uses the entire PlatformAirdrop object (except the signature). Then we verify the expiry, then check if it has been used before. Checked toggle to make sure that when the bit is unset it returns true, and that seems to be the case. Reversion should keep toggle from turning the bit off. Should work as intended.
@@ -285,16 +307,13 @@ contract SuperMinterV1_1 is ISuperMinterV1_1, EIP712 { | |||
d.maxMintable = c.maxMintable; | |||
d.affiliateFeeBPS = c.affiliateFeeBPS; | |||
d.mode = c.mode; | |||
d.flags = _MINT_CREATED_FLAG | LibOps.toFlag(usePlatformSigner, _USE_PLATFORM_SIGNER_FLAG); | |||
d.flags = _MINT_CREATED_FLAG; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like everything is going to use the platform signer. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's correct
_requireMintOpen(d); | ||
|
||
if (d.mode != PLATFORM_AIRDROP) revert InvalidMode(); | ||
_verifyAndClaimPlatfromAidropSignature(d, p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signature check, expiry check, and replay check are all in this method.
if (d.mode != PLATFORM_AIRDROP) revert InvalidMode(); | ||
_verifyAndClaimPlatfromAidropSignature(d, p); | ||
|
||
_incrementPlatformAirdropMinted(d, p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quantity check is inside this method
/* ------------------------- MINT --------------------------- */ | ||
|
||
ISoundEditionV2_1 edition = ISoundEditionV2_1(p.edition); | ||
fromTokenId = edition.airdrop(p.tier, p.to, p.signedQuantity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can only be reached if:
- the mint is a platform airdrop
- there is a valid platform signature by the platform specified during mint creation (
createEditionMint
sets the platform that is used to find the signer. - the signature hasn't expired
- the claim ticket number hasn't been used yet
- the minting doesn't go over the max mintable
As long as this matches the intended restrictions, then this should be good to go.
/** | ||
* @inheritdoc ISuperMinterV1_1 | ||
*/ | ||
function setSigner( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a change where the only signer used going forward is going to be the platform signer for the platform declared as part of the mint creation. This means no more declaring a specific signer, instead for VERIFY_SIGNATURE mint you'll declare a platform and the platform will will use their enrolled signer. Is this correct? The change on line 1119 seems to support this being the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct
Perfect summary, thank you Matt! |
91993c2
to
802a7d7
Compare
* 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]>
* 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]>
SuperMinterV1_1: