simplify getTransferValidationFunction(), add test

ProjectOpenSea · Apr 1, 2024 · 9328779 · 9328779
1 parent 652c21d
commit 9328779
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 22 deletions.
diff --git a/src-upgradeable/src/ERC721ContractMetadataUpgradeable.sol b/src-upgradeable/src/ERC721ContractMetadataUpgradeable.sol
@@ -18,7 +18,7 @@ import {
     ILegacyCreatorToken
 } from "./interfaces/ICreatorToken.sol";
 
-import { ITransferValidator } from "./interfaces/ITransferValidator.sol";
+import { ITransferValidator721 } from "./interfaces/ITransferValidator.sol";
 
 import {
     ERC721AUpgradeable
@@ -304,9 +304,7 @@ contract ERC721ContractMetadataUpgradeable is
         pure
         returns (bytes4 functionSignature, bool isViewFunction)
     {
-        functionSignature = bytes4(
-            keccak256("validateTransfer(address,address,address,uint256)")
-        );
+        functionSignature = ITransferValidator721.validateTransfer.selector;
         isViewFunction = false;
     }
 
@@ -331,7 +329,7 @@ contract ERC721ContractMetadataUpgradeable is
         if (from != address(0) && to != address(0)) {
             // Call the transfer validator if one is set.
             if (_transferValidator != address(0)) {
-                ITransferValidator(_transferValidator).validateTransfer(
+                ITransferValidator721(_transferValidator).validateTransfer(
                     msg.sender,
                     from,
                     to,

diff --git a/src-upgradeable/src/interfaces/ITransferValidator.sol b/src-upgradeable/src/interfaces/ITransferValidator.sol
@@ -1,15 +1,17 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
-interface ITransferValidator {
+interface ITransferValidator721 {
     /// @notice Ensure that a transfer has been authorized for a specific tokenId
     function validateTransfer(
         address caller,
         address from,
         address to,
         uint256 tokenId
     ) external view;
+}
 
+interface ITransferValidator1155 {
     /// @notice Ensure that a transfer has been authorized for a specific amount of a specific tokenId, and reduce the transferable amount remaining
     function validateTransfer(
         address caller,

diff --git a/src/ERC721ContractMetadata.sol b/src/ERC721ContractMetadata.sol
@@ -16,7 +16,7 @@ import {
     ILegacyCreatorToken
 } from "./interfaces/ICreatorToken.sol";
 
-import { ITransferValidator } from "./interfaces/ITransferValidator.sol";
+import { ITransferValidator721 } from "./interfaces/ITransferValidator.sol";
 
 import { TwoStepOwnable } from "utility-contracts/TwoStepOwnable.sol";
 
@@ -294,9 +294,7 @@ contract ERC721ContractMetadata is
         pure
         returns (bytes4 functionSignature, bool isViewFunction)
     {
-        functionSignature = bytes4(
-            keccak256("validateTransfer(address,address,address,uint256)")
-        );
+        functionSignature = ITransferValidator721.validateTransfer.selector;
         isViewFunction = false;
     }
 
@@ -321,7 +319,7 @@ contract ERC721ContractMetadata is
         if (from != address(0) && to != address(0)) {
             // Call the transfer validator if one is set.
             if (_transferValidator != address(0)) {
-                ITransferValidator(_transferValidator).validateTransfer(
+                ITransferValidator721(_transferValidator).validateTransfer(
                     msg.sender,
                     from,
                     to,

diff --git a/src/clones/ERC721ContractMetadataCloneable.sol b/src/clones/ERC721ContractMetadataCloneable.sol
@@ -18,7 +18,7 @@ import {
     ILegacyCreatorToken
 } from "../interfaces/ICreatorToken.sol";
 
-import { ITransferValidator } from "../interfaces/ITransferValidator.sol";
+import { ITransferValidator721 } from "../interfaces/ITransferValidator.sol";
 
 import { TwoStepOwnable } from "utility-contracts/TwoStepOwnable.sol";
 
@@ -289,9 +289,7 @@ contract ERC721ContractMetadataCloneable is
         pure
         returns (bytes4 functionSignature, bool isViewFunction)
     {
-        functionSignature = bytes4(
-            keccak256("validateTransfer(address,address,address,uint256)")
-        );
+        functionSignature = ITransferValidator721.validateTransfer.selector;
         isViewFunction = false;
     }
 
@@ -316,7 +314,7 @@ contract ERC721ContractMetadataCloneable is
         if (from != address(0) && to != address(0)) {
             // Call the transfer validator if one is set.
             if (_transferValidator != address(0)) {
-                ITransferValidator(_transferValidator).validateTransfer(
+                ITransferValidator721(_transferValidator).validateTransfer(
                     msg.sender,
                     from,
                     to,

diff --git a/src/interfaces/ITransferValidator.sol b/src/interfaces/ITransferValidator.sol
@@ -1,17 +1,18 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
-interface ITransferValidator {
-    /// @notice Ensure that a transfer has been authorized for a specific tokenId.
+interface ITransferValidator721 {
+    /// @notice Ensure that a transfer has been authorized for a specific tokenId
     function validateTransfer(
         address caller,
         address from,
         address to,
         uint256 tokenId
     ) external view;
+}
 
-    /// @notice Ensure that a transfer has been authorized for a specific amount of
-    //          a specific tokenId, and reduce the transferable amount remaining.
+interface ITransferValidator1155 {
+    /// @notice Ensure that a transfer has been authorized for a specific amount of a specific tokenId, and reduce the transferable amount remaining
     function validateTransfer(
         address caller,
         address from,

diff --git a/src/test/MockTransferValidator.sol b/src/test/MockTransferValidator.sol
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: Unlicense
 pragma solidity 0.8.17;
 
-import { ITransferValidator } from "../interfaces/ITransferValidator.sol";
+import { ITransferValidator721, ITransferValidator1155 } from "../interfaces/ITransferValidator.sol";
 
-contract MockTransferValidator is ITransferValidator {
+contract MockTransferValidator is ITransferValidator721, ITransferValidator1155 {
     bool internal _revertOnValidate;
 
     constructor(bool revertOnValidate) {

diff --git a/test/foundry/ERC721TransferValidator.t.sol b/test/foundry/ERC721TransferValidator.t.sol
@@ -5,6 +5,10 @@ import { TestHelper } from "test/foundry/utils/TestHelper.sol";
 
 import { ERC721SeaDrop } from "seadrop/ERC721SeaDrop.sol";
 
+import {
+    ITransferValidator721
+} from "seadrop/interfaces/ITransferValidator.sol";
+
 import { MockTransferValidator } from "seadrop/test/MockTransferValidator.sol";
 
 import { TwoStepOwnable } from "utility-contracts/TwoStepOwnable.sol";
@@ -79,6 +83,16 @@ contract ERC721TransferValidatorTest is TestHelper {
         token_.safeTransferFrom(address(this), msg.sender, 2);
     }
 
+    function testGetTransferValidationFunction() public {
+        (bytes4 functionSignature, bool isViewFunction) = token_
+            .getTransferValidationFunction();
+        assertEq(
+            functionSignature,
+            ITransferValidator721.validateTransfer.selector
+        );
+        assertEq(isViewFunction, false);
+    }
+
     function onERC721Received(
         address,
         address,