From 627a251f93d1a6c0e7597c8774cf5039c596364d Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Sun, 6 Aug 2023 12:32:44 +0500 Subject: [PATCH 1/2] refactor: update ownerOfOverrideHook to return a tuple of owner and runSuper to conditionally run super.ownerOf if the hook says so --- src/ERC721ACH.sol | 16 ++++++++++++---- src/interfaces/IOwnerOfHook.sol | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/ERC721ACH.sol b/src/ERC721ACH.sol index 666ddef..3374961 100644 --- 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 index 74e009d..3c66fa2 100644 --- 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); } From 826e927d88a62c96cfae0784af0a30f68a0be1ff Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Sun, 6 Aug 2023 13:04:32 +0500 Subject: [PATCH 2/2] test: update tests to pass --- test/hooks/OwnerOfHook.t.sol | 2 +- test/utils/hooks/OwnerOfHookMock.sol | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/test/hooks/OwnerOfHook.t.sol b/test/hooks/OwnerOfHook.t.sol index 8e997ba..97d0fb7 100644 --- 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 index 8b97ab1..3c532ba 100644 --- 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 } }