From 29b005401ab024fb01cf5a36ed4ad25490d04ab9 Mon Sep 17 00:00:00 2001 From: Alain Nicolas Date: Mon, 25 Nov 2024 18:44:59 +0100 Subject: [PATCH] chore(contracts): supportsInterface should make use of inheritance --- contracts/src/abstracts/AbstractModule.sol | 8 ++++---- contracts/src/abstracts/AbstractModuleV2.sol | 8 ++++---- contracts/src/abstracts/AbstractPortal.sol | 8 ++++---- contracts/src/abstracts/AbstractPortalV2.sol | 8 ++++---- contracts/src/examples/portals/NFTPortal.sol | 9 ++------- contracts/src/interfaces/IPortal.sol | 4 +--- contracts/test/PortalRegistry.t.sol | 5 +++-- 7 files changed, 22 insertions(+), 28 deletions(-) diff --git a/contracts/src/abstracts/AbstractModule.sol b/contracts/src/abstracts/AbstractModule.sol index f604dbe0..df8340f2 100644 --- a/contracts/src/abstracts/AbstractModule.sol +++ b/contracts/src/abstracts/AbstractModule.sol @@ -2,14 +2,14 @@ pragma solidity 0.8.21; import { AttestationPayload } from "../types/Structs.sol"; -import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; /** * @title Abstract Module * @author Consensys * @notice Deprecated. Use the AbstractModuleV2 contract instead. */ -abstract contract AbstractModule is IERC165 { +abstract contract AbstractModule is ERC165 { /// @notice Error thrown when someone else than the portal's owner is trying to revoke error OnlyPortalOwner(); @@ -32,7 +32,7 @@ abstract contract AbstractModule is IERC165 { * @param interfaceID The ID of the interface to check. * @return A boolean indicating interface support. */ - function supportsInterface(bytes4 interfaceID) public pure virtual override returns (bool) { - return interfaceID == type(AbstractModule).interfaceId || interfaceID == type(IERC165).interfaceId; + function supportsInterface(bytes4 interfaceID) public view virtual override returns (bool) { + return interfaceID == type(AbstractModule).interfaceId || super.supportsInterface(interfaceID); } } diff --git a/contracts/src/abstracts/AbstractModuleV2.sol b/contracts/src/abstracts/AbstractModuleV2.sol index d77fc4ee..8ecc7f90 100644 --- a/contracts/src/abstracts/AbstractModuleV2.sol +++ b/contracts/src/abstracts/AbstractModuleV2.sol @@ -3,14 +3,14 @@ pragma solidity 0.8.21; import { OperationType } from "../types/Enums.sol"; import { AttestationPayload } from "../types/Structs.sol"; -import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; /** * @title Abstract Module V2 * @author Consensys * @notice Defines the minimal Module V2 interface */ -abstract contract AbstractModuleV2 is IERC165 { +abstract contract AbstractModuleV2 is ERC165 { /// @notice Error thrown when someone else than the portal's owner is trying to revoke error OnlyPortalOwner(); @@ -38,7 +38,7 @@ abstract contract AbstractModuleV2 is IERC165 { * @param interfaceID The ID of the interface to check. * @return A boolean indicating interface support. */ - function supportsInterface(bytes4 interfaceID) public pure virtual override returns (bool) { - return interfaceID == type(AbstractModuleV2).interfaceId || interfaceID == type(IERC165).interfaceId; + function supportsInterface(bytes4 interfaceID) public view virtual override returns (bool) { + return interfaceID == type(AbstractModuleV2).interfaceId || super.supportsInterface(interfaceID); } } diff --git a/contracts/src/abstracts/AbstractPortal.sol b/contracts/src/abstracts/AbstractPortal.sol index 9e5f9022..318e691f 100644 --- a/contracts/src/abstracts/AbstractPortal.sol +++ b/contracts/src/abstracts/AbstractPortal.sol @@ -6,7 +6,7 @@ import { ModuleRegistry } from "../ModuleRegistry.sol"; import { PortalRegistry } from "../PortalRegistry.sol"; import { OperationType } from "../types/Enums.sol"; import { AttestationPayload } from "../types/Structs.sol"; -import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import { IRouter } from "../interfaces/IRouter.sol"; import { IPortal } from "../interfaces/IPortal.sol"; @@ -15,7 +15,7 @@ import { IPortal } from "../interfaces/IPortal.sol"; * @author Consensys * @notice Deprecated. Use the AbstractPortalV2 contract instead. */ -abstract contract AbstractPortal is IPortal { +abstract contract AbstractPortal is IPortal, ERC165 { IRouter public router; address[] public modules; ModuleRegistry public moduleRegistry; @@ -259,11 +259,11 @@ abstract contract AbstractPortal is IPortal { * @param interfaceID the interface identifier checked in this call * @return The list of modules addresses linked to the Portal */ - function supportsInterface(bytes4 interfaceID) public pure virtual override returns (bool) { + function supportsInterface(bytes4 interfaceID) public view virtual override returns (bool) { return interfaceID == type(AbstractPortal).interfaceId || interfaceID == type(IPortal).interfaceId || - interfaceID == type(IERC165).interfaceId; + super.supportsInterface(interfaceID); } /** diff --git a/contracts/src/abstracts/AbstractPortalV2.sol b/contracts/src/abstracts/AbstractPortalV2.sol index 5a73751a..cce873da 100644 --- a/contracts/src/abstracts/AbstractPortalV2.sol +++ b/contracts/src/abstracts/AbstractPortalV2.sol @@ -6,7 +6,7 @@ import { ModuleRegistry } from "../ModuleRegistry.sol"; import { PortalRegistry } from "../PortalRegistry.sol"; import { OperationType } from "../types/Enums.sol"; import { AttestationPayload } from "../types/Structs.sol"; -import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import { IRouter } from "../interfaces/IRouter.sol"; import { IPortal } from "../interfaces/IPortal.sol"; @@ -17,7 +17,7 @@ import { IPortal } from "../interfaces/IPortal.sol"; * to be inherited. We strongly encourage all Portals to implement * this contract. */ -abstract contract AbstractPortalV2 is IPortal { +abstract contract AbstractPortalV2 is IPortal, ERC165 { IRouter public router; address[] public modules; ModuleRegistry public moduleRegistry; @@ -191,11 +191,11 @@ abstract contract AbstractPortalV2 is IPortal { * @param interfaceID the interface identifier checked in this call * @return The list of modules addresses linked to the Portal */ - function supportsInterface(bytes4 interfaceID) public pure virtual override returns (bool) { + function supportsInterface(bytes4 interfaceID) public view virtual override returns (bool) { return interfaceID == type(AbstractPortalV2).interfaceId || interfaceID == type(IPortal).interfaceId || - interfaceID == type(IERC165).interfaceId; + super.supportsInterface(interfaceID); } /** diff --git a/contracts/src/examples/portals/NFTPortal.sol b/contracts/src/examples/portals/NFTPortal.sol index e4abb7a2..1a5bf65c 100644 --- a/contracts/src/examples/portals/NFTPortal.sol +++ b/contracts/src/examples/portals/NFTPortal.sol @@ -3,7 +3,6 @@ pragma solidity 0.8.21; import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; import { IERC721, ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; -import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import { AbstractPortalV2 } from "../../abstracts/AbstractPortalV2.sol"; import { Attestation, AttestationPayload } from "../../types/Structs.sol"; import { IPortal } from "../../interfaces/IPortal.sol"; @@ -67,11 +66,7 @@ contract NFTPortal is AbstractPortalV2, ERC721 { * @param interfaceID the interface identifier checked in this call * @return The list of modules addresses linked to the Portal */ - function supportsInterface(bytes4 interfaceID) public pure virtual override(AbstractPortalV2, ERC721) returns (bool) { - return - interfaceID == type(AbstractPortalV2).interfaceId || - interfaceID == type(IPortal).interfaceId || - interfaceID == type(IERC165).interfaceId || - interfaceID == type(IERC721).interfaceId; + function supportsInterface(bytes4 interfaceID) public view virtual override(AbstractPortalV2, ERC721) returns (bool) { + return interfaceID == type(IERC721).interfaceId || super.supportsInterface(interfaceID); } } diff --git a/contracts/src/interfaces/IPortal.sol b/contracts/src/interfaces/IPortal.sol index 1968859c..3feffa2e 100644 --- a/contracts/src/interfaces/IPortal.sol +++ b/contracts/src/interfaces/IPortal.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; - /** * @title IPortal * @author Consensys @@ -10,7 +8,7 @@ import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol" * NOTE: A portal must implement this interface to registered on * the PortalRegistry contract. */ -interface IPortal is IERC165 { +interface IPortal { /** * @notice Get all the modules addresses used by the Portal * @return The list of modules addresses linked to the Portal diff --git a/contracts/test/PortalRegistry.t.sol b/contracts/test/PortalRegistry.t.sol index dab4e06a..8530cec4 100644 --- a/contracts/test/PortalRegistry.t.sol +++ b/contracts/test/PortalRegistry.t.sol @@ -250,10 +250,11 @@ contract PortalRegistryTest is Test { } function test_revoke() public { + address portalAddress = address(validPortalMock); vm.expectEmit(); - emit PortalRegistered(expectedName, expectedDescription, address(validPortalMock)); + emit PortalRegistered(expectedName, expectedDescription, portalAddress); vm.prank(user); - portalRegistry.register(address(validPortalMock), expectedName, expectedDescription, true, expectedOwnerName); + portalRegistry.register(portalAddress, expectedName, expectedDescription, true, expectedOwnerName); bool isRegistered = portalRegistry.isRegistered(address(validPortalMock)); assertEq(isRegistered, true);