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

refactor: deprecate supply royalty #267

Merged
merged 6 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/great-camels-impress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@zoralabs/zora-1155-contracts": minor
---

Supply royalties are no longer supported
50 changes: 13 additions & 37 deletions packages/1155-contracts/src/nft/ZoraCreator1155Impl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ contract ZoraCreator1155Impl is
_requireAdminOrRole(msg.sender, tokenIds[i], PERMISSION_BIT_MINTER);
}
}

_mintBatch(recipient, tokenIds, quantities, data);
}

Expand Down Expand Up @@ -553,61 +554,36 @@ contract ZoraCreator1155Impl is
return super.supportsInterface(interfaceId) || interfaceId == type(IZoraCreator1155).interfaceId || ERC1155Upgradeable.supportsInterface(interfaceId);
}

function _handleSupplyRoyalty(uint256 tokenId, uint256 mintAmount, bytes memory data) internal returns (uint256 totalRoyaltyMints) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we're ok removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup got the approval!

uint256 royaltyMintSchedule = royalties[tokenId].royaltyMintSchedule;
if (royaltyMintSchedule == 0) {
royaltyMintSchedule = royalties[CONTRACT_BASE_ID].royaltyMintSchedule;
}
if (royaltyMintSchedule == 0) {
// If we still have no schedule, return 0 supply royalty.
return 0;
}
uint256 maxSupply = tokens[tokenId].maxSupply;
uint256 totalMinted = tokens[tokenId].totalMinted;

totalRoyaltyMints = (mintAmount + (totalMinted % royaltyMintSchedule)) / (royaltyMintSchedule - 1);
totalRoyaltyMints = MathUpgradeable.min(totalRoyaltyMints, maxSupply - (mintAmount + totalMinted));
if (totalRoyaltyMints > 0) {
address royaltyRecipient = royalties[tokenId].royaltyRecipient;
if (royaltyRecipient == address(0)) {
royaltyRecipient = royalties[CONTRACT_BASE_ID].royaltyRecipient;
}
// If we have no recipient set, return 0 supply royalty.
if (royaltyRecipient == address(0)) {
return 0;
}
super._mint(royaltyRecipient, tokenId, totalRoyaltyMints, data);
}
}

/// Generic 1155 function overrides ///

/// @notice Mint function that 1) checks quantity and 2) handles supply royalty 3) keeps track of allowed tokens
/// @notice Mint function that 1) checks quantity 2) keeps track of allowed tokens
/// @param to to mint to
/// @param id token id to mint
/// @param amount of tokens to mint
/// @param data as specified by 1155 standard
function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal virtual override {
uint256 supplyRoyaltyMints = _handleSupplyRoyalty(id, amount, data);
_requireCanMintQuantity(id, amount + supplyRoyaltyMints);
_requireCanMintQuantity(id, amount);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need this check?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes for limited supply nfts.


tokens[id].totalMinted += amount;

super._mint(to, id, amount, data);
tokens[id].totalMinted += amount + supplyRoyaltyMints;
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we still want to track totalMinted ? are we using that anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah good catch - @iainnash wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just checked our frontend repo and they're using it - i'll add that back

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep we want to track this still.

}

/// @notice Mint batch function that 1) checks quantity and 2) handles supply royalty 3) keeps track of allowed tokens
/// @notice Mint batch function that 1) checks quantity and 2) keeps track of allowed tokens
/// @param to to mint to
/// @param ids token ids to mint
/// @param amounts of tokens to mint
/// @param data as specified by 1155 standard
function _mintBatch(address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data) internal virtual override {
super._mintBatch(to, ids, amounts, data);
uint256 numTokens = ids.length;

for (uint256 i = 0; i < ids.length; ++i) {
uint256 supplyRoyaltyMints = _handleSupplyRoyalty(ids[i], amounts[i], data);
_requireCanMintQuantity(ids[i], amounts[i] + supplyRoyaltyMints);
tokens[ids[i]].totalMinted += amounts[i] + supplyRoyaltyMints;
for (uint256 i; i < numTokens; ++i) {
_requireCanMintQuantity(ids[i], amounts[i]);

tokens[ids[i]].totalMinted += amounts[i];
}

super._mintBatch(to, ids, amounts, data);
}

/// @notice Burns a batch of tokens
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ abstract contract CreatorRoyaltiesControl is CreatorRoyaltiesStorageV1, SharedBa
}

function _updateRoyalties(uint256 tokenId, RoyaltyConfiguration memory configuration) internal {
// Don't allow 100% supply royalties
if (configuration.royaltyMintSchedule == 1) {
// Deprecate supply royalty support
if (configuration.royaltyMintSchedule != 0) {
revert InvalidMintSchedule();
}
// Don't allow setting royalties to burn address
Expand Down
24 changes: 6 additions & 18 deletions packages/1155-contracts/test/factory/ZoraCreator1155Factory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -77,38 +77,26 @@ contract ZoraCreator1155FactoryTest is Test {
assertEq(address(minters[2]), address(3));
}

function test_createContract(
string memory contractURI,
string memory name,
uint32 royaltyBPS,
uint32 royaltyMintSchedule,
address royaltyRecipient,
address payable admin
) external {
function test_createContract(string memory contractURI, string memory name, uint32 royaltyBPS, address royaltyRecipient, address payable admin) external {
// If the factory is the admin, the admin flag is cleared
// during multicall breaking a further test assumption.
// Additionally, this case makes no sense from a user perspective.
vm.assume(admin != payable(address(factory)));
vm.assume(royaltyMintSchedule != 1);
// Assume royalty recipient is not 0
vm.assume(royaltyRecipient != payable(address(0)));
bytes[] memory initSetup = new bytes[](1);
initSetup[0] = abi.encodeWithSelector(IZoraCreator1155.setupNewToken.selector, "ipfs://asdfadsf", 100);
address deployedAddress = factory.createContract(
contractURI,
name,
ICreatorRoyaltiesControl.RoyaltyConfiguration({
royaltyBPS: royaltyBPS,
royaltyRecipient: royaltyRecipient,
royaltyMintSchedule: royaltyMintSchedule
}),
ICreatorRoyaltiesControl.RoyaltyConfiguration({royaltyBPS: royaltyBPS, royaltyRecipient: royaltyRecipient, royaltyMintSchedule: 0}),
admin,
initSetup
);
ZoraCreator1155Impl target = ZoraCreator1155Impl(deployedAddress);

ICreatorRoyaltiesControl.RoyaltyConfiguration memory config = target.getRoyalties(0);
assertEq(config.royaltyMintSchedule, royaltyMintSchedule);
assertEq(config.royaltyMintSchedule, 0);
assertEq(config.royaltyBPS, royaltyBPS);
assertEq(config.royaltyRecipient, royaltyRecipient);
assertEq(target.permissions(0, admin), target.PERMISSION_BIT_ADMIN());
Expand Down Expand Up @@ -170,7 +158,7 @@ contract ZoraCreator1155FactoryTest is Test {
ICreatorRoyaltiesControl.RoyaltyConfiguration memory royaltyConfig = ICreatorRoyaltiesControl.RoyaltyConfiguration({
royaltyBPS: 10,
royaltyRecipient: vm.addr(5),
royaltyMintSchedule: 100
royaltyMintSchedule: 0
});
bytes[] memory initSetup = new bytes[](1);
initSetup[0] = abi.encodeWithSelector(IZoraCreator1155.setupNewToken.selector, "ipfs://asdfadsf", 100);
Expand Down Expand Up @@ -220,7 +208,7 @@ contract ZoraCreator1155FactoryTest is Test {
ICreatorRoyaltiesControl.RoyaltyConfiguration memory royaltyConfig = ICreatorRoyaltiesControl.RoyaltyConfiguration({
royaltyBPS: 10,
royaltyRecipient: vm.addr(5),
royaltyMintSchedule: 100
royaltyMintSchedule: 0
});
bytes[] memory initSetup = new bytes[](1);
initSetup[0] = abi.encodeWithSelector(IZoraCreator1155.setupNewToken.selector, "ipfs://asdfadsf", 100);
Expand All @@ -241,7 +229,7 @@ contract ZoraCreator1155FactoryTest is Test {
ICreatorRoyaltiesControl.RoyaltyConfiguration memory royaltyConfig = ICreatorRoyaltiesControl.RoyaltyConfiguration({
royaltyBPS: 10,
royaltyRecipient: vm.addr(5),
royaltyMintSchedule: 100
royaltyMintSchedule: 0
});
bytes[] memory initSetup = new bytes[](1);
initSetup[0] = abi.encodeWithSelector(IZoraCreator1155.setupNewToken.selector, "ipfs://asdfadsf", 100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ library Zora1155PremintFixtures {
}

function defaultRoyaltyConfig(address royaltyRecipient) internal pure returns (ICreatorRoyaltiesControl.RoyaltyConfiguration memory) {
return ICreatorRoyaltiesControl.RoyaltyConfiguration({royaltyBPS: 10, royaltyRecipient: royaltyRecipient, royaltyMintSchedule: 100});
return ICreatorRoyaltiesControl.RoyaltyConfiguration({royaltyBPS: 10, royaltyRecipient: royaltyRecipient, royaltyMintSchedule: 0});
}

function makeDefaultTokenCreationConfig(IMinter1155 fixedPriceMinter, address royaltyRecipient) internal pure returns (TokenCreationConfig memory) {
Expand Down
Loading
Loading