Merge branch 'Consensys:dev' into port

contracts/script/upgrade/checkImplementations.ts

@@ -21,8 +21,7 @@ async function main() {
 
   console.log("Checking PortalRegistry...");
   const PortalRegistry = await ethers.getContractFactory("PortalRegistry");
-  // @ts-expect-error-next-line - constructorArgs is not part of the type
-  await upgrades.validateImplementation(PortalRegistry, { constructorArgs: [false] });
+  await upgrades.validateImplementation(PortalRegistry);
   console.log("PortalRegistry OK");
 
   console.log("Checking SchemaRegistry...");

contracts/script/upgrade/checkUpgradeability.ts

@@ -31,10 +31,8 @@ async function main() {
   const portalRegistryProxyAddress = process.env.PORTAL_REGISTRY_ADDRESS ?? "";
   const PortalRegistry = await ethers.getContractFactory("PortalRegistry");
 
-  // @ts-expect-error-next-line - constructorArgs is not part of the type
   await upgrades.validateUpgrade(portalRegistryProxyAddress, PortalRegistry, {
     kind: "transparent",
-    constructorArgs: [false],
   });
 
   console.log("Checking SchemaRegistry...");

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
       );
   }

contracts/src/AttestationRegistry.sol

@@ -101,10 +101,12 @@ contract AttestationRegistry is RouterManager {
   }
 
   /**
-   * @notice Registers an attestation to the AttestationRegistry
-   * @param attestationPayload the attestation payload to create attestation and register it
-   * @param attester the account address issuing the attestation
-   * @dev This method is only callable by a registered Portal
+   * @notice Registers an attestation in the AttestationRegistry.
+   * @param attestationPayload The payload used to create and register the attestation.
+   * @param attester The address of the account issuing the attestation.
+   * @dev This function can only be called by a registered Portal.
+   * @dev While it might not align with typical business rules, it is technically
+   *      possible to register expired attestations.
    */
   function attest(AttestationPayload calldata attestationPayload, address attester) public onlyPortals(msg.sender) {
     // Verify the schema id exists
@@ -118,6 +120,7 @@ contract AttestationRegistry is RouterManager {
     attestationIdCounter++;
     // Generate the full attestation ID, padded with the chain prefix
     bytes32 id = generateAttestationId(attestationIdCounter);
+    assert(id != 0x0 && !isRegistered(id));
     // Create attestation
     attestations[id] = Attestation(
       id,
@@ -152,6 +155,7 @@ contract AttestationRegistry is RouterManager {
       attestationIdCounter++;
       // Generate the full attestation ID, padded with the chain prefix
       bytes32 id = generateAttestationId(attestationIdCounter);
+      assert(id != 0x0 && !isRegistered(id));
       // Create attestation
       attestations[id] = Attestation(
         id,
@@ -245,7 +249,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);
   }
 
   /**
@@ -293,20 +297,23 @@ contract AttestationRegistry is RouterManager {
   }
 
   /**
-   * @notice Checks if an address owns a given attestation following ERC-1155
+   * @notice Checks if an address owns a valid attestation following the ERC-1155 interface
    * @param account The address of the token holder
    * @param id ID of the attestation
    * @return The _owner's balance of the attestations on a given attestation ID
+   * @dev Only considers non-revoked, non-replaced and non-expired attestations
    */
   function balanceOf(address account, uint256 id) public view returns (uint256) {
     bytes32 attestationId = generateAttestationId(id);
     Attestation memory attestation = attestations[attestationId];
-    if (attestation.subject.length > 20 && keccak256(attestation.subject) == keccak256(abi.encode(account))) {
-      return 1;
-    }
-    if (attestation.subject.length == 20 && keccak256(attestation.subject) == keccak256(abi.encodePacked(account))) {
-      return 1;
-    }
+
+    if (attestation.attestationId == bytes32(0)) return 0;
+    if (attestation.revoked == true) return 0;
+    if (attestation.expirationDate != 0 && attestation.expirationDate < block.timestamp) return 0;
+
+    if (attestation.subject.length == 32 && abi.decode(attestation.subject, (address)) == account) return 1;
+    if (attestation.subject.length == 20 && address(uint160(bytes20(attestation.subject))) == account) return 1;
+
     return 0;
   }
 
@@ -340,6 +347,9 @@ contract AttestationRegistry is RouterManager {
    * @return The next attestation ID
    */
   function getNextAttestationId() public view returns (bytes32) {
-    return generateAttestationId(attestationIdCounter + 1);
+    uint256 nextattestationId = attestationIdCounter + 1;
+    bytes32 id = generateAttestationId(nextattestationId);
+    assert(id != 0x0 && !isRegistered(id));
+    return id;
   }
 }

contracts/src/PortalRegistry.sol

@@ -64,16 +64,17 @@ contract PortalRegistry is RouterManager {
   event IsTestnetUpdated(bool isTestnet);
 
   /// @custom:oz-upgrades-unsafe-allow constructor
-  constructor(bool _isTestnet) {
+  constructor() {
     _disableInitializers();
-    isTestnet = _isTestnet;
   }
 
   /**
-   * @notice Contract initialization
+   * @notice Contract initialization with testnet status
+   * @param _isTestnet Boolean indicating if the deployment is on a testnet
    */
-  function initialize() public initializer {
+  function initialize(bool _isTestnet) public initializer {
     __Ownable_init();
+    isTestnet = _isTestnet;
   }
 
   /**
@@ -221,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

contracts/src/SchemaRegistry.sol

@@ -29,8 +29,6 @@ contract SchemaRegistry is RouterManager {
   error SchemaContextAlreadyUpdated();
   /// @notice Error thrown when a non-allowlisted user tries to call a forbidden method
   error OnlyAllowlisted();
-  /// @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
@@ -72,16 +70,6 @@ contract SchemaRegistry is RouterManager {
     _;
   }
 
-  /**
-   * @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();
-    _;
-  }
-
   /**
    * @dev Changes the address for the Router
    * @param _router the new Router address

contracts/src/abstracts/AbstractPortal.sol

@@ -22,7 +22,7 @@ abstract contract AbstractPortal is IPortal, ERC165 {
   AttestationRegistry public attestationRegistry;
   PortalRegistry public portalRegistry;
 
-  /// @notice Error thrown when someone else than the portal's owner is trying to revoke
+  /// @notice Error thrown when someone else than the portal's owner is trying to revoke or replace
   error OnlyPortalOwner();
 
   /// @notice Error thrown when withdrawing funds fails
@@ -42,14 +42,19 @@ abstract contract AbstractPortal is IPortal, ERC165 {
     portalRegistry = PortalRegistry(router.getPortalRegistry());
   }
 
+  /// @notice Modifier to enforce only the portal owner can perform certain actions
+  modifier onlyPortalOwner() {
+    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
+    _;
+  }
+
   /**
    * @notice Withdraw funds from the Portal
    * @param to the address to send the funds to
    * @param amount the amount to withdraw
    * @dev Only the Portal owner can withdraw funds
    */
-  function withdraw(address payable to, uint256 amount) external virtual {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
+  function withdraw(address payable to, uint256 amount) external virtual onlyPortalOwner {
     (bool s, ) = to.call{ value: amount }("");
     if (!s) revert WithdrawFail();
   }
@@ -310,61 +315,57 @@ abstract contract AbstractPortal is IPortal, ERC165 {
     uint256 value
   ) internal virtual {}
 
+  /**
+   * @notice Optional method run when attesting a batch of payloads
+   * @param attestationsPayloads the payloads to attest
+   * @param validationPayloads the payloads to validate in order to issue the attestations
+   */
+  function _onBulkAttest(
+    AttestationPayload[] memory attestationsPayloads,
+    bytes[][] memory validationPayloads
+  ) internal virtual {}
+
   /**
    * @notice Optional method run when an attestation is replaced
    * @dev    IMPORTANT NOTE: By default, replacement is only possible by the portal owner
    * @param attestationId the ID of the attestation being replaced
    * @param attestationPayload the attestation payload to create attestation and register it
    * @param attester the address of the attester
    * @param value the value sent with the attestation
+   * @dev This method now uses the `onlyPortalOwner` modifier to enforce ownership rules
    */
   function _onReplace(
     bytes32 attestationId,
     AttestationPayload memory attestationPayload,
     address attester,
     uint256 value
-  ) internal virtual {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
-
-  /**
-   * @notice Optional method run when attesting a batch of payloads
-   * @param attestationsPayloads the payloads to attest
-   * @param validationPayloads the payloads to validate in order to issue the attestations
-   */
-  function _onBulkAttest(
-    AttestationPayload[] memory attestationsPayloads,
-    bytes[][] memory validationPayloads
-  ) internal virtual {}
+  ) internal virtual onlyPortalOwner {}
 
   /**
    * @notice Optional method run when replacing a batch of payloads
    * @dev    IMPORTANT NOTE: By default, bulk replacement is only possible by the portal owner
    * @param attestationIds the IDs of the attestations being replaced
    * @param attestationsPayloads the payloads to replace
    * @param validationPayloads the payloads to validate in order to replace the attestations
+   * @dev This method now uses the `onlyPortalOwner` modifier to enforce ownership rules
    */
   function _onBulkReplace(
     bytes32[] memory attestationIds,
     AttestationPayload[] memory attestationsPayloads,
     bytes[][] memory validationPayloads
-  ) internal virtual {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  ) internal virtual onlyPortalOwner {}
 
   /**
-   * @notice Optional method run when an attestation is revoked or replaced
+   * @notice Optional method run when an attestation is revoked
    * @dev    IMPORTANT NOTE: By default, revocation is only possible by the portal owner
+   * @dev This method now uses the `onlyPortalOwner` modifier to enforce ownership rules
    */
-  function _onRevoke(bytes32 /*attestationId*/) internal virtual {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  function _onRevoke(bytes32 attestationId) internal virtual onlyPortalOwner {}
 
   /**
    * @notice Optional method run when a batch of attestations are revoked or replaced
    * @dev    IMPORTANT NOTE: By default, revocation is only possible by the portal owner
+   * @dev This method now uses the `onlyPortalOwner` modifier to enforce ownership rules
    */
-  function _onBulkRevoke(bytes32[] memory /*attestationIds*/) internal virtual {
-    if (msg.sender != portalRegistry.getPortalByAddress(address(this)).ownerAddress) revert OnlyPortalOwner();
-  }
+  function _onBulkRevoke(bytes32[] memory attestationIds) internal virtual onlyPortalOwner {}
 }