feat: As a Dev, I want to have a minimal IPortal interface to facil…

…itate the creation of a `Portal` (#303) Co-authored-by: Satyajeet Kolhapure <[email protected]> Co-authored-by: Alain Nicolas <[email protected]>
Consensys · Oct 20, 2023 · e644262 · e644262
1 parent 506c139
commit e644262
Show file tree

Hide file tree

Showing 20 changed files with 120 additions and 43 deletions.
diff --git a/contracts/src/portal/DefaultPortal.sol → contracts/src/DefaultPortal.sol b/contracts/src/portal/DefaultPortal.sol → 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
@@ -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
diff --git a/contracts/src/example/MsgSenderModule.sol → .../src/examples/modules/MsgSenderModule.sol b/contracts/src/example/MsgSenderModule.sol → .../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 → ...ts/src/examples/modules/PayableModule.sol b/contracts/src/example/PayableModule.sol → ...ts/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 → contracts/src/examples/portals/EASPortal.sol b/contracts/src/example/EASPortal.sol → 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 → contracts/src/examples/portals/NFTPortal.sol b/contracts/src/example/NFTPortal.sol → 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
@@ -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
@@ -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 → contracts/test/DefaultPortal.t.sol b/contracts/test/portal/DefaultPortal.t.sol → 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 → contracts/test/mocks/MockModules.sol b/contracts/src/example/CorrectModule.sol → 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 {
+
+}