fix: CollectingTest refactored and passing

test/foundry/CollectingTest.t.sol

@@ -4,10 +4,11 @@ pragma solidity ^0.8.13;
 import './base/BaseTest.t.sol';
 import './helpers/SignatureHelpers.sol';
 import './helpers/CollectingHelpers.sol';
+import './MetaTxNegatives.t.sol';
 
 // TODO add check for _initialize() called for fork tests - check name and symbol set
 
-contract CollectingTest_Base is BaseTest, CollectingHelpers, SigSetup {
+contract CollectingTest is BaseTest, CollectingHelpers, SigSetup {
     uint256 constant collectorProfileOwnerPk = 0xC011EEC7012;
     address collectorProfileOwner;
     uint256 collectorProfileId;
@@ -15,30 +16,6 @@ contract CollectingTest_Base is BaseTest, CollectingHelpers, SigSetup {
     uint256 constant userWithoutProfilePk = 0x105312;
     address userWithoutProfile;
 
-    function _mockCollect() internal virtual returns (uint256) {
-        return
-            _collect(
-                mockCollectParams.collectorProfileId,
-                mockCollectParams.publicationCollectedProfileId,
-                mockCollectParams.publicationCollectedId,
-                mockCollectParams.collectModuleData
-            );
-    }
-
-    function _mockCollectWithSig(address delegatedSigner, uint256 signerPrivKey)
-        internal
-        virtual
-        returns (uint256)
-    {
-        bytes32 digest = _getCollectTypedDataHash(mockCollectParams, nonce, deadline);
-
-        return
-            _collectWithSig(
-                mockCollectParams,
-                _getSigStruct(delegatedSigner, signerPrivKey, digest, deadline)
-            );
-    }
-
     function setUp() public virtual override(SigSetup, TestSetup) {
         TestSetup.setUp();
         SigSetup.setUp();
@@ -53,11 +30,14 @@ contract CollectingTest_Base is BaseTest, CollectingHelpers, SigSetup {
 
         mockCollectParams.collectorProfileId = collectorProfileId;
     }
-}
 
-contract CollectingTest_Generic is CollectingTest_Base {
-    function setUp() public override {
-        CollectingTest_Base.setUp();
+    function _collect(
+        uint256, /* metaTxSignerPk */
+        address transactionExecutor,
+        DataTypes.CollectParams memory collectParams
+    ) internal virtual returns (uint256) {
+        vm.prank(transactionExecutor);
+        return hub.collect(collectParams);
     }
 
     // NEGATIVES
@@ -65,8 +45,7 @@ contract CollectingTest_Generic is CollectingTest_Base {
     // Also acts like a test for cannot collect specifying another (non-owned) profile as a parameter
     function testCannotCollectIfNotExecutor() public {
         vm.expectRevert(Errors.ExecutorInvalid.selector);
-        vm.startPrank(otherSigner);
-        _mockCollect();
+        _collect(collectorProfileOwnerPk, otherSigner, mockCollectParams);
     }
 
     function testCannotCollectIfNonexistantPub() public {
@@ -80,9 +59,8 @@ contract CollectingTest_Generic is CollectingTest_Base {
             0
         );
 
-        vm.startPrank(collectorProfileOwner);
         vm.expectRevert(Errors.CollectNotAllowed.selector);
-        _mockCollect();
+        _collect(collectorProfileOwnerPk, collectorProfileOwner, mockCollectParams);
         vm.stopPrank();
     }
 
@@ -97,17 +75,15 @@ contract CollectingTest_Generic is CollectingTest_Base {
             0
         );
 
-        vm.startPrank(collectorProfileOwner);
         vm.expectRevert(Errors.CollectNotAllowed.selector);
-        _mockCollect();
+        _collect(collectorProfileOwnerPk, collectorProfileOwner, mockCollectParams);
         vm.stopPrank();
     }
 
     function testCannotCollect_WithoutProfile() public {
         mockCollectParams.collectorProfileId = _getNextProfileId(); // Non-existent profile
-        vm.startPrank(userWithoutProfile);
         vm.expectRevert(Errors.TokenDoesNotExist.selector);
-        _mockCollect();
+        _collect(userWithoutProfilePk, userWithoutProfile, mockCollectParams);
         vm.stopPrank();
     }
 
@@ -115,8 +91,7 @@ contract CollectingTest_Generic is CollectingTest_Base {
         vm.prank(profileOwner);
         hub.setBlockStatus(newProfileId, _toUint256Array(collectorProfileId), _toBoolArray(true));
         vm.expectRevert(Errors.Blocked.selector);
-        vm.startPrank(collectorProfileOwner);
-        _mockCollect();
+        _collect(collectorProfileOwnerPk, collectorProfileOwner, mockCollectParams);
     }
 
     function testCannotCollectMirror() public {
@@ -129,9 +104,8 @@ contract CollectingTest_Generic is CollectingTest_Base {
         // Collecting the mirror
         mockCollectParams.publicationCollectedId = mirrorPubId;
 
-        vm.prank(collectorProfileOwner);
         vm.expectRevert(Errors.CollectNotAllowed.selector);
-        _mockCollect();
+        _collect(collectorProfileOwnerPk, collectorProfileOwner, mockCollectParams);
     }
 
     // SCENARIOS
@@ -139,9 +113,7 @@ contract CollectingTest_Generic is CollectingTest_Base {
     function testCollect() public {
         uint256 startNftId = _checkCollectNFTBefore();
 
-        vm.startPrank(collectorProfileOwner);
-        uint256 nftId = _mockCollect();
-        vm.stopPrank();
+        uint256 nftId = _collect(collectorProfileOwnerPk, collectorProfileOwner, mockCollectParams);
 
         _checkCollectNFTAfter(nftId, startNftId + 1);
     }
@@ -152,8 +124,7 @@ contract CollectingTest_Generic is CollectingTest_Base {
         vm.prank(profileOwner);
         hub.mirror(mockMirrorParams);
 
-        vm.prank(collectorProfileOwner);
-        uint256 nftId = _mockCollect();
+        uint256 nftId = _collect(collectorProfileOwnerPk, collectorProfileOwner, mockCollectParams);
 
         _checkCollectNFTAfter(nftId, startNftId + 1);
     }
@@ -170,9 +141,7 @@ contract CollectingTest_Generic is CollectingTest_Base {
         );
 
         // collect from executor
-        vm.startPrank(otherSigner);
-        uint256 nftId = _mockCollect();
-        vm.stopPrank();
+        uint256 nftId = _collect(otherSignerKey, otherSigner, mockCollectParams);
 
         _checkCollectNFTAfter(nftId, startNftId + 1);
     }
@@ -191,170 +160,58 @@ contract CollectingTest_Generic is CollectingTest_Base {
         );
 
         // collect from executor
-        vm.startPrank(otherSigner);
-        uint256 nftId = _mockCollect();
-        vm.stopPrank();
+        uint256 nftId = _collect(otherSignerKey, otherSigner, mockCollectParams);
 
         _checkCollectNFTAfter(nftId, startNftId + 1);
     }
 }
 
-contract CollectingTest_WithSig is CollectingTest_Base {
-    function setUp() public override {
-        CollectingTest_Base.setUp();
+contract CollectingTestMetaTx is CollectingTest, MetaTxNegatives {
+    mapping(address => uint256) cachedNonceByAddress;
+
+    function setUp() public override(CollectingTest, MetaTxNegatives) {
+        CollectingTest.setUp();
+        MetaTxNegatives.setUp();
+
+        cachedNonceByAddress[collectorProfileOwner] = _getSigNonce(collectorProfileOwner);
     }
 
-    // NEGATIVES
-
-    // Also acts like a test for cannot collect specifying another (non-owned) profile as a parameter
-    function testCannotCollectWithSigIfNotExecutor() public {
-        vm.expectRevert(Errors.ExecutorInvalid.selector);
-        _mockCollectWithSig({delegatedSigner: otherSigner, signerPrivKey: otherSignerKey});
-    }
-
-    function testCannotCollectWithSigIfNonexistantPub() public {
-        mockCollectParams.publicationCollectedId = 2;
-        // Check that the publication doesn't exist.
-        assertEq(
-            _getPub(
-                mockCollectParams.publicationCollectedProfileId,
-                mockCollectParams.publicationCollectedId
-            ).pointedProfileId,
-            0
+    function _collect(
+        uint256 metaTxSignerPk,
+        address transactionExecutor,
+        DataTypes.CollectParams memory collectParams
+    ) internal override returns (uint256) {
+        address signer = vm.addr(metaTxSignerPk);
+        uint256 deadline = type(uint256).max;
+        bytes32 digest = _getCollectTypedDataHash(
+            collectParams,
+            cachedNonceByAddress[signer],
+            deadline
         );
-
-        vm.expectRevert(Errors.PublicationDoesNotExist.selector);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: collectorProfileOwnerPk});
+        return
+            hub.collectWithSig(
+                collectParams,
+                _getSigStruct(transactionExecutor, metaTxSignerPk, digest, deadline)
+            );
     }
 
-    function testCannotCollectWithSigIfZeroPub() public {
-        mockCollectParams.publicationCollectedId = 0;
-        // Check that the publication doesn't exist.
-        assertEq(
-            _getPub(
-                mockCollectParams.publicationCollectedProfileId,
-                mockCollectParams.publicationCollectedId
-            ).pointedProfileId,
-            0
+    function _executeMetaTx(
+        uint256 signerPk,
+        uint256 nonce,
+        uint256 deadline
+    ) internal override {
+        _collectWithSig(
+            mockCollectParams,
+            _getSigStruct(
+                collectorProfileOwner,
+                signerPk,
+                _getCollectTypedDataHash(mockCollectParams, nonce, deadline),
+                deadline
+            )
         );
-
-        vm.expectRevert(Errors.PublicationDoesNotExist.selector);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: collectorProfileOwnerPk});
     }
 
-    function testCannotCollectWithSig_WithoutProfile() public {
-        mockCollectParams.collectorProfileId = _getNextProfileId(); // Non-existent profile
-        vm.expectRevert(Errors.TokenDoesNotExist.selector);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: userWithoutProfilePk});
-    }
-
-    function testCannotCollectWithSigOnExpiredDeadline() public {
-        deadline = block.timestamp - 1;
-        vm.expectRevert(Errors.SignatureExpired.selector);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: collectorProfileOwnerPk});
-    }
-
-    function testCannotCollectWithSigOnInvalidNonce() public {
-        nonce = 5;
-        vm.expectRevert(Errors.SignatureInvalid.selector);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: collectorProfileOwnerPk});
-    }
-
-    function testCannotCollectIfNonceWasIncrementedWithAnotherAction() public {
-        assertEq(_getSigNonce(collectorProfileOwner), nonce, 'Wrong nonce before posting');
-
-        uint256 expectedCollectId = _getCollectCount(
-            collectorProfileId,
-            mockCollectParams.publicationCollectedId
-        ) + 1;
-
-        uint256 nftId = _mockCollectWithSig({
-            delegatedSigner: address(0),
-            signerPrivKey: collectorProfileOwnerPk
-        });
-
-        assertEq(nftId, expectedCollectId, 'Wrong collectId');
-
-        assertTrue(_getSigNonce(collectorProfileOwner) != nonce, 'Wrong nonce after collecting');
-
-        vm.expectRevert(Errors.SignatureInvalid.selector);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: collectorProfileOwnerPk});
-    }
-
-    function testCannotCollectWithSigIfBlocked() public {
-        vm.prank(profileOwner);
-        hub.setBlockStatus(newProfileId, _toUint256Array(collectorProfileId), _toBoolArray(true));
-        vm.expectRevert(Errors.Blocked.selector);
-        vm.startPrank(collectorProfileOwner);
-        _mockCollectWithSig({delegatedSigner: address(0), signerPrivKey: collectorProfileOwnerPk});
-    }
-
-    // SCENARIOS
-
-    function testCollectWithSig() public {
-        uint256 startNftId = _checkCollectNFTBefore();
-
-        uint256 nftId = _mockCollectWithSig({
-            delegatedSigner: address(0),
-            signerPrivKey: collectorProfileOwnerPk
-        });
-
-        _checkCollectNFTAfter(nftId, startNftId + 1);
-    }
-
-    function testCollectWithSigMirror() public {
-        uint256 startNftId = _checkCollectNFTBefore();
-
-        vm.prank(profileOwner);
-        hub.mirror(mockMirrorParams);
-
-        uint256 nftId = _mockCollectWithSig({
-            delegatedSigner: address(0),
-            signerPrivKey: collectorProfileOwnerPk
-        });
-
-        _checkCollectNFTAfter(nftId, startNftId + 1);
-    }
-
-    function testExecutorCollectWithSig() public {
-        uint256 startNftId = _checkCollectNFTBefore();
-
-        // delegate power to executor
-        _changeDelegatedExecutorsConfig(
-            collectorProfileOwner,
-            collectorProfileId,
-            otherSigner,
-            true
-        );
-
-        // collect from executor
-        uint256 nftId = _mockCollectWithSig({
-            delegatedSigner: otherSigner,
-            signerPrivKey: otherSignerKey
-        });
-
-        _checkCollectNFTAfter(nftId, startNftId + 1);
-    }
-
-    function testExecutorCollectWithSigMirror() public {
-        uint256 startNftId = _checkCollectNFTBefore();
-
-        // mirror, then delegate power to executor
-        vm.prank(profileOwner);
-        hub.mirror(mockMirrorParams);
-        _changeDelegatedExecutorsConfig(
-            collectorProfileOwner,
-            collectorProfileId,
-            otherSigner,
-            true
-        );
-
-        // collect from executor
-        uint256 nftId = _mockCollectWithSig({
-            delegatedSigner: otherSigner,
-            signerPrivKey: otherSignerKey
-        });
-
-        _checkCollectNFTAfter(nftId, startNftId + 1);
+    function _getDefaultMetaTxSignerPk() internal pure override returns (uint256) {
+        return collectorProfileOwnerPk;
     }
 }