Skip to content

Commit

Permalink
chore(contracts): supportsInterface should make use of inheritance (#818
Browse files Browse the repository at this point in the history
)
  • Loading branch information
alainncls authored Nov 26, 2024
1 parent efde4d0 commit f7733ff
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 28 deletions.
8 changes: 4 additions & 4 deletions contracts/src/abstracts/AbstractModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
}
}
8 changes: 4 additions & 4 deletions contracts/src/abstracts/AbstractModuleV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
}
}
8 changes: 4 additions & 4 deletions contracts/src/abstracts/AbstractPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions contracts/src/abstracts/AbstractPortalV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
9 changes: 2 additions & 7 deletions contracts/src/examples/portals/NFTPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
}
}
4 changes: 1 addition & 3 deletions contracts/src/interfaces/IPortal.sol
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.21;

import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol";

/**
* @title IPortal
* @author Consensys
* @notice This contract is the interface to be implemented by any Portal.
* 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
Expand Down
5 changes: 3 additions & 2 deletions contracts/test/PortalRegistry.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit f7733ff

Please sign in to comment.