Merge pull request #16 from defientco/ahmedali8/ownerof-hook-changes

ownerOfHook returns tuple (owner, runSuper)
defientco · Aug 6, 2023 · fba41f6 · fba41f6
2 parents 0d47340 + 826e927
commit fba41f6
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 9 deletions.
diff --git a/src/ERC721ACH.sol b/src/ERC721ACH.sol
@@ -95,17 +95,25 @@ contract ERC721ACH is ERC721AC, IERC721ACH {
     }
 
     function ownerOf(uint256 tokenId) public view virtual override returns (address) {
-
+        address owner;
+        bool runSuper;
+
         IOwnerOfHook ownerOfHook = IOwnerOfHook(hooks[HookType.OwnerOf]);
 
         if (
             address(ownerOfHook) != address(0) &&
             ownerOfHook.useOwnerOfHook(tokenId)
         ) {
-            return ownerOfHook.ownerOfOverrideHook(tokenId);
-        } 
+            (owner, runSuper) = ownerOfHook.ownerOfOverrideHook(tokenId);
+        } else {
+            runSuper = true;
+        }
 
-        return super.ownerOf(tokenId);
+        if (runSuper) {
+            owner = super.ownerOf(tokenId);
+        }
+
+        return owner;
     }
 
 

diff --git a/src/interfaces/IOwnerOfHook.sol b/src/interfaces/IOwnerOfHook.sol
@@ -27,9 +27,9 @@ interface IOwnerOfHook {
     /**
         @notice Provides a custom implementation for the owner retrieval process.
         @param tokenId The ID of the token whose owner is being retrieved.
-        @return The address of the owner of the token.
+        @return A tuple with The address of the owner of the token and A bool flag whether to run `super.ownerOf` or not
      */
     function ownerOfOverrideHook(
         uint256 tokenId
-    ) external view returns (address);
+    ) external view returns (address, bool);
 }
diff --git a/test/hooks/OwnerOfHook.t.sol b/test/hooks/OwnerOfHook.t.sol
@@ -44,10 +44,10 @@ contract OwnerOfHookTest is DSTest {
 
         test_setOwnerOfHook();
         erc721Mock.mint(DEFAULT_BUYER_ADDRESS, tokenId);
-
         assertEq(DEFAULT_BUYER_ADDRESS, erc721Mock.ownerOf(tokenId));
 
         hookMock.setHooksEnabled(true);
+        hookMock.setRevertOwnerOfOverrideHook(true);
         vm.expectRevert(OwnerOfHookMock.OwnerOfHook_Executed.selector);
         erc721Mock.ownerOf(tokenId);
     }

diff --git a/test/utils/hooks/OwnerOfHookMock.sol b/test/utils/hooks/OwnerOfHookMock.sol
@@ -7,9 +7,14 @@ contract OwnerOfHookMock is IOwnerOfHook {
     /// @notice hook was executed
     error OwnerOfHook_Executed();
 
+    bool public revertOwnerOfOverrideHook;
     bool public hooksEnabled;
     address public fixedOwner;
 
+    function setRevertOwnerOfOverrideHook(bool _enabled) public {
+        revertOwnerOfOverrideHook = _enabled;
+    }
+
     /// @notice toggle ownerOf hook.
     function setHooksEnabled(bool _enabled) public {
         hooksEnabled = _enabled;
@@ -31,7 +36,8 @@ contract OwnerOfHookMock is IOwnerOfHook {
     /// @notice custom implementation for ownerOf Hook.
     function ownerOfOverrideHook(
         uint256
-    ) external view override returns (address) {
-        revert OwnerOfHook_Executed();
+    ) external view override returns (address, bool) {
+        if (revertOwnerOfOverrideHook) revert OwnerOfHook_Executed();
+        return (address(0), true); // run super
     }
 }