Merge pull request #72 from lens-protocol/code-changes

Code improvements and fixes

.gitignore

@@ -133,3 +133,5 @@ dist
 deployment-zk/*
 !deployment-zk/lensSepoliaTestnet/
 cache_forge/solidity-files-cache.json
+
+lcov.info

contracts/actions/account/TippingAccountAction.sol

@@ -1,12 +1,13 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {BaseAccountAction} from "contracts/actions/account/base/BaseAccountAction.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {KeyValue} from "contracts/core/types/Types.sol";
 import {MetadataBased} from "contracts/core/base/MetadataBased.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 contract TippingAccountAction is BaseAccountAction, MetadataBased {
     using SafeERC20 for IERC20;
@@ -40,7 +41,7 @@ contract TippingAccountAction is BaseAccountAction, MetadataBased {
                 erc20Token = abi.decode(params[i].value, (address));
             }
         }
-        require(tipAmount > 0);
+        require(tipAmount > 0, Errors.InvalidParameter());
         IERC20(erc20Token).safeTransferFrom(originalMsgSender, account, tipAmount);
         return "";
     }

contracts/actions/account/base/BaseAccountAction.sol

@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {KeyValue} from "contracts/core/types/Types.sol";
 import {BaseAction} from "contracts/actions/base/BaseAction.sol";
 import {IAccountAction} from "contracts/extensions/actions/ActionHub.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 abstract contract BaseAccountAction is BaseAction, IAccountAction {
     constructor(address actionHub) BaseAction(actionHub) {}
@@ -55,6 +56,6 @@ abstract contract BaseAccountAction is BaseAction, IAccountAction {
         bool, /* isDisabled */
         KeyValue[] calldata /* params */
     ) internal virtual returns (bytes memory) {
-        revert();
+        revert Errors.NotImplemented();
     }
 }

contracts/actions/base/BaseAction.sol

@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {UNIVERSAL_ACTION_MAGIC_VALUE} from "contracts/extensions/actions/ActionHub.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 abstract contract BaseAction {
     address immutable ACTION_HUB;
@@ -11,7 +12,7 @@ abstract contract BaseAction {
     bytes32 constant STORAGE__ACTION_CONFIGURED = 0x852bead036b7ef35b8026346140cc688bafe817a6c3491812e6d994b1bcda6d9;
 
     modifier onlyActionHub() {
-        require(msg.sender == ACTION_HUB);
+        require(msg.sender == ACTION_HUB, Errors.InvalidMsgSender());
         _;
     }
 
@@ -24,8 +25,8 @@ abstract contract BaseAction {
         assembly {
             configured := sload(STORAGE__ACTION_CONFIGURED)
         }
-        require(!configured);
-        require(originalMsgSender == address(0));
+        require(!configured, Errors.RedundantStateChange());
+        require(originalMsgSender == address(0), Errors.InvalidParameter());
         assembly {
             sstore(STORAGE__ACTION_CONFIGURED, 1)
         }

contracts/actions/post/TippingPostAction.sol

@@ -1,13 +1,14 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {BasePostAction} from "contracts/actions/post/base/BasePostAction.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {KeyValue} from "contracts/core/types/Types.sol";
 import {IFeed} from "contracts/core/interfaces/IFeed.sol";
 import {MetadataBased} from "contracts/core/base/MetadataBased.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 contract TippingPostAction is BasePostAction, MetadataBased {
     using SafeERC20 for IERC20;
@@ -41,7 +42,7 @@ contract TippingPostAction is BasePostAction, MetadataBased {
                 erc20Token = abi.decode(params[i].value, (address));
             }
         }
-        require(tipAmount > 0);
+        require(tipAmount > 0, Errors.InvalidParameter());
         address account = IFeed(feed).getPostAuthor(postId);
         IERC20(erc20Token).safeTransferFrom(originalMsgSender, account, tipAmount);
         return abi.encode(account);

contracts/actions/post/base/BasePostAction.sol

@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {KeyValue} from "contracts/core/types/Types.sol";
 import {BaseAction} from "contracts/actions/base/BaseAction.sol";
 import {IPostAction} from "contracts/extensions/actions/ActionHub.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 abstract contract BasePostAction is BaseAction, IPostAction {
     constructor(address actionHub) BaseAction(actionHub) {}
@@ -58,6 +59,6 @@ abstract contract BasePostAction is BaseAction, IPostAction {
         bool, /* isDisabled */
         KeyValue[] calldata /* params */
     ) internal virtual returns (bytes memory) {
-        revert();
+        revert Errors.NotImplemented();
     }
 }

contracts/actions/post/collect/IERC7572.sol

@@ -2,7 +2,7 @@
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
 // As appears in https://eips.ethereum.org/EIPS/eip-7572
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 /**
  * This specification standardizes contractURI() to return contract-level metadata. This is useful for dapps and

contracts/actions/post/collect/ISimpleCollectAction.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {IPostAction} from "contracts/extensions/actions/ActionHub.sol";
 

contracts/actions/post/collect/LensCollectedPost.sol

@@ -1,11 +1,12 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import "contracts/core/base/LensERC721.sol";
 import {IERC7572} from "contracts/actions/post/collect/IERC7572.sol";
 import {IFeed} from "contracts/core/interfaces/IFeed.sol";
 import {ITokenURIProvider} from "contracts/core/interfaces/ITokenURIProvider.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 /**
  * @notice A contract that represents a Lens Collected Post.
@@ -24,16 +25,14 @@ contract LensCollectedPost is LensERC721, IERC7572 {
     string internal _contractURI;
     address internal immutable _feed;
     uint256 internal immutable _postId;
-    bool internal immutable _isImmutable; // TODO: This can be replaced with bytes(_contentURISnapshot).length
     address internal immutable _collectAction;
 
     constructor(address feed, uint256 postId, bool isImmutable) {
         LensERC721._initialize("Lens Collected Post", "LCP", ITokenURIProvider(address(0)));
         string memory contentURI = IFeed(feed).getPost(postId).contentURI;
-        require(bytes(contentURI).length > 0, "Post content URI is empty");
+        require(bytes(contentURI).length > 0, Errors.InvalidParameter());
         _feed = feed;
         _postId = postId;
-        _isImmutable = isImmutable;
         _contractURI = contentURI;
         _collectAction = msg.sender;
         if (isImmutable) {
@@ -43,7 +42,7 @@ contract LensCollectedPost is LensERC721, IERC7572 {
     }
 
     function mint(address to, uint256 tokenId) external {
-        require(msg.sender == _collectAction, "Only CollectAction can mint");
+        require(msg.sender == _collectAction, Errors.InvalidMsgSender());
         _mint(to, tokenId);
     }
 
@@ -54,12 +53,12 @@ contract LensCollectedPost is LensERC721, IERC7572 {
     }
 
     function tokenURI(uint256 /*tokenId*/ ) public view override returns (string memory) {
-        if (_isImmutable) {
+        if (bytes(_contentURISnapshot).length > 0) {
             return _contentURISnapshot;
         } else {
             string memory contentURI = IFeed(_feed).getPost(_postId).contentURI;
             // TODO: If content was deleted - should we fail or return empty string?
-            require(bytes(contentURI).length > 0);
+            require(bytes(contentURI).length > 0, Errors.DoesNotExist());
             return contentURI;
         }
     }
@@ -69,6 +68,6 @@ contract LensCollectedPost is LensERC721, IERC7572 {
     // Disabling integrated LensERC721 tokenURIProvider
     // TODO: Is this approach more favorable than deploying the LensCollectedPostTokenURIProvider over and over?
     function _beforeTokenURIProviderSet(ITokenURIProvider /* tokenURIProvider */ ) internal pure override {
-        revert();
+        revert Errors.NotImplemented();
     }
 }

contracts/actions/post/collect/SimpleCollectAction.sol

@@ -1,18 +1,17 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {ISimpleCollectAction, CollectActionData} from "contracts/actions/post/collect/ISimpleCollectAction.sol";
 import {IFeed} from "contracts/core/interfaces/IFeed.sol";
 import {IGraph} from "contracts/core/interfaces/IGraph.sol";
-
 import {LensCollectedPost} from "contracts/actions/post/collect/LensCollectedPost.sol";
 import {BasePostAction} from "contracts/actions/post/base/BasePostAction.sol";
-
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {MetadataBased} from "contracts/core/base/MetadataBased.sol";
 import {KeyValue} from "contracts/core/types/Types.sol";
+import {Errors} from "contracts/core/types/Errors.sol";
 
 contract SimpleCollectAction is ISimpleCollectAction, BasePostAction, MetadataBased {
     using SafeERC20 for IERC20;
@@ -107,7 +106,7 @@ contract SimpleCollectAction is ISimpleCollectAction, BasePostAction, MetadataBa
             // Editing existing collect action config
             if (storedData.isImmutable) {
                 // TODO: Should we have two different bools? isImmutableConfig & isImmutableContentURI?
-                revert("Cannot edit immutable collect");
+                revert Errors.Immutable();
             } else {
                 storedData.amount = configData.amount;
                 storedData.collectLimit = configData.collectLimit;
@@ -152,9 +151,9 @@ contract SimpleCollectAction is ISimpleCollectAction, BasePostAction, MetadataBa
         _validateSenderIsAuthor(originalMsgSender, feed, postId);
         CollectActionData storage storedData = $collectDataStorage().collectData[feed][postId];
         // We don't check for existence of collect before disabling, because it might be useful to disable it initially
-        // require(storedData.collectionAddress != address(0), "Collect not configured for this post");
-        require(!storedData.isImmutable, "Cannot modify immutable collect");
-        require(storedData.isDisabled != isDisabled, "Already in desired state");
+        // require(storedData.collectionAddress != address(0), Errors.DoesNotExist());
+        require(!storedData.isImmutable, Errors.Immutable());
+        require(storedData.isDisabled != isDisabled, Errors.RedundantStateChange());
         storedData.isDisabled = isDisabled;
         return abi.encode(isDisabled);
     }
@@ -165,18 +164,18 @@ contract SimpleCollectAction is ISimpleCollectAction, BasePostAction, MetadataBa
 
     function _validateSenderIsAuthor(address sender, address feed, uint256 postId) internal virtual {
         if (sender != IFeed(feed).getPostAuthor(postId)) {
-            revert("Sender is not the author");
+            revert Errors.InvalidMsgSender();
         }
     }
 
     function _validateConfigureParams(CollectActionConfigureParams memory configData) internal virtual {
         if (configData.amount == 0) {
-            require(configData.token == address(0), "Invalid token");
+            require(configData.token == address(0), Errors.InvalidParameter());
         } else {
-            require(configData.token != address(0), "Invalid token");
+            require(configData.token != address(0), Errors.InvalidParameter());
         }
         if (configData.endTimestamp != 0 && configData.endTimestamp < block.timestamp) {
-            revert("Invalid params");
+            revert Errors.InvalidParameter();
         }
         if (configData.followerOnlyGraph != address(0)) {
             // Check if the Graph supports isFollowing() interface with two random addresses
@@ -209,24 +208,24 @@ contract SimpleCollectAction is ISimpleCollectAction, BasePostAction, MetadataBa
     ) internal virtual {
         CollectActionData storage data = $collectDataStorage().collectData[feed][postId];
 
-        require(data.collectionAddress != address(0), "Collect not configured for this post");
+        require(data.collectionAddress != address(0), Errors.DoesNotExist());
 
         if (data.endTimestamp != 0 && block.timestamp > data.endTimestamp) {
-            revert("Collect expired");
+            revert Errors.Expired();
         }
 
         if (data.collectLimit != 0 && data.currentCollects + 1 > data.collectLimit) {
-            revert("Collect limit exceeded");
+            revert Errors.LimitReached();
         }
 
         if (expectedParams.amount != data.amount || expectedParams.token != data.token) {
-            revert("Invalid expected amount and/or token");
+            revert Errors.InvalidParameter();
         }
 
         if (data.followerOnlyGraph != address(0)) {
             require(
                 IGraph(data.followerOnlyGraph).isFollowing(originalMsgSender, IFeed(feed).getPostAuthor(postId)),
-                "Not following"
+                Errors.NotFollowing()
             );
         }
 
@@ -236,12 +235,12 @@ contract SimpleCollectAction is ISimpleCollectAction, BasePostAction, MetadataBa
             require(
                 keccak256(bytes(contentURI))
                     == keccak256(bytes(LensCollectedPost(data.collectionAddress).tokenURI(data.currentCollects))),
-                "Invalid content URI"
+                Errors.InvalidParameter()
             );
         }
 
         if (data.isDisabled) {
-            revert("Collect is disabled");
+            revert Errors.Disabled();
         }
     }
 

contracts/core/access/AccessControlled.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.26;
 
 import {IAccessControl} from "contracts/core/interfaces/IAccessControl.sol";
 import {AccessControlLib} from "contracts/core/libraries/AccessControlLib.sol";

contracts/core/access/Ownable.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: UNLICENSED
 // Copyright (C) 2024 Lens Labs. All Rights Reserved.
-pragma solidity ^0.8.12;
+pragma solidity ^0.8.26;
+
+import {Errors} from "contracts/core/types/Errors.sol";
 
 abstract contract Ownable {
     event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
@@ -19,7 +21,7 @@ abstract contract Ownable {
     }
 
     modifier onlyOwner() {
-        require(msg.sender == $ownableStorage().owner);
+        require(msg.sender == $ownableStorage().owner, Errors.InvalidMsgSender());
         _;
     }