diff --git a/contracts/LensHub.sol b/contracts/LensHub.sol index 32e0230..bf0c572 100644 --- a/contracts/LensHub.sol +++ b/contracts/LensHub.sol @@ -126,8 +126,8 @@ contract LensHub is LensBaseERC721, VersionedInitializable, LensMultiState, Lens } /// @inheritdoc ILensHub - function whitelistActionModuleId(address actionModule, uint256 whitelistId) external override onlyGov { - GovernanceLib.whitelistActionModuleId(actionModule, whitelistId); + function whitelistActionModule(address actionModule, bool whitelist) external override onlyGov { + GovernanceLib.whitelistActionModule(actionModule, whitelist); } /////////////////////////////////////////// @@ -606,8 +606,10 @@ contract LensHub is LensBaseERC721, VersionedInitializable, LensMultiState, Lens } /// @inheritdoc ILensHub - function isActionModuleWhitelisted(address actionModule) external view override returns (bool) { - return _actionModuleWhitelistedId[actionModule] > 0; + function getActionModuleWhitelistData( + address actionModule + ) external view override returns (Types.ActionModuleWhitelistData memory) { + return _actionModuleWhitelistData[actionModule]; } /// @inheritdoc ILensHub @@ -736,7 +738,7 @@ contract LensHub is LensBaseERC721, VersionedInitializable, LensMultiState, Lens return COLLECT_NFT_IMPL; } - function getWhitelistedActionModuleById(uint256 id) external view returns (address) { + function getActionModuleById(uint256 id) external view override returns (address) { return _actionModuleById[id]; } diff --git a/contracts/base/LensHubStorage.sol b/contracts/base/LensHubStorage.sol index ea1eab4..8607364 100644 --- a/contracts/base/LensHubStorage.sol +++ b/contracts/base/LensHubStorage.sol @@ -16,11 +16,10 @@ abstract contract LensHubStorage { mapping(address => bool) internal _profileCreatorWhitelisted; // Slot 13 mapping(address => bool) internal _followModuleWhitelisted; // Slot 14 - // `_collectModuleWhitelisted` slot replaced by `_actionModuleWhitelistedId` in Lens V2. - // We will need to unwhitelist all the old modules before V2 upgrade so they don't conflict with the new bitmap. - // e.g. any address that was True as bool will be mapped to a new ID of 1. + // `_collectModuleWhitelisted` slot replaced by `_actionModuleWhitelistData` in Lens V2. + // All the old modules need to be unwhitelisted before V2 upgrade to avoid dirty storage. // mapping(address => bool) internal __DEPRECATED__collectModuleWhitelisted; - mapping(address => uint256) internal _actionModuleWhitelistedId; // Slot 15 + mapping(address => Types.ActionModuleWhitelistData) internal _actionModuleWhitelistData; // Slot 15 mapping(address => bool) internal _referenceModuleWhitelisted; // Slot 16 @@ -40,4 +39,5 @@ abstract contract LensHubStorage { mapping(uint256 => Types.DelegatedExecutorsConfig) internal _delegatedExecutorsConfigByProfileId; // Slot 25 mapping(uint256 => mapping(uint256 => bool)) internal _blockedStatus; // Slot 26, _blockedStatus[byProfile][profile] mapping(uint256 id => address actionModule) internal _actionModuleById; // Slot 27 + uint256 _maxActionModuleIdUsed; // Slot 28 } diff --git a/contracts/interfaces/ILensHub.sol b/contracts/interfaces/ILensHub.sol index 82c5c3d..1146c71 100644 --- a/contracts/interfaces/ILensHub.sol +++ b/contracts/interfaces/ILensHub.sol @@ -80,9 +80,9 @@ interface ILensHub { * @custom:permissions Governance. * * @param actionModule The action module contract address to add or remove from the whitelist. - * @param whitelistId The whitelist ID to set for the action module (0 if not whitelisted). + * @param whitelist True if the action module should be whitelisted, false if it should be unwhitelisted. */ - function whitelistActionModuleId(address actionModule, uint256 whitelistId) external; + function whitelistActionModule(address actionModule, bool whitelist) external; /** * @notice Creates a profile with the specified parameters, minting a Profile NFT to the given recipient. @@ -457,13 +457,16 @@ interface ILensHub { function isReferenceModuleWhitelisted(address referenceModule) external view returns (bool); /** - * @notice Returns whether or not an action module is whitelisted. + * @notice Returns whether or not an action module is whitelisted, and its ID assigned. + * @dev If the ID is zero, it means the module has never been whitelisted, so no ID assigned to it yet. * - * @param actionModule The address of the action module to check. + * @param actionModule The address of the action module to get whitelist data of. * - * @return bool True if the action module is whitelisted, false otherwise. + * @return ActionModuleWhitelistData The data containing the ID and whitelist status of the given module. */ - function isActionModuleWhitelisted(address actionModule) external view returns (bool); + function getActionModuleWhitelistData( + address actionModule + ) external view returns (Types.ActionModuleWhitelistData memory); /** * @notice Returns the currently configured governance address. @@ -631,8 +634,8 @@ interface ILensHub { /** * @notice Returns the action modules associated with a given publication in a bitmap. * The bitmap is a uint256 where each bit represents an action module: 1 if the publication uses it, and 0 if not. - * You can use getWhitelistedActionModuleById() to get the address of the action module associated with a given bit. - * Or + * You can use getActionModuleById() to get the address of the action module associated with a given bit. + * * * In the future this can be replaced with a getter that allows to query the bitmap by index, if there are more than * 256 action modules. @@ -644,6 +647,15 @@ interface ILensHub { */ function getActionModulesBitmap(uint256 profileId, uint256 pubId) external view returns (uint256); + /** + * @notice Returns the address of the action module associated with the given whitelist ID, address(0) if none. + * + * @param id The ID of the module whose address wants to be queried. + * + * @return address The address of the action module associated with the given ID. + */ + function getActionModuleById(uint256 id) external view returns (address); + /** * @notice Returns the publication (profileId & pubId) that a given publication is pointing to. * This is used to implement the "reference" feature of the platform and is used in: diff --git a/contracts/libraries/ActionLib.sol b/contracts/libraries/ActionLib.sol index 42b88f2..c366490 100644 --- a/contracts/libraries/ActionLib.sol +++ b/contracts/libraries/ActionLib.sol @@ -9,8 +9,6 @@ import {IPublicationActionModule} from 'contracts/interfaces/IPublicationActionM import {Errors} from 'contracts/libraries/constants/Errors.sol'; import {Events} from 'contracts/libraries/constants/Events.sol'; -import 'forge-std/console.sol'; - library ActionLib { function act( Types.PublicationActionParams calldata publicationActionParams, @@ -32,7 +30,7 @@ library ActionLib { ); address actionModuleAddress = publicationActionParams.actionModuleAddress; - uint256 actionModuleId = StorageLib.actionModuleWhitelistedId()[actionModuleAddress]; + uint256 actionModuleId = StorageLib.actionModuleWhitelistData()[actionModuleAddress].id; if (actionModuleId == 0) { revert Errors.ActionNotAllowed(); diff --git a/contracts/libraries/GovernanceLib.sol b/contracts/libraries/GovernanceLib.sol index 5004ee1..7e8d4e3 100644 --- a/contracts/libraries/GovernanceLib.sol +++ b/contracts/libraries/GovernanceLib.sol @@ -81,9 +81,27 @@ library GovernanceLib { emit Events.ReferenceModuleWhitelisted(referenceModule, whitelist, block.timestamp); } - function whitelistActionModuleId(address actionModule, uint256 whitelistId) external { - StorageLib.actionModuleWhitelistedId()[actionModule] = whitelistId; - StorageLib.actionModuleById()[whitelistId] = actionModule; - emit Events.ActionModuleWhitelistedId(actionModule, whitelistId, block.timestamp); + function whitelistActionModule(address actionModule, bool whitelist) external { + Types.ActionModuleWhitelistData memory actionModuleWhitelistData = StorageLib.actionModuleWhitelistData()[ + actionModule + ]; + + uint256 id; + if (actionModuleWhitelistData.id == 0) { + if (!whitelist) { + revert('ModuleNotWhitelisted'); + } + id = StorageLib.incrementMaxActionModuleIdUsed(); + + StorageLib.actionModuleWhitelistData()[actionModule] = Types.ActionModuleWhitelistData( + uint248(id), + whitelist + ); + StorageLib.actionModuleById()[id] = actionModule; + } else { + StorageLib.actionModuleWhitelistData()[actionModule].isWhitelisted = whitelist; + id = actionModuleWhitelistData.id; + } + emit Events.ActionModuleWhitelisted(actionModule, id, whitelist, block.timestamp); } } diff --git a/contracts/libraries/PublicationLib.sol b/contracts/libraries/PublicationLib.sol index 5ee0839..a61c500 100644 --- a/contracts/libraries/PublicationLib.sol +++ b/contracts/libraries/PublicationLib.sol @@ -417,12 +417,15 @@ library PublicationLib { uint256 i; while (i < actionModules.length) { - uint256 actionModuleId = StorageLib.actionModuleWhitelistedId()[actionModules[i]]; - if (actionModuleId == 0) { + Types.ActionModuleWhitelistData memory actionModuleWhitelistData = StorageLib.actionModuleWhitelistData()[ + actionModules[i] + ]; + + if (!actionModuleWhitelistData.isWhitelisted) { revert Errors.ActionModuleNotWhitelisted(); } - actionModuleBitmap |= 1 << (actionModuleId - 1); + actionModuleBitmap |= 1 << (actionModuleWhitelistData.id - 1); actionModuleInitResults[i] = IPublicationActionModule(actionModules[i]).initializePublicationAction( profileId, diff --git a/contracts/libraries/StorageLib.sol b/contracts/libraries/StorageLib.sol index dcb1857..75e559e 100644 --- a/contracts/libraries/StorageLib.sol +++ b/contracts/libraries/StorageLib.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.15; import {Types} from 'contracts/libraries/constants/Types.sol'; +import {Errors} from 'contracts/libraries/constants/Errors.sol'; library StorageLib { uint256 constant NAME_SLOT = 0; @@ -13,7 +14,7 @@ library StorageLib { uint256 constant PROTOCOL_STATE_SLOT = 12; uint256 constant PROFILE_CREATOR_WHITELIST_MAPPING_SLOT = 13; uint256 constant FOLLOW_MODULE_WHITELIST_MAPPING_SLOT = 14; - uint256 constant ACTION_MODULE_WHITELIST_ID_MAPPING_SLOT = 15; + uint256 constant ACTION_MODULE_WHITELIST_DATA_MAPPING_SLOT = 15; uint256 constant REFERENCE_MODULE_WHITELIST_MAPPING_SLOT = 16; // Slot 17 is deprecated in Lens V2. In V1 it was used for the dispatcher address by profile ID. uint256 constant PROFILE_ID_BY_HANDLE_HASH_MAPPING_SLOT = 18; @@ -27,6 +28,9 @@ library StorageLib { uint256 constant DELEGATED_EXECUTOR_CONFIG_MAPPING_SLOT = 25; uint256 constant BLOCKED_STATUS_MAPPING_SLOT = 26; uint256 constant ACTION_MODULE_BY_ID_SLOT = 27; + uint256 constant MAX_ACTION_MODULE_ID_USED_SLOT = 28; + + uint256 constant MAX_ACTION_MODULE_ID_SUPPORTED = 255; function getPublication( uint256 profileId, @@ -113,13 +117,13 @@ library StorageLib { } } - function actionModuleWhitelistedId() + function actionModuleWhitelistData() internal pure - returns (mapping(address => uint256) storage _actionModuleWhitelistedId) + returns (mapping(address => Types.ActionModuleWhitelistData) storage _actionModuleWhitelistData) { assembly { - _actionModuleWhitelistedId.slot := ACTION_MODULE_WHITELIST_ID_MAPPING_SLOT + _actionModuleWhitelistData.slot := ACTION_MODULE_WHITELIST_DATA_MAPPING_SLOT } } @@ -129,6 +133,18 @@ library StorageLib { } } + function incrementMaxActionModuleIdUsed() internal returns (uint256) { + uint256 incrementedId; + assembly { + incrementedId := add(sload(MAX_ACTION_MODULE_ID_USED_SLOT), 1) + sstore(MAX_ACTION_MODULE_ID_USED_SLOT, incrementedId) + } + if (incrementedId > MAX_ACTION_MODULE_ID_SUPPORTED) { + revert Errors.MaxActionModuleIdReached(); + } + return incrementedId; + } + function referenceModuleWhitelisted() internal pure diff --git a/contracts/libraries/ValidationLib.sol b/contracts/libraries/ValidationLib.sol index 124d57f..efe5719 100644 --- a/contracts/libraries/ValidationLib.sol +++ b/contracts/libraries/ValidationLib.sol @@ -48,12 +48,6 @@ library ValidationLib { } } - function validateCollectModuleWhitelisted(address collectModule) internal view { - if (StorageLib.actionModuleWhitelistedId()[collectModule] == 0) { - revert Errors.CollectModuleNotWhitelisted(); - } - } - function validateReferenceModuleWhitelisted(address referenceModule) internal view { if (!StorageLib.referenceModuleWhitelisted()[referenceModule]) { revert Errors.ReferenceModuleNotWhitelisted(); diff --git a/contracts/libraries/constants/Errors.sol b/contracts/libraries/constants/Errors.sol index 2356081..44d534f 100644 --- a/contracts/libraries/constants/Errors.sol +++ b/contracts/libraries/constants/Errors.sol @@ -67,6 +67,9 @@ library Errors { error InvalidPointedPub(); error ActionModuleNotWhitelisted(); + // Internal Errors + error MaxActionModuleIdReached(); // This means we need an upgrade + // Module Errors error InitParamsInvalid(); error FollowInvalid(); diff --git a/contracts/libraries/constants/Events.sol b/contracts/libraries/constants/Events.sol index 594648e..7e3ecdf 100644 --- a/contracts/libraries/constants/Events.sol +++ b/contracts/libraries/constants/Events.sol @@ -92,10 +92,16 @@ library Events { * @dev Emitted when a action module is added to or removed from the whitelist. * * @param actionModule The address of the action module. - * @param whitelistedId Id of the whitelisted action module (0 if not whitelisted). + * @param id Id of the whitelisted action module. + * @param whitelisted Whether or not the action module is being added to the whitelist. * @param timestamp The current block timestamp. */ - event ActionModuleWhitelistedId(address indexed actionModule, uint256 indexed whitelistedId, uint256 timestamp); + event ActionModuleWhitelisted( + address indexed actionModule, + uint256 indexed id, + bool indexed whitelisted, + uint256 timestamp + ); /** * @dev Emitted when a profile is created. diff --git a/contracts/libraries/constants/Types.sol b/contracts/libraries/constants/Types.sol index abffa18..1c83f0e 100644 --- a/contracts/libraries/constants/Types.sol +++ b/contracts/libraries/constants/Types.sol @@ -371,4 +371,9 @@ library Types { uint64 prevConfigNumber; uint64 maxConfigNumberSet; } + + struct ActionModuleWhitelistData { + uint248 id; + bool isWhitelisted; + } } diff --git a/contracts/misc/LensV2UpgradeContract.sol b/contracts/misc/LensV2UpgradeContract.sol index 17f79a5..2f8f968 100644 --- a/contracts/misc/LensV2UpgradeContract.sol +++ b/contracts/misc/LensV2UpgradeContract.sol @@ -120,8 +120,7 @@ contract LensV2UpgradeContract is ImmutableOwnable { uint256 newActionModulesToWhitelistLength = newActionModulesToWhitelist.length; uint256 i; while (i < newActionModulesToWhitelistLength) { - uint256 moduleId = i + 1; // Starting from 1 - GOVERNANCE.lensHub_whitelistActionModuleId(newActionModulesToWhitelist[i], moduleId); + GOVERNANCE.lensHub_whitelistActionModule(newActionModulesToWhitelist[i], true); unchecked { ++i; } diff --git a/contracts/misc/ModuleWhitelistProxy.sol b/contracts/misc/ModuleWhitelistProxy.sol deleted file mode 100644 index 2f6b6cf..0000000 --- a/contracts/misc/ModuleWhitelistProxy.sol +++ /dev/null @@ -1,25 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.15; - -import {ILensHub} from 'contracts/interfaces/ILensHub.sol'; -import {ImmutableOwnable} from 'contracts/misc/ImmutableOwnable.sol'; - -/** - * @title ModuleWhitelistProxy - * @author Lens Protocol - * - * @notice This is an ownable proxy contract that enforces ".lens" handle suffixes at profile creation. - * Only the owner can create profiles. - */ -contract ModuleWhitelistProxy is ImmutableOwnable { - uint256 latestId; - - constructor(address owner, address hub) ImmutableOwnable(owner, hub) {} - - function proxyWhitelistActionModule(address actionModule) external onlyOwner returns (uint256) { - ++latestId; - ILensHub(LENS_HUB).whitelistActionModuleId(actionModule, latestId); - return latestId; - } -} diff --git a/contracts/misc/access/Governance.sol b/contracts/misc/access/Governance.sol index 854d776..5f82ccd 100644 --- a/contracts/misc/access/Governance.sol +++ b/contracts/misc/access/Governance.sol @@ -53,11 +53,11 @@ contract Governance is ControllableByContract { LENS_HUB.whitelistReferenceModule(referenceModule, whitelist); } - function lensHub_whitelistActionModuleId( + function lensHub_whitelistActionModule( address actionModule, - uint256 whitelistId + bool whitelist ) external onlyOwnerOrControllerContract { - LENS_HUB.whitelistActionModuleId(actionModule, whitelistId); + LENS_HUB.whitelistActionModule(actionModule, whitelist); } // Interface to the Deprecated LensHub V1 to unwhitelist collect modules diff --git a/test/foundry/base/TestSetup.t.sol b/test/foundry/base/TestSetup.t.sol index 0c9e8a8..9a4172f 100644 --- a/test/foundry/base/TestSetup.t.sol +++ b/test/foundry/base/TestSetup.t.sol @@ -160,7 +160,7 @@ contract TestSetup is Test, ForkManagement, ArrayHelpers { vm.startPrank(governance); // Whitelist the MockActionModule. - hub.whitelistActionModuleId(address(mockActionModule), 1); + hub.whitelistActionModule(address(mockActionModule), true); // Whitelist the MockReferenceModule. hub.whitelistReferenceModule(address(mockReferenceModule), true);