I add fuzzing to afterTokenTransfers hooks.

defientco · Aug 6, 2023 · b742345 · b742345
1 parent ee6aee3
commit b742345
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 67 deletions.
diff --git a/src/interfaces/IAfterTokenTransfersHook.sol b/src/interfaces/IAfterTokenTransfersHook.sol
@@ -5,12 +5,11 @@ pragma solidity ^0.8.15;
 /// @dev Interface that defines hooks to be executed After token transfers.
 interface IAfterTokenTransfersHook {
     /**
-        @notice Emitted when the after token transfers hook is used.
-        @param from Address from which the tokens are being transferred.
-        @param to Address to which the tokens are being transferred.
-        @param startTokenId The starting ID of the tokens being transferred.
-        @param quantity The number of tokens being transferred.
-    
+     * @notice Emitted when the after token transfers hook is used.
+     * @param from Address from which the tokens are being transferred.
+     * @param to Address to which the tokens are being transferred.
+     * @param startTokenId The starting ID of the tokens being transferred.
+     * @param quantity The number of tokens being transferred.
      */
     event AfterTokenTransfersHookUsed(
         address from,
@@ -20,12 +19,11 @@ interface IAfterTokenTransfersHook {
     );
 
     /**
-        @notice Provides a custom implementation for the token transfers process.
-        @param from Address from which the tokens are being transferred.
-        @param to Address to which the tokens are being transferred.
-        @param startTokenId The starting ID of the tokens being transferred.
-        @param quantity The number of tokens being transferred.
-            
+     * @notice Provides a custom implementation for the token transfers process.
+     * @param from Address from which the tokens are being transferred.
+     * @param to Address to which the tokens are being transferred.
+     * @param startTokenId The starting ID of the tokens being transferred.
+     * @param quantity The number of tokens being transferred.
      */
     function afterTokenTransfersHook(
         address from,

diff --git a/test/hooks/AfterTokenTransfers.t.sol b/test/hooks/AfterTokenTransfers.t.sol
@@ -6,13 +6,11 @@ import {DSTest} from "ds-test/test.sol";
 import {ERC721ACHMock} from "../utils/ERC721ACHMock.sol";
 import {IERC721A} from "lib/ERC721A/contracts/IERC721A.sol";
 import {AfterTokenTransfersHookMock} from "../utils/hooks/AfterTokenTransfersHookMock.sol";
+import {HookUtils} from "../utils/HookUtils.sol";
 
 import {IERC721ACH} from "../../src/interfaces/IERC721ACH.sol";
 
-contract AfterTokenTransfersHookTest is DSTest {
-    Vm public constant vm = Vm(HEVM_ADDRESS);
-    address public constant DEFAULT_OWNER_ADDRESS = address(0xC0FFEE);
-    address public constant DEFAULT_BUYER_ADDRESS = address(0xBABE);
+contract AfterTokenTransfersHookTest is DSTest, HookUtils {
     ERC721ACHMock erc721Mock;
     AfterTokenTransfersHookMock hookMock;
 
@@ -45,40 +43,37 @@ contract AfterTokenTransfersHookTest is DSTest {
     }
 
     function test_afterTokenTransfersHook(
+        address _firstOwner,
+        address _secondOwner,
         uint256 startTokenId,
         uint256 quantity
     ) public {
-        vm.assume(quantity > 0);
-        vm.assume(startTokenId > 0);
+        _assumeGtZero(quantity);
+        _assumeGtZero(startTokenId);
         vm.assume(quantity < 10_000);
         vm.assume(quantity >= startTokenId);
+        _assumeNotNull(_firstOwner);
+        _assumeNotNull(_secondOwner);
 
         // Mint some tokens first
-        erc721Mock.mint(DEFAULT_BUYER_ADDRESS, quantity);
-
-        vm.prank(DEFAULT_BUYER_ADDRESS);
-        erc721Mock.transferFrom(
-            DEFAULT_BUYER_ADDRESS,
-            DEFAULT_OWNER_ADDRESS,
+        erc721Mock.mint(_firstOwner, quantity);
+        _assertNormalTransfer(
+            address(erc721Mock),
+            _firstOwner,
+            _secondOwner,
             startTokenId
         );
 
-        assertEq(DEFAULT_OWNER_ADDRESS, erc721Mock.ownerOf(startTokenId));
-
         // Verify hook override
         test_setAfterTokenTransfersHook();
-
-        hookMock.setHooksEnabled(true);
-        vm.expectRevert(
+        _assertTransferRevert(
+            address(erc721Mock),
+            _secondOwner,
+            _firstOwner,
+            startTokenId,
             AfterTokenTransfersHookMock
                 .AfterTokenTransfersHook_Executed
                 .selector
         );
-        vm.prank(DEFAULT_OWNER_ADDRESS);
-        erc721Mock.transferFrom(
-            DEFAULT_OWNER_ADDRESS,
-            DEFAULT_BUYER_ADDRESS,
-            startTokenId
-        );
     }
 }
diff --git a/test/hooks/BeforeTokenTransfers.t.sol b/test/hooks/BeforeTokenTransfers.t.sol
@@ -1,17 +1,14 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.15;
 
-import {Vm} from "forge-std/Vm.sol";
 import {DSTest} from "ds-test/test.sol";
 import {ERC721ACHMock} from "../utils/ERC721ACHMock.sol";
 import {IERC721A} from "lib/ERC721A/contracts/IERC721A.sol";
 import {BeforeTokenTransfersHookMock} from "../utils/hooks/BeforeTokenTransfersHookMock.sol";
+import {HookUtils} from "../utils/HookUtils.sol";
 import {IERC721ACH} from "../../src/interfaces/IERC721ACH.sol";
 
-contract BeforeTokenTransfersHookTest is DSTest {
-    Vm public constant vm = Vm(HEVM_ADDRESS);
-    address public constant DEFAULT_OWNER_ADDRESS = address(0xC0FFEE);
-    address public constant DEFAULT_BUYER_ADDRESS = address(0xBABE);
+contract BeforeTokenTransfersHookTest is DSTest, HookUtils {
     ERC721ACHMock erc721Mock;
     BeforeTokenTransfersHookMock hookMock;
     IERC721ACH.HookType constant BeforeTokenTransfers =
@@ -51,39 +48,29 @@ contract BeforeTokenTransfersHookTest is DSTest {
         _assumeGtZero(startTokenId);
         vm.assume(quantity < 10_000);
         vm.assume(quantity >= startTokenId);
-        _assumeNotBurn(_firstOwner);
-        _assumeNotBurn(_secondOwner);
+        _assumeNotNull(_firstOwner);
+        _assumeNotNull(_secondOwner);
 
         // Mint some tokens first
         erc721Mock.mint(_firstOwner, quantity);
-        _assertNormalTransfer(_firstOwner, _secondOwner, startTokenId);
+        _assertNormalTransfer(
+            address(erc721Mock),
+            _firstOwner,
+            _secondOwner,
+            startTokenId
+        );
 
         // Verify hook override
         test_setBeforeTokenTransfersHook();
-        _assertHookRevert(_secondOwner, _firstOwner, startTokenId);
-    }
-
-    function _assumeNotBurn(address _wallet) internal pure {
-        vm.assume(_wallet != address(0));
-    }
-
-    function _assumeGtZero(uint256 _num) internal pure {
-        vm.assume(_num > 0);
-    }
-
-    function _assertNormalTransfer(
-        address _from,
-        address _to,
-        uint256 _tokenId
-    ) public {
-        vm.prank(_from);
-        erc721Mock.transferFrom(_from, _to, _tokenId);
-
-        _assertOwner(_to, _tokenId);
-    }
-
-    function _assertOwner(address _owner, uint256 _token) internal {
-        assertEq(_owner, erc721Mock.ownerOf(_token));
+        _assertTransferRevert(
+            address(erc721Mock),
+            _secondOwner,
+            _firstOwner,
+            startTokenId,
+            BeforeTokenTransfersHookMock
+                .BeforeTokenTransfersHook_Executed
+                .selector
+        );
     }
 
     function _assertHookRevert(

diff --git a/test/utils/HookUtils.sol b/test/utils/HookUtils.sol
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.15;
+
+import {Vm} from "forge-std/Vm.sol";
+import {DSTest} from "ds-test/test.sol";
+import {ERC721ACHMock} from "./ERC721ACHMock.sol";
+import {IERC721A} from "lib/ERC721A/contracts/IERC721A.sol";
+import {BeforeTokenTransfersHookMock} from "../utils/hooks/BeforeTokenTransfersHookMock.sol";
+import {IERC721ACH} from "../../src/interfaces/IERC721ACH.sol";
+
+contract HookUtils is DSTest {
+    address public constant DEFAULT_OWNER_ADDRESS = address(0xC0FFEE);
+    address public constant DEFAULT_BUYER_ADDRESS = address(0xBABE);
+
+    Vm public constant vm = Vm(HEVM_ADDRESS);
+
+    function _assumeNotNull(address _wallet) internal pure {
+        vm.assume(_wallet != address(0));
+    }
+
+    function _assumeGtZero(uint256 _num) internal pure {
+        vm.assume(_num > 0);
+    }
+
+    function _assertNormalTransfer(
+        address _target,
+        address _from,
+        address _to,
+        uint256 _tokenId
+    ) public {
+        vm.prank(_from);
+        ERC721ACHMock(_target).transferFrom(_from, _to, _tokenId);
+
+        _assertOwner(_target, _to, _tokenId);
+    }
+
+    function _assertOwner(
+        address _target,
+        address _owner,
+        uint256 _token
+    ) internal {
+        assertEq(_owner, ERC721ACHMock(_target).ownerOf(_token));
+    }
+
+    function _assertTransferRevert(
+        address _target,
+        address _from,
+        address _to,
+        uint256 _tokenId,
+        bytes4 _err
+    ) internal {
+        vm.prank(_from);
+        vm.expectRevert(_err);
+        ERC721ACHMock(_target).transferFrom(_from, _to, _tokenId);
+    }
+}