Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(contracts): supportsInterface should make use of inheritance #818

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading