From 127a6da40583a8f47d1d3772b3881e97f2cca636 Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 30 Oct 2022 14:39:33 -0700 Subject: [PATCH 1/9] forge install: onchain-modules --- .gitmodules | 3 +++ lib/onchain-modules | 1 + 2 files changed, 4 insertions(+) create mode 160000 lib/onchain-modules diff --git a/.gitmodules b/.gitmodules index 2935700..52c4339 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "lib/micro-onchain-metadata-utils"] path = lib/micro-onchain-metadata-utils url = https://github.com/iainnash/micro-onchain-metadata-utils +[submodule "lib/onchain-modules"] + path = lib/onchain-modules + url = https://github.com/public-assembly/onchain-modules diff --git a/lib/onchain-modules b/lib/onchain-modules new file mode 160000 index 0000000..5dcfa74 --- /dev/null +++ b/lib/onchain-modules @@ -0,0 +1 @@ +Subproject commit 5dcfa74569a08f56c8dab625b8c034fd64506e6f From d330923098ce1fa3aa72ca336e80c45e2a97613b Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 30 Oct 2022 16:56:25 -0700 Subject: [PATCH 2/9] first commit integrating access control logic --- remappings.txt | 3 +- src/Curator.sol | 156 ++++++++++++++++++++++++-------- src/CuratorStorageV1.sol | 10 +- src/interfaces/ICurator.sol | 22 +++-- src/interfaces/ICuratorInfo.sol | 7 +- 5 files changed, 146 insertions(+), 52 deletions(-) diff --git a/remappings.txt b/remappings.txt index 6df5311..50fdd0f 100644 --- a/remappings.txt +++ b/remappings.txt @@ -1,4 +1,5 @@ forge-std/=lib/forge-std/src/ micro-onchain-metadata-utils/=lib/micro-onchain-metadata-utils/src/ @openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/ -@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/ \ No newline at end of file +@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/ +onchain-modules/=lib/onchain-modules/tokenized-access-control/src \ No newline at end of file diff --git a/src/Curator.sol b/src/Curator.sol index 6fccca7..4c859d0 100644 --- a/src/Curator.sol +++ b/src/Curator.sol @@ -9,6 +9,7 @@ import { ICuratorFactory } from "./interfaces/ICuratorFactory.sol"; import { CuratorSkeletonNFT } from "./CuratorSkeletonNFT.sol"; import { IMetadataRenderer } from "./interfaces/IMetadataRenderer.sol"; import { CuratorStorageV1 } from "./CuratorStorageV1.sol"; +import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; /** * @notice Base contract for curation functioanlity. Inherits ERC721 standard from CuratorSkeletonNFT.sol @@ -38,9 +39,33 @@ contract Curator is /// @notice Reference to factory contract ICuratorFactory private immutable curatorFactory; + // /// @notice Modifier that ensures curation functionality is active and not frozen + // modifier onlyActive() { + // if (isPaused && msg.sender != owner()) { + // revert CURATION_PAUSED(); + // } + + // if (frozenAt != 0 && frozenAt < block.timestamp) { + // revert CURATION_FROZEN(); + // } + + // _; + // } + + // /// @notice Modifier that restricts entry access to an admin or curator + // /// @param listingId to check access for + // modifier onlyCuratorOrAdmin(uint256 listingId) { + // if (owner() != msg.sender && idToListing[listingId].curator != msg.sender) { + // revert ACCESS_NOT_ALLOWED(); + // } + + // _; + // } + + // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx /// @notice Modifier that ensures curation functionality is active and not frozen modifier onlyActive() { - if (isPaused && msg.sender != owner()) { + if (isPaused && IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 2) { revert CURATION_PAUSED(); } @@ -51,15 +76,31 @@ contract Curator is _; } + + modifier onlyOwnerOrAdminAccess() { + if ( + IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 3 && + owner() != msg.sender + ) { + revert ACCESS_NOT_ALLOWED(); + } + + _; + } + /// @notice Modifier that restricts entry access to an admin or curator /// @param listingId to check access for - modifier onlyCuratorOrAdmin(uint256 listingId) { - if (owner() != msg.sender && idToListing[listingId].curator != msg.sender) { + modifier onlyCuratorOrManagerAccess(uint256 listingId) { + if ( + IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 2 && + idToListing[listingId].curator != msg.sender + ) { revert ACCESS_NOT_ALLOWED(); } _; } + // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx /// @notice Global constructor – these variables will not change with further proxy deploys /// @param _curatorFactory Curator Factory Address @@ -67,37 +108,44 @@ contract Curator is curatorFactory = ICuratorFactory(_curatorFactory); } - /// @dev Create a new curation contract /// @param _owner User that owns and can accesss contract admin functionality /// @param _name Contract name /// @param _symbol Contract symbol - /// @param _curationPass ERC721 contract whose ownership gates access to curation functionality + // @param _curationPass ERC721 contract whose ownership gates access to curation functionality /// @param _pause Sets curation active state upon initialization /// @param _curationLimit Sets cap for number of listings that can be curated at any time. Doubles as MaxSupply check. 0 = uncapped /// @param _renderer Renderer contract to use /// @param _rendererInitializer Bytes encoded string to pass into renderer. Leave blank if using SVGMetadataRenderer /// @param _initialListings Array of Listing structs to curate (aka mint) upon initialization + /// @param _accessControl access control contract to use + /// @param _accessControlInitializer Bytes encoded data to pass into accessControl. Leave blank if ??? function initialize( address _owner, string memory _name, string memory _symbol, - address _curationPass, + // address _curationPass, bool _pause, uint256 _curationLimit, address _renderer, bytes memory _rendererInitializer, - Listing[] memory _initialListings + Listing[] memory _initialListings, + address _accessControl, + bytes memory _accessControlInitializer ) external initializer { // Setup owner role __Ownable_init(_owner); // Setup contract name + symbol contractName = _name; contractSymbol = _symbol; - // Setup curation pass. MUST be set to a valid ERC721 address - curationPass = IERC721Upgradeable(_curationPass); + + // // Setup curation pass. MUST be set to a valid ERC721 address + // curationPass = IERC721Upgradeable(_curationPass); + // Setup metadata renderer _updateRenderer(IMetadataRenderer(_renderer), _rendererInitializer); + // Setup accessControl + _updateAccessControl(IAccessControlRegistry(_accessControl), _accessControlInitializer); // Setup initial curation active state if (_pause) { _setCurationPaused(_pause); @@ -147,7 +195,7 @@ contract Curator is /// @dev Allows contract owner to update curation limit /// @param newLimit new curationLimit to assign - function updateCurationLimit(uint256 newLimit) external onlyOwner { + function updateCurationLimit(uint256 newLimit) external onlyOwnerOrAdminAccess { _updateCurationLimit(newLimit); } @@ -163,7 +211,7 @@ contract Curator is /// @dev Allows contract owner to freeze all contract functionality starting from a given Unix timestamp /// @param timestamp unix timestamp in seconds - function freezeAt(uint256 timestamp) external onlyOwner { + function freezeAt(uint256 timestamp) external onlyOwnerOrAdminAccess { // Prevents owner from adjusting freezeAt time if contract alrady frozen if (frozenAt != 0 && frozenAt < block.timestamp) { @@ -176,7 +224,7 @@ contract Curator is /// @dev Allows contract owner to update renderer address and pass in an optional initializer for the new renderer /// @param _newRenderer address of new renderer /// @param _rendererInitializer bytes encoded string value passed into new renderer - function updateRenderer(address _newRenderer, bytes memory _rendererInitializer) external onlyOwner { + function updateRenderer(address _newRenderer, bytes memory _rendererInitializer) external onlyOwnerOrAdminAccess { _updateRenderer(IMetadataRenderer(_newRenderer), _rendererInitializer); } @@ -190,17 +238,48 @@ contract Curator is emit SetRenderer(address(renderer)); } - /// @dev Allows contract owner to update the ERC721 Curation Pass being used to restrict access to curation functionality - /// @param _curationPass address of new ERC721 Curation Pass - function updateCurationPass(IERC721Upgradeable _curationPass) public onlyOwner { - curationPass = _curationPass; - emit CurationPassUpdated(msg.sender, address(_curationPass)); + + + //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + + /// @dev Allows contract owner to update accessControl address and pass in an optional initializer for the new accessControl + /// @param _newAccessControl address of new accessControl + /// @param _accessControlInitializer bytes encoded data passed into new accessControl + function updateAccessControl(address _newAccessControl, bytes memory _accessControlInitializer) external onlyOwnerOrAdminAccess { + _updateAccessControl(IAccessControlRegistry(_newAccessControl), _accessControlInitializer); + } + + function _updateAccessControl(IAccessControlRegistry _newAccessControl, bytes memory _accessControlInitializer) internal { + accessControl = _newAccessControl; + + // If data provided, call initalize to new renderer replacement. + if (_accessControlInitializer.length > 0) { + accessControl.initializeWithData(_accessControlInitializer); + } + emit SetAccessControl(address(accessControl)); } + //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + + + + + + + + + // /// @dev Allows contract owner to update the ERC721 Curation Pass being used to restrict access to curation functionality + // /// @param _curationPass address of new ERC721 Curation Pass + // function updateCurationPass(IERC721Upgradeable _curationPass) public onlyOwner { + // curationPass = _curationPass; + + // emit CurationPassUpdated(msg.sender, address(_curationPass)); + // } + /// @dev Allows contract owner to update the ERC721 Curation Pass being used to restrict access to curation functionality /// @param _setPaused boolean of new curation active state - function setCurationPaused(bool _setPaused) public onlyOwner { + function setCurationPaused(bool _setPaused) public onlyOwnerOrAdminAccess { // Prevents owner from updating the curation active state to the current active state if (isPaused == _setPaused) { @@ -228,23 +307,27 @@ contract Curator is /// @param listings array of Listing structs function addListings(Listing[] memory listings) external onlyActive { - // Access control for non owners to acess addListings functionality - if (msg.sender != owner()) { + // // Access control for non owners to acess addListings functionality + // if (msg.sender != owner()) { - // ensures that curationPass is a valid ERC721 address - if (address(curationPass).code.length == 0) { - revert PASS_REQUIRED(); - } - - // checks if non-owner msg.sender owns the Curation Pass - try curationPass.balanceOf(msg.sender) returns (uint256 count) { - if (count == 0) { - revert PASS_REQUIRED(); - } - } catch { - revert PASS_REQUIRED(); - } - } + // Access control for non manager/admins to acess addListings functionality + if (IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 1) { + revert ACCESS_NOT_ALLOWED(); + } + // // ensures that curationPass is a valid ERC721 address + // if (address(curationPass).code.length == 0) { + // revert PASS_REQUIRED(); + // } + + // // checks if non-owner msg.sender owns the Curation Pass + // try curationPass.balanceOf(msg.sender) returns (uint256 count) { + // if (count == 0) { + // revert PASS_REQUIRED(); + // } + // } catch { + // revert PASS_REQUIRED(); + // } + // } _addListings(listings, msg.sender); } @@ -283,7 +366,7 @@ contract Curator is } // prevents non-owners from updating the SortOrder on a listingRecord they did not curate themselves - function _setSortOrder(uint256 listingId, int32 sortOrder) internal onlyCuratorOrAdmin(listingId) { + function _setSortOrder(uint256 listingId, int32 sortOrder) internal onlyCuratorOrManagerAccess(listingId) { idToListing[listingId].sortOrder = sortOrder; } @@ -303,7 +386,6 @@ contract Curator is _burnTokenWithChecks(listingId); } - /// @dev allows owner or curators to burn specfic listingRecord NFTs which also removes them from the listings mapping /// @param listingIds array of listingIds to burn function removeListings(uint256[] calldata listingIds) external onlyActive { @@ -357,7 +439,7 @@ contract Curator is return renderer.contractURI(); } - function _burnTokenWithChecks(uint256 listingId) internal onlyActive onlyCuratorOrAdmin(listingId) { + function _burnTokenWithChecks(uint256 listingId) internal onlyActive onlyCuratorOrManagerAccess(listingId) { Listing memory _listing = idToListing[listingId]; // Process NFT Burn _burn(listingId); diff --git a/src/CuratorStorageV1.sol b/src/CuratorStorageV1.sol index 212065e..d156ad3 100644 --- a/src/CuratorStorageV1.sol +++ b/src/CuratorStorageV1.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.15; import { ICurator } from "./interfaces/ICurator.sol"; import { IERC721Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721Upgradeable.sol"; import { IMetadataRenderer } from "./interfaces/IMetadataRenderer.sol"; +import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; /** @@ -17,9 +18,12 @@ abstract contract CuratorStorageV1 is ICurator { /// @notice Standard ERC721 symbol for the curator contract string internal contractSymbol; - /// Curation pass as an ERC721 that allows other users to curate. - /// @notice Address to ERC721 with `balanceOf` function. - IERC721Upgradeable public curationPass; + // Curation pass as an ERC721 that allows other users to curate. + // @notice Address to ERC721 with `balanceOf` function. + // IERC721Upgradeable public curationPass; + + /// @notice Address of the accessControl contract + IAccessControlRegistry public accessControl; /// Stores virtual mapping array length parameters /// @notice Array total size (total size) diff --git a/src/interfaces/ICurator.sol b/src/interfaces/ICurator.sol index 66f65ea..4a5d988 100644 --- a/src/interfaces/ICurator.sol +++ b/src/interfaces/ICurator.sol @@ -59,9 +59,12 @@ interface ICurator { /// @notice Emitted when a listing is removed event ListingRemoved(address indexed curator, Listing listing); - /// @notice The curation pass has been updated for the curation contract - /// @dev Any users that have already curated something still can delete their curation. - event CurationPassUpdated(address indexed owner, address curationPass); + // @notice The curation pass has been updated for the curation contract + // @dev Any users that have already curated something still can delete their curation. + // event CurationPassUpdated(address indexed owner, address curationPass); + + /// @notice A new accessControl is set + event SetAccessControl(address); /// @notice A new renderer is set event SetRenderer(address); @@ -78,11 +81,11 @@ interface ICurator { /// @notice This contract is scheduled to be frozen event ScheduledFreeze(uint256 timestamp); - /// @notice Pass is required to manage curation but not held by attempted updater. - error PASS_REQUIRED(); + // /// @notice Pass is required to manage curation but not held by attempted updater. + // error PASS_REQUIRED(); - /// @notice Only the curator of a listing (or owner) can manage that curation - error ONLY_CURATOR(); + // @notice Only the curator of a listing (or owner) can manage that curation + // error ONLY_CURATOR(); /// @notice Wrong curator for the listing when attempting to access the listing. error WRONG_CURATOR_FOR_LISTING(address setCurator, address expectedCurator); @@ -115,11 +118,12 @@ interface ICurator { address _owner, string memory _name, string memory _symbol, - address _curationPass, bool _pause, uint256 _curationLimit, address _renderer, bytes memory _rendererInitializer, - Listing[] memory _initialListings + Listing[] memory _initialListings, + address _accessControl, + bytes memory _accessControlInitializer ) external; } diff --git a/src/interfaces/ICuratorInfo.sol b/src/interfaces/ICuratorInfo.sol index a4e5a94..94dce8e 100644 --- a/src/interfaces/ICuratorInfo.sol +++ b/src/interfaces/ICuratorInfo.sol @@ -1,12 +1,15 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol"; +// import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol"; +import { IAccessControlRegistry } from "./interfaces/IAccessControlRegistry.sol"; interface ICuratorInfo { function name() external view returns (string memory); - function curationPass() external view returns (IERC721Metadata); + function accessControl() external view returns (IAccessControlRegistry); + + // function curationPass() external view returns (IERC721Metadata); function owner() external view returns (address); } From 06e0eee50e89e4121baeb281e7aff0975cb9119a Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 30 Oct 2022 16:56:39 -0700 Subject: [PATCH 3/9] forge install: onchain-modules --- .gitmodules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitmodules b/.gitmodules index 52c4339..00a71c4 100644 --- a/.gitmodules +++ b/.gitmodules @@ -6,4 +6,4 @@ url = https://github.com/iainnash/micro-onchain-metadata-utils [submodule "lib/onchain-modules"] path = lib/onchain-modules - url = https://github.com/public-assembly/onchain-modules + url = https://github.com/public--assembly/onchain-modules From 1c3d073eee6af671281b2991a8d95766c68cb89b Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 30 Oct 2022 16:59:27 -0700 Subject: [PATCH 4/9] forge install: onchain-modules --- .gitmodules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitmodules b/.gitmodules index 00a71c4..52c4339 100644 --- a/.gitmodules +++ b/.gitmodules @@ -6,4 +6,4 @@ url = https://github.com/iainnash/micro-onchain-metadata-utils [submodule "lib/onchain-modules"] path = lib/onchain-modules - url = https://github.com/public--assembly/onchain-modules + url = https://github.com/public-assembly/onchain-modules From 46d3f8c01dd300bf7962f618b3b507f8dd0ea5d3 Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 30 Oct 2022 17:59:17 -0700 Subject: [PATCH 5/9] finished draft of new access control -- NO TESTS --- lib/forge-std | 2 +- lib/micro-onchain-metadata-utils | 2 +- lib/onchain-modules | 2 +- src/Curator.sol | 87 +++------------------------- src/CuratorFactory.sol | 20 ++++++- src/CuratorStorageV1.sol | 6 -- src/interfaces/ICurator.sol | 14 +---- src/interfaces/ICuratorFactory.sol | 2 + src/interfaces/ICuratorInfo.sol | 5 +- src/renderer/SVGMetadataRenderer.sol | 15 ++--- 10 files changed, 38 insertions(+), 117 deletions(-) diff --git a/lib/forge-std b/lib/forge-std index 82e6f53..2a2ce36 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 82e6f5376c15341629ca23098e0b32d303f44f02 +Subproject commit 2a2ce3692b8c1523b29de3ec9d961ee9fbbc43a6 diff --git a/lib/micro-onchain-metadata-utils b/lib/micro-onchain-metadata-utils index ae854fd..46a60e2 160000 --- a/lib/micro-onchain-metadata-utils +++ b/lib/micro-onchain-metadata-utils @@ -1 +1 @@ -Subproject commit ae854fd4c981384dc602c846e4c1900e4aacd115 +Subproject commit 46a60e298132a9bec578cdaa6bd71bf0775f411e diff --git a/lib/onchain-modules b/lib/onchain-modules index 5dcfa74..1d9e08e 160000 --- a/lib/onchain-modules +++ b/lib/onchain-modules @@ -1 +1 @@ -Subproject commit 5dcfa74569a08f56c8dab625b8c034fd64506e6f +Subproject commit 1d9e08e2af820fc7b7c5952de42ce84876455d33 diff --git a/src/Curator.sol b/src/Curator.sol index 4c859d0..84b0b8c 100644 --- a/src/Curator.sol +++ b/src/Curator.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import { IERC721Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721Upgradeable.sol"; import { UUPS } from "./lib/proxy/UUPS.sol"; import { ICurator } from "./interfaces/ICurator.sol"; import { Ownable } from "./lib/utils/Ownable.sol"; @@ -39,30 +38,6 @@ contract Curator is /// @notice Reference to factory contract ICuratorFactory private immutable curatorFactory; - // /// @notice Modifier that ensures curation functionality is active and not frozen - // modifier onlyActive() { - // if (isPaused && msg.sender != owner()) { - // revert CURATION_PAUSED(); - // } - - // if (frozenAt != 0 && frozenAt < block.timestamp) { - // revert CURATION_FROZEN(); - // } - - // _; - // } - - // /// @notice Modifier that restricts entry access to an admin or curator - // /// @param listingId to check access for - // modifier onlyCuratorOrAdmin(uint256 listingId) { - // if (owner() != msg.sender && idToListing[listingId].curator != msg.sender) { - // revert ACCESS_NOT_ALLOWED(); - // } - - // _; - // } - - // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx /// @notice Modifier that ensures curation functionality is active and not frozen modifier onlyActive() { if (isPaused && IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 2) { @@ -76,7 +51,7 @@ contract Curator is _; } - + /// @notice Modifier that restricts entry access to an owner or admin modifier onlyOwnerOrAdminAccess() { if ( IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 3 && @@ -88,7 +63,7 @@ contract Curator is _; } - /// @notice Modifier that restricts entry access to an admin or curator + /// @notice Modifier that restricts entry access to an manager/admin or curator /// @param listingId to check access for modifier onlyCuratorOrManagerAccess(uint256 listingId) { if ( @@ -100,7 +75,6 @@ contract Curator is _; } - // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx /// @notice Global constructor – these variables will not change with further proxy deploys /// @param _curatorFactory Curator Factory Address @@ -112,36 +86,30 @@ contract Curator is /// @param _owner User that owns and can accesss contract admin functionality /// @param _name Contract name /// @param _symbol Contract symbol - // @param _curationPass ERC721 contract whose ownership gates access to curation functionality /// @param _pause Sets curation active state upon initialization /// @param _curationLimit Sets cap for number of listings that can be curated at any time. Doubles as MaxSupply check. 0 = uncapped /// @param _renderer Renderer contract to use /// @param _rendererInitializer Bytes encoded string to pass into renderer. Leave blank if using SVGMetadataRenderer - /// @param _initialListings Array of Listing structs to curate (aka mint) upon initialization /// @param _accessControl access control contract to use - /// @param _accessControlInitializer Bytes encoded data to pass into accessControl. Leave blank if ??? + /// @param _accessControlInitializer Bytes encoded data to pass into accessControl. Leave blank if ??? + /// @param _initialListings Array of Listing structs to curate (aka mint) upon initialization function initialize( address _owner, string memory _name, string memory _symbol, - // address _curationPass, bool _pause, uint256 _curationLimit, address _renderer, bytes memory _rendererInitializer, - Listing[] memory _initialListings, address _accessControl, - bytes memory _accessControlInitializer + bytes memory _accessControlInitializer, + Listing[] memory _initialListings ) external initializer { // Setup owner role __Ownable_init(_owner); // Setup contract name + symbol contractName = _name; contractSymbol = _symbol; - - // // Setup curation pass. MUST be set to a valid ERC721 address - // curationPass = IERC721Upgradeable(_curationPass); - // Setup metadata renderer _updateRenderer(IMetadataRenderer(_renderer), _rendererInitializer); // Setup accessControl @@ -238,11 +206,6 @@ contract Curator is emit SetRenderer(address(renderer)); } - - - - //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx - /// @dev Allows contract owner to update accessControl address and pass in an optional initializer for the new accessControl /// @param _newAccessControl address of new accessControl /// @param _accessControlInitializer bytes encoded data passed into new accessControl @@ -260,23 +223,6 @@ contract Curator is emit SetAccessControl(address(accessControl)); } - //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx - - - - - - - - - // /// @dev Allows contract owner to update the ERC721 Curation Pass being used to restrict access to curation functionality - // /// @param _curationPass address of new ERC721 Curation Pass - // function updateCurationPass(IERC721Upgradeable _curationPass) public onlyOwner { - // curationPass = _curationPass; - - // emit CurationPassUpdated(msg.sender, address(_curationPass)); - // } - /// @dev Allows contract owner to update the ERC721 Curation Pass being used to restrict access to curation functionality /// @param _setPaused boolean of new curation active state function setCurationPaused(bool _setPaused) public onlyOwnerOrAdminAccess { @@ -305,29 +251,12 @@ contract Curator is /// @dev Allows owner or curator to curate Listings --> which mints a listingRecord token to the msg.sender /// @param listings array of Listing structs - function addListings(Listing[] memory listings) external onlyActive { - - // // Access control for non owners to acess addListings functionality - // if (msg.sender != owner()) { + function addListings(Listing[] memory listings) external onlyActive { - // Access control for non manager/admins to acess addListings functionality + // Access control to prevent non curators/manager/admins from accessing if (IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 1) { revert ACCESS_NOT_ALLOWED(); } - // // ensures that curationPass is a valid ERC721 address - // if (address(curationPass).code.length == 0) { - // revert PASS_REQUIRED(); - // } - - // // checks if non-owner msg.sender owns the Curation Pass - // try curationPass.balanceOf(msg.sender) returns (uint256 count) { - // if (count == 0) { - // revert PASS_REQUIRED(); - // } - // } catch { - // revert PASS_REQUIRED(); - // } - // } _addListings(listings, msg.sender); } diff --git a/src/CuratorFactory.sol b/src/CuratorFactory.sol index cadc307..1f039bf 100644 --- a/src/CuratorFactory.sol +++ b/src/CuratorFactory.sol @@ -14,6 +14,8 @@ import { Curator } from "./Curator.sol"; abstract contract CuratorFactoryStorageV1 { address public defaultMetadataRenderer; + address public defaultAccessControl; + mapping(address => mapping(address => bool)) internal isUpgrade; uint256[50] __gap; @@ -41,25 +43,36 @@ contract CuratorFactory is ICuratorFactory, UUPS, Ownable, CuratorFactoryStorage emit HasNewMetadataRenderer(_renderer); } - function initialize(address _owner, address _defaultMetadataRenderer) external initializer { + function setDefaultAccessControl(address _accessControl) external { + defaultAccessControl = _accessControl; + + emit HasNewAccessControl(_accessControl); + } + + function initialize(address _owner, address _defaultMetadataRenderer, address _defaultAccessControl) external initializer { __Ownable_init(_owner); defaultMetadataRenderer = _defaultMetadataRenderer; + defaultAccessControl = _defaultAccessControl; } function deploy( address curationManager, string memory name, string memory symbol, - address curationPass, bool initialPause, uint256 curationLimit, address renderer, bytes memory rendererInitializer, + address accessControl, + bytes memory accessControlInitializer, ICurator.Listing[] memory listings ) external returns (address curator) { if (renderer == address(0)) { renderer = defaultMetadataRenderer; } + if (accessControl == address(0)) { + accessControl = defaultAccessControl; + } curator = address( new ERC1967Proxy( @@ -69,11 +82,12 @@ contract CuratorFactory is ICuratorFactory, UUPS, Ownable, CuratorFactoryStorage curationManager, name, symbol, - curationPass, initialPause, curationLimit, renderer, rendererInitializer, + accessControl, + accessControlInitializer, listings ) ) diff --git a/src/CuratorStorageV1.sol b/src/CuratorStorageV1.sol index d156ad3..9fd5eaa 100644 --- a/src/CuratorStorageV1.sol +++ b/src/CuratorStorageV1.sol @@ -2,11 +2,9 @@ pragma solidity 0.8.15; import { ICurator } from "./interfaces/ICurator.sol"; -import { IERC721Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721Upgradeable.sol"; import { IMetadataRenderer } from "./interfaces/IMetadataRenderer.sol"; import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; - /** @notice Curator storage variables contract. @author iain@zora.co @@ -18,10 +16,6 @@ abstract contract CuratorStorageV1 is ICurator { /// @notice Standard ERC721 symbol for the curator contract string internal contractSymbol; - // Curation pass as an ERC721 that allows other users to curate. - // @notice Address to ERC721 with `balanceOf` function. - // IERC721Upgradeable public curationPass; - /// @notice Address of the accessControl contract IAccessControlRegistry public accessControl; diff --git a/src/interfaces/ICurator.sol b/src/interfaces/ICurator.sol index 4a5d988..63aadde 100644 --- a/src/interfaces/ICurator.sol +++ b/src/interfaces/ICurator.sol @@ -59,10 +59,6 @@ interface ICurator { /// @notice Emitted when a listing is removed event ListingRemoved(address indexed curator, Listing listing); - // @notice The curation pass has been updated for the curation contract - // @dev Any users that have already curated something still can delete their curation. - // event CurationPassUpdated(address indexed owner, address curationPass); - /// @notice A new accessControl is set event SetAccessControl(address); @@ -81,12 +77,6 @@ interface ICurator { /// @notice This contract is scheduled to be frozen event ScheduledFreeze(uint256 timestamp); - // /// @notice Pass is required to manage curation but not held by attempted updater. - // error PASS_REQUIRED(); - - // @notice Only the curator of a listing (or owner) can manage that curation - // error ONLY_CURATOR(); - /// @notice Wrong curator for the listing when attempting to access the listing. error WRONG_CURATOR_FOR_LISTING(address setCurator, address expectedCurator); @@ -122,8 +112,8 @@ interface ICurator { uint256 _curationLimit, address _renderer, bytes memory _rendererInitializer, - Listing[] memory _initialListings, address _accessControl, - bytes memory _accessControlInitializer + bytes memory _accessControlInitializer, + Listing[] memory _initialListings ) external; } diff --git a/src/interfaces/ICuratorFactory.sol b/src/interfaces/ICuratorFactory.sol index 36cb0cf..2460559 100644 --- a/src/interfaces/ICuratorFactory.sol +++ b/src/interfaces/ICuratorFactory.sol @@ -12,6 +12,8 @@ interface ICuratorFactory { event RegisteredUpgradePath(address implFrom, address implTo); /// @notice Emitted when a new metadata renderer is set event HasNewMetadataRenderer(address); + /// @notice Emitted when a new accessControl is set + event HasNewAccessControl(address); /// @notice Getter to determine if a contract upgrade path is valid. function isValidUpgrade(address baseImpl, address newImpl) external view returns (bool); diff --git a/src/interfaces/ICuratorInfo.sol b/src/interfaces/ICuratorInfo.sol index 94dce8e..304ec95 100644 --- a/src/interfaces/ICuratorInfo.sol +++ b/src/interfaces/ICuratorInfo.sol @@ -1,15 +1,12 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -// import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol"; -import { IAccessControlRegistry } from "./interfaces/IAccessControlRegistry.sol"; +import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; interface ICuratorInfo { function name() external view returns (string memory); function accessControl() external view returns (IAccessControlRegistry); - // function curationPass() external view returns (IERC721Metadata); - function owner() external view returns (address); } diff --git a/src/renderer/SVGMetadataRenderer.sol b/src/renderer/SVGMetadataRenderer.sol index 2a02053..ff659ba 100644 --- a/src/renderer/SVGMetadataRenderer.sol +++ b/src/renderer/SVGMetadataRenderer.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.15; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IMetadataRenderer } from "../interfaces/IMetadataRenderer.sol"; -import { ICuratorInfo, IERC721Metadata } from "../interfaces/ICuratorInfo.sol"; +import { ICuratorInfo } from "../interfaces/ICuratorInfo.sol"; import { IZoraDrop } from "../interfaces/IZoraDrop.sol"; import { ICurator } from "../interfaces/ICurator.sol"; @@ -119,9 +119,9 @@ contract SVGMetadataRenderer is IMetadataRenderer { ICuratorInfo curation = ICuratorInfo(msg.sender); MetadataBuilder.JSONItem[] memory items = new MetadataBuilder.JSONItem[](3); - string memory curationName = "Untitled NFT"; + string memory curationName = "Untitled Access Control"; - try curation.curationPass().name() returns (string memory result) { + try curation.accessControl().name() returns (string memory result) { curationName = result; } catch {} @@ -137,10 +137,7 @@ contract SVGMetadataRenderer is IMetadataRenderer { "The curation pass for this NFT is ", curationName, "\\n\\nThese NFTs only mark curations and are non-transferrable." - "\\n\\nView or manage this curation at: " - "https://public---assembly.com/curation/", - Strings.toHexString(msg.sender), - "\\n\\nA project of public assembly." + "\\n\\nA project of Public Assembly." ); items[1].quote = true; items[2].key = MetadataJSONKeys.keyImage; @@ -219,9 +216,7 @@ contract SVGMetadataRenderer is IMetadataRenderer { Strings.toHexString(listing.curator), "\\n\\nTo remove this curation, burn the NFT. " "\\n\\nThis NFT is non-transferrable. " - "\\n\\nView or manage this curation at: " - "https://public---assembly.com/curation/", - Strings.toHexString(msg.sender) + "\\n\\nA project of Public Assembly." ); items[1].quote = true; items[2].key = MetadataJSONKeys.keyImage; From 57707591e01064ab41ce4477e80010f8ba38e580 Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 6 Nov 2022 18:17:22 -0800 Subject: [PATCH 6/9] minor updates --- lib/forge-std | 2 +- lib/micro-onchain-metadata-utils | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/forge-std b/lib/forge-std index 2a2ce36..82e6f53 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 2a2ce3692b8c1523b29de3ec9d961ee9fbbc43a6 +Subproject commit 82e6f5376c15341629ca23098e0b32d303f44f02 diff --git a/lib/micro-onchain-metadata-utils b/lib/micro-onchain-metadata-utils index 46a60e2..ae854fd 160000 --- a/lib/micro-onchain-metadata-utils +++ b/lib/micro-onchain-metadata-utils @@ -1 +1 @@ -Subproject commit 46a60e298132a9bec578cdaa6bd71bf0775f411e +Subproject commit ae854fd4c981384dc602c846e4c1900e4aacd115 From b05f51090c22231496d3106c9719c78ded50360b Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 6 Nov 2022 18:17:28 -0800 Subject: [PATCH 7/9] forge install: onchain --- .gitmodules | 3 +++ lib/onchain | 1 + 2 files changed, 4 insertions(+) create mode 160000 lib/onchain diff --git a/.gitmodules b/.gitmodules index 52c4339..cfc56dd 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,3 +7,6 @@ [submodule "lib/onchain-modules"] path = lib/onchain-modules url = https://github.com/public-assembly/onchain-modules +[submodule "lib/onchain"] + path = lib/onchain + url = https://github.com/public-assembly/onchain diff --git a/lib/onchain b/lib/onchain new file mode 160000 index 0000000..252d72a --- /dev/null +++ b/lib/onchain @@ -0,0 +1 @@ +Subproject commit 252d72a4c804b1dcb5d0eb33867bd1704d22a8a7 From 20a2e437ebb60500066bf6554a40d7ea1d76ff87 Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 6 Nov 2022 22:12:17 -0800 Subject: [PATCH 8/9] access role enums + updated onlyActive --- .gitmodules | 3 --- lib/onchain-modules | 1 - remappings.txt | 2 +- src/Curator.sol | 29 +++++++++++++++++++++-------- src/CuratorStorageV1.sol | 2 +- src/interfaces/ICuratorInfo.sol | 2 +- 6 files changed, 24 insertions(+), 15 deletions(-) delete mode 160000 lib/onchain-modules diff --git a/.gitmodules b/.gitmodules index cfc56dd..6d0ab03 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,9 +4,6 @@ [submodule "lib/micro-onchain-metadata-utils"] path = lib/micro-onchain-metadata-utils url = https://github.com/iainnash/micro-onchain-metadata-utils -[submodule "lib/onchain-modules"] - path = lib/onchain-modules - url = https://github.com/public-assembly/onchain-modules [submodule "lib/onchain"] path = lib/onchain url = https://github.com/public-assembly/onchain diff --git a/lib/onchain-modules b/lib/onchain-modules deleted file mode 160000 index 1d9e08e..0000000 --- a/lib/onchain-modules +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 1d9e08e2af820fc7b7c5952de42ce84876455d33 diff --git a/remappings.txt b/remappings.txt index 50fdd0f..9ecd82c 100644 --- a/remappings.txt +++ b/remappings.txt @@ -2,4 +2,4 @@ forge-std/=lib/forge-std/src/ micro-onchain-metadata-utils/=lib/micro-onchain-metadata-utils/src/ @openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/ @openzeppelin/contracts/=node_modules/@openzeppelin/contracts/ -onchain-modules/=lib/onchain-modules/tokenized-access-control/src \ No newline at end of file +onchain/=lib/onchain/tokenized-access-control/src \ No newline at end of file diff --git a/src/Curator.sol b/src/Curator.sol index 84b0b8c..d0bc59e 100644 --- a/src/Curator.sol +++ b/src/Curator.sol @@ -8,7 +8,7 @@ import { ICuratorFactory } from "./interfaces/ICuratorFactory.sol"; import { CuratorSkeletonNFT } from "./CuratorSkeletonNFT.sol"; import { IMetadataRenderer } from "./interfaces/IMetadataRenderer.sol"; import { CuratorStorageV1 } from "./CuratorStorageV1.sol"; -import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; +import { IAccessControlRegistry } from "onchain/interfaces/IAccessControlRegistry.sol"; /** * @notice Base contract for curation functioanlity. Inherits ERC721 standard from CuratorSkeletonNFT.sol @@ -35,14 +35,27 @@ contract Curator is uint16 public constant CURATION_TYPE_WALLET = 5; uint16 public constant CURATION_TYPE_ZORA_ERC721 = 6; + enum AccessRoles { + noAccess + curator, + manager, + admin + } + + AccessRoles public accessRoles; + /// @notice Reference to factory contract ICuratorFactory private immutable curatorFactory; /// @notice Modifier that ensures curation functionality is active and not frozen modifier onlyActive() { - if (isPaused && IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 2) { + if (isPaused + & (IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.manager + || msg.sender != owner()) + ) { revert CURATION_PAUSED(); - } + } + if (frozenAt != 0 && frozenAt < block.timestamp) { revert CURATION_FROZEN(); @@ -54,8 +67,8 @@ contract Curator is /// @notice Modifier that restricts entry access to an owner or admin modifier onlyOwnerOrAdminAccess() { if ( - IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 3 && - owner() != msg.sender + IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.admin + && owner() != msg.sender ) { revert ACCESS_NOT_ALLOWED(); } @@ -67,8 +80,8 @@ contract Curator is /// @param listingId to check access for modifier onlyCuratorOrManagerAccess(uint256 listingId) { if ( - IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 2 && - idToListing[listingId].curator != msg.sender + IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.manager + && idToListing[listingId].curator != msg.sender ) { revert ACCESS_NOT_ALLOWED(); } @@ -254,7 +267,7 @@ contract Curator is function addListings(Listing[] memory listings) external onlyActive { // Access control to prevent non curators/manager/admins from accessing - if (IAccessControlRegistry(accessControl).getAccessLevel(msg.sender) < 1) { + if (IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.curator ) { revert ACCESS_NOT_ALLOWED(); } diff --git a/src/CuratorStorageV1.sol b/src/CuratorStorageV1.sol index 9fd5eaa..7178a78 100644 --- a/src/CuratorStorageV1.sol +++ b/src/CuratorStorageV1.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.15; import { ICurator } from "./interfaces/ICurator.sol"; import { IMetadataRenderer } from "./interfaces/IMetadataRenderer.sol"; -import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; +import { IAccessControlRegistry } from "onchain/interfaces/IAccessControlRegistry.sol"; /** @notice Curator storage variables contract. diff --git a/src/interfaces/ICuratorInfo.sol b/src/interfaces/ICuratorInfo.sol index 304ec95..fe9c7a9 100644 --- a/src/interfaces/ICuratorInfo.sol +++ b/src/interfaces/ICuratorInfo.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import { IAccessControlRegistry } from "onchain-modules/interfaces/IAccessControlRegistry.sol"; +import { IAccessControlRegistry } from "onchain/interfaces/IAccessControlRegistry.sol"; interface ICuratorInfo { function name() external view returns (string memory); From 5ce462f6e4198fcd44e8258f1882796865685744 Mon Sep 17 00:00:00 2001 From: 0xTranqui <thinkandchill11@gmail.com> Date: Sun, 6 Nov 2022 22:14:08 -0800 Subject: [PATCH 9/9] replace & with && --- src/Curator.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Curator.sol b/src/Curator.sol index d0bc59e..59b0448 100644 --- a/src/Curator.sol +++ b/src/Curator.sol @@ -50,8 +50,8 @@ contract Curator is /// @notice Modifier that ensures curation functionality is active and not frozen modifier onlyActive() { if (isPaused - & (IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.manager - || msg.sender != owner()) + && (IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.manager + && msg.sender != owner()) ) { revert CURATION_PAUSED(); } @@ -66,9 +66,8 @@ contract Curator is /// @notice Modifier that restricts entry access to an owner or admin modifier onlyOwnerOrAdminAccess() { - if ( - IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.admin - && owner() != msg.sender + if (IAccessControlRegistry(accessControl).getAccessLevel(address(this), msg.sender) < accessRoles.admin + && owner() != msg.sender ) { revert ACCESS_NOT_ALLOWED(); }