chore(contracts)(sdk): Optimize gas efficiency when fetching a Portal (…

…#838)
Consensys · Nov 28, 2024 · bc9809b · bc9809b
1 parent 874a467
commit bc9809b
Show file tree

Hide file tree

Showing 24 changed files with 162 additions and 28 deletions.
diff --git a/contracts/src/AttestationReader.sol b/contracts/src/AttestationReader.sol
@@ -98,7 +98,7 @@ contract AttestationReader is RouterManager {
         bytes32(0),
         subject,
         veraxAttestation.attester,
-        PortalRegistry(router.getPortalRegistry()).getPortalByAddress(veraxAttestation.portal).isRevocable,
+        PortalRegistry(router.getPortalRegistry()).getPortalRevocability(veraxAttestation.portal),
         veraxAttestation.attestationData
       );
   }

diff --git a/contracts/src/AttestationRegistry.sol b/contracts/src/AttestationRegistry.sol
@@ -247,7 +247,7 @@ contract AttestationRegistry is RouterManager {
    */
   function isRevocable(address portalId) public view returns (bool) {
     PortalRegistry portalRegistry = PortalRegistry(router.getPortalRegistry());
-    return portalRegistry.getPortalByAddress(portalId).isRevocable;
+    return portalRegistry.getPortalRevocability(portalId);
   }
 
   /**

diff --git a/contracts/src/PortalRegistry.sol b/contracts/src/PortalRegistry.sol
@@ -222,6 +222,26 @@ contract PortalRegistry is RouterManager {
     return portals[id];
   }
 
+  /**
+   * @notice Get the owner address of a Portal
+   * @param portalAddress The address of the Portal
+   * @return The Portal owner address
+   */
+  function getPortalOwner(address portalAddress) external view returns (address) {
+    if (!isRegistered(portalAddress)) revert PortalNotRegistered();
+    return portals[portalAddress].ownerAddress;
+  }
+
+  /**
+   * @notice Get a Portal's revocability
+   * @param portalAddress The address of the Portal
+   * @return The Portal revocability
+   */
+  function getPortalRevocability(address portalAddress) external view returns (bool) {
+    if (!isRegistered(portalAddress)) revert PortalNotRegistered();
+    return portals[portalAddress].isRevocable;
+  }
+
   /**
    * @notice Check if a Portal is registered
    * @param id The address of the Portal

diff --git a/contracts/src/examples/portals/PausablePortal.sol b/contracts/src/examples/portals/PausablePortal.sol
@@ -70,9 +70,7 @@ contract PausablePortal is AbstractPortalV2, Pausable, Ownable {
     AttestationPayload memory /*attestationPayload*/,
     address /*attester*/,
     uint256 /*value*/
-  ) internal virtual override whenNotPaused {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  ) internal virtual override whenNotPaused {}
 
   /**
    * @inheritdoc AbstractPortalV2
@@ -82,23 +80,17 @@ contract PausablePortal is AbstractPortalV2, Pausable, Ownable {
     bytes32[] memory /*attestationIds*/,
     AttestationPayload[] memory /*attestationsPayloads*/,
     bytes[][] memory /*validationPayloads*/
-  ) internal virtual override whenNotPaused {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  ) internal virtual override whenNotPaused {}
 
   /**
    * @inheritdoc AbstractPortalV2
    * @dev Only the owner of the portal can revoke an attestation
    */
-  function _onRevoke(bytes32 /*attestationId*/) internal virtual override whenNotPaused {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  function _onRevoke(bytes32 /*attestationId*/) internal virtual override whenNotPaused {}
 
   /**
    * @inheritdoc AbstractPortalV2
    * @dev Only the owner of the portal can bulk revoke attestations
    */
-  function _onBulkRevoke(bytes32[] memory /*attestationIds*/) internal virtual override whenNotPaused {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  function _onBulkRevoke(bytes32[] memory /*attestationIds*/) internal virtual override whenNotPaused {}
 }
diff --git a/contracts/src/stdlib/ECDSAModule.sol b/contracts/src/stdlib/ECDSAModule.sol
@@ -29,7 +29,7 @@ contract ECDSAModule is AbstractModule {
   event SignersAuthorized(address indexed portal, address[] signers, bool[] authorizationStatus);
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/ECDSAModuleV2.sol b/contracts/src/stdlib/ECDSAModuleV2.sol
@@ -31,7 +31,7 @@ contract ECDSAModuleV2 is AbstractModuleV2 {
   event SignersAuthorized(address indexed portal, address[] signers, bool[] authorizationStatus);
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/ERC1271Module.sol b/contracts/src/stdlib/ERC1271Module.sol
@@ -27,7 +27,7 @@ contract ERC1271Module is ERC1271, AbstractModule {
   error InvalidSignature(string msg);
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/ERC1271ModuleV2.sol b/contracts/src/stdlib/ERC1271ModuleV2.sol
@@ -38,7 +38,7 @@ contract ERC1271ModuleV2 is AbstractModuleV2 {
   event SignersAuthorized(address indexed portal, address[] signers, bool[] authorizationStatus);
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/FeeModule.sol b/contracts/src/stdlib/FeeModule.sol
@@ -21,7 +21,7 @@ contract FeeModule is AbstractModule {
   error InvalidAttestationFee();
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/FeeModuleV2.sol b/contracts/src/stdlib/FeeModuleV2.sol
@@ -23,7 +23,7 @@ contract FeeModuleV2 is AbstractModuleV2 {
   error InvalidAttestationFee();
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/SchemaModule.sol b/contracts/src/stdlib/SchemaModule.sol
@@ -22,7 +22,7 @@ contract SchemaModule is AbstractModule {
   error SchemaNotAuthorized();
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/SchemaModuleV2.sol b/contracts/src/stdlib/SchemaModuleV2.sol
@@ -26,7 +26,7 @@ contract SchemaModuleV2 is AbstractModuleV2 {
   event SchemasAuthorized(address indexed portal, bytes32[] schemas, bool[] authorizedStatus);
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/SenderModule.sol b/contracts/src/stdlib/SenderModule.sol
@@ -21,7 +21,7 @@ contract SenderModule is AbstractModule {
   error UnauthorizedSender();
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/src/stdlib/SenderModuleV2.sol b/contracts/src/stdlib/SenderModuleV2.sol
@@ -25,7 +25,7 @@ contract SenderModuleV2 is AbstractModuleV2 {
   event SendersAuthorized(address indexed portal, address[] senders, bool[] authorizedStatus);
 
   modifier onlyPortalOwner(address portal) {
-    if (msg.sender != portalRegistry.getPortalByAddress(portal).ownerAddress) revert OnlyPortalOwner();
+    if (msg.sender != portalRegistry.getPortalOwner(portal)) revert OnlyPortalOwner();
     _;
   }
 

diff --git a/contracts/test/AttestationReader.t.sol b/contracts/test/AttestationReader.t.sol
@@ -156,7 +156,7 @@ contract AttestationReaderTest is Test {
       bytes32(0),
       abi.decode(attestationPayload.subject, (address)),
       attester,
-      PortalRegistry(router.getPortalRegistry()).getPortalByAddress(portal).isRevocable,
+      PortalRegistry(router.getPortalRegistry()).getPortalRevocability(portal),
       attestationPayload.attestationData
     );
     return attestation;

diff --git a/contracts/test/DefaultPortal.t.sol b/contracts/test/DefaultPortal.t.sol
@@ -51,7 +51,7 @@ contract DefaultPortalTest is Test {
     assertEq(address(defaultPortal.moduleRegistry()), address(moduleRegistryMock));
     assertEq(address(defaultPortal.attestationRegistry()), address(attestationRegistryMock));
     assertEq(address(defaultPortal.portalRegistry()), address(portalRegistryMock));
-    assertEq(portalRegistryMock.getPortalByAddress(address(defaultPortal)).ownerAddress, portalOwner);
+    assertEq(portalRegistryMock.getPortalOwner(address(defaultPortal)), portalOwner);
   }
 
   function test_getModules() public view {

diff --git a/contracts/test/DefaultPortalV2.t.sol b/contracts/test/DefaultPortalV2.t.sol
@@ -51,7 +51,7 @@ contract DefaultPortalV2Test is Test {
     assertEq(address(defaultPortal.moduleRegistry()), address(moduleRegistryMock));
     assertEq(address(defaultPortal.attestationRegistry()), address(attestationRegistryMock));
     assertEq(address(defaultPortal.portalRegistry()), address(portalRegistryMock));
-    assertEq(portalRegistryMock.getPortalByAddress(address(defaultPortal)).ownerAddress, portalOwner);
+    assertEq(portalRegistryMock.getPortalOwner(address(defaultPortal)), portalOwner);
   }
 
   function test_getModules() public view {

diff --git a/contracts/test/PortalRegistry.t.sol b/contracts/test/PortalRegistry.t.sol
@@ -332,11 +332,39 @@ contract PortalRegistryTest is Test {
     portalRegistry.deployDefaultPortal(modules, expectedName, expectedDescription, true, expectedOwnerName);
   }
 
-  function test_getPortals_PortalNotRegistered() public {
+  function test_getPortalByAddress_PortalNotRegistered() public {
     vm.expectRevert(PortalRegistry.PortalNotRegistered.selector);
     portalRegistry.getPortalByAddress(address(validPortalMock));
   }
 
+  function test_getPortalOwner() public {
+    address portalAddress = address(validPortalMock);
+    vm.prank(user);
+    portalRegistry.register(portalAddress, expectedName, expectedDescription, true, expectedOwnerName);
+
+    address ownerAddress = portalRegistry.getPortalOwner(portalAddress);
+    assertEq(ownerAddress, user);
+  }
+
+  function test_getPortalOwner_PortalNotRegistered() public {
+    vm.expectRevert(PortalRegistry.PortalNotRegistered.selector);
+    portalRegistry.getPortalOwner(address(validPortalMock));
+  }
+
+  function test_getPortalRevocability() public {
+    address portalAddress = address(validPortalMock);
+    vm.prank(user);
+    portalRegistry.register(portalAddress, expectedName, expectedDescription, true, expectedOwnerName);
+
+    bool revocability = portalRegistry.getPortalRevocability(portalAddress);
+    assertEq(revocability, true);
+  }
+
+  function test_getPortalRevocability_PortalNotRegistered() public {
+    vm.expectRevert(PortalRegistry.PortalNotRegistered.selector);
+    portalRegistry.getPortalRevocability(address(validPortalMock));
+  }
+
   function test_isRegistered() public {
     assertEq(portalRegistry.isRegistered(address(validPortalMock)), false);
     vm.prank(user);

diff --git a/contracts/test/mocks/PortalRegistryMock.sol b/contracts/test/mocks/PortalRegistryMock.sol
@@ -32,6 +32,14 @@ contract PortalRegistryMock {
     return portals[id];
   }
 
+  function getPortalOwner(address portalAddress) external view returns (address) {
+    return portals[portalAddress].ownerAddress;
+  }
+
+  function getPortalRevocability(address portalAddress) external view returns (bool) {
+    return portals[portalAddress].isRevocable;
+  }
+
   function setIssuer(address issuer) public {
     issuers[issuer] = true;
   }

diff --git a/contracts/test/mocks/PortalRegistryNotAllowlistedMock.sol b/contracts/test/mocks/PortalRegistryNotAllowlistedMock.sol
@@ -32,6 +32,14 @@ contract PortalRegistryNotAllowlistedMock {
     return portals[id];
   }
 
+  function getPortalOwner(address portalAddress) external view returns (address) {
+    return portals[portalAddress].ownerAddress;
+  }
+
+  function getPortalRevocability(address portalAddress) external view returns (bool) {
+    return portals[portalAddress].isRevocable;
+  }
+
   function setIssuer(address issuer) public {
     issuers[issuer] = true;
   }

diff --git a/sdk/doc/cli-examples.md b/sdk/doc/cli-examples.md
@@ -41,6 +41,10 @@
 
     pnpm portal getPortalByAddress '{\"portalAddress\":\"0x8b833796869b5debb9b06370d6d47016f0d7973b\"}'
 
+    pnpm portal getPortalOwner '{\"portalAddress\":\"0x8b833796869b5debb9b06370d6d47016f0d7973b\"}'
+
+    pnpm portal getPortalRevocability '{\"portalAddress\":\"0x8b833796869b5debb9b06370d6d47016f0d7973b\"}'
+
     pnpm portal isPortalRegistered '{\"portalAddress\":\"0x8b833796869b5debb9b06370d6d47016f0d7973b\"}'
 
     pnpm portal getPortalsNumber

diff --git a/sdk/examples/portal/portalExamples.ts b/sdk/examples/portal/portalExamples.ts
@@ -399,6 +399,24 @@ export default class PortalExamples {
       console.log(await this.veraxSdk.portal.getPortalByAddress(portalAddress));
     }
 
+    if (methodName.toLowerCase() == "getPortalOwner".toLowerCase() || methodName == "") {
+      let params;
+      if (argv !== "") params = JSON.parse(argv);
+      const portalAddress = params?.portalAddress
+        ? (params.portalAddress as Address)
+        : "0x8b833796869b5debb9b06370d6d47016f0d7973b";
+      console.log(await this.veraxSdk.portal.getPortalOwner(portalAddress));
+    }
+
+    if (methodName.toLowerCase() == "getPortalRevocability".toLowerCase() || methodName == "") {
+      let params;
+      if (argv !== "") params = JSON.parse(argv);
+      const portalAddress = params?.portalAddress
+        ? (params.portalAddress as Address)
+        : "0x8b833796869b5debb9b06370d6d47016f0d7973b";
+      console.log(await this.veraxSdk.portal.getPortalRevocability(portalAddress));
+    }
+
     if (methodName.toLowerCase() == "getPortalsNumber".toLowerCase() || methodName == "") {
       console.log(await this.veraxSdk.portal.getPortalsNumber());
     }

diff --git a/sdk/src/abi/PortalRegistry.ts b/sdk/src/abi/PortalRegistry.ts
@@ -195,6 +195,44 @@ export const abiPortalRegistry = [
     stateMutability: "view",
     type: "function",
   },
+  {
+    inputs: [
+      {
+        internalType: "address",
+        name: "portalAddress",
+        type: "address",
+      },
+    ],
+    name: "getPortalOwner",
+    outputs: [
+      {
+        internalType: "address",
+        name: "",
+        type: "address",
+      },
+    ],
+    stateMutability: "view",
+    type: "function",
+  },
+  {
+    inputs: [
+      {
+        internalType: "address",
+        name: "portalAddress",
+        type: "address",
+      },
+    ],
+    name: "getPortalRevocability",
+    outputs: [
+      {
+        internalType: "bool",
+        name: "",
+        type: "bool",
+      },
+    ],
+    stateMutability: "view",
+    type: "function",
+  },
   {
     inputs: [],
     name: "initialize",

diff --git a/sdk/src/dataMapper/PortalDataMapper.ts b/sdk/src/dataMapper/PortalDataMapper.ts
@@ -302,6 +302,24 @@ export default class PortalDataMapper extends BaseDataMapper<Portal, Portal_filt
     });
   }
 
+  async getPortalOwner(address: Address) {
+    return await this.web3Client.readContract({
+      address: this.conf.portalRegistryAddress,
+      abi: abiPortalRegistry,
+      functionName: "getPortalOwner",
+      args: [address],
+    });
+  }
+
+  async getPortalRevocability(address: Address) {
+    return await this.web3Client.readContract({
+      address: this.conf.portalRegistryAddress,
+      abi: abiPortalRegistry,
+      functionName: "getPortalRevocability",
+      args: [address],
+    });
+  }
+
   async getPortalsNumber() {
     return super.findTotalCount();
   }