Merge pull request #94 from lens-protocol/test/nfts-and-royalties

test: NFTs and royalties

contracts/CollectNFT.sol

@@ -14,6 +14,9 @@ import {ActionRestricted} from 'contracts/modules/ActionRestricted.sol';
  * @title CollectNFT
  * @author Lens Protocol
  *
+ * @dev This is the CollectNFT for Lens V2, it differs from LegacyCollectNFT that it's restricted to be called by an
+ * action module instead of LensHub.
+ *
  * @notice This is the NFT contract that is minted upon collecting a given publication. It is cloned upon
  * the first collect for a given publication, and the token URI points to the original publication's contentURI.
  */

contracts/FollowNFT.sol

@@ -47,7 +47,9 @@ contract FollowNFT is HubRestricted, LensBaseERC721, ERC2981CollectionRoyalties,
     /// @inheritdoc IFollowNFT
     function initialize(uint256 profileId) external override {
         // This is called right after deployment by the LensHub, so we can skip the onlyHub check.
-        if (_initialized) revert Errors.Initialized();
+        if (_initialized) {
+            revert Errors.Initialized();
+        }
         _initialized = true;
         _followedProfileId = profileId;
         _setRoyalty(1000); // 10% of royalties

contracts/misc/LegacyCollectNFT.sol

@@ -13,6 +13,9 @@ import {LensBaseERC721} from 'contracts/base/LensBaseERC721.sol';
  * @title CollectNFT
  * @author Lens Protocol
  *
+ * @dev This is the Legacy CollectNFT that is used for Legacy Lens V1 publications. The new CollectNFT was introduced in
+ * Lens V2 with the only difference that it is restricted by the Action Module instead of the LensHub.
+ *
  * @notice This is the NFT contract that is minted upon collecting a given publication. It is cloned upon
  * the first collect for a given publication, and the token URI points to the original publication's contentURI.
  */

test/CollectNFTTest.t.sol

@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.13;
+
+import 'test/base/BaseTest.t.sol';
+import 'test/ERC721Test.t.sol';
+import {CollectPublicationAction} from 'contracts/modules/act/collect/CollectPublicationAction.sol';
+import {CollectNFT} from 'contracts/CollectNFT.sol';
+import {MockCollectModule} from 'test/mocks/MockCollectModule.sol';
+
+contract CollectNFTTest is BaseTest, ERC721Test {
+    using stdJson for string;
+
+    function testCollectNFTTest() public {
+        // Prevents being counted in Foundry Coverage
+    }
+
+    CollectPublicationAction collectPublicationAction;
+    address mockCollectModule;
+    CollectNFT collectNFT;
+    address collectNFTImpl;
+
+    Types.PublicationActionParams collectActionParams;
+
+    // Deploy CollectPublicationAction
+    constructor() TestSetup() {
+        if (fork && keyExists(string(abi.encodePacked('.', forkEnv, '.CollectNFTImpl')))) {
+            collectNFTImpl = json.readAddress(string(abi.encodePacked('.', forkEnv, '.CollectNFTImpl')));
+            console.log('Found CollectNFTImpl deployed at:', address(collectNFTImpl));
+        }
+
+        if (fork && keyExists(string(abi.encodePacked('.', forkEnv, '.CollectPublicationAction')))) {
+            collectPublicationAction = CollectPublicationAction(
+                json.readAddress(string(abi.encodePacked('.', forkEnv, '.CollectPublicationAction')))
+            );
+            console.log('Found collectPublicationAction deployed at:', address(collectPublicationAction));
+        }
+
+        // Both deployed - need to verify if they are linked
+        if (collectNFTImpl != address(0) && address(collectPublicationAction) != address(0)) {
+            if (CollectNFT(collectNFTImpl).ACTION_MODULE() == address(collectPublicationAction)) {
+                console.log('CollectNFTImpl and CollectPublicationAction already deployed and linked');
+                return;
+            }
+        }
+
+        uint256 deployerNonce = vm.getNonce(deployer);
+
+        address predictedCollectPublicationAction = computeCreateAddress(deployer, deployerNonce);
+        address predictedCollectNFTImpl = computeCreateAddress(deployer, deployerNonce + 1);
+
+        vm.startPrank(deployer);
+        collectPublicationAction = new CollectPublicationAction(
+            address(hub),
+            predictedCollectNFTImpl,
+            address(moduleGlobals)
+        );
+        collectNFTImpl = address(new CollectNFT(address(hub), address(collectPublicationAction)));
+        vm.stopPrank();
+
+        vm.prank(governance);
+        hub.whitelistActionModule(address(collectPublicationAction), true);
+
+        assertEq(
+            address(collectPublicationAction),
+            predictedCollectPublicationAction,
+            'CollectPublicationAction deployed address mismatch'
+        );
+        assertEq(collectNFTImpl, predictedCollectNFTImpl, 'CollectNFTImpl deployed address mismatch');
+
+        vm.label(address(collectPublicationAction), 'CollectPublicationAction');
+        vm.label(collectNFTImpl, 'CollectNFTImpl');
+    }
+
+    function setUp() public override {
+        super.setUp();
+
+        // Deploy & Whitelist MockCollectModule
+        mockCollectModule = address(new MockCollectModule());
+        vm.prank(moduleGlobals.getGovernance());
+        collectPublicationAction.whitelistCollectModule(mockCollectModule, true);
+
+        Types.PostParams memory postParams = _getDefaultPostParams();
+        postParams.actionModules[0] = address(collectPublicationAction);
+        postParams.actionModulesInitDatas[0] = abi.encode(mockCollectModule, abi.encode(true));
+
+        vm.prank(defaultAccount.owner);
+        uint256 pubId = hub.post(postParams);
+
+        collectActionParams = Types.PublicationActionParams({
+            publicationActedProfileId: defaultAccount.profileId,
+            publicationActedId: pubId,
+            actorProfileId: defaultAccount.profileId,
+            referrerProfileIds: _emptyUint256Array(),
+            referrerPubIds: _emptyUint256Array(),
+            actionModuleAddress: address(collectPublicationAction),
+            actionModuleData: abi.encode(true)
+        });
+
+        vm.prank(defaultAccount.owner);
+        hub.act(collectActionParams);
+
+        collectNFT = CollectNFT(collectPublicationAction.getCollectData(defaultAccount.profileId, pubId).collectNFT);
+    }
+
+    function _mintERC721(address to) internal virtual override returns (uint256) {
+        collectActionParams.actorProfileId = _createProfile(to);
+        vm.prank(to);
+        bytes memory actResult = hub.act(collectActionParams);
+        (uint256 tokenId, ) = abi.decode(actResult, (uint256, bytes));
+        return tokenId;
+    }
+
+    function _burnERC721(uint256 tokenId) internal virtual override {
+        collectNFT.burn(tokenId);
+    }
+
+    function _getERC721TokenAddress() internal view virtual override returns (address) {
+        return address(collectNFT);
+    }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Scenarios
+    //////////////////////////////////////////////////////////
+
+    function testSupportsErc2981Interface() public {
+        assertTrue(collectNFT.supportsInterface(bytes4(keccak256('royaltyInfo(uint256,uint256)'))));
+    }
+
+    function testDefaultRoyaltiesAreSetTo10Percent(uint256 tokenId) public {
+        uint256 salePrice = 100;
+        uint256 expectedRoyalties = 10;
+
+        (address receiver, uint256 royalties) = collectNFT.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, defaultAccount.owner);
+        assertEq(royalties, expectedRoyalties);
+    }
+
+    function testSetRoyalties(uint256 royaltiesInBasisPoints, uint256 tokenId, uint256 salePrice) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        uint256 salePriceTimesRoyalties;
+        unchecked {
+            salePriceTimesRoyalties = salePrice * royaltiesInBasisPoints;
+            // Fuzz prices that does not generate overflow, otherwise royaltyInfo will revert
+            vm.assume(salePrice == 0 || salePriceTimesRoyalties / salePrice == basisPoints);
+        }
+
+        vm.prank(defaultAccount.owner);
+        collectNFT.setRoyalty(royaltiesInBasisPoints);
+
+        (address receiver, uint256 royalties) = collectNFT.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, defaultAccount.owner);
+        assertEq(royalties, salePriceTimesRoyalties / basisPoints);
+    }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Negatives
+    //////////////////////////////////////////////////////////
+
+    function testCannotSetRoyaltiesIf_NotOwnerOfProfileAuthoringCollectedPublication(
+        address nonCollectionOwner,
+        uint256 royaltiesInBasisPoints
+    ) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        vm.assume(nonCollectionOwner != defaultAccount.owner);
+
+        vm.prank(nonCollectionOwner);
+        vm.expectRevert(Errors.NotProfileOwner.selector);
+        collectNFT.setRoyalty(royaltiesInBasisPoints);
+    }
+
+    function testCannotSetRoyaltiesIf_ExceedsBasisPoints(uint256 royaltiesInBasisPoints) public {
+        uint256 basisPoints = 10000;
+        vm.assume(royaltiesInBasisPoints > basisPoints);
+
+        vm.prank(defaultAccount.owner);
+        vm.expectRevert(Errors.InvalidParameter.selector);
+        collectNFT.setRoyalty(royaltiesInBasisPoints);
+    }
+}

test/FollowNFTTest.t.sol

@@ -1229,4 +1229,67 @@ contract FollowNFTTest is BaseTest, ERC721Test {
 
         assertEq(followNFT.getFollowApproved(followTokenId), 0);
     }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Scenarios
+    //////////////////////////////////////////////////////////
+
+    function testSupportsErc2981Interface() public {
+        assertTrue(followNFT.supportsInterface(bytes4(keccak256('royaltyInfo(uint256,uint256)'))));
+    }
+
+    function testDefaultRoyaltiesAreSetTo10Percent(uint256 tokenId) public {
+        uint256 salePrice = 100;
+        uint256 expectedRoyalties = 10;
+
+        (address receiver, uint256 royalties) = followNFT.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, targetProfileOwner);
+        assertEq(royalties, expectedRoyalties);
+    }
+
+    function testSetRoyalties(uint256 royaltiesInBasisPoints, uint256 tokenId, uint256 salePrice) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        uint256 salePriceTimesRoyalties;
+        unchecked {
+            salePriceTimesRoyalties = salePrice * royaltiesInBasisPoints;
+            // Fuzz prices that does not generate overflow, otherwise royaltyInfo will revert
+            vm.assume(salePrice == 0 || salePriceTimesRoyalties / salePrice == basisPoints);
+        }
+
+        vm.prank(targetProfileOwner);
+        followNFT.setRoyalty(royaltiesInBasisPoints);
+
+        (address receiver, uint256 royalties) = followNFT.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, targetProfileOwner);
+        assertEq(royalties, salePriceTimesRoyalties / basisPoints);
+    }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Negatives
+    //////////////////////////////////////////////////////////
+
+    function testCannotSetRoyaltiesIf_NotTargetProfileOwner(
+        address nonTargetProfileOwner,
+        uint256 royaltiesInBasisPoints
+    ) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        vm.assume(nonTargetProfileOwner != targetProfileOwner);
+
+        vm.prank(nonTargetProfileOwner);
+        vm.expectRevert(Errors.NotProfileOwner.selector);
+        followNFT.setRoyalty(royaltiesInBasisPoints);
+    }
+
+    function testCannotSetRoyaltiesIf_ExceedsBasisPoints(uint256 royaltiesInBasisPoints) public {
+        uint256 basisPoints = 10000;
+        vm.assume(royaltiesInBasisPoints > basisPoints);
+
+        vm.prank(targetProfileOwner);
+        vm.expectRevert(Errors.InvalidParameter.selector);
+        followNFT.setRoyalty(royaltiesInBasisPoints);
+    }
 }

test/GovernanceFunctions.t.sol

@@ -14,8 +14,7 @@ contract GovernanceFunctionsTest is BaseTest {
     function testCannot_SetGovernance_IfNotGovernance(address nonGovernanceCaller, address randomAddress) public {
         vm.assume(nonGovernanceCaller != governance);
         vm.assume(nonGovernanceCaller != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceCaller != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceCaller));
 
         vm.expectRevert(Errors.NotGovernance.selector);
         vm.prank(nonGovernanceCaller);
@@ -25,8 +24,7 @@ contract GovernanceFunctionsTest is BaseTest {
     function testCannot_SetEmergencyAdmin_IfNotGovernance(address nonGovernanceCaller, address randomAddress) public {
         vm.assume(nonGovernanceCaller != governance);
         vm.assume(nonGovernanceCaller != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceCaller != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceCaller));
 
         vm.expectRevert(Errors.NotGovernance.selector);
         vm.prank(nonGovernanceCaller);
@@ -39,8 +37,7 @@ contract GovernanceFunctionsTest is BaseTest {
     ) public {
         vm.assume(nonGovernanceOrEmergencyAdmin != governance);
         vm.assume(nonGovernanceOrEmergencyAdmin != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceOrEmergencyAdmin != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceOrEmergencyAdmin));
 
         state = uint8(bound(state, uint8(Types.ProtocolState.Unpaused), uint8(Types.ProtocolState.Paused)));
 
@@ -94,8 +91,7 @@ contract GovernanceFunctionsTest is BaseTest {
     ) public {
         vm.assume(nonGovernanceCaller != governance);
         vm.assume(nonGovernanceCaller != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceCaller != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceCaller));
 
         vm.expectRevert(Errors.NotGovernance.selector);
         vm.prank(nonGovernanceCaller);
@@ -109,8 +105,7 @@ contract GovernanceFunctionsTest is BaseTest {
     ) public {
         vm.assume(nonGovernanceCaller != governance);
         vm.assume(nonGovernanceCaller != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceCaller != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceCaller));
 
         vm.expectRevert(Errors.NotGovernance.selector);
         vm.prank(nonGovernanceCaller);
@@ -124,8 +119,7 @@ contract GovernanceFunctionsTest is BaseTest {
     ) public {
         vm.assume(nonGovernanceCaller != governance);
         vm.assume(nonGovernanceCaller != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceCaller != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceCaller));
 
         vm.expectRevert(Errors.NotGovernance.selector);
         vm.prank(nonGovernanceCaller);
@@ -139,8 +133,7 @@ contract GovernanceFunctionsTest is BaseTest {
     ) public {
         vm.assume(nonGovernanceCaller != governance);
         vm.assume(nonGovernanceCaller != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
-        vm.assume(nonGovernanceCaller != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceCaller));
 
         vm.expectRevert(Errors.NotGovernance.selector);
         vm.prank(nonGovernanceCaller);

test/LegacyCollectNFTTest.t.sol

@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.13;
+
+import 'test/base/BaseTest.t.sol';
+import 'test/ERC721Test.t.sol';
+import {LegacyCollectNFT} from 'contracts/misc/LegacyCollectNFT.sol';
+import {MockDeprecatedCollectModule} from 'test/mocks/MockDeprecatedCollectModule.sol';
+
+contract LegacyCollectNFTTest is BaseTest, ERC721Test {
+    using stdJson for string;
+
+    function testLegacyCollectNFTTest() public {
+        // Prevents being counted in Foundry Coverage
+    }
+
+    Types.CollectParams defaultCollectParams;
+    address mockDeprecatedCollectModule;
+    LegacyCollectNFT collectNFT;
+    address collectNFTImpl;
+
+    function setUp() public override {
+        super.setUp();
+
+        mockDeprecatedCollectModule = address(new MockDeprecatedCollectModule());
+
+        // Create a V1 pub
+        vm.prank(defaultAccount.owner);
+        uint256 pubId = hub.post(_getDefaultPostParams());
+
+        _toLegacyV1Pub(defaultAccount.profileId, pubId, address(0), mockDeprecatedCollectModule);
+
+        defaultCollectParams = Types.CollectParams({
+            publicationCollectedProfileId: defaultAccount.profileId,
+            publicationCollectedId: pubId,
+            collectorProfileId: defaultAccount.profileId,
+            referrerProfileId: 0,
+            referrerPubId: 0,
+            collectModuleData: abi.encode(true)
+        });
+
+        vm.prank(defaultAccount.owner);
+        hub.collect(defaultCollectParams);
+
+        collectNFT = LegacyCollectNFT(hub.getPub(defaultAccount.profileId, pubId).__DEPRECATED__collectNFT);
+    }
+
+    function _mintERC721(address to) internal virtual override returns (uint256) {
+        defaultCollectParams.collectorProfileId = _createProfile(to);
+        vm.prank(to);
+        uint256 tokenId = hub.collect(defaultCollectParams);
+        return tokenId;
+    }
+
+    function _burnERC721(uint256 tokenId) internal virtual override {
+        collectNFT.burn(tokenId);
+    }
+
+    function _getERC721TokenAddress() internal view virtual override returns (address) {
+        return address(collectNFT);
+    }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Scenarios
+    //////////////////////////////////////////////////////////
+
+    function testSupportsErc2981Interface() public {
+        assertTrue(collectNFT.supportsInterface(bytes4(keccak256('royaltyInfo(uint256,uint256)'))));
+    }
+
+    function testDefaultRoyaltiesAreSetTo10Percent(uint256 tokenId) public {
+        uint256 salePrice = 100;
+        uint256 expectedRoyalties = 10;
+
+        (address receiver, uint256 royalties) = collectNFT.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, defaultAccount.owner);
+        assertEq(royalties, expectedRoyalties);
+    }
+
+    function testSetRoyalties(uint256 royaltiesInBasisPoints, uint256 tokenId, uint256 salePrice) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        uint256 salePriceTimesRoyalties;
+        unchecked {
+            salePriceTimesRoyalties = salePrice * royaltiesInBasisPoints;
+            // Fuzz prices that does not generate overflow, otherwise royaltyInfo will revert
+            vm.assume(salePrice == 0 || salePriceTimesRoyalties / salePrice == basisPoints);
+        }
+
+        vm.prank(defaultAccount.owner);
+        collectNFT.setRoyalty(royaltiesInBasisPoints);
+
+        (address receiver, uint256 royalties) = collectNFT.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, defaultAccount.owner);
+        assertEq(royalties, salePriceTimesRoyalties / basisPoints);
+    }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Negatives
+    //////////////////////////////////////////////////////////
+
+    function testCannotSetRoyaltiesIf_NotOwnerOfProfileAuthoringCollectedPublication(
+        address nonCollectionOwner,
+        uint256 royaltiesInBasisPoints
+    ) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        vm.assume(nonCollectionOwner != defaultAccount.owner);
+
+        vm.prank(nonCollectionOwner);
+        vm.expectRevert(Errors.NotProfileOwner.selector);
+        collectNFT.setRoyalty(royaltiesInBasisPoints);
+    }
+
+    function testCannotSetRoyaltiesIf_ExceedsBasisPoints(uint256 royaltiesInBasisPoints) public {
+        uint256 basisPoints = 10000;
+        vm.assume(royaltiesInBasisPoints > basisPoints);
+
+        vm.prank(defaultAccount.owner);
+        vm.expectRevert(Errors.InvalidParameter.selector);
+        collectNFT.setRoyalty(royaltiesInBasisPoints);
+    }
+}

test/ProfileNFTTest.t.sol

@@ -20,4 +20,65 @@ contract ProfileNFTTest is BaseTest, ERC721Test {
     function _getERC721TokenAddress() internal view virtual override returns (address) {
         return address(hub);
     }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Scenarios
+    //////////////////////////////////////////////////////////
+
+    function testSupportsErc2981Interface() public {
+        assertTrue(hub.supportsInterface(bytes4(keccak256('royaltyInfo(uint256,uint256)'))));
+    }
+
+    function testDefaultRoyaltiesAreSetToZero(uint256 tokenId, uint256 salePrice) public {
+        (address receiver, uint256 royalties) = hub.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, treasury);
+        assertEq(royalties, 0);
+    }
+
+    function testSetRoyalties(uint256 royaltiesInBasisPoints, uint256 tokenId, uint256 salePrice) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        uint256 salePriceTimesRoyalties;
+        unchecked {
+            salePriceTimesRoyalties = salePrice * royaltiesInBasisPoints;
+            // Fuzz prices that does not generate overflow, otherwise royaltyInfo will revert
+            vm.assume(salePrice == 0 || salePriceTimesRoyalties / salePrice == basisPoints);
+        }
+
+        vm.prank(governance);
+        hub.setRoyalty(royaltiesInBasisPoints);
+
+        (address receiver, uint256 royalties) = hub.royaltyInfo(tokenId, salePrice);
+
+        assertEq(receiver, treasury);
+        assertEq(royalties, salePriceTimesRoyalties / basisPoints);
+    }
+
+    //////////////////////////////////////////////////////////
+    // ERC-2981 Royalties - Negatives
+    //////////////////////////////////////////////////////////
+
+    function testCannotSetRoyaltiesIf_NotGovernance(
+        address nonGovernanceAddress,
+        uint256 royaltiesInBasisPoints
+    ) public {
+        uint256 basisPoints = 10000;
+        royaltiesInBasisPoints = bound(royaltiesInBasisPoints, 0, basisPoints);
+        vm.assume(nonGovernanceAddress != governance);
+        vm.assume(!_isLensHubProxyAdmin(nonGovernanceAddress));
+
+        vm.prank(nonGovernanceAddress);
+        vm.expectRevert(Errors.NotGovernance.selector);
+        hub.setRoyalty(royaltiesInBasisPoints);
+    }
+
+    function testCannotSetRoyaltiesIf_ExceedsBasisPoints(uint256 royaltiesInBasisPoints) public {
+        uint256 basisPoints = 10000;
+        vm.assume(royaltiesInBasisPoints > basisPoints);
+
+        vm.prank(governance);
+        vm.expectRevert(Errors.InvalidParameter.selector);
+        hub.setRoyalty(royaltiesInBasisPoints);
+    }
 }

test/base/BaseTest.t.sol

@@ -5,6 +5,7 @@ import 'test/base/TestSetup.t.sol';
 import 'contracts/libraries/constants/Types.sol';
 import {Typehash} from 'contracts/libraries/constants/Typehash.sol';
 import {Strings} from '@openzeppelin/contracts/utils/Strings.sol';
+import {StorageLib} from 'contracts/libraries/StorageLib.sol';
 
 contract BaseTest is TestSetup {
     using Strings for string;
@@ -22,6 +23,11 @@ contract BaseTest is TestSetup {
         return bound(fuzzedUint256, 1, ISSECP256K1_CURVE_ORDER - 1);
     }
 
+    function _isLensHubProxyAdmin(address proxyAdminCandidate) internal view returns (bool) {
+        address proxyAdmin = address(uint160(uint256(vm.load(address(hub), ADMIN_SLOT))));
+        return proxyAdminCandidate == proxyAdmin;
+    }
+
     function _getSetProfileMetadataURITypedDataHash(
         uint256 profileId,
         string memory metadataURI,

test/base/TestSetup.t.sol

@@ -77,6 +77,7 @@ contract TestSetup is Test, ForkManagement, ArrayHelpers {
     LensHandles lensHandles;
     TokenHandleRegistry tokenHandleRegistry;
 
+    // TODO: Avoid constructors in favour of setUp function - Failing asserts in constructor won't make the test fail!
     constructor() {
         if (bytes(forkEnv).length > 0) {
             loadBaseAddresses(forkEnv);
@@ -146,10 +147,12 @@ contract TestSetup is Test, ForkManagement, ArrayHelpers {
         governance = _loadAddressAs('GOVERNANCE');
         treasury = _loadAddressAs('TREASURY');
         modulesGovernance = _loadAddressAs('MODULES_GOVERNANCE');
-        vm.label(proxyAdmin, 'HUB_PROXY_ADMIN');
 
         TREASURY_FEE_BPS = 50;
 
+        moduleGlobals = new ModuleGlobals(modulesGovernance, treasury, TREASURY_FEE_BPS);
+        vm.label(address(moduleGlobals), 'MODULE_GLOBALS');
+
         ///////////////////////////////////////// Start deployments.
         vm.startPrank(deployer);
 
@@ -165,7 +168,7 @@ contract TestSetup is Test, ForkManagement, ArrayHelpers {
         // Deploy implementation contracts.
         // TODO: Last 3 addresses are for the follow modules for migration purposes.
         hubImpl = new LensHubInitializable({
-            moduleGlobals: address(0),
+            moduleGlobals: address(moduleGlobals),
             followNFTImpl: followNFTAddr,
             collectNFTImpl: legacyCollectNFTAddr,
             lensHandlesAddress: lensHandlesProxyAddr,
@@ -219,9 +222,6 @@ contract TestSetup is Test, ForkManagement, ArrayHelpers {
         mockReferenceModule = new MockReferenceModule();
         vm.label(address(mockReferenceModule), 'MOCK_REFERENCE_MODULE');
 
-        moduleGlobals = new ModuleGlobals(modulesGovernance, treasury, TREASURY_FEE_BPS);
-        vm.label(address(moduleGlobals), 'MODULE_GLOBALS');
-
         vm.stopPrank();
         ///////////////////////////////////////// End deployments.
 

test/modules/act/collect/BaseFeeCollectModule.t.sol

@@ -483,9 +483,7 @@ contract BaseFeeCollectModule_ProcessCollect is BaseFeeCollectModuleBase {
             vm.assume(transactionExecutor != address(0));
             vm.assume(collectorProfileOwner != address(0));
             vm.assume(recipient != address(0));
-            bytes32 ADMIN_SLOT = bytes32(uint256(keccak256('eip1967.proxy.admin')) - 1);
-            address proxyAdmin = address(uint160(uint256(vm.load(hubProxyAddr, ADMIN_SLOT))));
-            vm.assume(collectorProfileOwner != proxyAdmin);
+            vm.assume(!_isLensHubProxyAdmin(collectorProfileOwner));
         }
 
         exampleInitData.amount = amount;

test/namespaces/LensHandlesTest.t.sol

@@ -29,8 +29,7 @@ contract LensHandlesTest is BaseTest {
         vm.assume(owner != otherAddress);
         vm.assume(owner != address(0));
         vm.assume(otherAddress != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(lensHandles), ADMIN_SLOT))));
-        vm.assume(otherAddress != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(otherAddress));
 
         string memory handle = 'handle';
 
@@ -51,8 +50,7 @@ contract LensHandlesTest is BaseTest {
         vm.assume(otherAddress != address(hub));
         vm.assume(otherAddress != lensHandles.OWNER());
         vm.assume(!hub.isProfileCreatorWhitelisted(otherAddress));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(lensHandles), ADMIN_SLOT))));
-        vm.assume(otherAddress != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(otherAddress));
 
         string memory handle = 'handle';
 
@@ -266,8 +264,7 @@ contract LensHandlesTest is BaseTest {
 
     function testBurn(address owner) public {
         vm.assume(owner != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(lensHandles), ADMIN_SLOT))));
-        vm.assume(owner != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(owner));
 
         string memory handle = 'handle';
 
@@ -303,8 +300,7 @@ contract LensHandlesTest is BaseTest {
         vm.assume(whitelistedProfileCreator != address(0));
         vm.assume(whitelistedProfileCreator != address(hub));
         vm.assume(whitelistedProfileCreator != lensHandles.OWNER());
-        address proxyAdmin = address(uint160(uint256(vm.load(address(lensHandles), ADMIN_SLOT))));
-        vm.assume(whitelistedProfileCreator != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(whitelistedProfileCreator));
 
         vm.prank(governance);
         hub.whitelistProfileCreator(whitelistedProfileCreator, true);

test/namespaces/TokenHandleRegistryTest.t.sol

@@ -25,8 +25,7 @@ contract TokenHandleRegistryTest is BaseTest {
     function testCannot_MigrationLink_IfNotHub(address otherAddress) public {
         vm.assume(otherAddress != address(hub));
         vm.assume(otherAddress != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(otherAddress != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(otherAddress));
 
         vm.expectRevert(RegistryErrors.OnlyLensHub.selector);
 
@@ -37,8 +36,7 @@ contract TokenHandleRegistryTest is BaseTest {
     function testCannot_Link_IfNotHoldingProfile(address otherAddress) public {
         vm.assume(otherAddress != hub.ownerOf(profileId));
         vm.assume(otherAddress != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(otherAddress != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(otherAddress));
 
         vm.prank(initialHandleHolder);
         lensHandles.transferFrom(initialHandleHolder, otherAddress, handleId);
@@ -52,8 +50,7 @@ contract TokenHandleRegistryTest is BaseTest {
     function testCannot_Link_IfNotHoldingHandle(address otherAddress) public {
         vm.assume(otherAddress != lensHandles.ownerOf(handleId));
         vm.assume(otherAddress != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(otherAddress != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(otherAddress));
 
         vm.prank(initialProfileHolder);
         hub.transferFrom(initialProfileHolder, otherAddress, profileId);
@@ -86,8 +83,7 @@ contract TokenHandleRegistryTest is BaseTest {
         vm.assume(otherAddress != lensHandles.ownerOf(handleId));
         vm.assume(otherAddress != hub.ownerOf(profileId));
         vm.assume(otherAddress != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(otherAddress != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(otherAddress));
 
         vm.prank(address(hub));
         tokenHandleRegistry.migrationLink(handleId, profileId);
@@ -204,8 +200,7 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testFreshLink(address holder) public {
         vm.assume(holder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(holder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(holder));
 
         vm.prank(initialHandleHolder);
         lensHandles.transferFrom(initialHandleHolder, holder, handleId);
@@ -228,13 +223,12 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testLink_AfterHandleWasMoved(address firstHolder, address newHolder) public {
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -275,13 +269,12 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testLink_AfterProfileWasMoved(address firstHolder, address newHolder) public {
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -327,14 +320,13 @@ contract TokenHandleRegistryTest is BaseTest {
         address thirdHolder = makeAddr('THIRD_HOLDER');
 
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
         vm.assume(firstHolder != thirdHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -392,8 +384,7 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink(address holder) public {
         vm.assume(holder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(holder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(holder));
         vm.assume(holder != initialProfileHolder);
         vm.assume(holder != initialHandleHolder);
 
@@ -424,13 +415,12 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_ByProfileOwner_IfHandleWasMoved(address firstHolder, address newHolder) public {
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -463,13 +453,12 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_ByNewHandleOwner_IfHandleWasMoved(address firstHolder, address newHolder) public {
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -502,13 +491,12 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_ByHandleOwner_IfProfileWasMoved(address firstHolder, address newHolder) public {
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -541,13 +529,12 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_ByNewProfileOwner_IfProfileWasMoved(address firstHolder, address newHolder) public {
         vm.assume(firstHolder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(firstHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(firstHolder));
         vm.assume(firstHolder != initialProfileHolder);
         vm.assume(firstHolder != initialHandleHolder);
 
         vm.assume(newHolder != address(0));
-        vm.assume(newHolder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(newHolder));
         vm.assume(newHolder != initialProfileHolder);
         vm.assume(newHolder != initialHandleHolder);
 
@@ -580,8 +567,7 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_IfHandleWasBurned(address holder) public {
         vm.assume(holder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(holder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(holder));
         vm.assume(holder != initialProfileHolder);
         vm.assume(holder != initialHandleHolder);
 
@@ -617,8 +603,7 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_IfProfileWasBurned(address holder) public {
         vm.assume(holder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(holder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(holder));
         vm.assume(holder != initialProfileHolder);
         vm.assume(holder != initialHandleHolder);
 
@@ -654,8 +639,7 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_IfHandleWasBurned_CalledByNotOwner(address holder) public {
         vm.assume(holder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(holder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(holder));
         vm.assume(holder != initialProfileHolder);
         vm.assume(holder != initialHandleHolder);
 
@@ -692,8 +676,7 @@ contract TokenHandleRegistryTest is BaseTest {
 
     function testUnlink_IfProfileWasBurned_CalledByNotOwner(address holder) public {
         vm.assume(holder != address(0));
-        address proxyAdmin = address(uint160(uint256(vm.load(address(tokenHandleRegistry), ADMIN_SLOT))));
-        vm.assume(holder != proxyAdmin);
+        vm.assume(!_isLensHubProxyAdmin(holder));
         vm.assume(holder != initialProfileHolder);
         vm.assume(holder != initialHandleHolder);