From 6a5e213b9ed8f38f84d2397dcf61771d038c4b8e Mon Sep 17 00:00:00 2001 From: Hadrienlc Date: Fri, 26 Nov 2021 09:54:21 +0700 Subject: [PATCH 1/3] use isPrivate in MinterAccessControl --- .../contracts/access/MinterAccessControl.sol | 30 +------ tokens/contracts/erc-1155/ERC1155Lazy.sol | 4 +- tokens/contracts/erc-1155/ERC1155Rarible.sol | 5 +- .../erc-721-minimal/ERC721LazyMinimal.sol | 4 +- .../erc-721-minimal/ERC721RaribleMinimal.sol | 5 +- tokens/contracts/erc-721/ERC721Lazy.sol | 4 +- tokens/contracts/erc-721/ERC721Rarible.sol | 1 - tokens/test-access-control.sh | 5 ++ tokens/test/MinterAccessControl.test.js | 33 +++++++ .../access/MinterAccessControlTest.sol | 13 +++ tokens/test/erc-1155/ERC1155Rarible.test.js | 87 ------------------- .../test/erc-1155/ERC1155RaribleUser.test.js | 74 ++++++++++++++++ .../erc-1155/MinterAccessControl1155.test.js | 59 ------------- .../test/erc-721-minimal/RaribleUser.test.js | 64 ++++++++++++++ .../erc-721/MinterAccessControl721.test.js | 53 ----------- tokens/test/erc-721/Rarible.test.js | 77 ---------------- 16 files changed, 202 insertions(+), 316 deletions(-) create mode 100755 tokens/test-access-control.sh create mode 100644 tokens/test/MinterAccessControl.test.js create mode 100644 tokens/test/contracts/access/MinterAccessControlTest.sol delete mode 100644 tokens/test/erc-1155/MinterAccessControl1155.test.js delete mode 100644 tokens/test/erc-721/MinterAccessControl721.test.js diff --git a/tokens/contracts/access/MinterAccessControl.sol b/tokens/contracts/access/MinterAccessControl.sol index a6d824880..b8b47a3af 100644 --- a/tokens/contracts/access/MinterAccessControl.sol +++ b/tokens/contracts/access/MinterAccessControl.sol @@ -10,14 +10,11 @@ contract MinterAccessControl is Initializable, OwnableUpgradeable { using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet; EnumerableSetUpgradeable.AddressSet private _minters; - bool public minterAccessControlEnabled; - - event MinterAccessControlEnabled(); - event MinterAccessControlDisabled(); + event MinterGranted(address indexed account); event MinterRevoked(address indexed account); - function __MinterAccessControl_init() internal initializer { + function __MinterAccessControl_init() external initializer { __Ownable_init_unchained(); __MinterAccessControl_init_unchained(); } @@ -25,25 +22,6 @@ contract MinterAccessControl is Initializable, OwnableUpgradeable { function __MinterAccessControl_init_unchained() internal initializer { } - /** - * @dev Enable minter control - * When enabled, only addresses added to `grantMinter` will be allowed to mint - */ - function enableMinterAccessControl() external onlyOwner { - require(!minterAccessControlEnabled, "MinterAccessControl: Already enabled"); - minterAccessControlEnabled = true; - emit MinterAccessControlEnabled(); - } - - /** - * @dev Disable minter control - */ - function disableMinterAccessControl() external onlyOwner { - require(minterAccessControlEnabled, "MinterAccessControl: Already disabled"); - minterAccessControlEnabled = false; - emit MinterAccessControlDisabled(); - } - /** * @dev Add `_minter` to the list of allowed minters. */ @@ -63,10 +41,10 @@ contract MinterAccessControl is Initializable, OwnableUpgradeable { } /** - * @dev Returns `true` if minterControl is not enabled or `account` has been granted to minters. + * @dev Returns `true` if `account` has been granted to minters. */ function isValidMinter(address account) public view returns (bool) { - return !minterAccessControlEnabled || _minters.contains(account); + return _minters.contains(account); } uint256[50] private __gap; diff --git a/tokens/contracts/erc-1155/ERC1155Lazy.sol b/tokens/contracts/erc-1155/ERC1155Lazy.sol index 858834eac..2dc9b935c 100644 --- a/tokens/contracts/erc-1155/ERC1155Lazy.sol +++ b/tokens/contracts/erc-1155/ERC1155Lazy.sol @@ -9,9 +9,8 @@ import "@rarible/royalties-upgradeable/contracts/RoyaltiesV2Upgradeable.sol"; import "@rarible/lazy-mint/contracts/erc-1155/IERC1155LazyMint.sol"; import "./Mint1155Validator.sol"; import "./ERC1155BaseURI.sol"; -import "../access/MinterAccessControl.sol"; -abstract contract ERC1155Lazy is IERC1155LazyMint, ERC1155BaseURI, Mint1155Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl, MinterAccessControl { +abstract contract ERC1155Lazy is IERC1155LazyMint, ERC1155BaseURI, Mint1155Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl { using SafeMathUpgradeable for uint; bytes4 private constant _INTERFACE_ID_ERC165 = 0x01ffc9a7; @@ -60,7 +59,6 @@ abstract contract ERC1155Lazy is IERC1155LazyMint, ERC1155BaseURI, Mint1155Valid address sender = _msgSender(); require(minter == sender || isApprovedForAll(minter, sender), "ERC1155: transfer caller is not approved"); - require(isValidMinter(minter), "ERC1155: minter not granted"); require(_amount > 0, "amount incorrect"); if (supply[data.tokenId] == 0) { diff --git a/tokens/contracts/erc-1155/ERC1155Rarible.sol b/tokens/contracts/erc-1155/ERC1155Rarible.sol index a41ee3e98..9d2b9204b 100644 --- a/tokens/contracts/erc-1155/ERC1155Rarible.sol +++ b/tokens/contracts/erc-1155/ERC1155Rarible.sol @@ -4,8 +4,9 @@ pragma solidity 0.7.6; pragma abicoder v2; import "./ERC1155Base.sol"; +import "../access/MinterAccessControl.sol"; -contract ERC1155Rarible is ERC1155Base { +contract ERC1155Rarible is ERC1155Base, MinterAccessControl { /// @dev true if collection is private, false if public bool isPrivate; @@ -50,7 +51,7 @@ contract ERC1155Rarible is ERC1155Base { function mintAndTransfer(LibERC1155LazyMint.Mint1155Data memory data, address to, uint256 _amount) public override { if (isPrivate){ - require(owner() == data.creators[0].account, "minter is not the owner"); + require(owner() == data.creators[0].account || isValidMinter(data.creators[0].account), "minter not granted or not owner"); } super.mintAndTransfer(data, to, _amount); } diff --git a/tokens/contracts/erc-721-minimal/ERC721LazyMinimal.sol b/tokens/contracts/erc-721-minimal/ERC721LazyMinimal.sol index 2fc806a39..95c089529 100644 --- a/tokens/contracts/erc-721-minimal/ERC721LazyMinimal.sol +++ b/tokens/contracts/erc-721-minimal/ERC721LazyMinimal.sol @@ -10,9 +10,8 @@ import "@rarible/lazy-mint/contracts/erc-721/IERC721LazyMint.sol"; import "../Mint721Validator.sol"; import "@openzeppelin/contracts-upgradeable/math/SafeMathUpgradeable.sol"; import "./ERC721URI.sol"; -import "../access/MinterAccessControl.sol"; -abstract contract ERC721LazyMinimal is IERC721LazyMint, ERC721UpgradeableMinimal, Mint721Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl, ERC721URI, MinterAccessControl { +abstract contract ERC721LazyMinimal is IERC721LazyMint, ERC721UpgradeableMinimal, Mint721Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl, ERC721URI { using SafeMathUpgradeable for uint; bytes4 private constant _INTERFACE_ID_ERC165 = 0x01ffc9a7; @@ -55,7 +54,6 @@ abstract contract ERC721LazyMinimal is IERC721LazyMint, ERC721UpgradeableMinimal require(minter == data.creators[0].account, "tokenId incorrect"); require(data.creators.length == data.signatures.length); require(minter == sender || isApprovedForAll(minter, sender), "ERC721: transfer caller is not owner nor approved"); - require(isValidMinter(minter), "ERC721: minter not granted"); bytes32 hash = LibERC721LazyMint.hash(data); for (uint i = 0; i < data.creators.length; i++) { diff --git a/tokens/contracts/erc-721-minimal/ERC721RaribleMinimal.sol b/tokens/contracts/erc-721-minimal/ERC721RaribleMinimal.sol index ddfc78e4c..d80317502 100644 --- a/tokens/contracts/erc-721-minimal/ERC721RaribleMinimal.sol +++ b/tokens/contracts/erc-721-minimal/ERC721RaribleMinimal.sol @@ -4,8 +4,9 @@ pragma solidity 0.7.6; pragma abicoder v2; import "./ERC721BaseMinimal.sol"; +import "../access/MinterAccessControl.sol"; -contract ERC721RaribleMinimal is ERC721BaseMinimal { +contract ERC721RaribleMinimal is ERC721BaseMinimal, MinterAccessControl { /// @dev true if collection is private, false if public bool isPrivate; @@ -50,7 +51,7 @@ contract ERC721RaribleMinimal is ERC721BaseMinimal { function mintAndTransfer(LibERC721LazyMint.Mint721Data memory data, address to) public override virtual { if (isPrivate){ - require(owner() == data.creators[0].account, "minter is not the owner"); + require(owner() == data.creators[0].account || isValidMinter(data.creators[0].account), "minter not granted or not owner"); } super.mintAndTransfer(data, to); } diff --git a/tokens/contracts/erc-721/ERC721Lazy.sol b/tokens/contracts/erc-721/ERC721Lazy.sol index 6b4a8e890..4d1fc5c0a 100644 --- a/tokens/contracts/erc-721/ERC721Lazy.sol +++ b/tokens/contracts/erc-721/ERC721Lazy.sol @@ -8,9 +8,8 @@ import "@rarible/royalties/contracts/impl/RoyaltiesV2Impl.sol"; import "@rarible/royalties-upgradeable/contracts/RoyaltiesV2Upgradeable.sol"; import "@rarible/lazy-mint/contracts/erc-721/IERC721LazyMint.sol"; import "../Mint721Validator.sol"; -import "../access/MinterAccessControl.sol"; -abstract contract ERC721Lazy is IERC721LazyMint, ERC721Upgradeable, Mint721Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl, MinterAccessControl { +abstract contract ERC721Lazy is IERC721LazyMint, ERC721Upgradeable, Mint721Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl { using SafeMathUpgradeable for uint; using EnumerableSetUpgradeable for EnumerableSetUpgradeable.UintSet; using EnumerableMapUpgradeable for EnumerableMapUpgradeable.UintToAddressMap; @@ -55,7 +54,6 @@ abstract contract ERC721Lazy is IERC721LazyMint, ERC721Upgradeable, Mint721Valid require(minter == data.creators[0].account, "tokenId incorrect"); require(data.creators.length == data.signatures.length); require(minter == sender || isApprovedForAll(minter, sender), "ERC721: transfer caller is not owner nor approved"); - require(isValidMinter(minter), "ERC721: minter not granted"); bytes32 hash = LibERC721LazyMint.hash(data); for (uint i = 0; i < data.creators.length; i++) { diff --git a/tokens/contracts/erc-721/ERC721Rarible.sol b/tokens/contracts/erc-721/ERC721Rarible.sol index 09ebe9f40..48f1d5f85 100644 --- a/tokens/contracts/erc-721/ERC721Rarible.sol +++ b/tokens/contracts/erc-721/ERC721Rarible.sol @@ -23,7 +23,6 @@ contract ERC721Rarible is ERC721Base { __Ownable_init_unchained(); __ERC721Burnable_init_unchained(); __Mint721Validator_init_unchained(); - __MinterAccessControl_init_unchained(); __HasContractURI_init_unchained(contractURI); __ERC721_init_unchained(_name, _symbol); diff --git a/tokens/test-access-control.sh b/tokens/test-access-control.sh new file mode 100755 index 000000000..9ec112a93 --- /dev/null +++ b/tokens/test-access-control.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +truffle test ./test/erc-721-minimal/RaribleUser.test.js \ + ./test/erc-1155/ERC1155RaribleUser.test.js \ + ./test/MinterAccessControl.test.js \ + ./test/contracts/access/MinterAccessControlTest.sol \ No newline at end of file diff --git a/tokens/test/MinterAccessControl.test.js b/tokens/test/MinterAccessControl.test.js new file mode 100644 index 000000000..311677937 --- /dev/null +++ b/tokens/test/MinterAccessControl.test.js @@ -0,0 +1,33 @@ +const Testing = artifacts.require("MinterAccessControl.sol"); +const TestingV2 = artifacts.require("MinterAccessControlTest.sol"); + +const { expectThrow } = require('@daonomic/tests-common'); +const { deployProxy, upgradeProxy } = require('@openzeppelin/truffle-upgrades'); + +function creators(list) { + const value = 10000 / list.length + return list.map(account => ({ account, value })) +} + +contract("MinterAccessControl", accounts => { + let token; + let tokenOwner = accounts[9]; + + beforeEach(async () => { + token = await deployProxy(Testing, [], { initializer: '__MinterAccessControl_init' }); + await token.transferOwnership(tokenOwner); + }); + + it("conserve minter access control after upgrade", async () => { + const minter = accounts[1]; + + await token.grantMinter(minter, {from: tokenOwner}) + assert.equal(await token.isValidMinter(minter), true); + + // upgrade contract + const newInstance = await upgradeProxy(token.address, TestingV2); + assert.equal(await newInstance.version(), 2); + + assert.equal(await newInstance.isValidMinter(minter), true); + }); +}); \ No newline at end of file diff --git a/tokens/test/contracts/access/MinterAccessControlTest.sol b/tokens/test/contracts/access/MinterAccessControlTest.sol new file mode 100644 index 000000000..d940c5e29 --- /dev/null +++ b/tokens/test/contracts/access/MinterAccessControlTest.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "../../../contracts/access/MinterAccessControl.sol"; + +contract MinterAccessControlTest is MinterAccessControl { + + function version() public view returns (uint256) { + return 2; + } + +} diff --git a/tokens/test/erc-1155/ERC1155Rarible.test.js b/tokens/test/erc-1155/ERC1155Rarible.test.js index bd4bfc4ec..782eb3eeb 100644 --- a/tokens/test/erc-1155/ERC1155Rarible.test.js +++ b/tokens/test/erc-1155/ERC1155Rarible.test.js @@ -382,93 +382,6 @@ contract("ERC1155Rarible", accounts => { ); }); - it("mint and transfer with minter access control", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - let supply = 5; - let mint = 2; - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}) - ); - - await token.grantMinter(minter, {from: tokenOwner}); - assert.equal(await token.isValidMinter(minter), true); - assert.equal(await token.isValidMinter(transferTo), false); - - await token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}); - assert.equal(await token.uri(tokenId), "ipfs:/" + tokenURI); - assert.equal(await token.balanceOf(transferTo, tokenId), mint); - assert.equal(await token.balanceOf(minter, tokenId), 0); - }); - - it("mint and transfer with minter access control and minter signature", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - let supply = 5; - let mint = 2; - - const signature = await getSignature(tokenId, tokenURI, supply, creators([minter]), [], minter); - - let whiteListProxy = accounts[5]; - await token.setDefaultApproval(whiteListProxy, true, {from: tokenOwner}); - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) - ); - - await token.grantMinter(minter, {from: tokenOwner}); - assert.equal(await token.isValidMinter(minter), true); - assert.equal(await token.isValidMinter(whiteListProxy), false); - - await token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) - assert.equal(await token.uri(tokenId), "ipfs:/" + tokenURI); - assert.equal(await token.balanceOf(transferTo, tokenId), mint); - assert.equal(await token.balanceOf(minter, tokenId), 0); - }); - - it("mint and transfer with minter access control and wrong minter signature", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - let supply = 5; - let mint = 2; - - const signature = await getSignature(tokenId, tokenURI, supply, creators([minter]), [], transferTo); - - let whiteListProxy = accounts[5]; - await token.setDefaultApproval(whiteListProxy, true, {from: tokenOwner}); - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) - ); - - await token.grantMinter(minter, {from: tokenOwner}); - assert.equal(await token.isValidMinter(minter), true); - assert.equal(await token.isValidMinter(whiteListProxy), false); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) - ); - }); - it("standard transfer from owner", async () => { let minter = accounts[1]; const tokenId = minter + "b00000000000000000000001"; diff --git a/tokens/test/erc-1155/ERC1155RaribleUser.test.js b/tokens/test/erc-1155/ERC1155RaribleUser.test.js index 60745d1df..a4b841eec 100644 --- a/tokens/test/erc-1155/ERC1155RaribleUser.test.js +++ b/tokens/test/erc-1155/ERC1155RaribleUser.test.js @@ -177,6 +177,80 @@ contract("ERC1155RaribleUser", accounts => { await checkCreators(tokenId, [minter]); }); + it("mint and transfer with minter access control", async () => { + const minter = accounts[1]; + let transferTo = accounts[2]; + + const tokenId = minter + "b00000000000000000000001"; + const tokenURI = "//uri"; + let supply = 5; + let mint = 2; + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}) + ); + + await token.grantMinter(minter, {from: tokenOwner}); + assert.equal(await token.isValidMinter(minter), true); + assert.equal(await token.isValidMinter(transferTo), false); + + await token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}); + assert.equal(await token.uri(tokenId), "ipfs:/" + tokenURI); + assert.equal(await token.balanceOf(transferTo, tokenId), mint); + assert.equal(await token.balanceOf(minter, tokenId), 0); + }); + + it("mint and transfer with minter access control and minter signature", async () => { + const minter = accounts[1]; + let transferTo = accounts[2]; + + const tokenId = minter + "b00000000000000000000001"; + const tokenURI = "//uri"; + let supply = 5; + let mint = 2; + + const signature = await getSignature(tokenId, tokenURI, supply, creators([minter]), [], minter); + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) + ); + + await token.setApprovalForAll(whiteListProxy, true, {from: minter}) + await token.grantMinter(minter, {from: tokenOwner}); + assert.equal(await token.isValidMinter(minter), true); + assert.equal(await token.isValidMinter(whiteListProxy), false); + + await token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) + assert.equal(await token.uri(tokenId), "ipfs:/" + tokenURI); + assert.equal(await token.balanceOf(transferTo, tokenId), mint); + assert.equal(await token.balanceOf(minter, tokenId), 0); + }); + + it("mint and transfer with minter access control and wrong minter signature", async () => { + const minter = accounts[1]; + let transferTo = accounts[2]; + + const tokenId = minter + "b00000000000000000000001"; + const tokenURI = "//uri"; + let supply = 5; + let mint = 2; + + const signature = await getSignature(tokenId, tokenURI, supply, creators([minter]), [], transferTo); + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) + ); + + await token.setApprovalForAll(whiteListProxy, true, {from: minter}) + await token.grantMinter(minter, {from: tokenOwner}); + assert.equal(await token.isValidMinter(minter), true); + assert.equal(await token.isValidMinter(whiteListProxy), false); + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [signature]], transferTo, mint, {from: whiteListProxy}) + ); + }); + async function getSignature(tokenId, tokenURI, supply, creators, fees, account) { return sign(account, tokenId, tokenURI, supply, creators, fees, token.address); } diff --git a/tokens/test/erc-1155/MinterAccessControl1155.test.js b/tokens/test/erc-1155/MinterAccessControl1155.test.js deleted file mode 100644 index 1d1d7e040..000000000 --- a/tokens/test/erc-1155/MinterAccessControl1155.test.js +++ /dev/null @@ -1,59 +0,0 @@ -const Testing = artifacts.require("ERC1155Rarible.sol"); - -const { expectThrow } = require('@daonomic/tests-common'); -const { deployProxy, upgradeProxy } = require('@openzeppelin/truffle-upgrades'); - -function creators(list) { - const value = 10000 / list.length - return list.map(account => ({ account, value })) -} - -contract("MinterAccessControl1155", accounts => { - let token; - let tokenOwner = accounts[9]; - const zeroWord = "0x0000000000000000000000000000000000000000000000000000000000000000"; - const name = 'FreeMintable'; - const ZERO = "0x0000000000000000000000000000000000000000"; - - beforeEach(async () => { - token = await deployProxy(Testing, [name, "TST", "ipfs:/", "ipfs:/"], { initializer: "__ERC1155Rarible_init" }); - await token.transferOwnership(tokenOwner); - }); - - it("conserve minter access control after upgrade", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - let supply = 5; - let mint = 2; - - - console.log(`owner: ${await token.owner()}, expected: ${tokenOwner}`); - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}) - ); - - // upgrade contract - const newInstance = await upgradeProxy(token.address, Testing); - assert.equal(await newInstance.minterAccessControlEnabled(), true); - - await expectThrow( - newInstance.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}) - ); - - await newInstance.grantMinter(minter, {from: tokenOwner}); - assert.equal(await newInstance.isValidMinter(minter), true); - assert.equal(await newInstance.isValidMinter(transferTo), false); - - await newInstance.mintAndTransfer([tokenId, tokenURI, supply, creators([minter]), [], [zeroWord]], transferTo, mint, {from: minter}); - assert.equal(await newInstance.uri(tokenId), "ipfs:/" + tokenURI); - assert.equal(await newInstance.balanceOf(transferTo, tokenId), mint); - assert.equal(await newInstance.balanceOf(minter, tokenId), 0); - }); -}); \ No newline at end of file diff --git a/tokens/test/erc-721-minimal/RaribleUser.test.js b/tokens/test/erc-721-minimal/RaribleUser.test.js index 587970116..f74ebdb49 100644 --- a/tokens/test/erc-721-minimal/RaribleUser.test.js +++ b/tokens/test/erc-721-minimal/RaribleUser.test.js @@ -124,6 +124,70 @@ contract("ERC721RaribleUser minimal", accounts => { ); }); + it("mint and transfer with minter access control", async () => { + const minter = accounts[1]; + let transferTo = accounts[2]; + + const tokenId = minter + "b00000000000000000000001"; + const tokenURI = "//uri"; + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) + ); + + await token.grantMinter(minter, {from: tokenOwner}) + assert.equal(await token.isValidMinter(minter), true); + assert.equal(await token.isValidMinter(transferTo), false); + + await token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) + assert.equal(await token.ownerOf(tokenId), transferTo); + }); + + it("mint and transfer with minter access control and minter signature", async () => { + const minter = accounts[1]; + let transferTo = accounts[2]; + + const tokenId = minter + "b00000000000000000000001"; + const tokenURI = "//uri"; + + const signature = await getSignature(tokenId, tokenURI, creators([minter]), [], minter); + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) + ); + + await token.setApprovalForAll(whiteListProxy, true, {from: minter}) + await token.grantMinter(minter, {from: tokenOwner}) + assert.equal(await token.isValidMinter(minter), true); + assert.equal(await token.isValidMinter(whiteListProxy), false); + + await token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) + assert.equal(await token.ownerOf(tokenId), transferTo); + }); + + it("mint and transfer with minter access control and wrong minter signature", async () => { + const minter = accounts[1]; + let transferTo = accounts[2]; + + const tokenId = minter + "b00000000000000000000001"; + const tokenURI = "//uri"; + + const signature = await getSignature(tokenId, tokenURI, creators([minter]), [], transferTo); + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) + ); + + await token.setApprovalForAll(whiteListProxy, true, {from: minter}) + await token.grantMinter(minter, {from: tokenOwner}) + assert.equal(await token.isValidMinter(minter), true); + assert.equal(await token.isValidMinter(whiteListProxy), false); + + await expectThrow( + token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) + ); + }); + function getSignature(tokenId, tokenURI, fees, creators, account) { return sign(account, tokenId, tokenURI, fees, creators, token.address); } diff --git a/tokens/test/erc-721/MinterAccessControl721.test.js b/tokens/test/erc-721/MinterAccessControl721.test.js deleted file mode 100644 index 083e48f20..000000000 --- a/tokens/test/erc-721/MinterAccessControl721.test.js +++ /dev/null @@ -1,53 +0,0 @@ -const Testing = artifacts.require("ERC721RaribleMinimal.sol"); - -const { expectThrow } = require('@daonomic/tests-common'); -const { deployProxy, upgradeProxy } = require('@openzeppelin/truffle-upgrades'); - -function creators(list) { - const value = 10000 / list.length - return list.map(account => ({ account, value })) -} - -contract("MinterAccessControl721", accounts => { - let token; - let tokenOwner = accounts[9]; - const name = 'FreeMintableRarible'; - const chainId = 1; - const zeroWord = "0x0000000000000000000000000000000000000000000000000000000000000000"; - const ZERO = "0x0000000000000000000000000000000000000000"; - - beforeEach(async () => { - token = await deployProxy(Testing, [name, "RARI", "https://ipfs.rarible.com", "https://ipfs.rarible.com"], { initializer: "__ERC721Rarible_init" }); - await token.transferOwnership(tokenOwner); - }); - - it("conserve minter access control after upgrade", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) - ); - - // upgrade contract - const newInstance = await upgradeProxy(token.address, Testing); - assert.equal(await newInstance.minterAccessControlEnabled(), true); - - await expectThrow( - newInstance.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) - ); - - await newInstance.grantMinter(minter, {from: tokenOwner}) - assert.equal(await newInstance.isValidMinter(minter), true); - assert.equal(await newInstance.isValidMinter(transferTo), false); - - await token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) - assert.equal(await token.ownerOf(tokenId), transferTo); - }); -}); \ No newline at end of file diff --git a/tokens/test/erc-721/Rarible.test.js b/tokens/test/erc-721/Rarible.test.js index 2313a2170..ad1dd1acf 100644 --- a/tokens/test/erc-721/Rarible.test.js +++ b/tokens/test/erc-721/Rarible.test.js @@ -396,83 +396,6 @@ contract("ERC721Rarible", accounts => { ); }); - it("mint and transfer with minter access control", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) - ); - - await token.grantMinter(minter, {from: tokenOwner}) - assert.equal(await token.isValidMinter(minter), true); - assert.equal(await token.isValidMinter(transferTo), false); - - await token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [zeroWord]], transferTo, {from: minter}) - assert.equal(await token.ownerOf(tokenId), transferTo); - }); - - it("mint and transfer with minter access control and minter signature", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - const signature = await getSignature(tokenId, tokenURI, creators([minter]), [], minter); - - let whiteListProxy = accounts[5]; - await token.setDefaultApproval(whiteListProxy, true, {from: tokenOwner}); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) - ); - - await token.grantMinter(minter, {from: tokenOwner}) - assert.equal(await token.isValidMinter(minter), true); - assert.equal(await token.isValidMinter(whiteListProxy), false); - - await token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) - assert.equal(await token.ownerOf(tokenId), transferTo); - }); - - it("mint and transfer with minter access control and wrong minter signature", async () => { - const minter = accounts[1]; - let transferTo = accounts[2]; - - const tokenId = minter + "b00000000000000000000001"; - const tokenURI = "//uri"; - - await token.enableMinterAccessControl({from: tokenOwner}); - assert.equal(await token.minterAccessControlEnabled(), true); - - const signature = await getSignature(tokenId, tokenURI, creators([minter]), [], transferTo); - - let whiteListProxy = accounts[5]; - await token.setDefaultApproval(whiteListProxy, true, {from: tokenOwner}); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) - ); - - await token.grantMinter(minter, {from: tokenOwner}) - assert.equal(await token.isValidMinter(minter), true); - assert.equal(await token.isValidMinter(whiteListProxy), false); - - await expectThrow( - token.mintAndTransfer([tokenId, tokenURI, creators([minter]), [], [signature]], transferTo, {from: whiteListProxy}) - ); - }); - it("standard transfer from owner", async () => { let minter = accounts[1]; const tokenId = minter + "b00000000000000000000001"; From eb3607e22bf9f7bc4dae59c4b08fa945ea1f2ef7 Mon Sep 17 00:00:00 2001 From: Hadrienlc Date: Thu, 2 Dec 2021 11:45:22 +0700 Subject: [PATCH 2/3] fix: minter access control typos and tests --- .../contracts/access/MinterAccessControl.sol | 2 +- tokens/contracts/erc-1155/ERC1155Lazy.sol | 2 +- tokens/contracts/erc-1155/ERC1155Rarible.sol | 2 +- tokens/test-access-control.sh | 5 ----- tokens/test/MinterAccessControl.test.js | 6 +++--- .../access/MinterAccessControlTest.sol | 13 ------------- .../access/MinterAccessControlTestV1.sol | 15 +++++++++++++++ .../access/MinterAccessControlTestV2.sol | 19 +++++++++++++++++++ 8 files changed, 40 insertions(+), 24 deletions(-) delete mode 100755 tokens/test-access-control.sh delete mode 100644 tokens/test/contracts/access/MinterAccessControlTest.sol create mode 100644 tokens/test/contracts/access/MinterAccessControlTestV1.sol create mode 100644 tokens/test/contracts/access/MinterAccessControlTestV2.sol diff --git a/tokens/contracts/access/MinterAccessControl.sol b/tokens/contracts/access/MinterAccessControl.sol index b8b47a3af..24a6850bc 100644 --- a/tokens/contracts/access/MinterAccessControl.sol +++ b/tokens/contracts/access/MinterAccessControl.sol @@ -14,7 +14,7 @@ contract MinterAccessControl is Initializable, OwnableUpgradeable { event MinterGranted(address indexed account); event MinterRevoked(address indexed account); - function __MinterAccessControl_init() external initializer { + function __MinterAccessControl_init() internal initializer { __Ownable_init_unchained(); __MinterAccessControl_init_unchained(); } diff --git a/tokens/contracts/erc-1155/ERC1155Lazy.sol b/tokens/contracts/erc-1155/ERC1155Lazy.sol index 2dc9b935c..3d30e454a 100644 --- a/tokens/contracts/erc-1155/ERC1155Lazy.sol +++ b/tokens/contracts/erc-1155/ERC1155Lazy.sol @@ -10,7 +10,7 @@ import "@rarible/lazy-mint/contracts/erc-1155/IERC1155LazyMint.sol"; import "./Mint1155Validator.sol"; import "./ERC1155BaseURI.sol"; -abstract contract ERC1155Lazy is IERC1155LazyMint, ERC1155BaseURI, Mint1155Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl { +abstract contract ERC1155Lazy is IERC1155LazyMint, ERC1155BaseURI, Mint1155Validator, RoyaltiesV2Upgradeable, RoyaltiesV2Impl { using SafeMathUpgradeable for uint; bytes4 private constant _INTERFACE_ID_ERC165 = 0x01ffc9a7; diff --git a/tokens/contracts/erc-1155/ERC1155Rarible.sol b/tokens/contracts/erc-1155/ERC1155Rarible.sol index 9d2b9204b..4de68a996 100644 --- a/tokens/contracts/erc-1155/ERC1155Rarible.sol +++ b/tokens/contracts/erc-1155/ERC1155Rarible.sol @@ -51,7 +51,7 @@ contract ERC1155Rarible is ERC1155Base, MinterAccessControl { function mintAndTransfer(LibERC1155LazyMint.Mint1155Data memory data, address to, uint256 _amount) public override { if (isPrivate){ - require(owner() == data.creators[0].account || isValidMinter(data.creators[0].account), "minter not granted or not owner"); + require(owner() == data.creators[0].account || isValidMinter(data.creators[0].account), "minter not granted or not owner"); } super.mintAndTransfer(data, to, _amount); } diff --git a/tokens/test-access-control.sh b/tokens/test-access-control.sh deleted file mode 100755 index 9ec112a93..000000000 --- a/tokens/test-access-control.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env bash -truffle test ./test/erc-721-minimal/RaribleUser.test.js \ - ./test/erc-1155/ERC1155RaribleUser.test.js \ - ./test/MinterAccessControl.test.js \ - ./test/contracts/access/MinterAccessControlTest.sol \ No newline at end of file diff --git a/tokens/test/MinterAccessControl.test.js b/tokens/test/MinterAccessControl.test.js index 311677937..5e16b7b75 100644 --- a/tokens/test/MinterAccessControl.test.js +++ b/tokens/test/MinterAccessControl.test.js @@ -1,5 +1,5 @@ -const Testing = artifacts.require("MinterAccessControl.sol"); -const TestingV2 = artifacts.require("MinterAccessControlTest.sol"); +const TestingV1 = artifacts.require("MinterAccessControlTestV1.sol"); +const TestingV2 = artifacts.require("MinterAccessControlTestV2.sol"); const { expectThrow } = require('@daonomic/tests-common'); const { deployProxy, upgradeProxy } = require('@openzeppelin/truffle-upgrades'); @@ -14,7 +14,7 @@ contract("MinterAccessControl", accounts => { let tokenOwner = accounts[9]; beforeEach(async () => { - token = await deployProxy(Testing, [], { initializer: '__MinterAccessControl_init' }); + token = await deployProxy(TestingV1, [], { initializer: 'initialize' }); await token.transferOwnership(tokenOwner); }); diff --git a/tokens/test/contracts/access/MinterAccessControlTest.sol b/tokens/test/contracts/access/MinterAccessControlTest.sol deleted file mode 100644 index d940c5e29..000000000 --- a/tokens/test/contracts/access/MinterAccessControlTest.sol +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity >=0.6.0 <0.8.0; - -import "../../../contracts/access/MinterAccessControl.sol"; - -contract MinterAccessControlTest is MinterAccessControl { - - function version() public view returns (uint256) { - return 2; - } - -} diff --git a/tokens/test/contracts/access/MinterAccessControlTestV1.sol b/tokens/test/contracts/access/MinterAccessControlTestV1.sol new file mode 100644 index 000000000..9c5e3222e --- /dev/null +++ b/tokens/test/contracts/access/MinterAccessControlTestV1.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "../../../contracts/access/MinterAccessControl.sol"; + +contract MinterAccessControlTestV1 is MinterAccessControl { + + function initialize() external initializer { + __Ownable_init_unchained(); + __MinterAccessControl_init_unchained(); + } + + uint256[50] private __gap; +} diff --git a/tokens/test/contracts/access/MinterAccessControlTestV2.sol b/tokens/test/contracts/access/MinterAccessControlTestV2.sol new file mode 100644 index 000000000..5830d80f3 --- /dev/null +++ b/tokens/test/contracts/access/MinterAccessControlTestV2.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "../../../contracts/access/MinterAccessControl.sol"; + +contract MinterAccessControlTestV2 is MinterAccessControl { + + function initialize() external initializer { + __Ownable_init_unchained(); + __MinterAccessControl_init_unchained(); + } + + function version() public view returns (uint256) { + return 2; + } + + uint256[50] private __gap; +} From 1e8f3235e6d82a7f93fc5e37564e76bea73a3593 Mon Sep 17 00:00:00 2001 From: Hadrienlc Date: Mon, 6 Dec 2021 16:56:06 +0700 Subject: [PATCH 3/3] set MinterAccessControl abstract --- tokens/contracts/access/MinterAccessControl.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokens/contracts/access/MinterAccessControl.sol b/tokens/contracts/access/MinterAccessControl.sol index 24a6850bc..f5c72afee 100644 --- a/tokens/contracts/access/MinterAccessControl.sol +++ b/tokens/contracts/access/MinterAccessControl.sol @@ -6,7 +6,7 @@ import "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/utils/EnumerableSetUpgradeable.sol"; -contract MinterAccessControl is Initializable, OwnableUpgradeable { +abstract contract MinterAccessControl is Initializable, OwnableUpgradeable { using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet; EnumerableSetUpgradeable.AddressSet private _minters;