From 129ab6f59fc658766f44a7cfa7387a9653cbc2e5 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Wed, 8 Dec 2021 16:10:27 -0400 Subject: [PATCH 01/51] Add initial contracts for royalties --- contracts/interfaces/draft-IERC2981.sol | 7 ++ .../ERC721/extensions/draft-ERC721Royalty.sol | 69 +++++++++++++++++++ .../extensions/draft-IERC721Royalty.sol | 31 +++++++++ 3 files changed, 107 insertions(+) create mode 100644 contracts/interfaces/draft-IERC2981.sol create mode 100644 contracts/token/ERC721/extensions/draft-ERC721Royalty.sol create mode 100644 contracts/token/ERC721/extensions/draft-IERC721Royalty.sol diff --git a/contracts/interfaces/draft-IERC2981.sol b/contracts/interfaces/draft-IERC2981.sol new file mode 100644 index 00000000000..1d531d9a9de --- /dev/null +++ b/contracts/interfaces/draft-IERC2981.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/draft-ERC721Royalty.sol) + +pragma solidity ^0.8.0; + +import "../token/ERC721/extensions/draft-IERC721Royalty.sol"; + diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol new file mode 100644 index 00000000000..fa46a44fdc4 --- /dev/null +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/draft-ERC721Royalty.sol) + +pragma solidity ^0.8.0; + +import "./draft-IERC721Royalty.sol"; + +/** + * @dev TBD + * + * _Available since v4.5._ + */ +abstract contract ERC721Royalty is IERC721Royalty { + struct RoyaltyInfo { + address receiver; + uint256 royaltyAmount; + } + + RoyaltyInfo private _royaltyInfo; + mapping(uint256 => RoyaltyInfo) private _tokenRoyalty; + + /*@dev Sets tokens royalties + * + * Requirements: + * - `tokenId` must be already mined. + * - `recipient` cannot be the zero address. + * - `value` must indicate the percentage value using two decimals. + */ + function _setTokenRoyalty( + uint256 tokenId, + address recipient, + uint256 value + ) internal { + require(value <= 10000, 'ERC2981Royalties: Too high'); + _tokenRoyalty[tokenId] = RoyaltyInfo(recipient, value); + } + + /*@dev Sets global royalty + * + * Requirements: + * - `recipient` cannot be the zero address. + * - `value` must indicate the percentage value using two decimals. + */ + function _setRoyalty( + address recipient, + uint256 value + ) internal { + _royaltyInfo = RoyaltyInfo(recipient, value); + } + + /** + * @dev See {IERC721Royalty-royaltyInfo} + */ + function royaltyInfo( + uint256 _tokenId, + uint256 _salePrice + ) external view returns ( + address receiver, + uint256 royaltyAmount + ){ + if(_tokenRoyalty[_tokenId].receiver != address(0)){ + receiver = _tokenRoyalty[_tokenId].receiver; + royaltyAmount = (_salePrice * _tokenRoyalty[_tokenId].royaltyAmount) / 10000; + }else{ + receiver = _royaltyInfo.receiver; + royaltyAmount = (_salePrice * _royaltyInfo.royaltyAmount) / 10000; + } + } +} diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol new file mode 100644 index 00000000000..7ebc9a85844 --- /dev/null +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/draft-ERC721Royalty.sol) + +pragma solidity ^0.8.0; + +import "../../../interfaces/IERC165.sol"; + +/** + * @dev Interface for the NFT Royalty Standard + * + * _Available since v4.5._ + */ +interface IERC721Royalty is IERC165{ + + /** + * @dev Called with the sale price to determine how much royalty + * is owed and to whom. + * + * Requirements: + * - `_tokenId` must be already mined, and have its royalty info set + * - `_salePrice` cannot be the zero. + * + */ + function royaltyInfo( + uint256 _tokenId, + uint256 _salePrice + ) external view returns ( + address receiver, + uint256 royaltyAmount + ); +} From 3fa2485687964a38497a5cbc939f9033b9887377 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 9 Dec 2021 15:37:10 -0400 Subject: [PATCH 02/51] Update interface helper/add tests --- contracts/mocks/ERC721RoyaltyMock.sol | 21 ++++ .../ERC721/extensions/draft-ERC721Royalty.sol | 26 ++-- .../extensions/draft-IERC721Royalty.sol | 2 +- .../ERC721/extensions/ERC721Royalty.test.js | 118 ++++++++++++++++++ .../SupportsInterface.behavior.js | 3 + 5 files changed, 160 insertions(+), 10 deletions(-) create mode 100644 contracts/mocks/ERC721RoyaltyMock.sol create mode 100644 test/token/ERC721/extensions/ERC721Royalty.test.js diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol new file mode 100644 index 00000000000..fbcaba4c669 --- /dev/null +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../token/ERC721/extensions/draft-ERC721Royalty.sol"; + +contract ERC721RoyaltyMock is ERC721Royalty { + bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a; + + constructor () { + _registerInterface(_INTERFACE_ID_ERC2981); + } + + function setTokenRoyalty(uint256 tokenId, address recipient, uint256 value) public { + _setTokenRoyalty(tokenId, recipient, value); + } + + function setRoyalty(address recipient, uint256 value) public { + _setRoyalty(recipient, value); + } +} diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index fa46a44fdc4..745e4346a08 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -4,13 +4,15 @@ pragma solidity ^0.8.0; import "./draft-IERC721Royalty.sol"; +import "../../../utils/introspection/ERC165Storage.sol"; +import "hardhat/console.sol"; /** * @dev TBD * * _Available since v4.5._ */ -abstract contract ERC721Royalty is IERC721Royalty { +abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { struct RoyaltyInfo { address receiver; uint256 royaltyAmount; @@ -30,8 +32,10 @@ abstract contract ERC721Royalty is IERC721Royalty { uint256 tokenId, address recipient, uint256 value - ) internal { - require(value <= 10000, 'ERC2981Royalties: Too high'); + ) internal virtual { + require(value < 100, 'ERC2981: Royalty percentage is too high'); + require(recipient != address(0), "ERC2981: Invalid recipient"); + _tokenRoyalty[tokenId] = RoyaltyInfo(recipient, value); } @@ -39,12 +43,15 @@ abstract contract ERC721Royalty is IERC721Royalty { * * Requirements: * - `recipient` cannot be the zero address. - * - `value` must indicate the percentage value using two decimals. + * - `value` must indicate the percentage value. */ function _setRoyalty( address recipient, uint256 value - ) internal { + ) internal virtual { + require(value < 100, 'ERC2981: Royalty percentage is too high'); + require(recipient != address(0), "ERC2981: Invalid recipient"); + _royaltyInfo = RoyaltyInfo(recipient, value); } @@ -54,16 +61,17 @@ abstract contract ERC721Royalty is IERC721Royalty { function royaltyInfo( uint256 _tokenId, uint256 _salePrice - ) external view returns ( + ) external view override returns ( address receiver, uint256 royaltyAmount ){ if(_tokenRoyalty[_tokenId].receiver != address(0)){ - receiver = _tokenRoyalty[_tokenId].receiver; - royaltyAmount = (_salePrice * _tokenRoyalty[_tokenId].royaltyAmount) / 10000; + RoyaltyInfo memory royalty = _tokenRoyalty[_tokenId]; + receiver = royalty.receiver; + royaltyAmount = (_salePrice * royalty.royaltyAmount) / 100; }else{ receiver = _royaltyInfo.receiver; - royaltyAmount = (_salePrice * _royaltyInfo.royaltyAmount) / 10000; + royaltyAmount = (_salePrice * _royaltyInfo.royaltyAmount) / 100; } } } diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol index 7ebc9a85844..ff4c3c57c98 100644 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -10,7 +10,7 @@ import "../../../interfaces/IERC165.sol"; * * _Available since v4.5._ */ -interface IERC721Royalty is IERC165{ +interface IERC721Royalty is IERC165 { /** * @dev Called with the sale price to determine how much royalty diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js new file mode 100644 index 00000000000..a08aaee194a --- /dev/null +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -0,0 +1,118 @@ +const { BN, constants, expectRevert } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); +const { ZERO_ADDRESS } = constants; + +const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior'); + +const ERC721RoyaltyMock = artifacts.require('ERC721RoyaltyMock'); + +contract('ERC721Royalty', function (accounts) { + const [ account1 ] = accounts; + const tokenId1 = new BN('1'); + const tokenId2 = new BN('2'); + const salePrice = new BN('1000'); + const royaltyAmount = new BN('10'); + + beforeEach(async function () { + this.token = await ERC721RoyaltyMock.new(); + }); + + it("supports interface", async function () { + shouldSupportInterfaces(['ERC2981']); + }); + + describe("global royalty", function () { + beforeEach(async function () { + await this.token.setRoyalty(account1, royaltyAmount); + }); + + it("updates royalty amount", async function () { + const newAmount = new BN('25'); + let royalty = new BN((salePrice * royaltyAmount)/100); + //Initial royalty check + const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(initInfo.receiver).to.be.equal(account1); + expect(initInfo.royaltyAmount).to.be.bignumber.equal(royalty); + + //Updated royalty check + await this.token.setRoyalty(account1, newAmount); + royalty = new BN((salePrice * newAmount)/100); + const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(newInfo.receiver).to.be.equal(account1); + expect(newInfo.royaltyAmount).to.be.bignumber.equal(royalty); + }); + + it("holds same royalty value for different tokens", async function () { + const newAmount = new BN('20'); + await this.token.setRoyalty(account1, newAmount); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + + expect(token1Info.royaltyAmount).to.be.bignumber.equal(token2Info.royaltyAmount); + + }); + + it("reverts if invalid parameters", async function () { + await expectRevert( + this.token.setRoyalty(ZERO_ADDRESS, royaltyAmount), + 'ERC2981: Invalid recipient', + ); + + await expectRevert( + this.token.setRoyalty(account1, new BN('100')), + 'ERC2981: Royalty percentage is too high', + ); + }); + }); + + describe("token based royalty", function () { + beforeEach(async function () { + await this.token.setTokenRoyalty(tokenId1, account1, royaltyAmount); + }); + + it("updates royalty amount", async function () { + const newAmount = new BN('25'); + let royalty = new BN((salePrice * royaltyAmount)/100); + //Initial royalty check + const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(initInfo.receiver).to.be.equal(account1); + expect(initInfo.royaltyAmount).to.be.bignumber.equal(royalty); + + //Updated royalty check + await this.token.setTokenRoyalty(tokenId1, account1, newAmount); + royalty = new BN((salePrice * newAmount)/100); + const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(newInfo.receiver).to.be.equal(account1); + expect(newInfo.royaltyAmount).to.be.bignumber.equal(royalty); + }); + + it("holds different values for different tokens", async function () { + const newAmount = new BN('20'); + await this.token.setTokenRoyalty(tokenId2, account1, newAmount); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + + //must be different even at the same SalePrice + expect(token1Info.royaltyAmount).to.not.be.equal(token2Info.royaltyAmount); + + }); + + it("reverts if invalid parameters", async function () { + await expectRevert( + this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyAmount), + 'ERC2981: Invalid recipient', + ); + + await expectRevert( + this.token.setTokenRoyalty(tokenId1, account1, new BN('100')), + 'ERC2981: Royalty percentage is too high', + ); + }); + }); +}); diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index 2027b4057d0..3f6f5228b64 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -74,6 +74,9 @@ const INTERFACES = { 'proposalEta(uint256)', 'queue(address[],uint256[],bytes[],bytes32)', ], + ERC2981: [ + 'royaltyInfo(uint256,uint256)', + ], }; const INTERFACE_IDS = {}; From 7fb8bfd8b7fcd8b4cf5bec9fc34919b22984b225 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Fri, 10 Dec 2021 13:28:37 -0400 Subject: [PATCH 03/51] Update 2981 tests --- contracts/interfaces/draft-IERC2981.sol | 1 - contracts/mocks/ERC721RoyaltyMock.sol | 8 +++- .../ERC721/extensions/draft-ERC721Royalty.sol | 48 +++++++++---------- .../extensions/draft-IERC721Royalty.sol | 28 +++++------ .../ERC721/extensions/ERC721Royalty.test.js | 40 +++++++--------- 5 files changed, 58 insertions(+), 67 deletions(-) diff --git a/contracts/interfaces/draft-IERC2981.sol b/contracts/interfaces/draft-IERC2981.sol index 1d531d9a9de..890b3ceefeb 100644 --- a/contracts/interfaces/draft-IERC2981.sol +++ b/contracts/interfaces/draft-IERC2981.sol @@ -4,4 +4,3 @@ pragma solidity ^0.8.0; import "../token/ERC721/extensions/draft-IERC721Royalty.sol"; - diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index fbcaba4c669..b8d5301783a 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -7,11 +7,15 @@ import "../token/ERC721/extensions/draft-ERC721Royalty.sol"; contract ERC721RoyaltyMock is ERC721Royalty { bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a; - constructor () { + constructor() { _registerInterface(_INTERFACE_ID_ERC2981); } - function setTokenRoyalty(uint256 tokenId, address recipient, uint256 value) public { + function setTokenRoyalty( + uint256 tokenId, + address recipient, + uint256 value + ) public { _setTokenRoyalty(tokenId, recipient, value); } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 745e4346a08..e9f88c1e191 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -22,34 +22,31 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { mapping(uint256 => RoyaltyInfo) private _tokenRoyalty; /*@dev Sets tokens royalties - * - * Requirements: - * - `tokenId` must be already mined. - * - `recipient` cannot be the zero address. - * - `value` must indicate the percentage value using two decimals. - */ + * + * Requirements: + * - `tokenId` must be already mined. + * - `recipient` cannot be the zero address. + * - `value` must indicate the percentage value using two decimals. + */ function _setTokenRoyalty( uint256 tokenId, address recipient, uint256 value ) internal virtual { - require(value < 100, 'ERC2981: Royalty percentage is too high'); + require(value < 100, "ERC2981: Royalty percentage is too high"); require(recipient != address(0), "ERC2981: Invalid recipient"); _tokenRoyalty[tokenId] = RoyaltyInfo(recipient, value); } /*@dev Sets global royalty - * - * Requirements: - * - `recipient` cannot be the zero address. - * - `value` must indicate the percentage value. - */ - function _setRoyalty( - address recipient, - uint256 value - ) internal virtual { - require(value < 100, 'ERC2981: Royalty percentage is too high'); + * + * Requirements: + * - `recipient` cannot be the zero address. + * - `value` must indicate the percentage value. + */ + function _setRoyalty(address recipient, uint256 value) internal virtual { + require(value < 100, "ERC2981: Royalty percentage is too high"); require(recipient != address(0), "ERC2981: Invalid recipient"); _royaltyInfo = RoyaltyInfo(recipient, value); @@ -58,18 +55,17 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { /** * @dev See {IERC721Royalty-royaltyInfo} */ - function royaltyInfo( - uint256 _tokenId, - uint256 _salePrice - ) external view override returns ( - address receiver, - uint256 royaltyAmount - ){ - if(_tokenRoyalty[_tokenId].receiver != address(0)){ + function royaltyInfo(uint256 _tokenId, uint256 _salePrice) + external + view + override + returns (address receiver, uint256 royaltyAmount) + { + if (_tokenRoyalty[_tokenId].receiver != address(0)) { RoyaltyInfo memory royalty = _tokenRoyalty[_tokenId]; receiver = royalty.receiver; royaltyAmount = (_salePrice * royalty.royaltyAmount) / 100; - }else{ + } else { receiver = _royaltyInfo.receiver; royaltyAmount = (_salePrice * _royaltyInfo.royaltyAmount) / 100; } diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol index ff4c3c57c98..4275f623786 100644 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -11,21 +11,17 @@ import "../../../interfaces/IERC165.sol"; * _Available since v4.5._ */ interface IERC721Royalty is IERC165 { - /** - * @dev Called with the sale price to determine how much royalty - * is owed and to whom. - * - * Requirements: - * - `_tokenId` must be already mined, and have its royalty info set - * - `_salePrice` cannot be the zero. - * - */ - function royaltyInfo( - uint256 _tokenId, - uint256 _salePrice - ) external view returns ( - address receiver, - uint256 royaltyAmount - ); + * @dev Called with the sale price to determine how much royalty + * is owed and to whom. + * + * Requirements: + * - `_tokenId` must be already mined, and have its royalty info set + * - `_salePrice` cannot be the zero. + * + */ + function royaltyInfo(uint256 _tokenId, uint256 _salePrice) + external + view + returns (address receiver, uint256 royaltyAmount); } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index a08aaee194a..174c2264494 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -17,34 +17,32 @@ contract('ERC721Royalty', function (accounts) { this.token = await ERC721RoyaltyMock.new(); }); - it("supports interface", async function () { - shouldSupportInterfaces(['ERC2981']); - }); + shouldSupportInterfaces(['ERC2981']); - describe("global royalty", function () { + describe('global royalty', function () { beforeEach(async function () { await this.token.setRoyalty(account1, royaltyAmount); }); - it("updates royalty amount", async function () { + it('updates royalty amount', async function () { const newAmount = new BN('25'); - let royalty = new BN((salePrice * royaltyAmount)/100); - //Initial royalty check + let royalty = new BN((salePrice * royaltyAmount) / 100); + // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(initInfo.receiver).to.be.equal(account1); expect(initInfo.royaltyAmount).to.be.bignumber.equal(royalty); - //Updated royalty check + // Updated royalty check await this.token.setRoyalty(account1, newAmount); - royalty = new BN((salePrice * newAmount)/100); + royalty = new BN((salePrice * newAmount) / 100); const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(newInfo.receiver).to.be.equal(account1); expect(newInfo.royaltyAmount).to.be.bignumber.equal(royalty); }); - it("holds same royalty value for different tokens", async function () { + it('holds same royalty value for different tokens', async function () { const newAmount = new BN('20'); await this.token.setRoyalty(account1, newAmount); @@ -52,10 +50,9 @@ contract('ERC721Royalty', function (accounts) { const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); expect(token1Info.royaltyAmount).to.be.bignumber.equal(token2Info.royaltyAmount); - }); - it("reverts if invalid parameters", async function () { + it('reverts if invalid parameters', async function () { await expectRevert( this.token.setRoyalty(ZERO_ADDRESS, royaltyAmount), 'ERC2981: Invalid recipient', @@ -68,42 +65,41 @@ contract('ERC721Royalty', function (accounts) { }); }); - describe("token based royalty", function () { + describe('token based royalty', function () { beforeEach(async function () { await this.token.setTokenRoyalty(tokenId1, account1, royaltyAmount); }); - it("updates royalty amount", async function () { + it('updates royalty amount', async function () { const newAmount = new BN('25'); - let royalty = new BN((salePrice * royaltyAmount)/100); - //Initial royalty check + let royalty = new BN((salePrice * royaltyAmount) / 100); + // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(initInfo.receiver).to.be.equal(account1); expect(initInfo.royaltyAmount).to.be.bignumber.equal(royalty); - //Updated royalty check + // Updated royalty check await this.token.setTokenRoyalty(tokenId1, account1, newAmount); - royalty = new BN((salePrice * newAmount)/100); + royalty = new BN((salePrice * newAmount) / 100); const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(newInfo.receiver).to.be.equal(account1); expect(newInfo.royaltyAmount).to.be.bignumber.equal(royalty); }); - it("holds different values for different tokens", async function () { + it('holds different values for different tokens', async function () { const newAmount = new BN('20'); await this.token.setTokenRoyalty(tokenId2, account1, newAmount); const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - //must be different even at the same SalePrice + // must be different even at the same SalePrice expect(token1Info.royaltyAmount).to.not.be.equal(token2Info.royaltyAmount); - }); - it("reverts if invalid parameters", async function () { + it('reverts if invalid parameters', async function () { await expectRevert( this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyAmount), 'ERC2981: Invalid recipient', From e979b938623e7be1b97f930cde342960b82bc097 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Fri, 10 Dec 2021 13:47:49 -0400 Subject: [PATCH 04/51] Add documentation for 2981 implementation --- CHANGELOG.md | 1 + .../token/ERC721/extensions/draft-ERC721Royalty.sol | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0733af7cb91..57f9c761350 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased + * `ERC2891`: add a new extension of `ERC721` to handle royalty information.([]()) * `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977)) * `Math`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984)) * Preset contracts are now deprecated in favor of [Contracts Wizard](https://wizard.openzeppelin.com). ([#2986](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2986)) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index e9f88c1e191..7289e26dfcd 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -8,7 +8,11 @@ import "../../../utils/introspection/ERC165Storage.sol"; import "hardhat/console.sol"; /** - * @dev TBD + * @dev Implementation of the ERC721 Royalty extension allowing royalty information to be stored and retrieved, as defined in + * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. + * + * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setRoyalty} method to set a global + * royalty information. * * _Available since v4.5._ */ @@ -21,7 +25,8 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { RoyaltyInfo private _royaltyInfo; mapping(uint256 => RoyaltyInfo) private _tokenRoyalty; - /*@dev Sets tokens royalties + /* + * @dev Sets tokens royalties * * Requirements: * - `tokenId` must be already mined. @@ -39,7 +44,9 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { _tokenRoyalty[tokenId] = RoyaltyInfo(recipient, value); } - /*@dev Sets global royalty + /* + * + * @dev Sets global royalty * * Requirements: * - `recipient` cannot be the zero address. From 5f4499a5ccbbb36c478ba6579edc87a98c28a73a Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 08:19:28 -0400 Subject: [PATCH 05/51] Rename setRoyalty function --- contracts/mocks/ERC721RoyaltyMock.sol | 4 ++-- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index b8d5301783a..5ec35171d59 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -19,7 +19,7 @@ contract ERC721RoyaltyMock is ERC721Royalty { _setTokenRoyalty(tokenId, recipient, value); } - function setRoyalty(address recipient, uint256 value) public { - _setRoyalty(recipient, value); + function setGlobalRoyalty (address recipient, uint256 value) public { + _setGlobalRoyalty(recipient, value); } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 7289e26dfcd..f9da27fdc0b 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -11,7 +11,7 @@ import "hardhat/console.sol"; * @dev Implementation of the ERC721 Royalty extension allowing royalty information to be stored and retrieved, as defined in * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. * - * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setRoyalty} method to set a global + * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setGlobalRoyalty} method to set a global * royalty information. * * _Available since v4.5._ @@ -52,7 +52,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { * - `recipient` cannot be the zero address. * - `value` must indicate the percentage value. */ - function _setRoyalty(address recipient, uint256 value) internal virtual { + function _setGlobalRoyalty(address recipient, uint256 value) internal virtual { require(value < 100, "ERC2981: Royalty percentage is too high"); require(recipient != address(0), "ERC2981: Invalid recipient"); From 0f814dc8bc8ee0226a60a1b94fe2e44ae0998b01 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 08:31:21 -0400 Subject: [PATCH 06/51] Rename variables --- contracts/interfaces/IERC2981.sol | 4 +- contracts/mocks/ERC721RoyaltyMock.sol | 8 ++-- .../ERC721/extensions/draft-ERC721Royalty.sol | 44 +++++++++---------- .../extensions/draft-IERC721Royalty.sol | 2 +- .../ERC721/extensions/ERC721Royalty.test.js | 32 +++++++------- 5 files changed, 45 insertions(+), 45 deletions(-) diff --git a/contracts/interfaces/IERC2981.sol b/contracts/interfaces/IERC2981.sol index 20a417c64d6..cf624d0e78c 100644 --- a/contracts/interfaces/IERC2981.sol +++ b/contracts/interfaces/IERC2981.sol @@ -14,10 +14,10 @@ interface IERC2981 is IERC165 { * @param tokenId - the NFT asset queried for royalty information * @param salePrice - the sale price of the NFT asset specified by `tokenId` * @return receiver - address of who should be sent the royalty payment - * @return royaltyAmount - the royalty payment amount for `salePrice` + * @return royaltyFraction - the royalty payment amount for `salePrice` */ function royaltyInfo(uint256 tokenId, uint256 salePrice) external view - returns (address receiver, uint256 royaltyAmount); + returns (address receiver, uint256 royaltyFraction); } diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 5ec35171d59..437b95f387f 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -14,12 +14,12 @@ contract ERC721RoyaltyMock is ERC721Royalty { function setTokenRoyalty( uint256 tokenId, address recipient, - uint256 value + uint256 fraction ) public { - _setTokenRoyalty(tokenId, recipient, value); + _setTokenRoyalty(tokenId, recipient, fraction); } - function setGlobalRoyalty (address recipient, uint256 value) public { - _setGlobalRoyalty(recipient, value); + function setGlobalRoyalty (address recipient, uint256 fraction) public { + _setGlobalRoyalty(recipient, fraction); } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index f9da27fdc0b..a47c8d5fe2a 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -19,29 +19,29 @@ import "hardhat/console.sol"; abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { struct RoyaltyInfo { address receiver; - uint256 royaltyAmount; + uint256 royaltyFraction; } - RoyaltyInfo private _royaltyInfo; - mapping(uint256 => RoyaltyInfo) private _tokenRoyalty; + RoyaltyInfo private _globalRoyaltyInfo; + mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; /* * @dev Sets tokens royalties * * Requirements: * - `tokenId` must be already mined. - * - `recipient` cannot be the zero address. - * - `value` must indicate the percentage value using two decimals. + * - `receiver` cannot be the zero address. + * - `fraction` must indicate the percentage fraction using two decimals. */ function _setTokenRoyalty( uint256 tokenId, - address recipient, - uint256 value + address receiver, + uint256 fraction ) internal virtual { - require(value < 100, "ERC2981: Royalty percentage is too high"); - require(recipient != address(0), "ERC2981: Invalid recipient"); + require(fraction < 100, "ERC2981: Royalty percentage is too high"); + require(receiver != address(0), "ERC2981: Invalid receiver"); - _tokenRoyalty[tokenId] = RoyaltyInfo(recipient, value); + _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); } /* @@ -49,14 +49,14 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { * @dev Sets global royalty * * Requirements: - * - `recipient` cannot be the zero address. - * - `value` must indicate the percentage value. + * - `receiver` cannot be the zero address. + * - `fraction` must indicate the percentage fraction. */ - function _setGlobalRoyalty(address recipient, uint256 value) internal virtual { - require(value < 100, "ERC2981: Royalty percentage is too high"); - require(recipient != address(0), "ERC2981: Invalid recipient"); + function _setGlobalRoyalty(address receiver, uint256 fraction) internal virtual { + require(fraction < 100, "ERC2981: Royalty percentage is too high"); + require(receiver != address(0), "ERC2981: Invalid receiver"); - _royaltyInfo = RoyaltyInfo(recipient, value); + _globalRoyaltyInfo = RoyaltyInfo(receiver, fraction); } /** @@ -66,15 +66,15 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { external view override - returns (address receiver, uint256 royaltyAmount) + returns (address receiver, uint256 royaltyFraction) { - if (_tokenRoyalty[_tokenId].receiver != address(0)) { - RoyaltyInfo memory royalty = _tokenRoyalty[_tokenId]; + if (_tokenRoyaltyInfo[_tokenId].receiver != address(0)) { + RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; receiver = royalty.receiver; - royaltyAmount = (_salePrice * royalty.royaltyAmount) / 100; + royaltyFraction = (_salePrice * royalty.royaltyFraction) / 100; } else { - receiver = _royaltyInfo.receiver; - royaltyAmount = (_salePrice * _royaltyInfo.royaltyAmount) / 100; + receiver = _globalRoyaltyInfo.receiver; + royaltyFraction = (_salePrice * _globalRoyaltyInfo.royaltyFraction) / 100; } } } diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol index 4275f623786..cb2cc98bc1d 100644 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -23,5 +23,5 @@ interface IERC721Royalty is IERC165 { function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view - returns (address receiver, uint256 royaltyAmount); + returns (address receiver, uint256 royaltyFraction); } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 174c2264494..8444733533f 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -11,7 +11,7 @@ contract('ERC721Royalty', function (accounts) { const tokenId1 = new BN('1'); const tokenId2 = new BN('2'); const salePrice = new BN('1000'); - const royaltyAmount = new BN('10'); + const royaltyFraction = new BN('10'); beforeEach(async function () { this.token = await ERC721RoyaltyMock.new(); @@ -21,45 +21,45 @@ contract('ERC721Royalty', function (accounts) { describe('global royalty', function () { beforeEach(async function () { - await this.token.setRoyalty(account1, royaltyAmount); + await this.token.setGlobalRoyalty (account1, royaltyFraction); }); it('updates royalty amount', async function () { const newAmount = new BN('25'); - let royalty = new BN((salePrice * royaltyAmount) / 100); + let royalty = new BN((salePrice * royaltyFraction) / 100); // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(initInfo.receiver).to.be.equal(account1); - expect(initInfo.royaltyAmount).to.be.bignumber.equal(royalty); + expect(initInfo.royaltyFraction).to.be.bignumber.equal(royalty); // Updated royalty check - await this.token.setRoyalty(account1, newAmount); + await this.token.setGlobalRoyalty (account1, newAmount); royalty = new BN((salePrice * newAmount) / 100); const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(newInfo.receiver).to.be.equal(account1); - expect(newInfo.royaltyAmount).to.be.bignumber.equal(royalty); + expect(newInfo.royaltyFraction).to.be.bignumber.equal(royalty); }); it('holds same royalty value for different tokens', async function () { const newAmount = new BN('20'); - await this.token.setRoyalty(account1, newAmount); + await this.token.setGlobalRoyalty (account1, newAmount); const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - expect(token1Info.royaltyAmount).to.be.bignumber.equal(token2Info.royaltyAmount); + expect(token1Info.royaltyFraction).to.be.bignumber.equal(token2Info.royaltyFraction); }); it('reverts if invalid parameters', async function () { await expectRevert( - this.token.setRoyalty(ZERO_ADDRESS, royaltyAmount), + this.token.setGlobalRoyalty (ZERO_ADDRESS, royaltyFraction), 'ERC2981: Invalid recipient', ); await expectRevert( - this.token.setRoyalty(account1, new BN('100')), + this.token.setGlobalRoyalty (account1, new BN('100')), 'ERC2981: Royalty percentage is too high', ); }); @@ -67,17 +67,17 @@ contract('ERC721Royalty', function (accounts) { describe('token based royalty', function () { beforeEach(async function () { - await this.token.setTokenRoyalty(tokenId1, account1, royaltyAmount); + await this.token.setTokenRoyalty(tokenId1, account1, royaltyFraction); }); it('updates royalty amount', async function () { const newAmount = new BN('25'); - let royalty = new BN((salePrice * royaltyAmount) / 100); + let royalty = new BN((salePrice * royaltyFraction) / 100); // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(initInfo.receiver).to.be.equal(account1); - expect(initInfo.royaltyAmount).to.be.bignumber.equal(royalty); + expect(initInfo.royaltyFraction).to.be.bignumber.equal(royalty); // Updated royalty check await this.token.setTokenRoyalty(tokenId1, account1, newAmount); @@ -85,7 +85,7 @@ contract('ERC721Royalty', function (accounts) { const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(newInfo.receiver).to.be.equal(account1); - expect(newInfo.royaltyAmount).to.be.bignumber.equal(royalty); + expect(newInfo.royaltyFraction).to.be.bignumber.equal(royalty); }); it('holds different values for different tokens', async function () { @@ -96,12 +96,12 @@ contract('ERC721Royalty', function (accounts) { const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); // must be different even at the same SalePrice - expect(token1Info.royaltyAmount).to.not.be.equal(token2Info.royaltyAmount); + expect(token1Info.royaltyFraction).to.not.be.equal(token2Info.royaltyFraction); }); it('reverts if invalid parameters', async function () { await expectRevert( - this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyAmount), + this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyFraction), 'ERC2981: Invalid recipient', ); From d8a82c831f9c46ed0607833a14441ee7a8a02749 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 09:05:57 -0400 Subject: [PATCH 07/51] Remove ERC165Storage inheritance --- contracts/mocks/ERC721RoyaltyMock.sol | 4 ++-- .../ERC721/extensions/draft-ERC721Royalty.sol | 14 +++++++++++--- test/token/ERC721/extensions/ERC721Royalty.test.js | 4 ++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 437b95f387f..37e7a4f725a 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -7,8 +7,8 @@ import "../token/ERC721/extensions/draft-ERC721Royalty.sol"; contract ERC721RoyaltyMock is ERC721Royalty { bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a; - constructor() { - _registerInterface(_INTERFACE_ID_ERC2981); + constructor() public { + } function setTokenRoyalty( diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index a47c8d5fe2a..c563d7330b4 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -4,8 +4,7 @@ pragma solidity ^0.8.0; import "./draft-IERC721Royalty.sol"; -import "../../../utils/introspection/ERC165Storage.sol"; -import "hardhat/console.sol"; +import "../../../utils/introspection/ERC165.sol"; /** * @dev Implementation of the ERC721 Royalty extension allowing royalty information to be stored and retrieved, as defined in @@ -16,7 +15,7 @@ import "hardhat/console.sol"; * * _Available since v4.5._ */ -abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { +abstract contract ERC721Royalty is ERC165, IERC721Royalty { struct RoyaltyInfo { address receiver; uint256 royaltyFraction; @@ -77,4 +76,13 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC165Storage { royaltyFraction = (_salePrice * _globalRoyaltyInfo.royaltyFraction) / 100; } } + + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { + return + interfaceId == type(IERC721Royalty).interfaceId || + super.supportsInterface(interfaceId); + } } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 8444733533f..87221db243d 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -55,7 +55,7 @@ contract('ERC721Royalty', function (accounts) { it('reverts if invalid parameters', async function () { await expectRevert( this.token.setGlobalRoyalty (ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid recipient', + 'ERC2981: Invalid receiver', ); await expectRevert( @@ -102,7 +102,7 @@ contract('ERC721Royalty', function (accounts) { it('reverts if invalid parameters', async function () { await expectRevert( this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid recipient', + 'ERC2981: Invalid receiver', ); await expectRevert( From 646380b3587440f25045ae7660767f1a6f4938cc Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 11:38:23 -0400 Subject: [PATCH 08/51] Add different denominator logic --- contracts/mocks/ERC721RoyaltyMock.sol | 6 +-- .../ERC721/extensions/draft-ERC721Royalty.sol | 31 +++++++----- .../ERC721/extensions/ERC721Royalty.test.js | 50 +++++++++---------- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 37e7a4f725a..06f757ebbb4 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -7,9 +7,7 @@ import "../token/ERC721/extensions/draft-ERC721Royalty.sol"; contract ERC721RoyaltyMock is ERC721Royalty { bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a; - constructor() public { - - } + constructor() public {} function setTokenRoyalty( uint256 tokenId, @@ -19,7 +17,7 @@ contract ERC721RoyaltyMock is ERC721Royalty { _setTokenRoyalty(tokenId, recipient, fraction); } - function setGlobalRoyalty (address recipient, uint256 fraction) public { + function setGlobalRoyalty(address recipient, uint256 fraction) public { _setGlobalRoyalty(recipient, fraction); } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index c563d7330b4..22ea3169ce6 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -49,7 +49,8 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { * * Requirements: * - `receiver` cannot be the zero address. - * - `fraction` must indicate the percentage fraction. + * - `fraction` must indicate the percentage fraction. Needs to be set appropriately + * according to the _feeDenominator granularity. */ function _setGlobalRoyalty(address receiver, uint256 fraction) internal virtual { require(fraction < 100, "ERC2981: Royalty percentage is too high"); @@ -61,28 +62,34 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { /** * @dev See {IERC721Royalty-royaltyInfo} */ - function royaltyInfo(uint256 _tokenId, uint256 _salePrice) - external - view - override - returns (address receiver, uint256 royaltyFraction) - { + function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { + address receiver; + uint256 royaltyAmount; + if (_tokenRoyaltyInfo[_tokenId].receiver != address(0)) { RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; receiver = royalty.receiver; - royaltyFraction = (_salePrice * royalty.royaltyFraction) / 100; + royaltyAmount = (_salePrice * royalty.royaltyFraction) / _feeDenominator(); } else { receiver = _globalRoyaltyInfo.receiver; - royaltyFraction = (_salePrice * _globalRoyaltyInfo.royaltyFraction) / 100; + royaltyAmount = (_salePrice * _globalRoyaltyInfo.royaltyFraction) / _feeDenominator(); } + + return (receiver, royaltyAmount); } /** * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { - return - interfaceId == type(IERC721Royalty).interfaceId || - super.supportsInterface(interfaceId); + return interfaceId == type(IERC721Royalty).interfaceId || super.supportsInterface(interfaceId); + } + + /** + * @dev Returns the percentage granularity being used. The default denominator is 10000 + * but it can be customized by an override. + */ + function _feeDenominator() internal pure virtual returns (uint256) { + return 10000; } } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 87221db243d..52e73f71674 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -21,45 +21,45 @@ contract('ERC721Royalty', function (accounts) { describe('global royalty', function () { beforeEach(async function () { - await this.token.setGlobalRoyalty (account1, royaltyFraction); + await this.token.setGlobalRoyalty(account1, royaltyFraction); }); it('updates royalty amount', async function () { - const newAmount = new BN('25'); - let royalty = new BN((salePrice * royaltyFraction) / 100); + const newPercentage = new BN('25'); + let royalty = new BN((salePrice * royaltyFraction) / 10000); // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); - expect(initInfo.receiver).to.be.equal(account1); - expect(initInfo.royaltyFraction).to.be.bignumber.equal(royalty); + expect(initInfo[0]).to.be.equal(account1); + expect(initInfo[1]).to.be.bignumber.equal(royalty); // Updated royalty check - await this.token.setGlobalRoyalty (account1, newAmount); - royalty = new BN((salePrice * newAmount) / 100); + await this.token.setGlobalRoyalty(account1, newPercentage); + royalty = new BN((salePrice * newPercentage) / 10000); const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); - expect(newInfo.receiver).to.be.equal(account1); - expect(newInfo.royaltyFraction).to.be.bignumber.equal(royalty); + expect(newInfo[0]).to.be.equal(account1); + expect(newInfo[1]).to.be.bignumber.equal(royalty); }); it('holds same royalty value for different tokens', async function () { - const newAmount = new BN('20'); - await this.token.setGlobalRoyalty (account1, newAmount); + const newPercentage = new BN('20'); + await this.token.setGlobalRoyalty(account1, newPercentage); const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - expect(token1Info.royaltyFraction).to.be.bignumber.equal(token2Info.royaltyFraction); + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); }); it('reverts if invalid parameters', async function () { await expectRevert( - this.token.setGlobalRoyalty (ZERO_ADDRESS, royaltyFraction), + this.token.setGlobalRoyalty(ZERO_ADDRESS, royaltyFraction), 'ERC2981: Invalid receiver', ); await expectRevert( - this.token.setGlobalRoyalty (account1, new BN('100')), + this.token.setGlobalRoyalty(account1, new BN('100')), 'ERC2981: Royalty percentage is too high', ); }); @@ -71,32 +71,32 @@ contract('ERC721Royalty', function (accounts) { }); it('updates royalty amount', async function () { - const newAmount = new BN('25'); - let royalty = new BN((salePrice * royaltyFraction) / 100); + const newPercentage = new BN('25'); + let royalty = new BN((salePrice * royaltyFraction) / 10000); // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); - expect(initInfo.receiver).to.be.equal(account1); - expect(initInfo.royaltyFraction).to.be.bignumber.equal(royalty); + expect(initInfo[0]).to.be.equal(account1); + expect(initInfo[1]).to.be.bignumber.equal(royalty); // Updated royalty check - await this.token.setTokenRoyalty(tokenId1, account1, newAmount); - royalty = new BN((salePrice * newAmount) / 100); + await this.token.setTokenRoyalty(tokenId1, account1, newPercentage); + royalty = new BN((salePrice * newPercentage) / 10000); const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); - expect(newInfo.receiver).to.be.equal(account1); - expect(newInfo.royaltyFraction).to.be.bignumber.equal(royalty); + expect(newInfo[0]).to.be.equal(account1); + expect(newInfo[1]).to.be.bignumber.equal(royalty); }); it('holds different values for different tokens', async function () { - const newAmount = new BN('20'); - await this.token.setTokenRoyalty(tokenId2, account1, newAmount); + const newPercentage = new BN('20'); + await this.token.setTokenRoyalty(tokenId2, account1, newPercentage); const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); // must be different even at the same SalePrice - expect(token1Info.royaltyFraction).to.not.be.equal(token2Info.royaltyFraction); + expect(token1Info[1]).to.not.be.equal(token2Info.royaltyFraction); }); it('reverts if invalid parameters', async function () { From 94932688f79946b660e568749154d9984410b324 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 12:01:17 -0400 Subject: [PATCH 09/51] Refactor royaltyInfo function --- .../ERC721/extensions/draft-ERC721Royalty.sol | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 22ea3169ce6..fe4d60db1d5 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -63,19 +63,17 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { * @dev See {IERC721Royalty-royaltyInfo} */ function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { - address receiver; - uint256 royaltyAmount; + RoyaltyInfo memory royalty; if (_tokenRoyaltyInfo[_tokenId].receiver != address(0)) { - RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; - receiver = royalty.receiver; - royaltyAmount = (_salePrice * royalty.royaltyFraction) / _feeDenominator(); + royalty = _tokenRoyaltyInfo[_tokenId]; } else { - receiver = _globalRoyaltyInfo.receiver; - royaltyAmount = (_salePrice * _globalRoyaltyInfo.royaltyFraction) / _feeDenominator(); + royalty = _globalRoyaltyInfo; } - return (receiver, royaltyAmount); + uint256 royaltyAmount = (_salePrice * royalty.royaltyFraction) / _feeDenominator(); + + return (royalty.receiver, royaltyAmount); } /** From 76fa5b2a0e5c680a97949a81cd0078a9f5a5d460 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 12:34:52 -0400 Subject: [PATCH 10/51] Add validations to set royalty --- .../ERC721/extensions/draft-ERC721Royalty.sol | 24 +++++++++++++++++-- .../ERC721/extensions/ERC721Royalty.test.js | 18 ++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index fe4d60db1d5..dee77943a36 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -37,7 +37,8 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { address receiver, uint256 fraction ) internal virtual { - require(fraction < 100, "ERC2981: Royalty percentage is too high"); + require(fraction > 0, "ERC2981: Royalty percentage is too low"); + require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid receiver"); _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); @@ -53,7 +54,8 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { * according to the _feeDenominator granularity. */ function _setGlobalRoyalty(address receiver, uint256 fraction) internal virtual { - require(fraction < 100, "ERC2981: Royalty percentage is too high"); + require(fraction > 0, "ERC2981: Royalty percentage is too low"); + require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid receiver"); _globalRoyaltyInfo = RoyaltyInfo(receiver, fraction); @@ -90,4 +92,22 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { function _feeDenominator() internal pure virtual returns (uint256) { return 10000; } + + /** + * @dev Destroys `tokenId`. + * The royalty information is cleared when the token is burned. + * + * Requirements: + * + * - `tokenId` must exist. + * + * Emits a {Transfer} event. + + function _burn(uint256 tokenId) internal virtual override { + super._burn(tokenId); + + if (_tokenRoyaltyInfo[tokenId].royaltyFraction != 0) { + delete _tokenRoyaltyInfo[tokenId]; + } + } */ } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 52e73f71674..cea8f1c050d 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -59,8 +59,13 @@ contract('ERC721Royalty', function (accounts) { ); await expectRevert( - this.token.setGlobalRoyalty(account1, new BN('100')), - 'ERC2981: Royalty percentage is too high', + this.token.setGlobalRoyalty(account1, new BN('0')), + 'ERC2981: Royalty percentage is too low', + ); + + await expectRevert( + this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), + 'ERC2981: Royalty percentage will exceed salePrice', ); }); }); @@ -106,8 +111,13 @@ contract('ERC721Royalty', function (accounts) { ); await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('100')), - 'ERC2981: Royalty percentage is too high', + this.token.setTokenRoyalty(tokenId1, account1, new BN('0')), + 'ERC2981: Royalty percentage is too low', + ); + + await expectRevert( + this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), + 'ERC2981: Royalty percentage will exceed salePrice', ); }); }); From f64275b20fc31aac9bf000f5ef75cb0e21dc556a Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 16:05:36 -0400 Subject: [PATCH 11/51] Inherit from ERC721, include burn override --- CHANGELOG.md | 2 +- contracts/mocks/ERC721RoyaltyMock.sol | 12 +++++-- .../ERC721/extensions/draft-ERC721Royalty.sol | 34 +++++++++++++------ .../extensions/draft-IERC721Royalty.sol | 4 ++- .../ERC721/extensions/ERC721Royalty.test.js | 23 ++++++++++++- 5 files changed, 59 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f9c761350..64574a2be14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased - * `ERC2891`: add a new extension of `ERC721` to handle royalty information.([]()) + * `ERC2891`: add a new extension of `ERC721` to handle royalty information.([#3012](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3012)) * `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977)) * `Math`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984)) * Preset contracts are now deprecated in favor of [Contracts Wizard](https://wizard.openzeppelin.com). ([#2986](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2986)) diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 06f757ebbb4..b43adc4653b 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -5,9 +5,7 @@ pragma solidity ^0.8.0; import "../token/ERC721/extensions/draft-ERC721Royalty.sol"; contract ERC721RoyaltyMock is ERC721Royalty { - bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a; - - constructor() public {} + constructor(string memory name, string memory symbol) ERC721(name, symbol) {} function setTokenRoyalty( uint256 tokenId, @@ -20,4 +18,12 @@ contract ERC721RoyaltyMock is ERC721Royalty { function setGlobalRoyalty(address recipient, uint256 fraction) public { _setGlobalRoyalty(recipient, fraction); } + + function mint(address to, uint256 tokenId) public { + _mint(to, tokenId); + } + + function burn(uint256 tokenId) public { + _burn(tokenId); + } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index dee77943a36..a95bb135a96 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.0; +import "../ERC721.sol"; import "./draft-IERC721Royalty.sol"; import "../../../utils/introspection/ERC165.sol"; @@ -15,7 +16,7 @@ import "../../../utils/introspection/ERC165.sol"; * * _Available since v4.5._ */ -abstract contract ERC721Royalty is ERC165, IERC721Royalty { +abstract contract ERC721Royalty is IERC721Royalty, ERC721 { struct RoyaltyInfo { address receiver; uint256 royaltyFraction; @@ -24,7 +25,7 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { RoyaltyInfo private _globalRoyaltyInfo; mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; - /* + /** * @dev Sets tokens royalties * * Requirements: @@ -40,11 +41,12 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { require(fraction > 0, "ERC2981: Royalty percentage is too low"); require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid receiver"); + require(_exists(tokenId), "ERC2981: Nonexistent token"); _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); } - /* + /** * * @dev Sets global royalty * @@ -81,7 +83,7 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { /** * @dev See {IERC165-supportsInterface}. */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, IERC165) returns (bool) { return interfaceId == type(IERC721Royalty).interfaceId || super.supportsInterface(interfaceId); } @@ -93,6 +95,21 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { return 10000; } + /** + * @dev Removes `tokenId` royalty information. + * The royalty information is cleared when the token is burned. + * + * Requirements: + * + * - `tokenId` royalty information must exist. + * + */ + function _deleteTokenRoyalty(uint256 tokenId) internal virtual { + if (_tokenRoyaltyInfo[tokenId].royaltyFraction != 0) { + delete _tokenRoyaltyInfo[tokenId]; + } + } + /** * @dev Destroys `tokenId`. * The royalty information is cleared when the token is burned. @@ -102,12 +119,9 @@ abstract contract ERC721Royalty is ERC165, IERC721Royalty { * - `tokenId` must exist. * * Emits a {Transfer} event. - + */ function _burn(uint256 tokenId) internal virtual override { super._burn(tokenId); - - if (_tokenRoyaltyInfo[tokenId].royaltyFraction != 0) { - delete _tokenRoyaltyInfo[tokenId]; - } - } */ + _deleteTokenRoyalty(tokenId); + } } diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol index cb2cc98bc1d..37b5d1f54a6 100644 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -6,8 +6,10 @@ pragma solidity ^0.8.0; import "../../../interfaces/IERC165.sol"; /** - * @dev Interface for the NFT Royalty Standard + * @dev Interface for the NFT Royalty Standard. * + * A standardized way to retrieve royalty payment information for non-fungible tokens (NFTs) to enable universal + * support for royalty payments across all NFT marketplaces and ecosystem participants. * _Available since v4.5._ */ interface IERC721Royalty is IERC165 { diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index cea8f1c050d..23a0b3d5fe4 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -14,7 +14,10 @@ contract('ERC721Royalty', function (accounts) { const royaltyFraction = new BN('10'); beforeEach(async function () { - this.token = await ERC721RoyaltyMock.new(); + this.token = await ERC721RoyaltyMock.new('My Token', 'TKN'); + + await this.token.mint(account1, tokenId1); + await this.token.mint(account1, tokenId2); }); shouldSupportInterfaces(['ERC2981']); @@ -119,6 +122,24 @@ contract('ERC721Royalty', function (accounts) { this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), 'ERC2981: Royalty percentage will exceed salePrice', ); + + await expectRevert( + this.token.setTokenRoyalty(new BN('787'), account1, new BN('100')), + 'ERC2981: Nonexistent token', + ); + }); + + it('removes royalty information after burn', async function () { + await this.token.burn(tokenId1); + const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(tokenInfo[0]).to.be.equal(ZERO_ADDRESS); + expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); + + await expectRevert( + this.token.setTokenRoyalty(tokenId1, account1, new BN('100')), + 'ERC2981: Nonexistent token', + ); }); }); }); From d93ede819209f9ebb7cb48747466dfb12df6ac2a Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 17:10:04 -0400 Subject: [PATCH 12/51] Add tests coverage --- test/token/ERC721/extensions/ERC721Royalty.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 23a0b3d5fe4..78ad9248b63 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -20,6 +20,13 @@ contract('ERC721Royalty', function (accounts) { await this.token.mint(account1, tokenId2); }); + it('calls supports interface', async function () { + const result = await this.token.supportsInterface('0x2a55205a'); + + expect(result).to.not.be.undefined; + expect(result).to.be.true; + }); + shouldSupportInterfaces(['ERC2981']); describe('global royalty', function () { From 685945207109479a9112ab056476711daa729080 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 19:13:25 -0400 Subject: [PATCH 13/51] Refactor tests --- test/token/ERC721/extensions/ERC721Royalty.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 78ad9248b63..26c4f1ec061 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -22,9 +22,10 @@ contract('ERC721Royalty', function (accounts) { it('calls supports interface', async function () { const result = await this.token.supportsInterface('0x2a55205a'); + const expected = true; - expect(result).to.not.be.undefined; - expect(result).to.be.true; + expect(result).to.not.equal(undefined); + expect(result).to.equal(expected); }); shouldSupportInterfaces(['ERC2981']); From f4378c548c040e81436e4fc7058f4c8e2ce532d0 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 13 Dec 2021 19:14:42 -0400 Subject: [PATCH 14/51] Update contracts/token/ERC721/extensions/draft-IERC721Royalty.sol Co-authored-by: Francisco Giordano --- contracts/token/ERC721/extensions/draft-IERC721Royalty.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol index 37b5d1f54a6..4e1e0830654 100644 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -10,6 +10,7 @@ import "../../../interfaces/IERC165.sol"; * * A standardized way to retrieve royalty payment information for non-fungible tokens (NFTs) to enable universal * support for royalty payments across all NFT marketplaces and ecosystem participants. + * * _Available since v4.5._ */ interface IERC721Royalty is IERC165 { From 349fbf9023dea3637d3d3ef7a3a9943636791782 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Tue, 14 Dec 2021 08:15:06 -0400 Subject: [PATCH 15/51] Rename variable --- contracts/interfaces/IERC2981.sol | 4 ++-- contracts/token/ERC721/extensions/draft-IERC721Royalty.sol | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/interfaces/IERC2981.sol b/contracts/interfaces/IERC2981.sol index cf624d0e78c..20a417c64d6 100644 --- a/contracts/interfaces/IERC2981.sol +++ b/contracts/interfaces/IERC2981.sol @@ -14,10 +14,10 @@ interface IERC2981 is IERC165 { * @param tokenId - the NFT asset queried for royalty information * @param salePrice - the sale price of the NFT asset specified by `tokenId` * @return receiver - address of who should be sent the royalty payment - * @return royaltyFraction - the royalty payment amount for `salePrice` + * @return royaltyAmount - the royalty payment amount for `salePrice` */ function royaltyInfo(uint256 tokenId, uint256 salePrice) external view - returns (address receiver, uint256 royaltyFraction); + returns (address receiver, uint256 royaltyAmount); } diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol index 4e1e0830654..483ab9fd1ae 100644 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol @@ -26,5 +26,5 @@ interface IERC721Royalty is IERC165 { function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view - returns (address receiver, uint256 royaltyFraction); + returns (address receiver, uint256 royaltyAmount); } From 44e6e6d73dd1c6360cda3dd183c9e1a694111d2a Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Tue, 14 Dec 2021 08:29:42 -0400 Subject: [PATCH 16/51] Remove if --- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index a95bb135a96..ef89ba664ee 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -105,9 +105,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { * */ function _deleteTokenRoyalty(uint256 tokenId) internal virtual { - if (_tokenRoyaltyInfo[tokenId].royaltyFraction != 0) { - delete _tokenRoyaltyInfo[tokenId]; - } + delete _tokenRoyaltyInfo[tokenId]; } /** From 5fb5bfc55ebaafdf43b89313ce38cdccdbeaa311 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Tue, 14 Dec 2021 16:00:28 -0400 Subject: [PATCH 17/51] Add test case and global royalty delete --- contracts/mocks/ERC721RoyaltyMock.sol | 8 +++-- .../ERC721/extensions/draft-ERC721Royalty.sol | 32 ++++++++++------- .../ERC721/extensions/ERC721Royalty.test.js | 34 +++++++++++++------ 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index b43adc4653b..1ef41d5d458 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -10,12 +10,12 @@ contract ERC721RoyaltyMock is ERC721Royalty { function setTokenRoyalty( uint256 tokenId, address recipient, - uint256 fraction + uint96 fraction ) public { _setTokenRoyalty(tokenId, recipient, fraction); } - function setGlobalRoyalty(address recipient, uint256 fraction) public { + function setGlobalRoyalty(address recipient, uint96 fraction) public { _setGlobalRoyalty(recipient, fraction); } @@ -26,4 +26,8 @@ contract ERC721RoyaltyMock is ERC721Royalty { function burn(uint256 tokenId) public { _burn(tokenId); } + + function deleteRoyalty() public { + _deleteRoyalty(); + } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index ef89ba664ee..ad4fbb7ad44 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -19,12 +19,19 @@ import "../../../utils/introspection/ERC165.sol"; abstract contract ERC721Royalty is IERC721Royalty, ERC721 { struct RoyaltyInfo { address receiver; - uint256 royaltyFraction; + uint96 royaltyFraction; } RoyaltyInfo private _globalRoyaltyInfo; mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, IERC165) returns (bool) { + return interfaceId == type(IERC721Royalty).interfaceId || super.supportsInterface(interfaceId); + } + /** * @dev Sets tokens royalties * @@ -36,9 +43,8 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { function _setTokenRoyalty( uint256 tokenId, address receiver, - uint256 fraction + uint96 fraction ) internal virtual { - require(fraction > 0, "ERC2981: Royalty percentage is too low"); require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid receiver"); require(_exists(tokenId), "ERC2981: Nonexistent token"); @@ -55,8 +61,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { * - `fraction` must indicate the percentage fraction. Needs to be set appropriately * according to the _feeDenominator granularity. */ - function _setGlobalRoyalty(address receiver, uint256 fraction) internal virtual { - require(fraction > 0, "ERC2981: Royalty percentage is too low"); + function _setGlobalRoyalty(address receiver, uint96 fraction) internal virtual { require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid receiver"); @@ -80,18 +85,11 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { return (royalty.receiver, royaltyAmount); } - /** - * @dev See {IERC165-supportsInterface}. - */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, IERC165) returns (bool) { - return interfaceId == type(IERC721Royalty).interfaceId || super.supportsInterface(interfaceId); - } - /** * @dev Returns the percentage granularity being used. The default denominator is 10000 * but it can be customized by an override. */ - function _feeDenominator() internal pure virtual returns (uint256) { + function _feeDenominator() internal pure virtual returns (uint96) { return 10000; } @@ -108,6 +106,14 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { delete _tokenRoyaltyInfo[tokenId]; } + /** + * @dev Removes global royalty information. + * + */ + function _deleteRoyalty() internal virtual { + delete _globalRoyaltyInfo; + } + /** * @dev Destroys `tokenId`. * The royalty information is cleared when the token is burned. diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 26c4f1ec061..ab464052977 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -63,17 +63,36 @@ contract('ERC721Royalty', function (accounts) { expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); }); + it('can hold global and token royalty information', async function () { + const newPercentage = new BN('20'); + await this.token.setGlobalRoyalty(account1, newPercentage); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + }); + + it('Remove royalty information', async function () { + const newValue = new BN('0'); + await this.token.deleteRoyalty(); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + //Test royalty info is still persistent across all tokens + expect(token1Info[0]).to.be.bignumber.equal(token2Info[0]); + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + //Test information was deleted + expect(token1Info[0]).to.be.equal(ZERO_ADDRESS); + expect(token1Info[1]).to.be.bignumber.equal(newValue); + }); + it('reverts if invalid parameters', async function () { await expectRevert( this.token.setGlobalRoyalty(ZERO_ADDRESS, royaltyFraction), 'ERC2981: Invalid receiver', ); - await expectRevert( - this.token.setGlobalRoyalty(account1, new BN('0')), - 'ERC2981: Royalty percentage is too low', - ); - await expectRevert( this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), 'ERC2981: Royalty percentage will exceed salePrice', @@ -121,11 +140,6 @@ contract('ERC721Royalty', function (accounts) { 'ERC2981: Invalid receiver', ); - await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('0')), - 'ERC2981: Royalty percentage is too low', - ); - await expectRevert( this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), 'ERC2981: Royalty percentage will exceed salePrice', From b0f90c32d59106ad8f6aa99b0d3f953379c5e812 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Tue, 14 Dec 2021 16:17:26 -0400 Subject: [PATCH 18/51] Add mixed royalties test cases --- .../ERC721/extensions/ERC721Royalty.test.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index ab464052977..30c3f930d97 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -7,7 +7,7 @@ const { shouldSupportInterfaces } = require('../../../utils/introspection/Suppor const ERC721RoyaltyMock = artifacts.require('ERC721RoyaltyMock'); contract('ERC721Royalty', function (accounts) { - const [ account1 ] = accounts; + const [ account1, account2 ] = accounts; const tokenId1 = new BN('1'); const tokenId2 = new BN('2'); const salePrice = new BN('1000'); @@ -64,13 +64,20 @@ contract('ERC721Royalty', function (accounts) { }); it('can hold global and token royalty information', async function () { - const newPercentage = new BN('20'); - await this.token.setGlobalRoyalty(account1, newPercentage); + const newPercentage = new BN('30'); + const royalty = new BN((salePrice * newPercentage) / 10000); + + await this.token.setTokenRoyalty(tokenId2, account2, newPercentage); const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + // Tokens must not have same values + expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); + expect(token1Info[0]).to.not.be.equal(token2Info[0]); - expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + // Updated token must have new values + expect(token2Info[0]).to.be.equal(account2); + expect(token2Info[1]).to.be.bignumber.equal(royalty); }); it('Remove royalty information', async function () { @@ -79,10 +86,10 @@ contract('ERC721Royalty', function (accounts) { const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - //Test royalty info is still persistent across all tokens + // Test royalty info is still persistent across all tokens expect(token1Info[0]).to.be.bignumber.equal(token2Info[0]); expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); - //Test information was deleted + // Test information was deleted expect(token1Info[0]).to.be.equal(ZERO_ADDRESS); expect(token1Info[1]).to.be.bignumber.equal(newValue); }); From 85955fc7552a11379ef5e58eae52b5b51c5655c7 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Tue, 14 Dec 2021 16:22:23 -0400 Subject: [PATCH 19/51] Avoid doing ssload twice --- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index ad4fbb7ad44..378d8906815 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -72,11 +72,8 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { * @dev See {IERC721Royalty-royaltyInfo} */ function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { - RoyaltyInfo memory royalty; - - if (_tokenRoyaltyInfo[_tokenId].receiver != address(0)) { - royalty = _tokenRoyaltyInfo[_tokenId]; - } else { + RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; + if (royalty.receiver == address(0)) { royalty = _globalRoyaltyInfo; } From 9e3572d5a91b2e71fe8617487bb0b468716a6bae Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Wed, 15 Dec 2021 07:50:58 -0400 Subject: [PATCH 20/51] Avoid token exclussion from global royalties tests cases --- .../ERC721/extensions/draft-ERC721Royalty.sol | 3 ++- .../ERC721/extensions/ERC721Royalty.test.js | 20 ++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 378d8906815..59ada9cd178 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -46,7 +46,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { uint96 fraction ) internal virtual { require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); - require(receiver != address(0), "ERC2981: Invalid receiver"); + require(receiver != address(0), "ERC2981: Invalid parameters"); require(_exists(tokenId), "ERC2981: Nonexistent token"); _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); @@ -73,6 +73,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { */ function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; + if (royalty.receiver == address(0)) { royalty = _globalRoyaltyInfo; } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 30c3f930d97..dc6dfe29ef1 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -80,6 +80,24 @@ contract('ERC721Royalty', function (accounts) { expect(token2Info[1]).to.be.bignumber.equal(royalty); }); + it('can reset token after setting royalty', async function () { + const newPercentage = new BN('30'); + let royalty = new BN((salePrice * newPercentage) / 10000); + await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); + + const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + // Tokens must have own information + expect(tokenInfo[1]).to.be.bignumber.equal(royalty); + expect(tokenInfo[0]).to.be.equal(account2); + + await this.token.setTokenRoyalty(tokenId2, account1, new BN('0')); + const result = await this.token.royaltyInfo(tokenId2, salePrice); + // Token must not share global information + expect(result[0]).to.be.equal(account1); + expect(result[1]).to.be.bignumber.equal(new BN('0')); + }); + it('Remove royalty information', async function () { const newValue = new BN('0'); await this.token.deleteRoyalty(); @@ -144,7 +162,7 @@ contract('ERC721Royalty', function (accounts) { it('reverts if invalid parameters', async function () { await expectRevert( this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid receiver', + 'ERC2981: Invalid parameters', ); await expectRevert( From cd33397f208dbac9cbff28ddd91cf39eb5ff9cab Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Wed, 15 Dec 2021 08:04:16 -0400 Subject: [PATCH 21/51] Update variable type --- test/token/ERC721/extensions/ERC721Royalty.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index dc6dfe29ef1..85ef028e717 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -82,7 +82,7 @@ contract('ERC721Royalty', function (accounts) { it('can reset token after setting royalty', async function () { const newPercentage = new BN('30'); - let royalty = new BN((salePrice * newPercentage) / 10000); + const royalty = new BN((salePrice * newPercentage) / 10000); await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); From dffd19ed3c5e42c8eda122917b58d561b6c56d6e Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Wed, 15 Dec 2021 11:19:56 -0400 Subject: [PATCH 22/51] Rename function and update documentation --- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 59ada9cd178..b1b8b63bf33 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -93,14 +93,14 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { /** * @dev Removes `tokenId` royalty information. - * The royalty information is cleared when the token is burned. + * The royalty information is cleared and the token royalty fallbacks to the global royalty. * * Requirements: * * - `tokenId` royalty information must exist. * */ - function _deleteTokenRoyalty(uint256 tokenId) internal virtual { + function _resetTokenRoyalty(uint256 tokenId) internal virtual { delete _tokenRoyaltyInfo[tokenId]; } @@ -124,6 +124,6 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { */ function _burn(uint256 tokenId) internal virtual override { super._burn(tokenId); - _deleteTokenRoyalty(tokenId); + _resetTokenRoyalty(tokenId); } } From 98bbc5dc015bf1ed308e088f934dd6651c17cbac Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 13:46:13 -0400 Subject: [PATCH 23/51] Update contracts/token/ERC721/extensions/draft-ERC721Royalty.sol Co-authored-by: Francisco Giordano --- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index b1b8b63bf33..8399106cc11 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -14,6 +14,9 @@ import "../../../utils/introspection/ERC165.sol"; * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setGlobalRoyalty} method to set a global * royalty information. * + * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. + * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. + * * _Available since v4.5._ */ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { From 1fca44fba9962d38b1a2edca01d7bff9f76e8650 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 13:46:49 -0400 Subject: [PATCH 24/51] Update contracts/token/ERC721/extensions/draft-ERC721Royalty.sol Co-authored-by: Francisco Giordano --- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 8399106cc11..cdd271cfaad 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -36,7 +36,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { } /** - * @dev Sets tokens royalties + * @dev Sets the royalty info for a specific token id, overriding the default royalty info. * * Requirements: * - `tokenId` must be already mined. From 37ccc42173468e2ed2fdfa358bc5304c5b9e502e Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 13:46:58 -0400 Subject: [PATCH 25/51] Update contracts/token/ERC721/extensions/draft-ERC721Royalty.sol Co-authored-by: Francisco Giordano --- contracts/token/ERC721/extensions/draft-ERC721Royalty.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index cdd271cfaad..4028f40530f 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -56,8 +56,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { } /** - * - * @dev Sets global royalty + * @dev Sets the royalty info that tokens will default to. * * Requirements: * - `receiver` cannot be the zero address. From 5e4d4a4b0ffe9169e51ed74bd2483dfa1df12adf Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 13:59:10 -0400 Subject: [PATCH 26/51] Reorder tests and rename variables --- contracts/mocks/ERC721RoyaltyMock.sol | 6 +- .../ERC721/extensions/draft-ERC721Royalty.sol | 18 ++-- .../ERC721/extensions/ERC721Royalty.test.js | 92 ++++++++++--------- 3 files changed, 59 insertions(+), 57 deletions(-) diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 1ef41d5d458..6346ccfcca5 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -15,8 +15,8 @@ contract ERC721RoyaltyMock is ERC721Royalty { _setTokenRoyalty(tokenId, recipient, fraction); } - function setGlobalRoyalty(address recipient, uint96 fraction) public { - _setGlobalRoyalty(recipient, fraction); + function setDefaultRoyalty(address recipient, uint96 fraction) public { + _setDefaultRoyalty(recipient, fraction); } function mint(address to, uint256 tokenId) public { @@ -28,6 +28,6 @@ contract ERC721RoyaltyMock is ERC721Royalty { } function deleteRoyalty() public { - _deleteRoyalty(); + _deleteDefaultRoyalty(); } } diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol index 4028f40530f..2e7fd8a3cd7 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol @@ -11,7 +11,7 @@ import "../../../utils/introspection/ERC165.sol"; * @dev Implementation of the ERC721 Royalty extension allowing royalty information to be stored and retrieved, as defined in * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. * - * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setGlobalRoyalty} method to set a global + * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setDefaultRoyalty} method to set a default * royalty information. * * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. @@ -25,7 +25,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { uint96 royaltyFraction; } - RoyaltyInfo private _globalRoyaltyInfo; + RoyaltyInfo private _defaultRoyaltyInfo; mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; /** @@ -63,11 +63,11 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { * - `fraction` must indicate the percentage fraction. Needs to be set appropriately * according to the _feeDenominator granularity. */ - function _setGlobalRoyalty(address receiver, uint96 fraction) internal virtual { + function _setDefaultRoyalty(address receiver, uint96 fraction) internal virtual { require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid receiver"); - _globalRoyaltyInfo = RoyaltyInfo(receiver, fraction); + _defaultRoyaltyInfo = RoyaltyInfo(receiver, fraction); } /** @@ -77,7 +77,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; if (royalty.receiver == address(0)) { - royalty = _globalRoyaltyInfo; + royalty = _defaultRoyaltyInfo; } uint256 royaltyAmount = (_salePrice * royalty.royaltyFraction) / _feeDenominator(); @@ -95,7 +95,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { /** * @dev Removes `tokenId` royalty information. - * The royalty information is cleared and the token royalty fallbacks to the global royalty. + * The royalty information is cleared and the token royalty fallbacks to the default royalty. * * Requirements: * @@ -107,11 +107,11 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { } /** - * @dev Removes global royalty information. + * @dev Removes default royalty information. * */ - function _deleteRoyalty() internal virtual { - delete _globalRoyaltyInfo; + function _deleteDefaultRoyalty() internal virtual { + delete _defaultRoyaltyInfo; } /** diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 85ef028e717..70a3348037d 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -30,23 +30,25 @@ contract('ERC721Royalty', function (accounts) { shouldSupportInterfaces(['ERC2981']); - describe('global royalty', function () { + describe.only('default royalty', function () { beforeEach(async function () { - await this.token.setGlobalRoyalty(account1, royaltyFraction); + await this.token.setDefaultRoyalty(account1, royaltyFraction); }); + it('checks royalty is set', async function () { + const royalty = new BN((salePrice * royaltyFraction) / 10000); - it('updates royalty amount', async function () { - const newPercentage = new BN('25'); - let royalty = new BN((salePrice * royaltyFraction) / 10000); - // Initial royalty check const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(initInfo[0]).to.be.equal(account1); expect(initInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('updates royalty amount', async function () { + const newPercentage = new BN('25'); // Updated royalty check - await this.token.setGlobalRoyalty(account1, newPercentage); - royalty = new BN((salePrice * newPercentage) / 10000); + await this.token.setDefaultRoyalty(account1, newPercentage); + const royalty = new BN((salePrice * newPercentage) / 10000); const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); expect(newInfo[0]).to.be.equal(account1); @@ -55,7 +57,7 @@ contract('ERC721Royalty', function (accounts) { it('holds same royalty value for different tokens', async function () { const newPercentage = new BN('20'); - await this.token.setGlobalRoyalty(account1, newPercentage); + await this.token.setDefaultRoyalty(account1, newPercentage); const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); @@ -63,41 +65,6 @@ contract('ERC721Royalty', function (accounts) { expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); }); - it('can hold global and token royalty information', async function () { - const newPercentage = new BN('30'); - const royalty = new BN((salePrice * newPercentage) / 10000); - - await this.token.setTokenRoyalty(tokenId2, account2, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - // Tokens must not have same values - expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); - expect(token1Info[0]).to.not.be.equal(token2Info[0]); - - // Updated token must have new values - expect(token2Info[0]).to.be.equal(account2); - expect(token2Info[1]).to.be.bignumber.equal(royalty); - }); - - it('can reset token after setting royalty', async function () { - const newPercentage = new BN('30'); - const royalty = new BN((salePrice * newPercentage) / 10000); - await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); - - const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - // Tokens must have own information - expect(tokenInfo[1]).to.be.bignumber.equal(royalty); - expect(tokenInfo[0]).to.be.equal(account2); - - await this.token.setTokenRoyalty(tokenId2, account1, new BN('0')); - const result = await this.token.royaltyInfo(tokenId2, salePrice); - // Token must not share global information - expect(result[0]).to.be.equal(account1); - expect(result[1]).to.be.bignumber.equal(new BN('0')); - }); - it('Remove royalty information', async function () { const newValue = new BN('0'); await this.token.deleteRoyalty(); @@ -114,7 +81,7 @@ contract('ERC721Royalty', function (accounts) { it('reverts if invalid parameters', async function () { await expectRevert( - this.token.setGlobalRoyalty(ZERO_ADDRESS, royaltyFraction), + this.token.setDefaultRoyalty(ZERO_ADDRESS, royaltyFraction), 'ERC2981: Invalid receiver', ); @@ -188,5 +155,40 @@ contract('ERC721Royalty', function (accounts) { 'ERC2981: Nonexistent token', ); }); + + it('can reset token after setting royalty', async function () { + const newPercentage = new BN('30'); + const royalty = new BN((salePrice * newPercentage) / 10000); + await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); + + const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + // Tokens must have own information + expect(tokenInfo[1]).to.be.bignumber.equal(royalty); + expect(tokenInfo[0]).to.be.equal(account2); + + await this.token.setTokenRoyalty(tokenId2, account1, new BN('0')); + const result = await this.token.royaltyInfo(tokenId2, salePrice); + // Token must not share default information + expect(result[0]).to.be.equal(account1); + expect(result[1]).to.be.bignumber.equal(new BN('0')); + }); + + it('can hold default and token royalty information', async function () { + const newPercentage = new BN('30'); + const royalty = new BN((salePrice * newPercentage) / 10000); + + await this.token.setTokenRoyalty(tokenId2, account2, newPercentage); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + // Tokens must not have same values + expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); + expect(token1Info[0]).to.not.be.equal(token2Info[0]); + + // Updated token must have new values + expect(token2Info[0]).to.be.equal(account2); + expect(token2Info[1]).to.be.bignumber.equal(royalty); + }); }); }); From b37621c5fcc6e721aaa0323626f98b632a3943ac Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 14:00:00 -0400 Subject: [PATCH 27/51] Remove .only --- test/token/ERC721/extensions/ERC721Royalty.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 70a3348037d..97979aac0c5 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -30,7 +30,7 @@ contract('ERC721Royalty', function (accounts) { shouldSupportInterfaces(['ERC2981']); - describe.only('default royalty', function () { + describe('default royalty', function () { beforeEach(async function () { await this.token.setDefaultRoyalty(account1, royaltyFraction); }); From 79e01be8145fbc740a95da73e9e055db2ec12fed Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 14:10:08 -0400 Subject: [PATCH 28/51] Rename files --- contracts/interfaces/IERC2981.sol | 5 ++++ contracts/interfaces/draft-IERC2981.sol | 6 ---- contracts/mocks/ERC721RoyaltyMock.sol | 2 +- ...ft-ERC721Royalty.sol => ERC721Royalty.sol} | 10 +++---- .../extensions/draft-IERC721Royalty.sol | 30 ------------------- 5 files changed, 11 insertions(+), 42 deletions(-) delete mode 100644 contracts/interfaces/draft-IERC2981.sol rename contracts/token/ERC721/extensions/{draft-ERC721Royalty.sol => ERC721Royalty.sol} (92%) delete mode 100644 contracts/token/ERC721/extensions/draft-IERC721Royalty.sol diff --git a/contracts/interfaces/IERC2981.sol b/contracts/interfaces/IERC2981.sol index 20a417c64d6..4ebbaf28b5f 100644 --- a/contracts/interfaces/IERC2981.sol +++ b/contracts/interfaces/IERC2981.sol @@ -7,6 +7,11 @@ import "./IERC165.sol"; /** * @dev Interface for the NFT Royalty Standard + * + * A standardized way to retrieve royalty payment information for non-fungible tokens (NFTs) to enable universal + * support for royalty payments across all NFT marketplaces and ecosystem participants. + * + * _Available since v4.5._ */ interface IERC2981 is IERC165 { /** diff --git a/contracts/interfaces/draft-IERC2981.sol b/contracts/interfaces/draft-IERC2981.sol deleted file mode 100644 index 890b3ceefeb..00000000000 --- a/contracts/interfaces/draft-IERC2981.sol +++ /dev/null @@ -1,6 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/draft-ERC721Royalty.sol) - -pragma solidity ^0.8.0; - -import "../token/ERC721/extensions/draft-IERC721Royalty.sol"; diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 6346ccfcca5..93d5369a34e 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import "../token/ERC721/extensions/draft-ERC721Royalty.sol"; +import "../token/ERC721/extensions/ERC721Royalty.sol"; contract ERC721RoyaltyMock is ERC721Royalty { constructor(string memory name, string memory symbol) ERC721(name, symbol) {} diff --git a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol similarity index 92% rename from contracts/token/ERC721/extensions/draft-ERC721Royalty.sol rename to contracts/token/ERC721/extensions/ERC721Royalty.sol index 2e7fd8a3cd7..78950ca6ec4 100644 --- a/contracts/token/ERC721/extensions/draft-ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/draft-ERC721Royalty.sol) +// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/ERC721Royalty.sol) pragma solidity ^0.8.0; import "../ERC721.sol"; -import "./draft-IERC721Royalty.sol"; +import "../../../interfaces/IERC2981.sol"; import "../../../utils/introspection/ERC165.sol"; /** @@ -19,7 +19,7 @@ import "../../../utils/introspection/ERC165.sol"; * * _Available since v4.5._ */ -abstract contract ERC721Royalty is IERC721Royalty, ERC721 { +abstract contract ERC721Royalty is IERC2981, ERC721 { struct RoyaltyInfo { address receiver; uint96 royaltyFraction; @@ -32,7 +32,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, IERC165) returns (bool) { - return interfaceId == type(IERC721Royalty).interfaceId || super.supportsInterface(interfaceId); + return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); } /** @@ -71,7 +71,7 @@ abstract contract ERC721Royalty is IERC721Royalty, ERC721 { } /** - * @dev See {IERC721Royalty-royaltyInfo} + * @dev See {IERC2981-royaltyInfo} */ function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; diff --git a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol b/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol deleted file mode 100644 index 483ab9fd1ae..00000000000 --- a/contracts/token/ERC721/extensions/draft-IERC721Royalty.sol +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts v4.4.0 (token/ERC721/extensions/draft-ERC721Royalty.sol) - -pragma solidity ^0.8.0; - -import "../../../interfaces/IERC165.sol"; - -/** - * @dev Interface for the NFT Royalty Standard. - * - * A standardized way to retrieve royalty payment information for non-fungible tokens (NFTs) to enable universal - * support for royalty payments across all NFT marketplaces and ecosystem participants. - * - * _Available since v4.5._ - */ -interface IERC721Royalty is IERC165 { - /** - * @dev Called with the sale price to determine how much royalty - * is owed and to whom. - * - * Requirements: - * - `_tokenId` must be already mined, and have its royalty info set - * - `_salePrice` cannot be the zero. - * - */ - function royaltyInfo(uint256 _tokenId, uint256 _salePrice) - external - view - returns (address receiver, uint256 royaltyAmount); -} From fb6facf9cb961d6ed3b426106bb0d6addff307fe Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 15:04:23 -0400 Subject: [PATCH 29/51] Add royalty implementation without token inheritance, Add ERC1155 royalty implementation and tests --- contracts/mocks/ERC1155RoyaltyMock.sol | 42 ++++ .../ERC1155/extensions/ERC1155Royalty.sol | 48 +++++ .../token/ERC721/extensions/ERC721Royalty.sol | 91 +-------- contracts/token/common/ERC2981.sol | 106 ++++++++++ .../ERC1155/extensions/ERC1155Royalty.test.js | 187 ++++++++++++++++++ .../ERC721/extensions/ERC721Royalty.test.js | 10 - 6 files changed, 385 insertions(+), 99 deletions(-) create mode 100644 contracts/mocks/ERC1155RoyaltyMock.sol create mode 100644 contracts/token/ERC1155/extensions/ERC1155Royalty.sol create mode 100644 contracts/token/common/ERC2981.sol create mode 100644 test/token/ERC1155/extensions/ERC1155Royalty.test.js diff --git a/contracts/mocks/ERC1155RoyaltyMock.sol b/contracts/mocks/ERC1155RoyaltyMock.sol new file mode 100644 index 00000000000..8c7c2132fb3 --- /dev/null +++ b/contracts/mocks/ERC1155RoyaltyMock.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../token/ERC1155/extensions/ERC1155Royalty.sol"; + +contract ERC1155RoyaltyMock is ERC1155Royalty { + constructor(string memory uri) ERC1155(uri) {} + + function setTokenRoyalty( + uint256 tokenId, + address recipient, + uint96 fraction + ) public { + _setTokenRoyalty(tokenId, recipient, fraction); + } + + function setDefaultRoyalty(address recipient, uint96 fraction) public { + _setDefaultRoyalty(recipient, fraction); + } + + function mint( + address to, + uint256 id, + uint256 value, + bytes memory data + ) public { + _mint(to, id, value, data); + } + + function burn( + address from, + uint256 id, + uint256 amount + ) public { + _burn(from, id, amount); + } + + function deleteRoyalty() public { + _deleteDefaultRoyalty(); + } +} diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol new file mode 100644 index 00000000000..9ef541d1028 --- /dev/null +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.0 (token/ERC1155/extensions/ERC1155Royalty.sol) + +pragma solidity ^0.8.0; + +import "../ERC1155.sol"; +import "../../common/ERC2981.sol"; +import "../../../utils/introspection/ERC165.sol"; + +/** + * @dev Implementation of the ERC1155 Royalty extension allowing royalty information to be stored and retrieved, as defined in + * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. + * + * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setDefaultRoyalty} method to set a default + * royalty information. + * + * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. + * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. + * + * _Available since v4.5._ + */ +abstract contract ERC1155Royalty is ERC2981, ERC1155 { + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC1155, IERC165) returns (bool) { + return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); + } + + /** + * @dev Destroys `tokenId`. + * The royalty information is cleared when the token is burned. + * + * Requirements: + * + * - `tokenId` must exist. + * + * Emits a {Transfer} event. + */ + function _burn( + address from, + uint256 id, + uint256 amount + ) internal virtual override { + super._burn(from, id, amount); + _resetTokenRoyalty(id); + } +} diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol index 78950ca6ec4..5f8258657ae 100644 --- a/contracts/token/ERC721/extensions/ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.0; import "../ERC721.sol"; -import "../../../interfaces/IERC2981.sol"; +import "../../common/ERC2981.sol"; import "../../../utils/introspection/ERC165.sol"; /** @@ -19,15 +19,7 @@ import "../../../utils/introspection/ERC165.sol"; * * _Available since v4.5._ */ -abstract contract ERC721Royalty is IERC2981, ERC721 { - struct RoyaltyInfo { - address receiver; - uint96 royaltyFraction; - } - - RoyaltyInfo private _defaultRoyaltyInfo; - mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; - +abstract contract ERC721Royalty is ERC2981, ERC721 { /** * @dev See {IERC165-supportsInterface}. */ @@ -35,85 +27,6 @@ abstract contract ERC721Royalty is IERC2981, ERC721 { return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); } - /** - * @dev Sets the royalty info for a specific token id, overriding the default royalty info. - * - * Requirements: - * - `tokenId` must be already mined. - * - `receiver` cannot be the zero address. - * - `fraction` must indicate the percentage fraction using two decimals. - */ - function _setTokenRoyalty( - uint256 tokenId, - address receiver, - uint96 fraction - ) internal virtual { - require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); - require(receiver != address(0), "ERC2981: Invalid parameters"); - require(_exists(tokenId), "ERC2981: Nonexistent token"); - - _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); - } - - /** - * @dev Sets the royalty info that tokens will default to. - * - * Requirements: - * - `receiver` cannot be the zero address. - * - `fraction` must indicate the percentage fraction. Needs to be set appropriately - * according to the _feeDenominator granularity. - */ - function _setDefaultRoyalty(address receiver, uint96 fraction) internal virtual { - require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); - require(receiver != address(0), "ERC2981: Invalid receiver"); - - _defaultRoyaltyInfo = RoyaltyInfo(receiver, fraction); - } - - /** - * @dev See {IERC2981-royaltyInfo} - */ - function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { - RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; - - if (royalty.receiver == address(0)) { - royalty = _defaultRoyaltyInfo; - } - - uint256 royaltyAmount = (_salePrice * royalty.royaltyFraction) / _feeDenominator(); - - return (royalty.receiver, royaltyAmount); - } - - /** - * @dev Returns the percentage granularity being used. The default denominator is 10000 - * but it can be customized by an override. - */ - function _feeDenominator() internal pure virtual returns (uint96) { - return 10000; - } - - /** - * @dev Removes `tokenId` royalty information. - * The royalty information is cleared and the token royalty fallbacks to the default royalty. - * - * Requirements: - * - * - `tokenId` royalty information must exist. - * - */ - function _resetTokenRoyalty(uint256 tokenId) internal virtual { - delete _tokenRoyaltyInfo[tokenId]; - } - - /** - * @dev Removes default royalty information. - * - */ - function _deleteDefaultRoyalty() internal virtual { - delete _defaultRoyaltyInfo; - } - /** * @dev Destroys `tokenId`. * The royalty information is cleared when the token is burned. diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol new file mode 100644 index 00000000000..0c31a695316 --- /dev/null +++ b/contracts/token/common/ERC2981.sol @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.0 (token/common/ERC2981.sol) + +pragma solidity ^0.8.0; + +import "../../interfaces/IERC2981.sol"; +import "../../utils/introspection/ERC165.sol"; + +/** + * @dev Implementation of the Royalty Standard allowing royalty information to be stored and retrieved. + * + * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setDefaultRoyalty} method to set a default + * royalty information. + * + * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. + * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. + * + * _Available since v4.5._ + */ +abstract contract ERC2981 is IERC2981, ERC165 { + struct RoyaltyInfo { + address receiver; + uint96 royaltyFraction; + } + + RoyaltyInfo private _defaultRoyaltyInfo; + mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; + + /** + * @dev Sets the royalty info for a specific token id, overriding the default royalty info. + * + * Requirements: + * - `tokenId` must be already mined. + * - `receiver` cannot be the zero address. + * - `fraction` must indicate the percentage fraction using two decimals. + */ + function _setTokenRoyalty( + uint256 tokenId, + address receiver, + uint96 fraction + ) internal virtual { + require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); + require(receiver != address(0), "ERC2981: Invalid parameters"); + + _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); + } + + /** + * @dev Sets the royalty info that tokens will default to. + * + * Requirements: + * - `receiver` cannot be the zero address. + * - `fraction` must indicate the percentage fraction. Needs to be set appropriately + * according to the _feeDenominator granularity. + */ + function _setDefaultRoyalty(address receiver, uint96 fraction) internal virtual { + require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); + require(receiver != address(0), "ERC2981: Invalid receiver"); + + _defaultRoyaltyInfo = RoyaltyInfo(receiver, fraction); + } + + /** + * @dev See {IERC2981-royaltyInfo} + */ + function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { + RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; + + if (royalty.receiver == address(0)) { + royalty = _defaultRoyaltyInfo; + } + + uint256 royaltyAmount = (_salePrice * royalty.royaltyFraction) / _feeDenominator(); + + return (royalty.receiver, royaltyAmount); + } + + /** + * @dev Returns the percentage granularity being used. The default denominator is 10000 + * but it can be customized by an override. + */ + function _feeDenominator() internal pure virtual returns (uint96) { + return 10000; + } + + /** + * @dev Removes `tokenId` royalty information. + * The royalty information is cleared and the token royalty fallbacks to the default royalty. + * + * Requirements: + * + * - `tokenId` royalty information must exist. + * + */ + function _resetTokenRoyalty(uint256 tokenId) internal virtual { + delete _tokenRoyaltyInfo[tokenId]; + } + + /** + * @dev Removes default royalty information. + * + */ + function _deleteDefaultRoyalty() internal virtual { + delete _defaultRoyaltyInfo; + } +} diff --git a/test/token/ERC1155/extensions/ERC1155Royalty.test.js b/test/token/ERC1155/extensions/ERC1155Royalty.test.js new file mode 100644 index 00000000000..635b8c02a6f --- /dev/null +++ b/test/token/ERC1155/extensions/ERC1155Royalty.test.js @@ -0,0 +1,187 @@ +const { BN, constants, expectRevert } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); +const { ZERO_ADDRESS } = constants; + +const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior'); + +const ERC1155RoyaltyMock = artifacts.require('ERC1155RoyaltyMock'); + +contract('ERC1155Royalty', function (accounts) { + const [ account1, account2 ] = accounts; + const uri = 'https://token.com'; + const tokenId1 = new BN('1'); + const tokenId2 = new BN('2'); + const salePrice = new BN('1000'); + const royaltyFraction = new BN('10'); + const firstTokenAmount = new BN('42'); + const secondTokenAmount = new BN('23'); + + beforeEach(async function () { + this.token = await ERC1155RoyaltyMock.new(uri); + + await this.token.mint(account1, tokenId1, firstTokenAmount, '0x'); + await this.token.mint(account1, tokenId2, secondTokenAmount, '0x'); + }); + + it('calls supports interface', async function () { + const result = await this.token.supportsInterface('0x2a55205a'); + const expected = true; + + expect(result).to.not.equal(undefined); + expect(result).to.equal(expected); + }); + + shouldSupportInterfaces(['ERC2981']); + + describe('default royalty', function () { + beforeEach(async function () { + await this.token.setDefaultRoyalty(account1, royaltyFraction); + }); + it('checks royalty is set', async function () { + const royalty = new BN((salePrice * royaltyFraction) / 10000); + + const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(initInfo[0]).to.be.equal(account1); + expect(initInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('updates royalty amount', async function () { + const newPercentage = new BN('25'); + + // Updated royalty check + await this.token.setDefaultRoyalty(account1, newPercentage); + const royalty = new BN((salePrice * newPercentage) / 10000); + const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(newInfo[0]).to.be.equal(account1); + expect(newInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('holds same royalty value for different tokens', async function () { + const newPercentage = new BN('20'); + await this.token.setDefaultRoyalty(account1, newPercentage); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + }); + + it('Remove royalty information', async function () { + const newValue = new BN('0'); + await this.token.deleteRoyalty(); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + // Test royalty info is still persistent across all tokens + expect(token1Info[0]).to.be.bignumber.equal(token2Info[0]); + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + // Test information was deleted + expect(token1Info[0]).to.be.equal(ZERO_ADDRESS); + expect(token1Info[1]).to.be.bignumber.equal(newValue); + }); + + it('reverts if invalid parameters', async function () { + await expectRevert( + this.token.setDefaultRoyalty(ZERO_ADDRESS, royaltyFraction), + 'ERC2981: Invalid receiver', + ); + + await expectRevert( + this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), + 'ERC2981: Royalty percentage will exceed salePrice', + ); + }); + }); + + describe('token based royalty', function () { + beforeEach(async function () { + await this.token.setTokenRoyalty(tokenId1, account1, royaltyFraction); + }); + + it('updates royalty amount', async function () { + const newPercentage = new BN('25'); + let royalty = new BN((salePrice * royaltyFraction) / 10000); + // Initial royalty check + const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(initInfo[0]).to.be.equal(account1); + expect(initInfo[1]).to.be.bignumber.equal(royalty); + + // Updated royalty check + await this.token.setTokenRoyalty(tokenId1, account1, newPercentage); + royalty = new BN((salePrice * newPercentage) / 10000); + const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(newInfo[0]).to.be.equal(account1); + expect(newInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('holds different values for different tokens', async function () { + const newPercentage = new BN('20'); + await this.token.setTokenRoyalty(tokenId2, account1, newPercentage); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + + // must be different even at the same SalePrice + expect(token1Info[1]).to.not.be.equal(token2Info.royaltyFraction); + }); + + it('reverts if invalid parameters', async function () { + await expectRevert( + this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyFraction), + 'ERC2981: Invalid parameters', + ); + + await expectRevert( + this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), + 'ERC2981: Royalty percentage will exceed salePrice', + ); + }); + + it('removes royalty information after burn', async function () { + await this.token.burn(account1, tokenId1, firstTokenAmount); + const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + expect(tokenInfo[0]).to.be.equal(ZERO_ADDRESS); + expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); + }); + + it('can reset token after setting royalty', async function () { + const newPercentage = new BN('30'); + const royalty = new BN((salePrice * newPercentage) / 10000); + await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); + + const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); + + // Tokens must have own information + expect(tokenInfo[1]).to.be.bignumber.equal(royalty); + expect(tokenInfo[0]).to.be.equal(account2); + + await this.token.setTokenRoyalty(tokenId2, account1, new BN('0')); + const result = await this.token.royaltyInfo(tokenId2, salePrice); + // Token must not share default information + expect(result[0]).to.be.equal(account1); + expect(result[1]).to.be.bignumber.equal(new BN('0')); + }); + + it('can hold default and token royalty information', async function () { + const newPercentage = new BN('30'); + const royalty = new BN((salePrice * newPercentage) / 10000); + + await this.token.setTokenRoyalty(tokenId2, account2, newPercentage); + + const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); + const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); + // Tokens must not have same values + expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); + expect(token1Info[0]).to.not.be.equal(token2Info[0]); + + // Updated token must have new values + expect(token2Info[0]).to.be.equal(account2); + expect(token2Info[1]).to.be.bignumber.equal(royalty); + }); + }); +}); diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 97979aac0c5..322f4917b3d 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -136,11 +136,6 @@ contract('ERC721Royalty', function (accounts) { this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), 'ERC2981: Royalty percentage will exceed salePrice', ); - - await expectRevert( - this.token.setTokenRoyalty(new BN('787'), account1, new BN('100')), - 'ERC2981: Nonexistent token', - ); }); it('removes royalty information after burn', async function () { @@ -149,11 +144,6 @@ contract('ERC721Royalty', function (accounts) { expect(tokenInfo[0]).to.be.equal(ZERO_ADDRESS); expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); - - await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('100')), - 'ERC2981: Nonexistent token', - ); }); it('can reset token after setting royalty', async function () { From 3d826f8fd64d69621bbc73ef47e8f4850ead5774 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 15:06:41 -0400 Subject: [PATCH 30/51] Remove double supportinterface test --- test/token/ERC1155/extensions/ERC1155Royalty.test.js | 8 -------- test/token/ERC721/extensions/ERC721Royalty.test.js | 8 -------- 2 files changed, 16 deletions(-) diff --git a/test/token/ERC1155/extensions/ERC1155Royalty.test.js b/test/token/ERC1155/extensions/ERC1155Royalty.test.js index 635b8c02a6f..f808d74055c 100644 --- a/test/token/ERC1155/extensions/ERC1155Royalty.test.js +++ b/test/token/ERC1155/extensions/ERC1155Royalty.test.js @@ -23,14 +23,6 @@ contract('ERC1155Royalty', function (accounts) { await this.token.mint(account1, tokenId2, secondTokenAmount, '0x'); }); - it('calls supports interface', async function () { - const result = await this.token.supportsInterface('0x2a55205a'); - const expected = true; - - expect(result).to.not.equal(undefined); - expect(result).to.equal(expected); - }); - shouldSupportInterfaces(['ERC2981']); describe('default royalty', function () { diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 322f4917b3d..2b27741dba9 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -20,14 +20,6 @@ contract('ERC721Royalty', function (accounts) { await this.token.mint(account1, tokenId2); }); - it('calls supports interface', async function () { - const result = await this.token.supportsInterface('0x2a55205a'); - const expected = true; - - expect(result).to.not.equal(undefined); - expect(result).to.equal(expected); - }); - shouldSupportInterfaces(['ERC2981']); describe('default royalty', function () { From 7180b20851a9c94316347a21c6229d8f357998b4 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 15:38:56 -0400 Subject: [PATCH 31/51] Add the supportInterface override on the royalty base contract --- contracts/token/ERC1155/extensions/ERC1155Royalty.sol | 2 +- contracts/token/ERC721/extensions/ERC721Royalty.sol | 2 +- contracts/token/common/ERC2981.sol | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol index 9ef541d1028..4753aa54f57 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -23,7 +23,7 @@ abstract contract ERC1155Royalty is ERC2981, ERC1155 { /** * @dev See {IERC165-supportsInterface}. */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC1155, IERC165) returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC1155, ERC2981) returns (bool) { return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); } diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol index 5f8258657ae..c5aaf21b31b 100644 --- a/contracts/token/ERC721/extensions/ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -23,7 +23,7 @@ abstract contract ERC721Royalty is ERC2981, ERC721 { /** * @dev See {IERC165-supportsInterface}. */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, IERC165) returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC2981) returns (bool) { return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); } diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol index 0c31a695316..d141589c10d 100644 --- a/contracts/token/common/ERC2981.sol +++ b/contracts/token/common/ERC2981.sol @@ -26,6 +26,13 @@ abstract contract ERC2981 is IERC2981, ERC165 { RoyaltyInfo private _defaultRoyaltyInfo; mapping(uint256 => RoyaltyInfo) private _tokenRoyaltyInfo; + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { + return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); + } + /** * @dev Sets the royalty info for a specific token id, overriding the default royalty info. * From 8cf5939df4ba43c9fa5ff1bf0e9bb9ba7cce5022 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 16:53:47 -0400 Subject: [PATCH 32/51] Add Royalty tests behavior --- .../ERC1155/extensions/ERC1155Royalty.test.js | 159 ++--------------- .../ERC721/extensions/ERC721Royalty.test.js | 161 ++---------------- test/token/common/ERC2981.behavior.js | 159 +++++++++++++++++ 3 files changed, 180 insertions(+), 299 deletions(-) create mode 100644 test/token/common/ERC2981.behavior.js diff --git a/test/token/ERC1155/extensions/ERC1155Royalty.test.js b/test/token/ERC1155/extensions/ERC1155Royalty.test.js index f808d74055c..bd3963590dc 100644 --- a/test/token/ERC1155/extensions/ERC1155Royalty.test.js +++ b/test/token/ERC1155/extensions/ERC1155Royalty.test.js @@ -1,8 +1,9 @@ -const { BN, constants, expectRevert } = require('@openzeppelin/test-helpers'); +const { BN, constants } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); +const { describe } = require('yargs'); const { ZERO_ADDRESS } = constants; -const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior'); +const { shouldBehaveLikeERC2981 } = require('../../common/ERC2981.behavior'); const ERC1155RoyaltyMock = artifacts.require('ERC1155RoyaltyMock'); @@ -12,7 +13,6 @@ contract('ERC1155Royalty', function (accounts) { const tokenId1 = new BN('1'); const tokenId2 = new BN('2'); const salePrice = new BN('1000'); - const royaltyFraction = new BN('10'); const firstTokenAmount = new BN('42'); const secondTokenAmount = new BN('23'); @@ -21,118 +21,14 @@ contract('ERC1155Royalty', function (accounts) { await this.token.mint(account1, tokenId1, firstTokenAmount, '0x'); await this.token.mint(account1, tokenId2, secondTokenAmount, '0x'); + this.account1 = account1; + this.account2 = account2; + this.tokenId1 = tokenId1; + this.tokenId2 = tokenId2; + this.salePrice = salePrice; }); - shouldSupportInterfaces(['ERC2981']); - - describe('default royalty', function () { - beforeEach(async function () { - await this.token.setDefaultRoyalty(account1, royaltyFraction); - }); - it('checks royalty is set', async function () { - const royalty = new BN((salePrice * royaltyFraction) / 10000); - - const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(initInfo[0]).to.be.equal(account1); - expect(initInfo[1]).to.be.bignumber.equal(royalty); - }); - - it('updates royalty amount', async function () { - const newPercentage = new BN('25'); - - // Updated royalty check - await this.token.setDefaultRoyalty(account1, newPercentage); - const royalty = new BN((salePrice * newPercentage) / 10000); - const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(newInfo[0]).to.be.equal(account1); - expect(newInfo[1]).to.be.bignumber.equal(royalty); - }); - - it('holds same royalty value for different tokens', async function () { - const newPercentage = new BN('20'); - await this.token.setDefaultRoyalty(account1, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - - expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); - }); - - it('Remove royalty information', async function () { - const newValue = new BN('0'); - await this.token.deleteRoyalty(); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - // Test royalty info is still persistent across all tokens - expect(token1Info[0]).to.be.bignumber.equal(token2Info[0]); - expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); - // Test information was deleted - expect(token1Info[0]).to.be.equal(ZERO_ADDRESS); - expect(token1Info[1]).to.be.bignumber.equal(newValue); - }); - - it('reverts if invalid parameters', async function () { - await expectRevert( - this.token.setDefaultRoyalty(ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid receiver', - ); - - await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), - 'ERC2981: Royalty percentage will exceed salePrice', - ); - }); - }); - - describe('token based royalty', function () { - beforeEach(async function () { - await this.token.setTokenRoyalty(tokenId1, account1, royaltyFraction); - }); - - it('updates royalty amount', async function () { - const newPercentage = new BN('25'); - let royalty = new BN((salePrice * royaltyFraction) / 10000); - // Initial royalty check - const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(initInfo[0]).to.be.equal(account1); - expect(initInfo[1]).to.be.bignumber.equal(royalty); - - // Updated royalty check - await this.token.setTokenRoyalty(tokenId1, account1, newPercentage); - royalty = new BN((salePrice * newPercentage) / 10000); - const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(newInfo[0]).to.be.equal(account1); - expect(newInfo[1]).to.be.bignumber.equal(royalty); - }); - - it('holds different values for different tokens', async function () { - const newPercentage = new BN('20'); - await this.token.setTokenRoyalty(tokenId2, account1, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - - // must be different even at the same SalePrice - expect(token1Info[1]).to.not.be.equal(token2Info.royaltyFraction); - }); - - it('reverts if invalid parameters', async function () { - await expectRevert( - this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid parameters', - ); - - await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), - 'ERC2981: Royalty percentage will exceed salePrice', - ); - }); - + describe('token specific functions', function () { it('removes royalty information after burn', async function () { await this.token.burn(account1, tokenId1, firstTokenAmount); const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); @@ -140,40 +36,7 @@ contract('ERC1155Royalty', function (accounts) { expect(tokenInfo[0]).to.be.equal(ZERO_ADDRESS); expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); }); - - it('can reset token after setting royalty', async function () { - const newPercentage = new BN('30'); - const royalty = new BN((salePrice * newPercentage) / 10000); - await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); - - const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - // Tokens must have own information - expect(tokenInfo[1]).to.be.bignumber.equal(royalty); - expect(tokenInfo[0]).to.be.equal(account2); - - await this.token.setTokenRoyalty(tokenId2, account1, new BN('0')); - const result = await this.token.royaltyInfo(tokenId2, salePrice); - // Token must not share default information - expect(result[0]).to.be.equal(account1); - expect(result[1]).to.be.bignumber.equal(new BN('0')); - }); - - it('can hold default and token royalty information', async function () { - const newPercentage = new BN('30'); - const royalty = new BN((salePrice * newPercentage) / 10000); - - await this.token.setTokenRoyalty(tokenId2, account2, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - // Tokens must not have same values - expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); - expect(token1Info[0]).to.not.be.equal(token2Info[0]); - - // Updated token must have new values - expect(token2Info[0]).to.be.equal(account2); - expect(token2Info[1]).to.be.bignumber.equal(royalty); - }); }); + + shouldBehaveLikeERC2981(); }); diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 2b27741dba9..9ef2ee24e69 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -1,135 +1,28 @@ -const { BN, constants, expectRevert } = require('@openzeppelin/test-helpers'); -const { expect } = require('chai'); +const { BN, constants } = require('@openzeppelin/test-helpers'); +const ERC721RoyaltyMock = artifacts.require('ERC721RoyaltyMock'); const { ZERO_ADDRESS } = constants; -const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior'); - -const ERC721RoyaltyMock = artifacts.require('ERC721RoyaltyMock'); +const { shouldBehaveLikeERC2981 } = require('../../common/ERC2981.behavior'); contract('ERC721Royalty', function (accounts) { const [ account1, account2 ] = accounts; const tokenId1 = new BN('1'); const tokenId2 = new BN('2'); const salePrice = new BN('1000'); - const royaltyFraction = new BN('10'); beforeEach(async function () { this.token = await ERC721RoyaltyMock.new('My Token', 'TKN'); await this.token.mint(account1, tokenId1); await this.token.mint(account1, tokenId2); + this.account1 = account1; + this.account2 = account2; + this.tokenId1 = tokenId1; + this.tokenId2 = tokenId2; + this.salePrice = salePrice; }); - shouldSupportInterfaces(['ERC2981']); - - describe('default royalty', function () { - beforeEach(async function () { - await this.token.setDefaultRoyalty(account1, royaltyFraction); - }); - it('checks royalty is set', async function () { - const royalty = new BN((salePrice * royaltyFraction) / 10000); - - const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(initInfo[0]).to.be.equal(account1); - expect(initInfo[1]).to.be.bignumber.equal(royalty); - }); - - it('updates royalty amount', async function () { - const newPercentage = new BN('25'); - - // Updated royalty check - await this.token.setDefaultRoyalty(account1, newPercentage); - const royalty = new BN((salePrice * newPercentage) / 10000); - const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(newInfo[0]).to.be.equal(account1); - expect(newInfo[1]).to.be.bignumber.equal(royalty); - }); - - it('holds same royalty value for different tokens', async function () { - const newPercentage = new BN('20'); - await this.token.setDefaultRoyalty(account1, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - - expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); - }); - - it('Remove royalty information', async function () { - const newValue = new BN('0'); - await this.token.deleteRoyalty(); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - // Test royalty info is still persistent across all tokens - expect(token1Info[0]).to.be.bignumber.equal(token2Info[0]); - expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); - // Test information was deleted - expect(token1Info[0]).to.be.equal(ZERO_ADDRESS); - expect(token1Info[1]).to.be.bignumber.equal(newValue); - }); - - it('reverts if invalid parameters', async function () { - await expectRevert( - this.token.setDefaultRoyalty(ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid receiver', - ); - - await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), - 'ERC2981: Royalty percentage will exceed salePrice', - ); - }); - }); - - describe('token based royalty', function () { - beforeEach(async function () { - await this.token.setTokenRoyalty(tokenId1, account1, royaltyFraction); - }); - - it('updates royalty amount', async function () { - const newPercentage = new BN('25'); - let royalty = new BN((salePrice * royaltyFraction) / 10000); - // Initial royalty check - const initInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(initInfo[0]).to.be.equal(account1); - expect(initInfo[1]).to.be.bignumber.equal(royalty); - - // Updated royalty check - await this.token.setTokenRoyalty(tokenId1, account1, newPercentage); - royalty = new BN((salePrice * newPercentage) / 10000); - const newInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(newInfo[0]).to.be.equal(account1); - expect(newInfo[1]).to.be.bignumber.equal(royalty); - }); - - it('holds different values for different tokens', async function () { - const newPercentage = new BN('20'); - await this.token.setTokenRoyalty(tokenId2, account1, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - - // must be different even at the same SalePrice - expect(token1Info[1]).to.not.be.equal(token2Info.royaltyFraction); - }); - - it('reverts if invalid parameters', async function () { - await expectRevert( - this.token.setTokenRoyalty(tokenId1, ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid parameters', - ); - - await expectRevert( - this.token.setTokenRoyalty(tokenId1, account1, new BN('11000')), - 'ERC2981: Royalty percentage will exceed salePrice', - ); - }); - + describe('token specific functions', function () { it('removes royalty information after burn', async function () { await this.token.burn(tokenId1); const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); @@ -137,40 +30,6 @@ contract('ERC721Royalty', function (accounts) { expect(tokenInfo[0]).to.be.equal(ZERO_ADDRESS); expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); }); - - it('can reset token after setting royalty', async function () { - const newPercentage = new BN('30'); - const royalty = new BN((salePrice * newPercentage) / 10000); - await this.token.setTokenRoyalty(tokenId1, account2, newPercentage); - - const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - // Tokens must have own information - expect(tokenInfo[1]).to.be.bignumber.equal(royalty); - expect(tokenInfo[0]).to.be.equal(account2); - - await this.token.setTokenRoyalty(tokenId2, account1, new BN('0')); - const result = await this.token.royaltyInfo(tokenId2, salePrice); - // Token must not share default information - expect(result[0]).to.be.equal(account1); - expect(result[1]).to.be.bignumber.equal(new BN('0')); - }); - - it('can hold default and token royalty information', async function () { - const newPercentage = new BN('30'); - const royalty = new BN((salePrice * newPercentage) / 10000); - - await this.token.setTokenRoyalty(tokenId2, account2, newPercentage); - - const token1Info = await this.token.royaltyInfo(tokenId1, salePrice); - const token2Info = await this.token.royaltyInfo(tokenId2, salePrice); - // Tokens must not have same values - expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); - expect(token1Info[0]).to.not.be.equal(token2Info[0]); - - // Updated token must have new values - expect(token2Info[0]).to.be.equal(account2); - expect(token2Info[1]).to.be.bignumber.equal(royalty); - }); }); + shouldBehaveLikeERC2981(); }); diff --git a/test/token/common/ERC2981.behavior.js b/test/token/common/ERC2981.behavior.js new file mode 100644 index 00000000000..47fa96a595f --- /dev/null +++ b/test/token/common/ERC2981.behavior.js @@ -0,0 +1,159 @@ +const { BN, constants, expectRevert } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); +const { ZERO_ADDRESS } = constants; + +const { shouldSupportInterfaces } = require('../../utils/introspection/SupportsInterface.behavior'); + +function shouldBehaveLikeERC2981 () { + const royaltyFraction = new BN('10'); + + shouldSupportInterfaces(['ERC2981']); + + describe('default royalty', function () { + beforeEach(async function () { + await this.token.setDefaultRoyalty(this.account1, royaltyFraction); + }); + it('checks royalty is set', async function () { + const royalty = new BN((this.salePrice * royaltyFraction) / 10000); + + const initInfo = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + + expect(initInfo[0]).to.be.equal(this.account1); + expect(initInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('updates royalty amount', async function () { + const newPercentage = new BN('25'); + + // Updated royalty check + await this.token.setDefaultRoyalty(this.account1, newPercentage); + const royalty = new BN((this.salePrice * newPercentage) / 10000); + const newInfo = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + + expect(newInfo[0]).to.be.equal(this.account1); + expect(newInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('holds same royalty value for different tokens', async function () { + const newPercentage = new BN('20'); + await this.token.setDefaultRoyalty(this.account1, newPercentage); + + const token1Info = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + const token2Info = await this.token.royaltyInfo(this.tokenId2, this.salePrice); + + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + }); + + it('Remove royalty information', async function () { + const newValue = new BN('0'); + await this.token.deleteRoyalty(); + + const token1Info = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + const token2Info = await this.token.royaltyInfo(this.tokenId2, this.salePrice); + // Test royalty info is still persistent across all tokens + expect(token1Info[0]).to.be.bignumber.equal(token2Info[0]); + expect(token1Info[1]).to.be.bignumber.equal(token2Info[1]); + // Test information was deleted + expect(token1Info[0]).to.be.equal(ZERO_ADDRESS); + expect(token1Info[1]).to.be.bignumber.equal(newValue); + }); + + it('reverts if invalid parameters', async function () { + await expectRevert( + this.token.setDefaultRoyalty(ZERO_ADDRESS, royaltyFraction), + 'ERC2981: Invalid receiver', + ); + + await expectRevert( + this.token.setTokenRoyalty(this.tokenId1, this.account1, new BN('11000')), + 'ERC2981: Royalty percentage will exceed salePrice', + ); + }); + }); + + describe('token based royalty', function () { + beforeEach(async function () { + await this.token.setTokenRoyalty(this.tokenId1, this.account1, royaltyFraction); + }); + + it('updates royalty amount', async function () { + const newPercentage = new BN('25'); + let royalty = new BN((this.salePrice * royaltyFraction) / 10000); + // Initial royalty check + const initInfo = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + + expect(initInfo[0]).to.be.equal(this.account1); + expect(initInfo[1]).to.be.bignumber.equal(royalty); + + // Updated royalty check + await this.token.setTokenRoyalty(this.tokenId1, this.account1, newPercentage); + royalty = new BN((this.salePrice * newPercentage) / 10000); + const newInfo = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + + expect(newInfo[0]).to.be.equal(this.account1); + expect(newInfo[1]).to.be.bignumber.equal(royalty); + }); + + it('holds different values for different tokens', async function () { + const newPercentage = new BN('20'); + await this.token.setTokenRoyalty(this.tokenId2, this.account1, newPercentage); + + const token1Info = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + const token2Info = await this.token.royaltyInfo(this.tokenId2, this.salePrice); + + // must be different even at the same this.salePrice + expect(token1Info[1]).to.not.be.equal(token2Info.royaltyFraction); + }); + + it('reverts if invalid parameters', async function () { + await expectRevert( + this.token.setTokenRoyalty(this.tokenId1, ZERO_ADDRESS, royaltyFraction), + 'ERC2981: Invalid parameters', + ); + + await expectRevert( + this.token.setTokenRoyalty(this.tokenId1, this.account1, new BN('11000')), + 'ERC2981: Royalty percentage will exceed salePrice', + ); + }); + + it('can reset token after setting royalty', async function () { + const newPercentage = new BN('30'); + const royalty = new BN((this.salePrice * newPercentage) / 10000); + await this.token.setTokenRoyalty(this.tokenId1, this.account2, newPercentage); + + const tokenInfo = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + + // Tokens must have own information + expect(tokenInfo[1]).to.be.bignumber.equal(royalty); + expect(tokenInfo[0]).to.be.equal(this.account2); + + await this.token.setTokenRoyalty(this.tokenId2, this.account1, new BN('0')); + const result = await this.token.royaltyInfo(this.tokenId2, this.salePrice); + // Token must not share default information + expect(result[0]).to.be.equal(this.account1); + expect(result[1]).to.be.bignumber.equal(new BN('0')); + }); + + it('can hold default and token royalty information', async function () { + const newPercentage = new BN('30'); + const royalty = new BN((this.salePrice * newPercentage) / 10000); + + await this.token.setTokenRoyalty(this.tokenId2, this.account2, newPercentage); + + const token1Info = await this.token.royaltyInfo(this.tokenId1, this.salePrice); + const token2Info = await this.token.royaltyInfo(this.tokenId2, this.salePrice); + // Tokens must not have same values + expect(token1Info[1]).to.not.be.bignumber.equal(token2Info[1]); + expect(token1Info[0]).to.not.be.equal(token2Info[0]); + + // Updated token must have new values + expect(token2Info[0]).to.be.equal(this.account2); + expect(token2Info[1]).to.be.bignumber.equal(royalty); + }); + }); +} + +module.exports = { + shouldBehaveLikeERC2981, +}; From 1ce08dad987af892452841060d19ed511ea7e054 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Thu, 16 Dec 2021 17:12:04 -0400 Subject: [PATCH 33/51] Update ERC1155 royalty test file --- test/token/ERC1155/extensions/ERC1155Royalty.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/token/ERC1155/extensions/ERC1155Royalty.test.js b/test/token/ERC1155/extensions/ERC1155Royalty.test.js index bd3963590dc..36b96b766de 100644 --- a/test/token/ERC1155/extensions/ERC1155Royalty.test.js +++ b/test/token/ERC1155/extensions/ERC1155Royalty.test.js @@ -1,6 +1,5 @@ const { BN, constants } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); -const { describe } = require('yargs'); const { ZERO_ADDRESS } = constants; const { shouldBehaveLikeERC2981 } = require('../../common/ERC2981.behavior'); From 7ab210f4054f6ddc235b3df172fdb46c7ca7747d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 20 Dec 2021 14:28:57 +0100 Subject: [PATCH 34/51] cleanup ERC165 override --- contracts/token/ERC1155/extensions/ERC1155Royalty.sol | 2 +- contracts/token/ERC721/extensions/ERC721Royalty.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol index 4753aa54f57..32827771534 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -24,7 +24,7 @@ abstract contract ERC1155Royalty is ERC2981, ERC1155 { * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(ERC1155, ERC2981) returns (bool) { - return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); + return super.supportsInterface(interfaceId); } /** diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol index c5aaf21b31b..587aa6d1831 100644 --- a/contracts/token/ERC721/extensions/ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -24,7 +24,7 @@ abstract contract ERC721Royalty is ERC2981, ERC721 { * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC2981) returns (bool) { - return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); + return super.supportsInterface(interfaceId); } /** From c2c11bb52a3f6e8aa951e9e7774e4a322776a55f Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 20 Dec 2021 11:12:09 -0400 Subject: [PATCH 35/51] Update burn implementation for ERC1155 --- .../ERC1155/extensions/ERC1155Royalty.sol | 23 +++++++++++-------- .../ERC1155/extensions/ERC1155Royalty.test.js | 15 +++++++++++- .../ERC721/extensions/ERC721Royalty.test.js | 5 ++++ test/token/common/ERC2981.behavior.js | 1 + 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol index 32827771534..9caa93b7e12 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; -import "../ERC1155.sol"; +import "./ERC1155Supply.sol"; import "../../common/ERC2981.sol"; import "../../../utils/introspection/ERC165.sol"; @@ -11,15 +11,17 @@ import "../../../utils/introspection/ERC165.sol"; * @dev Implementation of the ERC1155 Royalty extension allowing royalty information to be stored and retrieved, as defined in * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. * - * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setDefaultRoyalty} method to set a default - * royalty information. + * Adds the {_setTokenRoyalty} methods to set the token royalty information, the {_setDefaultRoyalty} method to set a default + * royalty information, and the override of the burn method to restore royalty information. * * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. * * _Available since v4.5._ + * WARNING: Be aware not to inherit from ERC1155Supply when inheriting from this contract since some operations will be more expensive. + * */ -abstract contract ERC1155Royalty is ERC2981, ERC1155 { +abstract contract ERC1155Royalty is ERC2981, ERC1155Supply { /** * @dev See {IERC165-supportsInterface}. */ @@ -28,14 +30,13 @@ abstract contract ERC1155Royalty is ERC2981, ERC1155 { } /** - * @dev Destroys `tokenId`. - * The royalty information is cleared when the token is burned. + * @dev Destroys `amount` tokens of token type `id` from `from` + * The royalty information is cleared when the token is burned and there are no more tokens minted for that id. * * Requirements: * - * - `tokenId` must exist. - * - * Emits a {Transfer} event. + * - `from` cannot be the zero address. + * - `from` must have at least `amount` tokens of token type `id`. */ function _burn( address from, @@ -43,6 +44,8 @@ abstract contract ERC1155Royalty is ERC2981, ERC1155 { uint256 amount ) internal virtual override { super._burn(from, id, amount); - _resetTokenRoyalty(id); + if (!exists(id)) { + _resetTokenRoyalty(id); + } } } diff --git a/test/token/ERC1155/extensions/ERC1155Royalty.test.js b/test/token/ERC1155/extensions/ERC1155Royalty.test.js index 36b96b766de..c97d52d86e1 100644 --- a/test/token/ERC1155/extensions/ERC1155Royalty.test.js +++ b/test/token/ERC1155/extensions/ERC1155Royalty.test.js @@ -11,6 +11,7 @@ contract('ERC1155Royalty', function (accounts) { const uri = 'https://token.com'; const tokenId1 = new BN('1'); const tokenId2 = new BN('2'); + const royalty = new BN('200'); const salePrice = new BN('1000'); const firstTokenAmount = new BN('42'); const secondTokenAmount = new BN('23'); @@ -28,7 +29,19 @@ contract('ERC1155Royalty', function (accounts) { }); describe('token specific functions', function () { - it('removes royalty information after burn', async function () { + beforeEach(async function () { + await this.token.setTokenRoyalty(tokenId1, account1, royalty); + }); + + it('keeps royalty information after burning single token for id', async function () { + await this.token.burn(account1, tokenId1, new BN('1')); + const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); + const result = new BN((this.salePrice * royalty) / 10000); + expect(tokenInfo[0]).to.be.equal(account1); + expect(tokenInfo[1]).to.be.bignumber.equal(result); + }); + + it('removes royalty information after burning all tokens for id', async function () { await this.token.burn(account1, tokenId1, firstTokenAmount); const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 9ef2ee24e69..29b28dd00a3 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -8,6 +8,7 @@ contract('ERC721Royalty', function (accounts) { const [ account1, account2 ] = accounts; const tokenId1 = new BN('1'); const tokenId2 = new BN('2'); + const royalty = new BN('200'); const salePrice = new BN('1000'); beforeEach(async function () { @@ -23,6 +24,10 @@ contract('ERC721Royalty', function (accounts) { }); describe('token specific functions', function () { + beforeEach(async function () { + await this.token.setTokenRoyalty(tokenId1, account1, royalty); + }); + it('removes royalty information after burn', async function () { await this.token.burn(tokenId1); const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); diff --git a/test/token/common/ERC2981.behavior.js b/test/token/common/ERC2981.behavior.js index 47fa96a595f..3196342e783 100644 --- a/test/token/common/ERC2981.behavior.js +++ b/test/token/common/ERC2981.behavior.js @@ -13,6 +13,7 @@ function shouldBehaveLikeERC2981 () { beforeEach(async function () { await this.token.setDefaultRoyalty(this.account1, royaltyFraction); }); + it('checks royalty is set', async function () { const royalty = new BN((this.salePrice * royaltyFraction) / 10000); From 55aa14fb00e43c8065c3169925b7659ca9638a93 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Mon, 20 Dec 2021 16:19:19 -0400 Subject: [PATCH 36/51] Add warning detail --- contracts/token/ERC1155/extensions/ERC1155Royalty.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol index 9caa93b7e12..db7b7c4931d 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -17,8 +17,10 @@ import "../../../utils/introspection/ERC165.sol"; * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. * + * WARNING: Be aware not to inherit from ERC1155Supply when inheriting from this contract, this would cause an expensive + * minting operation. + * * _Available since v4.5._ - * WARNING: Be aware not to inherit from ERC1155Supply when inheriting from this contract since some operations will be more expensive. * */ abstract contract ERC1155Royalty is ERC2981, ERC1155Supply { From 2845801e2ed365ab3976cb862f695085b2b13c00 Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Tue, 21 Dec 2021 17:56:40 -0400 Subject: [PATCH 37/51] Add warning details --- contracts/token/ERC1155/extensions/ERC1155Royalty.sol | 5 +++-- slither.db.json | 0 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 slither.db.json diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol index db7b7c4931d..051ac96e0a1 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -17,8 +17,9 @@ import "../../../utils/introspection/ERC165.sol"; * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. * - * WARNING: Be aware not to inherit from ERC1155Supply when inheriting from this contract, this would cause an expensive - * minting operation. + * WARNING: This contract inherits from ERC1155Supply, which has a higher minting cost than the base ERC1155 contract. To avoid this + * cost use the ERC1155 base contract and inherit ERC2981 directly, with the downside that burning all the supply for a token will + * not delete the royalty info from storage. * * _Available since v4.5._ * diff --git a/slither.db.json b/slither.db.json new file mode 100644 index 00000000000..e69de29bb2d From 90feaf4adab376a6724c1cba887f1584ae0d0aaf Mon Sep 17 00:00:00 2001 From: JulissaDantes Date: Wed, 22 Dec 2021 08:27:12 -0400 Subject: [PATCH 38/51] Update changelog after latest changes --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64574a2be14..6d014346a61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased - * `ERC2891`: add a new extension of `ERC721` to handle royalty information.([#3012](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3012)) + * `ERC2891`: add implementation of the royalty standard, and the respective extensions for `ERC721` and `ERC1155`.([#3012](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3012)) * `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977)) * `Math`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984)) * Preset contracts are now deprecated in favor of [Contracts Wizard](https://wizard.openzeppelin.com). ([#2986](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2986)) From da0e9bcfac06e7fc0115480013af57fad153516c Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Jan 2022 21:33:31 -0300 Subject: [PATCH 39/51] whitespace --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d014346a61..ca070d3436b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased - * `ERC2891`: add implementation of the royalty standard, and the respective extensions for `ERC721` and `ERC1155`.([#3012](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3012)) + * `ERC2891`: add implementation of the royalty standard, and the respective extensions for `ERC721` and `ERC1155`. ([#3012](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3012)) * `GovernorTimelockControl`: improve the `state()` function to have it reflect cases where a proposal has been canceled directly on the timelock. ([#2977](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2977)) * `Math`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984)) * Preset contracts are now deprecated in favor of [Contracts Wizard](https://wizard.openzeppelin.com). ([#2986](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2986)) From 2a848dfef3787bbeef8764bce999996b9e0f8eb1 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Jan 2022 21:34:07 -0300 Subject: [PATCH 40/51] rename deleteRoyalty -> deleteDefaultRoyalty --- contracts/mocks/ERC1155RoyaltyMock.sol | 2 +- contracts/mocks/ERC721RoyaltyMock.sol | 2 +- test/token/common/ERC2981.behavior.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/mocks/ERC1155RoyaltyMock.sol b/contracts/mocks/ERC1155RoyaltyMock.sol index 8c7c2132fb3..6ddbf4e4547 100644 --- a/contracts/mocks/ERC1155RoyaltyMock.sol +++ b/contracts/mocks/ERC1155RoyaltyMock.sol @@ -36,7 +36,7 @@ contract ERC1155RoyaltyMock is ERC1155Royalty { _burn(from, id, amount); } - function deleteRoyalty() public { + function deleteDefaultRoyalty() public { _deleteDefaultRoyalty(); } } diff --git a/contracts/mocks/ERC721RoyaltyMock.sol b/contracts/mocks/ERC721RoyaltyMock.sol index 93d5369a34e..83a9074e2c1 100644 --- a/contracts/mocks/ERC721RoyaltyMock.sol +++ b/contracts/mocks/ERC721RoyaltyMock.sol @@ -27,7 +27,7 @@ contract ERC721RoyaltyMock is ERC721Royalty { _burn(tokenId); } - function deleteRoyalty() public { + function deleteDefaultRoyalty() public { _deleteDefaultRoyalty(); } } diff --git a/test/token/common/ERC2981.behavior.js b/test/token/common/ERC2981.behavior.js index 3196342e783..db270f377b9 100644 --- a/test/token/common/ERC2981.behavior.js +++ b/test/token/common/ERC2981.behavior.js @@ -47,7 +47,7 @@ function shouldBehaveLikeERC2981 () { it('Remove royalty information', async function () { const newValue = new BN('0'); - await this.token.deleteRoyalty(); + await this.token.deleteDefaultRoyalty(); const token1Info = await this.token.royaltyInfo(this.tokenId1, this.salePrice); const token2Info = await this.token.royaltyInfo(this.tokenId2, this.salePrice); From 40214df1245b7f7118340fe487c6c3116e8c7ad2 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Jan 2022 21:35:36 -0300 Subject: [PATCH 41/51] whitespace --- contracts/token/ERC1155/extensions/ERC1155Royalty.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol index 051ac96e0a1..059af61819e 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol @@ -22,7 +22,6 @@ import "../../../utils/introspection/ERC165.sol"; * not delete the royalty info from storage. * * _Available since v4.5._ - * */ abstract contract ERC1155Royalty is ERC2981, ERC1155Supply { /** From bf10a4f4f5e9f0ea6e4553aafb9f09e708256bb0 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Jan 2022 22:13:16 -0300 Subject: [PATCH 42/51] remove slither.db.json --- slither.db.json | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 slither.db.json diff --git a/slither.db.json b/slither.db.json deleted file mode 100644 index e69de29bb2d..00000000000 From a927dc73bd6476ba4701f69d085354bdfdddd911 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 17:07:32 -0300 Subject: [PATCH 43/51] improve docs for ERC2981 --- contracts/interfaces/IERC2981.sol | 9 +++------ contracts/token/common/ERC2981.sol | 11 ++++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/contracts/interfaces/IERC2981.sol b/contracts/interfaces/IERC2981.sol index 4ebbaf28b5f..8f7616b08bf 100644 --- a/contracts/interfaces/IERC2981.sol +++ b/contracts/interfaces/IERC2981.sol @@ -6,7 +6,7 @@ pragma solidity ^0.8.0; import "./IERC165.sol"; /** - * @dev Interface for the NFT Royalty Standard + * @dev Interface for the NFT Royalty Standard. * * A standardized way to retrieve royalty payment information for non-fungible tokens (NFTs) to enable universal * support for royalty payments across all NFT marketplaces and ecosystem participants. @@ -15,11 +15,8 @@ import "./IERC165.sol"; */ interface IERC2981 is IERC165 { /** - * @dev Called with the sale price to determine how much royalty is owed and to whom. - * @param tokenId - the NFT asset queried for royalty information - * @param salePrice - the sale price of the NFT asset specified by `tokenId` - * @return receiver - address of who should be sent the royalty payment - * @return royaltyAmount - the royalty payment amount for `salePrice` + * @dev Returns how much royalty is owed and to whom, based on a sale price that may be denominated in any unit of + * exchange. The royalty amount is denominated and should be payed in that same unit of exchange. */ function royaltyInfo(uint256 tokenId, uint256 salePrice) external diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol index d141589c10d..8350dd57581 100644 --- a/contracts/token/common/ERC2981.sol +++ b/contracts/token/common/ERC2981.sol @@ -7,13 +7,14 @@ import "../../interfaces/IERC2981.sol"; import "../../utils/introspection/ERC165.sol"; /** - * @dev Implementation of the Royalty Standard allowing royalty information to be stored and retrieved. + * @dev Implementation of the NFT Royalty Standard, a standardized way to retrieve royalty payment information. * - * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setDefaultRoyalty} method to set a default - * royalty information. + * Royalty information can be specified globally for all token ids via {_setDefaultRoyalty}, and/or individually for + * specific token ids via {_setTokenRoyalty}. The latter takes precedence over the first. * - * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. - * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. + * IMPORTANT: ERC-2981 only specifies a way to signal royalty information and does not enforce its payment. See + * https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. Marketplaces are expected to + * voluntarily pay royalties together with sales, but note that this standard is not yet widely supported. * * _Available since v4.5._ */ From 1f7eeae2557ba1205dffada5f79544246856c679 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 17:13:33 -0300 Subject: [PATCH 44/51] remove ERC1155Royalty, not safe to reset royalties if supply goes to zero --- contracts/mocks/ERC1155RoyaltyMock.sol | 42 --------------- .../ERC1155/extensions/ERC1155Royalty.sol | 53 ------------------ .../ERC1155/extensions/ERC1155Royalty.test.js | 54 ------------------- 3 files changed, 149 deletions(-) delete mode 100644 contracts/mocks/ERC1155RoyaltyMock.sol delete mode 100644 contracts/token/ERC1155/extensions/ERC1155Royalty.sol delete mode 100644 test/token/ERC1155/extensions/ERC1155Royalty.test.js diff --git a/contracts/mocks/ERC1155RoyaltyMock.sol b/contracts/mocks/ERC1155RoyaltyMock.sol deleted file mode 100644 index 6ddbf4e4547..00000000000 --- a/contracts/mocks/ERC1155RoyaltyMock.sol +++ /dev/null @@ -1,42 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; - -import "../token/ERC1155/extensions/ERC1155Royalty.sol"; - -contract ERC1155RoyaltyMock is ERC1155Royalty { - constructor(string memory uri) ERC1155(uri) {} - - function setTokenRoyalty( - uint256 tokenId, - address recipient, - uint96 fraction - ) public { - _setTokenRoyalty(tokenId, recipient, fraction); - } - - function setDefaultRoyalty(address recipient, uint96 fraction) public { - _setDefaultRoyalty(recipient, fraction); - } - - function mint( - address to, - uint256 id, - uint256 value, - bytes memory data - ) public { - _mint(to, id, value, data); - } - - function burn( - address from, - uint256 id, - uint256 amount - ) public { - _burn(from, id, amount); - } - - function deleteDefaultRoyalty() public { - _deleteDefaultRoyalty(); - } -} diff --git a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol b/contracts/token/ERC1155/extensions/ERC1155Royalty.sol deleted file mode 100644 index 059af61819e..00000000000 --- a/contracts/token/ERC1155/extensions/ERC1155Royalty.sol +++ /dev/null @@ -1,53 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts v4.4.0 (token/ERC1155/extensions/ERC1155Royalty.sol) - -pragma solidity ^0.8.0; - -import "./ERC1155Supply.sol"; -import "../../common/ERC2981.sol"; -import "../../../utils/introspection/ERC165.sol"; - -/** - * @dev Implementation of the ERC1155 Royalty extension allowing royalty information to be stored and retrieved, as defined in - * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. - * - * Adds the {_setTokenRoyalty} methods to set the token royalty information, the {_setDefaultRoyalty} method to set a default - * royalty information, and the override of the burn method to restore royalty information. - * - * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. - * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. - * - * WARNING: This contract inherits from ERC1155Supply, which has a higher minting cost than the base ERC1155 contract. To avoid this - * cost use the ERC1155 base contract and inherit ERC2981 directly, with the downside that burning all the supply for a token will - * not delete the royalty info from storage. - * - * _Available since v4.5._ - */ -abstract contract ERC1155Royalty is ERC2981, ERC1155Supply { - /** - * @dev See {IERC165-supportsInterface}. - */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC1155, ERC2981) returns (bool) { - return super.supportsInterface(interfaceId); - } - - /** - * @dev Destroys `amount` tokens of token type `id` from `from` - * The royalty information is cleared when the token is burned and there are no more tokens minted for that id. - * - * Requirements: - * - * - `from` cannot be the zero address. - * - `from` must have at least `amount` tokens of token type `id`. - */ - function _burn( - address from, - uint256 id, - uint256 amount - ) internal virtual override { - super._burn(from, id, amount); - if (!exists(id)) { - _resetTokenRoyalty(id); - } - } -} diff --git a/test/token/ERC1155/extensions/ERC1155Royalty.test.js b/test/token/ERC1155/extensions/ERC1155Royalty.test.js deleted file mode 100644 index c97d52d86e1..00000000000 --- a/test/token/ERC1155/extensions/ERC1155Royalty.test.js +++ /dev/null @@ -1,54 +0,0 @@ -const { BN, constants } = require('@openzeppelin/test-helpers'); -const { expect } = require('chai'); -const { ZERO_ADDRESS } = constants; - -const { shouldBehaveLikeERC2981 } = require('../../common/ERC2981.behavior'); - -const ERC1155RoyaltyMock = artifacts.require('ERC1155RoyaltyMock'); - -contract('ERC1155Royalty', function (accounts) { - const [ account1, account2 ] = accounts; - const uri = 'https://token.com'; - const tokenId1 = new BN('1'); - const tokenId2 = new BN('2'); - const royalty = new BN('200'); - const salePrice = new BN('1000'); - const firstTokenAmount = new BN('42'); - const secondTokenAmount = new BN('23'); - - beforeEach(async function () { - this.token = await ERC1155RoyaltyMock.new(uri); - - await this.token.mint(account1, tokenId1, firstTokenAmount, '0x'); - await this.token.mint(account1, tokenId2, secondTokenAmount, '0x'); - this.account1 = account1; - this.account2 = account2; - this.tokenId1 = tokenId1; - this.tokenId2 = tokenId2; - this.salePrice = salePrice; - }); - - describe('token specific functions', function () { - beforeEach(async function () { - await this.token.setTokenRoyalty(tokenId1, account1, royalty); - }); - - it('keeps royalty information after burning single token for id', async function () { - await this.token.burn(account1, tokenId1, new BN('1')); - const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); - const result = new BN((this.salePrice * royalty) / 10000); - expect(tokenInfo[0]).to.be.equal(account1); - expect(tokenInfo[1]).to.be.bignumber.equal(result); - }); - - it('removes royalty information after burning all tokens for id', async function () { - await this.token.burn(account1, tokenId1, firstTokenAmount); - const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); - - expect(tokenInfo[0]).to.be.equal(ZERO_ADDRESS); - expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); - }); - }); - - shouldBehaveLikeERC2981(); -}); From e2e4a56b300a456b23d1fe2d7cf82c2490099a90 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 17:16:33 -0300 Subject: [PATCH 45/51] improve ERC721Royalty docs --- contracts/token/ERC721/extensions/ERC721Royalty.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol index 587aa6d1831..5dd0f6e17ca 100644 --- a/contracts/token/ERC721/extensions/ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -8,14 +8,14 @@ import "../../common/ERC2981.sol"; import "../../../utils/introspection/ERC165.sol"; /** - * @dev Implementation of the ERC721 Royalty extension allowing royalty information to be stored and retrieved, as defined in - * https://eips.ethereum.org/EIPS/eip-2981[EIP-2981]. + * @dev Extension of ERC721 with the NFT Royalty Standard, a standardized way to retrieve royalty payment information. * - * Adds the {_setTokenRoyalty} methods to set the token royalty information, and {_setDefaultRoyalty} method to set a default - * royalty information. + * Royalty information can be specified globally for all token ids via {_setDefaultRoyalty}, and/or individually for + * specific token ids via {_setTokenRoyalty}. The latter takes precedence over the first. * - * NOTE: As specified in EIP-2981, royalties are technically optional and payment is not enforced by this contract. - * See https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. + * IMPORTANT: ERC-2981 only specifies a way to signal royalty information and does not enforce its payment. See + * https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. Marketplaces are expected to + * voluntarily pay royalties together with sales, but note that this standard is not yet widely supported. * * _Available since v4.5._ */ From fb239db80a81d8787c252121615a5c0c82beb914 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 17:46:39 -0300 Subject: [PATCH 46/51] add ERC721Royalty to ERC721 docs --- contracts/token/ERC721/README.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/token/ERC721/README.adoc b/contracts/token/ERC721/README.adoc index 83547f2134d..92959576bc1 100644 --- a/contracts/token/ERC721/README.adoc +++ b/contracts/token/ERC721/README.adoc @@ -24,6 +24,7 @@ Additionally there are a few of other extensions: * {ERC721URIStorage}: A more flexible but more expensive way of storing metadata. * {ERC721Votes}: Support for voting and vote delegation. +* {ERC721Royalty}: A way to signal royalty information following ERC2981. * {ERC721Pausable}: A primitive to pause contract operation. * {ERC721Burnable}: A way for token holders to burn their own tokens. @@ -53,6 +54,8 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel {{ERC721Votes}} +{{ERC721Royalty}} + == Presets These contracts are preconfigured combinations of the above features. They can be used through inheritance or as models to copy and paste their source code. From a00c10d619dbd0e5ba6d4f2ab13856a4995e768f Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 18:54:47 -0300 Subject: [PATCH 47/51] improve docs and reason strings --- contracts/token/common/ERC2981.sol | 46 +++++++++++++-------------- contracts/token/common/README.adoc | 10 ++++++ scripts/gen-nav.js | 11 ++++--- test/token/common/ERC2981.behavior.js | 6 ++-- 4 files changed, 42 insertions(+), 31 deletions(-) create mode 100644 contracts/token/common/README.adoc diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol index 8350dd57581..f53d8af74f1 100644 --- a/contracts/token/common/ERC2981.sol +++ b/contracts/token/common/ERC2981.sol @@ -12,6 +12,9 @@ import "../../utils/introspection/ERC165.sol"; * Royalty information can be specified globally for all token ids via {_setDefaultRoyalty}, and/or individually for * specific token ids via {_setTokenRoyalty}. The latter takes precedence over the first. * + * Royalty is specified as a fraction of sale price. {_feeDenominator} is overridable but defaults to 10000, meaning the + * fee is specified in basis points by default. + * * IMPORTANT: ERC-2981 only specifies a way to signal royalty information and does not enforce its payment. See * https://eips.ethereum.org/EIPS/eip-2981#optional-royalty-payments[Rationale] in the EIP. Marketplaces are expected to * voluntarily pay royalties together with sales, but note that this standard is not yet widely supported. @@ -35,41 +38,42 @@ abstract contract ERC2981 is IERC2981, ERC165 { } /** - * @dev Sets the royalty info for a specific token id, overriding the default royalty info. + * @dev Sets the royalty information for a specific token id, overriding the global default. * * Requirements: - * - `tokenId` must be already mined. + * + * - `tokenId` must be already minted. * - `receiver` cannot be the zero address. - * - `fraction` must indicate the percentage fraction using two decimals. + * - `feeNumerator` cannot be greater than the fee denominator. */ function _setTokenRoyalty( uint256 tokenId, address receiver, - uint96 fraction + uint96 feeNumerator ) internal virtual { - require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); + require(feeNumerator <= _feeDenominator(), "ERC2981: royalty fee will exceed salePrice"); require(receiver != address(0), "ERC2981: Invalid parameters"); - _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, fraction); + _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, feeNumerator); } /** - * @dev Sets the royalty info that tokens will default to. + * @dev Sets the royalty information that all ids in this contract will default to. * * Requirements: + * * - `receiver` cannot be the zero address. - * - `fraction` must indicate the percentage fraction. Needs to be set appropriately - * according to the _feeDenominator granularity. + * - `feeNumerator` cannot be greater than the fee denominator. */ - function _setDefaultRoyalty(address receiver, uint96 fraction) internal virtual { - require(fraction <= _feeDenominator(), "ERC2981: Royalty percentage will exceed salePrice"); - require(receiver != address(0), "ERC2981: Invalid receiver"); + function _setDefaultRoyalty(address receiver, uint96 feeNumerator) internal virtual { + require(feeNumerator <= _feeDenominator(), "ERC2981: royalty fee will exceed salePrice"); + require(receiver != address(0), "ERC2981: invalid receiver"); - _defaultRoyaltyInfo = RoyaltyInfo(receiver, fraction); + _defaultRoyaltyInfo = RoyaltyInfo(receiver, feeNumerator); } /** - * @dev See {IERC2981-royaltyInfo} + * @inheritdoc IERC2981 */ function royaltyInfo(uint256 _tokenId, uint256 _salePrice) external view override returns (address, uint256) { RoyaltyInfo memory royalty = _tokenRoyaltyInfo[_tokenId]; @@ -84,21 +88,16 @@ abstract contract ERC2981 is IERC2981, ERC165 { } /** - * @dev Returns the percentage granularity being used. The default denominator is 10000 - * but it can be customized by an override. + * @dev The denominator with which to interpret the fee set in {_setTokenRoyalty} and {_setDefaultRoyalty} as a + * fraction of the sale price. Defaults to 10000 so fees are expressed in basis points, but may be customized by an + * override. */ function _feeDenominator() internal pure virtual returns (uint96) { return 10000; } /** - * @dev Removes `tokenId` royalty information. - * The royalty information is cleared and the token royalty fallbacks to the default royalty. - * - * Requirements: - * - * - `tokenId` royalty information must exist. - * + * @dev Resets royalty information for the token id to the global default. */ function _resetTokenRoyalty(uint256 tokenId) internal virtual { delete _tokenRoyaltyInfo[tokenId]; @@ -106,7 +105,6 @@ abstract contract ERC2981 is IERC2981, ERC165 { /** * @dev Removes default royalty information. - * */ function _deleteDefaultRoyalty() internal virtual { delete _defaultRoyaltyInfo; diff --git a/contracts/token/common/README.adoc b/contracts/token/common/README.adoc new file mode 100644 index 00000000000..af61674645e --- /dev/null +++ b/contracts/token/common/README.adoc @@ -0,0 +1,10 @@ += Common (Tokens) + +Functionality that is common to multiple token standards. + +* {ERC2981}: NFT Royalties compatible with both ERC721 and ERC1155. +** For ERC721 consider {ERC721Royalty} which clears the royalty information from storage on burn. + +== Contracts + +{{ERC2981}} diff --git a/scripts/gen-nav.js b/scripts/gen-nav.js index e5a76a8164e..403321eae96 100644 --- a/scripts/gen-nav.js +++ b/scripts/gen-nav.js @@ -13,10 +13,13 @@ const files = proc.execFileSync( console.log('.API'); function getPageTitle (directory) { - if (directory === 'metatx') { - return 'Meta Transactions'; - } else { - return startCase(directory); + switch (directory) { + case 'metatx': + return 'Meta Transactions'; + case 'common': + return 'Common (Tokens)'; + default: + return startCase(directory); } } diff --git a/test/token/common/ERC2981.behavior.js b/test/token/common/ERC2981.behavior.js index db270f377b9..0d906a919ed 100644 --- a/test/token/common/ERC2981.behavior.js +++ b/test/token/common/ERC2981.behavior.js @@ -62,12 +62,12 @@ function shouldBehaveLikeERC2981 () { it('reverts if invalid parameters', async function () { await expectRevert( this.token.setDefaultRoyalty(ZERO_ADDRESS, royaltyFraction), - 'ERC2981: Invalid receiver', + 'ERC2981: invalid receiver', ); await expectRevert( this.token.setTokenRoyalty(this.tokenId1, this.account1, new BN('11000')), - 'ERC2981: Royalty percentage will exceed salePrice', + 'ERC2981: royalty fee will exceed salePrice', ); }); }); @@ -114,7 +114,7 @@ function shouldBehaveLikeERC2981 () { await expectRevert( this.token.setTokenRoyalty(this.tokenId1, this.account1, new BN('11000')), - 'ERC2981: Royalty percentage will exceed salePrice', + 'ERC2981: royalty fee will exceed salePrice', ); }); From 3035b32c1b6caf1a7b61a0ba8328882f0ec1253e Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 18:59:24 -0300 Subject: [PATCH 48/51] reorder functions more naturally --- contracts/token/common/ERC2981.sol | 76 +++++++++++++++--------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol index f53d8af74f1..abc89ac57f6 100644 --- a/contracts/token/common/ERC2981.sol +++ b/contracts/token/common/ERC2981.sol @@ -37,41 +37,6 @@ abstract contract ERC2981 is IERC2981, ERC165 { return interfaceId == type(IERC2981).interfaceId || super.supportsInterface(interfaceId); } - /** - * @dev Sets the royalty information for a specific token id, overriding the global default. - * - * Requirements: - * - * - `tokenId` must be already minted. - * - `receiver` cannot be the zero address. - * - `feeNumerator` cannot be greater than the fee denominator. - */ - function _setTokenRoyalty( - uint256 tokenId, - address receiver, - uint96 feeNumerator - ) internal virtual { - require(feeNumerator <= _feeDenominator(), "ERC2981: royalty fee will exceed salePrice"); - require(receiver != address(0), "ERC2981: Invalid parameters"); - - _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, feeNumerator); - } - - /** - * @dev Sets the royalty information that all ids in this contract will default to. - * - * Requirements: - * - * - `receiver` cannot be the zero address. - * - `feeNumerator` cannot be greater than the fee denominator. - */ - function _setDefaultRoyalty(address receiver, uint96 feeNumerator) internal virtual { - require(feeNumerator <= _feeDenominator(), "ERC2981: royalty fee will exceed salePrice"); - require(receiver != address(0), "ERC2981: invalid receiver"); - - _defaultRoyaltyInfo = RoyaltyInfo(receiver, feeNumerator); - } - /** * @inheritdoc IERC2981 */ @@ -97,10 +62,18 @@ abstract contract ERC2981 is IERC2981, ERC165 { } /** - * @dev Resets royalty information for the token id to the global default. + * @dev Sets the royalty information that all ids in this contract will default to. + * + * Requirements: + * + * - `receiver` cannot be the zero address. + * - `feeNumerator` cannot be greater than the fee denominator. */ - function _resetTokenRoyalty(uint256 tokenId) internal virtual { - delete _tokenRoyaltyInfo[tokenId]; + function _setDefaultRoyalty(address receiver, uint96 feeNumerator) internal virtual { + require(feeNumerator <= _feeDenominator(), "ERC2981: royalty fee will exceed salePrice"); + require(receiver != address(0), "ERC2981: invalid receiver"); + + _defaultRoyaltyInfo = RoyaltyInfo(receiver, feeNumerator); } /** @@ -109,4 +82,31 @@ abstract contract ERC2981 is IERC2981, ERC165 { function _deleteDefaultRoyalty() internal virtual { delete _defaultRoyaltyInfo; } + + /** + * @dev Sets the royalty information for a specific token id, overriding the global default. + * + * Requirements: + * + * - `tokenId` must be already minted. + * - `receiver` cannot be the zero address. + * - `feeNumerator` cannot be greater than the fee denominator. + */ + function _setTokenRoyalty( + uint256 tokenId, + address receiver, + uint96 feeNumerator + ) internal virtual { + require(feeNumerator <= _feeDenominator(), "ERC2981: royalty fee will exceed salePrice"); + require(receiver != address(0), "ERC2981: Invalid parameters"); + + _tokenRoyaltyInfo[tokenId] = RoyaltyInfo(receiver, feeNumerator); + } + + /** + * @dev Resets royalty information for the token id to the global default. + */ + function _resetTokenRoyalty(uint256 tokenId) internal virtual { + delete _tokenRoyaltyInfo[tokenId]; + } } From ff12eb3a638b3e56c25ddc324dda16cacf1c9b2c Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 19:00:40 -0300 Subject: [PATCH 49/51] wording --- contracts/token/common/ERC2981.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol index abc89ac57f6..56260a25aed 100644 --- a/contracts/token/common/ERC2981.sol +++ b/contracts/token/common/ERC2981.sol @@ -104,7 +104,7 @@ abstract contract ERC2981 is IERC2981, ERC165 { } /** - * @dev Resets royalty information for the token id to the global default. + * @dev Resets royalty information for the token id back to the global default. */ function _resetTokenRoyalty(uint256 tokenId) internal virtual { delete _tokenRoyaltyInfo[tokenId]; From e8d141c27b5fa6bb858c19a98803596a7ba4657b Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 19:02:15 -0300 Subject: [PATCH 50/51] lint --- scripts/gen-nav.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/gen-nav.js b/scripts/gen-nav.js index 403321eae96..a03fbd69634 100644 --- a/scripts/gen-nav.js +++ b/scripts/gen-nav.js @@ -14,12 +14,12 @@ console.log('.API'); function getPageTitle (directory) { switch (directory) { - case 'metatx': - return 'Meta Transactions'; - case 'common': - return 'Common (Tokens)'; - default: - return startCase(directory); + case 'metatx': + return 'Meta Transactions'; + case 'common': + return 'Common (Tokens)'; + default: + return startCase(directory); } } From 877a8a14d7378255737bc215dd8f6ae497bef8f3 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 6 Jan 2022 19:09:57 -0300 Subject: [PATCH 51/51] simplify docs for ERC721Royalty --- contracts/token/ERC721/extensions/ERC721Royalty.sol | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol index 5dd0f6e17ca..d43d3c6d9c5 100644 --- a/contracts/token/ERC721/extensions/ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -8,7 +8,8 @@ import "../../common/ERC2981.sol"; import "../../../utils/introspection/ERC165.sol"; /** - * @dev Extension of ERC721 with the NFT Royalty Standard, a standardized way to retrieve royalty payment information. + * @dev Extension of ERC721 with the ERC2981 NFT Royalty Standard, a standardized way to retrieve royalty payment + * information. * * Royalty information can be specified globally for all token ids via {_setDefaultRoyalty}, and/or individually for * specific token ids via {_setTokenRoyalty}. The latter takes precedence over the first. @@ -28,14 +29,7 @@ abstract contract ERC721Royalty is ERC2981, ERC721 { } /** - * @dev Destroys `tokenId`. - * The royalty information is cleared when the token is burned. - * - * Requirements: - * - * - `tokenId` must exist. - * - * Emits a {Transfer} event. + * @dev See {ERC721-_burn}. This override additionally clears the royalty information for the token. */ function _burn(uint256 tokenId) internal virtual override { super._burn(tokenId);