fix: As an issuer, I can update another issuer's schema context (#319)

Co-authored-by: Alain Nicolas <[email protected]>
Co-authored-by: Satyajeet Kolhapure <[email protected]>

contracts/src/PortalRegistry.sol

@@ -6,6 +6,7 @@ import { OwnableUpgradeable } from "openzeppelin-contracts-upgradeable/contracts
 import { ERC165CheckerUpgradeable } from "openzeppelin-contracts-upgradeable/contracts/utils/introspection/ERC165CheckerUpgradeable.sol";
 import { AbstractPortal } from "./interface/AbstractPortal.sol";
 import { DefaultPortal } from "./DefaultPortal.sol";
+import { SchemaRegistry } from "./SchemaRegistry.sol";
 import { Portal } from "./types/Structs.sol";
 import { IRouter } from "./interface/IRouter.sol";
 import { IPortal } from "./interface/IPortal.sol";
@@ -81,6 +82,7 @@ contract PortalRegistry is OwnableUpgradeable {
    */
   function removeIssuer(address issuer) public onlyOwner {
     issuers[issuer] = false;
+    SchemaRegistry(router.getSchemaRegistry()).updateMatchingSchemaIssuers(issuer, msg.sender);
   }
 
   /**

contracts/src/SchemaRegistry.sol

@@ -15,13 +15,21 @@ contract SchemaRegistry is OwnableUpgradeable {
   IRouter public router;
   /// @dev The list of Schemas, accessed by their ID
   mapping(bytes32 id => Schema schema) private schemas;
+  /// @dev Associates a Schema ID with the address of the Issuer who created it
+  mapping(bytes32 id => address issuer) private schemasIssuers;
   /// @dev The list of Schema IDs
   bytes32[] public schemaIds;
 
   /// @notice Error thrown when an invalid Router address is given
   error RouterInvalid();
   /// @notice Error thrown when a non-issuer tries to call a method that can only be called by an issuer
   error OnlyIssuer();
+  /// @notice Error thrown when any address which is not a portal registry tries to call a method
+  error OnlyPortalRegistry();
+  /// @notice Error thrown when a non-assigned issuer tries to call a method that can only be called by an assigned issuer
+  error OnlyAssignedIssuer();
+  /// @notice Error thrown when an invalid Issuer address is given
+  error IssuerInvalid();
   /// @notice Error thrown when an identical Schema was already registered
   error SchemaAlreadyExists();
   /// @notice Error thrown when attempting to add a Schema without a name
@@ -56,6 +64,16 @@ contract SchemaRegistry is OwnableUpgradeable {
     _;
   }
 
+  /**
+   * @notice Checks if the caller is the portal registry.
+   * @param caller the caller address
+   */
+  modifier onlyPortalRegistry(address caller) {
+    bool isCallerPortalRegistry = router.getPortalRegistry() == caller;
+    if (!isCallerPortalRegistry) revert OnlyPortalRegistry();
+    _;
+  }
+
   /**
    * @notice Changes the address for the Router
    * @dev Only the registry owner can call this method
@@ -65,6 +83,34 @@ contract SchemaRegistry is OwnableUpgradeable {
     router = IRouter(_router);
   }
 
+  /**
+   * @notice Updates a given Schema's Issuer
+   * @param schemaId the Schema's ID
+   * @param issuer the address of the issuer who created the given Schema
+   * @dev Updates issuer for the given schemaId in the `schemaIssuers` mapping
+   *      The issuer must already be registered as an Issuer via the `PortalRegistry`
+   */
+  function updateSchemaIssuer(bytes32 schemaId, address issuer) public onlyOwner {
+    if (!isRegistered(schemaId)) revert SchemaNotRegistered();
+    if (issuer == address(0)) revert IssuerInvalid();
+    schemasIssuers[schemaId] = issuer;
+  }
+
+  /**
+   * @notice Updates issuer address for all schemas associated with old issuer address
+   * @param oldIssuer the address of old issuer
+   * @param newIssuer the address of new issuer
+   * @dev Finds all the schemaIds associated with old issuer and updates the mapping `schemasIssuers`
+   *      for schemaIds found with new issuer
+   */
+  function updateMatchingSchemaIssuers(address oldIssuer, address newIssuer) public onlyPortalRegistry(msg.sender) {
+    for (uint256 i = 0; i < schemaIds.length; i++) {
+      if (schemasIssuers[schemaIds[i]] == oldIssuer) {
+        schemasIssuers[schemaIds[i]] = newIssuer;
+      }
+    }
+  }
+
   /**
    * Generate an ID for a given schema
    * @param schema the string defining a schema
@@ -84,7 +130,8 @@ contract SchemaRegistry is OwnableUpgradeable {
    * @param description the Schema description
    * @param context the Schema context
    * @param schemaString the string defining a Schema
-   * @dev the Schema is stored in a mapping, its ID is added to an array of IDs and an event is emitted
+   * @dev The Schema is stored in the `schemas` mapping, its ID is added to an array of IDs and an event is emitted
+   *      The caller is assigned as the creator of the Schema, via the `schemasIssuers` mapping
    */
   function createSchema(
     string memory name,
@@ -103,6 +150,7 @@ contract SchemaRegistry is OwnableUpgradeable {
 
     schemas[schemaId] = Schema(name, description, context, schemaString);
     schemaIds.push(schemaId);
+    schemasIssuers[schemaId] = msg.sender;
     emit SchemaCreated(schemaId, name, description, context, schemaString);
   }
 
@@ -111,9 +159,11 @@ contract SchemaRegistry is OwnableUpgradeable {
    * @param schemaId the schema ID
    * @param context the Schema context
    * @dev Retrieve the Schema with given ID and update its context with new value
+   *      The caller must be the creator of the given Schema (through the `schemaIssuers` mapping)
    */
-  function updateContext(bytes32 schemaId, string memory context) public onlyIssuers(msg.sender) {
+  function updateContext(bytes32 schemaId, string memory context) public {
     if (!isRegistered(schemaId)) revert SchemaNotRegistered();
+    if (schemasIssuers[schemaId] != msg.sender) revert OnlyAssignedIssuer();
     schemas[schemaId].context = context;
   }
 

contracts/test/PortalRegistry.t.sol

@@ -7,6 +7,7 @@ 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 { SchemaRegistryMock } from "./mocks/SchemaRegistryMock.sol";
 import { ModuleRegistryMock } from "./mocks/ModuleRegistryMock.sol";
 import { ValidPortalMock } from "./mocks/ValidPortalMock.sol";
 import { InvalidPortalMock } from "./mocks/InvalidPortalMock.sol";
@@ -18,6 +19,7 @@ contract PortalRegistryTest is Test {
   PortalRegistry public portalRegistry;
   address public moduleRegistryAddress;
   address public attestationRegistryAddress;
+  address public schemaRegistryAddress;
   string public expectedName = "Name";
   string public expectedDescription = "Description";
   string public expectedOwnerName = "Owner Name";
@@ -37,11 +39,13 @@ contract PortalRegistryTest is Test {
 
     moduleRegistryAddress = address(new ModuleRegistryMock());
     attestationRegistryAddress = address(new AttestationRegistryMock());
+    schemaRegistryAddress = address(new SchemaRegistryMock());
     vm.prank(address(0));
     portalRegistry.updateRouter(address(router));
 
     router.updateModuleRegistry(moduleRegistryAddress);
     router.updateAttestationRegistry(attestationRegistryAddress);
+    router.updateSchemaRegistry(schemaRegistryAddress);
     vm.prank(address(0));
     portalRegistry.setIssuer(user);
 

contracts/test/SchemaRegistry.t.sol

@@ -17,6 +17,7 @@ contract SchemaRegistryTest is Test {
   string private expectedContext = "Context";
   string private expectedString = "this is a schema";
   address private user = makeAddr("user");
+  address private unassignedUser = makeAddr("unassignedUser");
 
   event SchemaCreated(bytes32 indexed id, string name, string description, string context, string schemaString);
   event Initialized(uint8 version);
@@ -32,6 +33,7 @@ contract SchemaRegistryTest is Test {
     portalRegistryAddress = address(portalRegistryMock);
     router.updatePortalRegistry(portalRegistryAddress);
     portalRegistryMock.setIssuer(user);
+    portalRegistryMock.setIssuer(unassignedUser);
   }
 
   function test_initialize_ContractAlreadyInitialized() public {
@@ -56,6 +58,42 @@ contract SchemaRegistryTest is Test {
     testSchemaRegistry.updateRouter(address(0));
   }
 
+  function test_updateSchemaIssuer() public {
+    vm.prank(user);
+    schemaRegistry.createSchema(expectedName, expectedDescription, expectedContext, expectedString);
+    vm.prank(address(0));
+    schemaRegistry.updateSchemaIssuer(expectedId, address(2));
+  }
+
+  function test_updateSchemaIssuer_SchemaNotRegistered() public {
+    vm.expectRevert(SchemaRegistry.SchemaNotRegistered.selector);
+    vm.prank(address(0));
+    schemaRegistry.updateSchemaIssuer(expectedId, address(0));
+  }
+
+  function test_updateSchemaIssuer_IssuerInvalid() public {
+    vm.prank(user);
+    schemaRegistry.createSchema(expectedName, expectedDescription, expectedContext, expectedString);
+    vm.prank(address(0));
+    vm.expectRevert(SchemaRegistry.IssuerInvalid.selector);
+    schemaRegistry.updateSchemaIssuer(expectedId, address(0));
+  }
+
+  function test_updateMatchingSchemaIssuers() public {
+    vm.prank(user);
+    schemaRegistry.createSchema(expectedName, expectedDescription, expectedContext, expectedString);
+    vm.prank(portalRegistryAddress);
+    schemaRegistry.updateMatchingSchemaIssuers(user, address(2));
+  }
+
+  function test_updateMatchingSchemaIssuers_OnlyPortalRegistry() public {
+    vm.prank(user);
+    schemaRegistry.createSchema(expectedName, expectedDescription, expectedContext, expectedString);
+    vm.prank(address(0));
+    vm.expectRevert(SchemaRegistry.OnlyPortalRegistry.selector);
+    schemaRegistry.updateMatchingSchemaIssuers(user, address(2));
+  }
+
   function test_getIdFromSchemaString() public {
     bytes32 id = schemaRegistry.getIdFromSchemaString(expectedString);
     assertEq(id, expectedId);
@@ -118,20 +156,22 @@ contract SchemaRegistryTest is Test {
     vm.stopPrank();
   }
 
-  function test_updateContext_OnlyIssuer() public {
-    vm.expectRevert(SchemaRegistry.OnlyIssuer.selector);
-    vm.startPrank(makeAddr("InvalidIssuer"));
-    schemaRegistry.updateContext(expectedId, "New context");
-    vm.stopPrank();
-  }
-
   function test_updateContext_SchemaNotRegistered() public {
     vm.startPrank(user);
     vm.expectRevert(SchemaRegistry.SchemaNotRegistered.selector);
     schemaRegistry.updateContext("Invalid ID", "New context");
     vm.stopPrank();
   }
 
+  function test_updateContext_OnlyAssignedIssuer() public {
+    vm.startPrank(user);
+    schemaRegistry.createSchema(expectedName, expectedDescription, expectedContext, expectedString);
+    vm.expectRevert(SchemaRegistry.OnlyAssignedIssuer.selector);
+    vm.startPrank(unassignedUser);
+    schemaRegistry.updateContext(expectedId, "New context");
+    vm.stopPrank();
+  }
+
   function test_updateContext_withEmptyContext() public {
     vm.startPrank(user);
     schemaRegistry.createSchema(expectedName, expectedDescription, expectedContext, expectedString);

contracts/test/mocks/SchemaRegistryMock.sol

@@ -26,4 +26,6 @@ contract SchemaRegistryMock {
   function isRegistered(bytes32 schemaId) public view returns (bool) {
     return schemas[schemaId];
   }
+
+  function updateMatchingSchemaIssuers(address oldIssuer, address newIssuer) public {}
 }