From e644262e206730e632667bbf21928e183f3f747b Mon Sep 17 00:00:00 2001 From: ollie <67156692+0xEillo@users.noreply.github.com> Date: Fri, 20 Oct 2023 12:09:59 +0200 Subject: [PATCH] feat: As a Dev, I want to have a minimal `IPortal` interface to facilitate the creation of a `Portal` (#303) Co-authored-by: Satyajeet Kolhapure Co-authored-by: Alain Nicolas --- contracts/src/{portal => }/DefaultPortal.sol | 2 +- contracts/src/PortalRegistry.sol | 5 ++-- contracts/src/example/IncorrectModule.sol | 11 -------- .../modules}/MsgSenderModule.sol | 4 +-- .../modules}/PayableModule.sol | 4 +-- .../portals}/EASPortal.sol | 4 +-- .../portals}/NFTPortal.sol | 4 +-- contracts/src/interface/AbstractPortal.sol | 17 ++++++++++--- contracts/src/interface/IPortal.sol | 25 +++++++++++++++++++ .../test/{portal => }/DefaultPortal.t.sol | 16 ++++++------ contracts/test/ModuleRegistry.t.sol | 4 +-- contracts/test/PortalRegistry.t.sol | 20 ++++++++++++++- contracts/test/example/EASPortal.t.sol | 2 +- contracts/test/example/MsgSenderModule.t.sol | 2 +- contracts/test/example/NFTPortal.t.sol | 2 +- contracts/test/example/PayableModule.t.sol | 2 +- .../integration/AttestationRegistryMass.t.sol | 2 +- contracts/test/mocks/EASRegistryMock.sol | 3 +++ .../test/mocks/IPortalImplementation.sol | 21 ++++++++++++++++ .../mocks/MockModules.sol} | 13 ++++++++-- 20 files changed, 120 insertions(+), 43 deletions(-) rename contracts/src/{portal => }/DefaultPortal.sol (91%) delete mode 100644 contracts/src/example/IncorrectModule.sol rename contracts/src/{example => examples/modules}/MsgSenderModule.sol (89%) rename contracts/src/{example => examples/modules}/PayableModule.sol (90%) rename contracts/src/{example => examples/portals}/EASPortal.sol (96%) rename contracts/src/{example => examples/portals}/NFTPortal.sol (94%) create mode 100644 contracts/src/interface/IPortal.sol rename contracts/test/{portal => }/DefaultPortal.t.sol (93%) create mode 100644 contracts/test/mocks/IPortalImplementation.sol rename contracts/{src/example/CorrectModule.sol => test/mocks/MockModules.sol} (64%) diff --git a/contracts/src/portal/DefaultPortal.sol b/contracts/src/DefaultPortal.sol similarity index 91% rename from contracts/src/portal/DefaultPortal.sol rename to contracts/src/DefaultPortal.sol index 0e28af63..4cdc9304 100644 --- a/contracts/src/portal/DefaultPortal.sol +++ b/contracts/src/DefaultPortal.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import { AbstractPortal } from "../interface/AbstractPortal.sol"; +import { AbstractPortal } from "./interface/AbstractPortal.sol"; /** * @title Default Portal diff --git a/contracts/src/PortalRegistry.sol b/contracts/src/PortalRegistry.sol index 47cfa53c..d0d008aa 100644 --- a/contracts/src/PortalRegistry.sol +++ b/contracts/src/PortalRegistry.sol @@ -5,9 +5,10 @@ import { OwnableUpgradeable } from "openzeppelin-contracts-upgradeable/contracts // solhint-disable-next-line max-line-length import { ERC165CheckerUpgradeable } from "openzeppelin-contracts-upgradeable/contracts/utils/introspection/ERC165CheckerUpgradeable.sol"; import { AbstractPortal } from "./interface/AbstractPortal.sol"; -import { DefaultPortal } from "./portal/DefaultPortal.sol"; +import { DefaultPortal } from "./DefaultPortal.sol"; import { Portal } from "./types/Structs.sol"; import { IRouter } from "./interface/IRouter.sol"; +import { IPortal } from "./interface/IPortal.sol"; /** * @title Portal Registry @@ -130,7 +131,7 @@ contract PortalRegistry is OwnableUpgradeable { if (bytes(ownerName).length == 0) revert PortalOwnerNameMissing(); // Check if portal has implemented AbstractPortal - if (!ERC165CheckerUpgradeable.supportsInterface(id, type(AbstractPortal).interfaceId)) revert PortalInvalid(); + if (!ERC165CheckerUpgradeable.supportsInterface(id, type(IPortal).interfaceId)) revert PortalInvalid(); // Get the array of modules implemented by the portal address[] memory modules = AbstractPortal(id).getModules(); diff --git a/contracts/src/example/IncorrectModule.sol b/contracts/src/example/IncorrectModule.sol deleted file mode 100644 index fc849a18..00000000 --- a/contracts/src/example/IncorrectModule.sol +++ /dev/null @@ -1,11 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.21; - -/** - * @title Incorrect Module - * @author Consensys - * @notice This contract illustrates an invalid Module that doesn't follow the AbstractModule interface - */ -contract IncorrectModule { - -} diff --git a/contracts/src/example/MsgSenderModule.sol b/contracts/src/examples/modules/MsgSenderModule.sol similarity index 89% rename from contracts/src/example/MsgSenderModule.sol rename to contracts/src/examples/modules/MsgSenderModule.sol index 5cdc3a95..03e4b411 100644 --- a/contracts/src/example/MsgSenderModule.sol +++ b/contracts/src/examples/modules/MsgSenderModule.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import { AbstractModule } from "../interface/AbstractModule.sol"; -import { AttestationPayload } from "../types/Structs.sol"; +import { AbstractModule } from "../../interface/AbstractModule.sol"; +import { AttestationPayload } from "../../types/Structs.sol"; /** * @title Msg Sender Module diff --git a/contracts/src/example/PayableModule.sol b/contracts/src/examples/modules/PayableModule.sol similarity index 90% rename from contracts/src/example/PayableModule.sol rename to contracts/src/examples/modules/PayableModule.sol index ddf1804c..77b5108e 100644 --- a/contracts/src/example/PayableModule.sol +++ b/contracts/src/examples/modules/PayableModule.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import { AbstractModule } from "../interface/AbstractModule.sol"; -import { AttestationPayload } from "../types/Structs.sol"; +import { AbstractModule } from "../../interface/AbstractModule.sol"; +import { AttestationPayload } from "../../types/Structs.sol"; import { Ownable } from "openzeppelin-contracts/contracts/access/Ownable.sol"; /** diff --git a/contracts/src/example/EASPortal.sol b/contracts/src/examples/portals/EASPortal.sol similarity index 96% rename from contracts/src/example/EASPortal.sol rename to contracts/src/examples/portals/EASPortal.sol index 74c6c7b1..2912ff4a 100644 --- a/contracts/src/example/EASPortal.sol +++ b/contracts/src/examples/portals/EASPortal.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import { AbstractPortal } from "../interface/AbstractPortal.sol"; -import { AttestationPayload } from "../types/Structs.sol"; +import { AbstractPortal } from "../../interface/AbstractPortal.sol"; +import { AttestationPayload } from "../../types/Structs.sol"; /** * @title EAS Portal diff --git a/contracts/src/example/NFTPortal.sol b/contracts/src/examples/portals/NFTPortal.sol similarity index 94% rename from contracts/src/example/NFTPortal.sol rename to contracts/src/examples/portals/NFTPortal.sol index 8e7a9963..102e50d6 100644 --- a/contracts/src/example/NFTPortal.sol +++ b/contracts/src/examples/portals/NFTPortal.sol @@ -3,8 +3,8 @@ pragma solidity 0.8.21; import { IERC721, ERC721 } from "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol"; import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/ERC165.sol"; -import { AbstractPortal } from "../interface/AbstractPortal.sol"; -import { Attestation, AttestationPayload } from "../types/Structs.sol"; +import { AbstractPortal } from "../../interface/AbstractPortal.sol"; +import { Attestation, AttestationPayload } from "../../types/Structs.sol"; /** * @title NFT Portal diff --git a/contracts/src/interface/AbstractPortal.sol b/contracts/src/interface/AbstractPortal.sol index 11abaf69..2a3f4b02 100644 --- a/contracts/src/interface/AbstractPortal.sol +++ b/contracts/src/interface/AbstractPortal.sol @@ -7,8 +7,16 @@ import { PortalRegistry } from "../PortalRegistry.sol"; import { AttestationPayload } from "../types/Structs.sol"; import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/ERC165.sol"; import { IRouter } from "../interface/IRouter.sol"; - -abstract contract AbstractPortal is IERC165 { +import { IPortal } from "../interface/IPortal.sol"; + +/** + * @title Abstract Portal + * @author Consensys + * @notice This contract is an abstract contract with basic Portal logic + * to be inherited. We strongly encourage all Portals to implement + * this contract. + */ +abstract contract AbstractPortal is IPortal { IRouter public router; address[] public modules; ModuleRegistry public moduleRegistry; @@ -139,7 +147,10 @@ abstract contract AbstractPortal is IERC165 { * @return The list of modules addresses linked to the Portal */ function supportsInterface(bytes4 interfaceID) public pure virtual override returns (bool) { - return interfaceID == type(AbstractPortal).interfaceId || interfaceID == type(IERC165).interfaceId; + return + interfaceID == type(AbstractPortal).interfaceId || + interfaceID == type(IPortal).interfaceId || + interfaceID == type(IERC165).interfaceId; } /** diff --git a/contracts/src/interface/IPortal.sol b/contracts/src/interface/IPortal.sol new file mode 100644 index 00000000..7ede1e2f --- /dev/null +++ b/contracts/src/interface/IPortal.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.21; + +import { IERC165 } from "openzeppelin-contracts/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 { + /** + * @notice Get all the modules addresses used by the Portal + * @return The list of modules addresses linked to the Portal + */ + function getModules() external view returns (address[] memory); + + /** + * @notice Defines the address of the entity issuing attestations to the subject + * @dev We strongly encourage a reflection when implementing this method + */ + function getAttester() external view returns (address); +} diff --git a/contracts/test/portal/DefaultPortal.t.sol b/contracts/test/DefaultPortal.t.sol similarity index 93% rename from contracts/test/portal/DefaultPortal.t.sol rename to contracts/test/DefaultPortal.t.sol index 7cc9e19f..f7451c6b 100644 --- a/contracts/test/portal/DefaultPortal.t.sol +++ b/contracts/test/DefaultPortal.t.sol @@ -2,15 +2,15 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; -import { AbstractPortal } from "../../src/interface/AbstractPortal.sol"; -import { DefaultPortal } from "../../src/portal/DefaultPortal.sol"; -import { AttestationPayload } from "../../src/types/Structs.sol"; -import { CorrectModule } from "../../src/example/CorrectModule.sol"; -import { AttestationRegistryMock } from "../mocks/AttestationRegistryMock.sol"; -import { ModuleRegistryMock } from "../mocks/ModuleRegistryMock.sol"; -import { PortalRegistryMock } from "../mocks/PortalRegistryMock.sol"; +import { AbstractPortal } from "../src/interface/AbstractPortal.sol"; +import { DefaultPortal } from "../src/DefaultPortal.sol"; +import { AttestationPayload } from "../src/types/Structs.sol"; +import { CorrectModule } from "./mocks/MockModules.sol"; +import { AttestationRegistryMock } from "./mocks/AttestationRegistryMock.sol"; +import { ModuleRegistryMock } from "./mocks/ModuleRegistryMock.sol"; +import { PortalRegistryMock } from "./mocks/PortalRegistryMock.sol"; import { ERC165Upgradeable } from "openzeppelin-contracts-upgradeable/contracts/utils/introspection/ERC165Upgradeable.sol"; -import { Router } from "../../src/Router.sol"; +import { Router } from "./../src/Router.sol"; contract DefaultPortalTest is Test { CorrectModule public correctModule = new CorrectModule(); diff --git a/contracts/test/ModuleRegistry.t.sol b/contracts/test/ModuleRegistry.t.sol index 5719a97e..86e236b5 100644 --- a/contracts/test/ModuleRegistry.t.sol +++ b/contracts/test/ModuleRegistry.t.sol @@ -3,8 +3,8 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; import { ModuleRegistry } from "../src/ModuleRegistry.sol"; -import { CorrectModule } from "../src/example/CorrectModule.sol"; -import { IncorrectModule } from "../src/example/IncorrectModule.sol"; +import { CorrectModule } from "./mocks/MockModules.sol"; +import { IncorrectModule } from "./mocks/MockModules.sol"; import { PortalRegistryMock } from "./mocks/PortalRegistryMock.sol"; import { AttestationPayload } from "../src/types/Structs.sol"; import { Router } from "../src/Router.sol"; diff --git a/contracts/test/PortalRegistry.t.sol b/contracts/test/PortalRegistry.t.sol index 5465994f..5c1e2abf 100644 --- a/contracts/test/PortalRegistry.t.sol +++ b/contracts/test/PortalRegistry.t.sol @@ -3,13 +3,14 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; import { PortalRegistry } from "../src/PortalRegistry.sol"; -import { CorrectModule } from "../src/example/CorrectModule.sol"; +import { CorrectModule } from "./mocks/MockModules.sol"; import { Portal } from "../src/types/Structs.sol"; import { Router } from "../src/Router.sol"; import { AttestationRegistryMock } from "./mocks/AttestationRegistryMock.sol"; import { ModuleRegistryMock } from "./mocks/ModuleRegistryMock.sol"; import { ValidPortalMock } from "./mocks/ValidPortalMock.sol"; import { InvalidPortalMock } from "./mocks/InvalidPortalMock.sol"; +import { IPortalImplementation } from "./mocks/IPortalImplementation.sol"; contract PortalRegistryTest is Test { address public user = makeAddr("user"); @@ -22,6 +23,7 @@ contract PortalRegistryTest is Test { string public expectedOwnerName = "Owner Name"; ValidPortalMock public validPortalMock; InvalidPortalMock public invalidPortalMock = new InvalidPortalMock(); + IPortalImplementation public iPortalImplementation = new IPortalImplementation(); event Initialized(uint8 version); event PortalRegistered(string name, string description, address portalAddress); @@ -83,6 +85,7 @@ contract PortalRegistryTest is Test { } function test_register() public { + // Register a portal implmenting AbstractPortal vm.expectEmit(); emit PortalRegistered(expectedName, expectedDescription, address(validPortalMock)); vm.prank(user); @@ -91,6 +94,21 @@ contract PortalRegistryTest is Test { uint256 portalCount = portalRegistry.getPortalsCount(); assertEq(portalCount, 1); + // Register a portal implementing IPortal + vm.expectEmit(); + emit PortalRegistered("IPortalImplementation", "IPortalImplementation description", address(iPortalImplementation)); + vm.prank(user); + portalRegistry.register( + address(iPortalImplementation), + "IPortalImplementation", + "IPortalImplementation description", + true, + expectedOwnerName + ); + + portalCount = portalRegistry.getPortalsCount(); + assertEq(portalCount, 2); + Portal memory expectedPortal = Portal( address(validPortalMock), user, diff --git a/contracts/test/example/EASPortal.t.sol b/contracts/test/example/EASPortal.t.sol index 7c12d6c0..db3c5353 100644 --- a/contracts/test/example/EASPortal.t.sol +++ b/contracts/test/example/EASPortal.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; -import { EASPortal } from "../../src/example/EASPortal.sol"; +import { EASPortal } from "../../src/examples/portals/EASPortal.sol"; import { Router } from "../../src/Router.sol"; import { AbstractPortal } from "../../src/interface/AbstractPortal.sol"; import { AttestationRegistryMock } from "../mocks/AttestationRegistryMock.sol"; diff --git a/contracts/test/example/MsgSenderModule.t.sol b/contracts/test/example/MsgSenderModule.t.sol index 9459f94f..6c94e536 100644 --- a/contracts/test/example/MsgSenderModule.t.sol +++ b/contracts/test/example/MsgSenderModule.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; import { AbstractModule } from "../../src/interface/AbstractModule.sol"; -import { MsgSenderModule } from "../../src/example/MsgSenderModule.sol"; +import { MsgSenderModule } from "../../src/examples/modules/MsgSenderModule.sol"; import { AttestationPayload } from "../../src/types/Structs.sol"; contract MsgSenderModuleTest is Test { diff --git a/contracts/test/example/NFTPortal.t.sol b/contracts/test/example/NFTPortal.t.sol index 6139c940..ab5c3de1 100644 --- a/contracts/test/example/NFTPortal.t.sol +++ b/contracts/test/example/NFTPortal.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; -import { NFTPortal } from "../../src/example/NFTPortal.sol"; +import { NFTPortal } from "../../src/examples/portals/NFTPortal.sol"; import { Router } from "../../src/Router.sol"; import { AbstractPortal } from "../../src/interface/AbstractPortal.sol"; import { AttestationPayload } from "../../src/types/Structs.sol"; diff --git a/contracts/test/example/PayableModule.t.sol b/contracts/test/example/PayableModule.t.sol index 445a9493..04677af3 100644 --- a/contracts/test/example/PayableModule.t.sol +++ b/contracts/test/example/PayableModule.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.21; import { Test } from "forge-std/Test.sol"; import { AbstractModule } from "../../src/interface/AbstractModule.sol"; -import { PayableModule } from "../../src/example/PayableModule.sol"; +import { PayableModule } from "../../src/examples/modules/PayableModule.sol"; import { AttestationPayload } from "../../src/types/Structs.sol"; contract PayableModuleTest is Test { diff --git a/contracts/test/integration/AttestationRegistryMass.t.sol b/contracts/test/integration/AttestationRegistryMass.t.sol index b1dfe395..21cd6bce 100644 --- a/contracts/test/integration/AttestationRegistryMass.t.sol +++ b/contracts/test/integration/AttestationRegistryMass.t.sol @@ -6,7 +6,7 @@ import { AttestationRegistry } from "../../src/AttestationRegistry.sol"; import { PortalRegistry } from "../../src/PortalRegistry.sol"; import { SchemaRegistry } from "../../src/SchemaRegistry.sol"; import { ModuleRegistry } from "../../src/ModuleRegistry.sol"; -import { DefaultPortal } from "../../src/portal/DefaultPortal.sol"; +import { DefaultPortal } from "../../src/DefaultPortal.sol"; import { Attestation, AttestationPayload } from "../../src/types/Structs.sol"; import { Router } from "../../src/Router.sol"; diff --git a/contracts/test/mocks/EASRegistryMock.sol b/contracts/test/mocks/EASRegistryMock.sol index b5770efe..f265018a 100644 --- a/contracts/test/mocks/EASRegistryMock.sol +++ b/contracts/test/mocks/EASRegistryMock.sol @@ -4,6 +4,9 @@ pragma solidity 0.8.21; import { IEAS, Attestation } from "../../src/interface/IEAS.sol"; contract EASRegistryMock is IEAS { + /// @dev This empty method prevents Foundry from counting this contract in code coverage + function test() public {} + mapping(bytes32 attestationId => Attestation attestation) private attestations; function getAttestation(bytes32 uid) external view override returns (Attestation memory) { diff --git a/contracts/test/mocks/IPortalImplementation.sol b/contracts/test/mocks/IPortalImplementation.sol new file mode 100644 index 00000000..ab5fe556 --- /dev/null +++ b/contracts/test/mocks/IPortalImplementation.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.21; + +import { IPortal } from "../../src/interface/IPortal.sol"; +import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/ERC165.sol"; + +contract IPortalImplementation is IPortal { + function test() public {} + + function getModules() external pure override returns (address[] memory) { + return new address[](0); + } + + function getAttester() external view override returns (address) { + return msg.sender; + } + + function supportsInterface(bytes4 interfaceID) public pure override returns (bool) { + return interfaceID == type(IPortal).interfaceId || interfaceID == type(IERC165).interfaceId; + } +} diff --git a/contracts/src/example/CorrectModule.sol b/contracts/test/mocks/MockModules.sol similarity index 64% rename from contracts/src/example/CorrectModule.sol rename to contracts/test/mocks/MockModules.sol index 3ccb9cbc..415fb4a5 100644 --- a/contracts/src/example/CorrectModule.sol +++ b/contracts/test/mocks/MockModules.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import { AbstractModule } from "../interface/AbstractModule.sol"; -import { AttestationPayload } from "../types/Structs.sol"; +import { AbstractModule } from "../../src/interface/AbstractModule.sol"; +import { AttestationPayload } from "../../src/types/Structs.sol"; /** * @title Correct Module @@ -21,3 +21,12 @@ contract CorrectModule is AbstractModule { uint256 /*value*/ ) public pure override {} } + +/** + * @title Incorrect Module + * @author Consensys + * @notice This contract illustrates an invalid Module that doesn't follow the AbstractModule interface + */ +contract IncorrectModule { + +}