From db42f3941aa040a5e22f2708b3e40405479f0e88 Mon Sep 17 00:00:00 2001 From: donosonaumczuk Date: Tue, 21 Feb 2023 02:29:07 +0000 Subject: [PATCH] fix: CollectingTest refactored and passing --- test/foundry/CollectingTest.t.sol | 259 +++++++----------------------- 1 file changed, 58 insertions(+), 201 deletions(-) diff --git a/test/foundry/CollectingTest.t.sol b/test/foundry/CollectingTest.t.sol index 51e613c..1f04537 100644 --- a/test/foundry/CollectingTest.t.sol +++ b/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; } }