chore: Cleanup and improvements

src/AttestationRegistry.sol

@@ -74,6 +74,7 @@ contract AttestationRegistry is OwnableUpgradeable {
 
   /**
    * @notice Changes the address for the Router
+   * @dev Only the registry owner can call this method
    */
   function updateRouter(address _router) public onlyOwner {
     if (_router == address(0)) revert RouterInvalid();

src/ModuleRegistry.sol

@@ -67,6 +67,7 @@ contract ModuleRegistry is OwnableUpgradeable {
 
   /**
    * @notice Changes the address for the Router
+   * @dev Only the registry owner can call this method
    */
   function updateRouter(address _router) public onlyOwner {
     if (_router == address(0)) revert RouterInvalid();
@@ -82,7 +83,8 @@ contract ModuleRegistry is OwnableUpgradeable {
     return contractAddress.code.length > 0;
   }
 
-  /** Register a Module, with its metadata and run some checks:
+  /**
+Register a Module, with its metadata and run some checks:
    * - mandatory name
    * - mandatory module's deployed smart contract address
    * - the module must be unique
@@ -110,7 +112,8 @@ contract ModuleRegistry is OwnableUpgradeable {
     emit ModuleRegistered(name, description, moduleAddress);
   }
 
-  /** Execute the run method for all given Modules that are registered
+  /**
+   * @notice Executes the run method for all given Modules that are registered
    * @param modulesAddresses the addresses of the registered modules
    * @param attestationPayload the payload to attest
    * @param validationPayloads the payloads to check for each module (one payload per module)
@@ -134,7 +137,8 @@ contract ModuleRegistry is OwnableUpgradeable {
     }
   }
 
-  /** Execute the modules validation for all attestations payloads for all given Modules that are registered
+  /**
+   * @notice Executes the modules validation for all attestations payloads for all given Modules that are registered
    * @param modulesAddresses the addresses of the registered modules
    * @param attestationsPayloads the payloads to attest
    * @param validationPayloads the payloads to check for each module

src/PortalRegistry.sol

@@ -59,6 +59,7 @@ contract PortalRegistry is OwnableUpgradeable {
 
   /**
    * @notice Changes the address for the Router
+   * @dev Only the registry owner can call this method
    */
   function updateRouter(address _router) public onlyOwner {
     if (_router == address(0)) revert RouterInvalid();

src/Router.sol

@@ -7,7 +7,7 @@ import { IRouter } from "./interface/IRouter.sol";
 /**
  * @title Router
  * @author Consensys
- * @notice This contract aims to provides a single entrypoint for the Verax registries
+ * @notice This contract aims to register the addresses of the Verax registries
  */
 contract Router is IRouter, OwnableUpgradeable {
   address private ATTESTATION_REGISTRY;
@@ -24,37 +24,57 @@ contract Router is IRouter, OwnableUpgradeable {
     __Ownable_init();
   }
 
+  /// @inheritdoc IRouter
   function getAttestationRegistry() external view override returns (address) {
     return ATTESTATION_REGISTRY;
   }
 
+  /**
+   * @notice Changes the address for the AttestationRegistry contract
+   * @param _attestationRegistry The new address of the AttestationRegistry contract
+   */
   function updateAttestationRegistry(address _attestationRegistry) public onlyOwner {
     ATTESTATION_REGISTRY = _attestationRegistry;
     emit AttestationRegistryUpdated(_attestationRegistry);
   }
 
+  /// @inheritdoc IRouter
   function getModuleRegistry() external view override returns (address) {
     return MODULE_REGISTRY;
   }
 
+  /**
+   * @notice Changes the address for the ModuleRegistry contract
+   * @param _moduleRegistry The new address of the ModuleRegistry contract
+   */
   function updateModuleRegistry(address _moduleRegistry) public onlyOwner {
     MODULE_REGISTRY = _moduleRegistry;
     emit ModuleRegistryUpdated(_moduleRegistry);
   }
 
+  /// @inheritdoc IRouter
   function getPortalRegistry() external view override returns (address) {
     return PORTAL_REGISTRY;
   }
 
+  /**
+   * @notice Changes the address for the PortalRegistry contract
+   * @param _portalRegistry The new address of the PortalRegistry contract
+   */
   function updatePortalRegistry(address _portalRegistry) public onlyOwner {
     PORTAL_REGISTRY = _portalRegistry;
     emit PortalRegistryUpdated(_portalRegistry);
   }
 
+  /// @inheritdoc IRouter
   function getSchemaRegistry() external view override returns (address) {
     return SCHEMA_REGISTRY;
   }
 
+  /**
+   * @notice Changes the address for the SchemaRegistry contract
+   * @param _schemaRegistry The new address of the SchemaRegistry contract
+   */
   function updateSchemaRegistry(address _schemaRegistry) public onlyOwner {
     SCHEMA_REGISTRY = _schemaRegistry;
     emit SchemaRegistryUpdated(_schemaRegistry);

src/SchemaRegistry.sol

@@ -58,6 +58,7 @@ contract SchemaRegistry is OwnableUpgradeable {
 
   /**
    * @notice Changes the address for the Router
+   * @dev Only the registry owner can call this method
    */
   function updateRouter(address _router) public onlyOwner {
     if (_router == address(0)) revert RouterInvalid();
@@ -74,7 +75,8 @@ contract SchemaRegistry is OwnableUpgradeable {
     return keccak256(abi.encodePacked(schema));
   }
 
-  /** Create a Schema, with its metadata and run some checks:
+  /**
+   * @notice Creates a Schema, with its metadata and runs some checks:
    * - mandatory name
    * - mandatory string defining the schema
    * - the Schema must be unique
@@ -104,7 +106,8 @@ contract SchemaRegistry is OwnableUpgradeable {
     emit SchemaCreated(schemaId, name, description, context, schemaString);
   }
 
-  /** Update a context field of schema :
+  /**
+   * @notice Updates the context of a given schema
    * @param schemaId the schema ID
    * @param context the Schema context
    * @dev Retrieve the Schema with given ID and update its context with new value

src/example/CorrectModule.sol

@@ -4,12 +4,19 @@ pragma solidity 0.8.21;
 import { AbstractModule } from "../interface/AbstractModule.sol";
 import { AttestationPayload } from "../types/Structs.sol";
 
+/**
+ * @title Correct Module
+ * @author Consensys
+ * @notice This contract illustrates a valid Module that follows the AbstractModule interface
+ */
 contract CorrectModule is AbstractModule {
+  /// @dev This empty method prevents Foundry from counting this contract in code coverage
   function test() public {}
 
+  /// @inheritdoc AbstractModule
   function run(
     AttestationPayload memory /*attestationPayload*/,
-    bytes memory validationPayload,
+    bytes memory /*validationPayload*/,
     address /*txSender*/,
     uint256 /*value*/
   ) public pure override {}

src/example/EASPortal.sol

@@ -6,6 +6,7 @@ import { AttestationPayload } from "../types/Structs.sol";
 
 /**
  * @title EAS Portal
+ * @author Consensys
  * @notice This is an example of how to maintain interoperability with EAS - https://attest.sh
  */
 contract EASPortal is AbstractPortal {
@@ -29,7 +30,7 @@ contract EASPortal is AbstractPortal {
     AttestationRequestData data;
   }
 
-  bytes32 private relationshipSchemaId = 0x89bd76e17fd84df8e1e448fa1b46dd8d97f7e8e806552b003f8386a5aebcb9f0;
+  bytes32 private _relationshipSchemaId = 0x89bd76e17fd84df8e1e448fa1b46dd8d97f7e8e806552b003f8386a5aebcb9f0;
 
   /// @notice Error thrown when reference attestation with refUID is not registered
   error ReferenceAttestationNotRegistered();
@@ -40,8 +41,14 @@ contract EASPortal is AbstractPortal {
 
   constructor(address[] memory modules, address router) AbstractPortal(modules, router) {}
 
+  /// @inheritdoc AbstractPortal
   function withdraw(address payable to, uint256 amount) external override {}
 
+  /**
+   * @notice Issues a Verax attestation based on an EAS attestation
+   * @param attestationRequest the EAS payload to attest
+   * @dev If a related EAS attestation exists, it will also be attested on Verax and linked via the dedicated Schema
+   */
   function attest(AttestationRequest memory attestationRequest) public payable {
     bytes[] memory validationPayload = new bytes[](0);
 
@@ -60,7 +67,7 @@ contract EASPortal is AbstractPortal {
       bytes32 attestationId = bytes32(abi.encode(attestationIdCounter));
 
       AttestationPayload memory relationshipAttestationPayload = AttestationPayload(
-        relationshipSchemaId,
+        _relationshipSchemaId,
         attestationRequest.data.expirationTime,
         abi.encodePacked(attestationRequest.data.refUID),
         abi.encode(attestationId, "EASrefUID", attestationRequest.data.refUID)
@@ -70,21 +77,38 @@ contract EASPortal is AbstractPortal {
     }
   }
 
+  /**
+   * @notice Issues Verax attestations in bulk, based on a list of EAS attestations
+   * @param attestationsRequests the EAS payloads to attest
+   */
   function bulkAttest(AttestationRequest[] memory attestationsRequests) external payable {
     for (uint256 i = 0; i < attestationsRequests.length; i++) {
       attest(attestationsRequests[i]);
     }
   }
 
+  /**
+   * @inheritdoc AbstractPortal
+   * @notice This portal doesn't allow for an attestation to be revoked
+   */
   function _onRevoke(bytes32 /*attestationId*/) internal pure override {
     revert NoRevocation();
   }
 
+  /**
+   * @inheritdoc AbstractPortal
+   * @notice This portal doesn't allow for attestations to be revoked
+   */
   function _onBulkRevoke(bytes32[] memory /*attestationIds*/) internal pure override {
     revert NoBulkRevocation();
   }
 
-  function _getAttester() public view override returns (address) {
+  /**
+   * @inheritdoc AbstractPortal
+   * @notice This portal considers the caller is the attester.
+   *         In other words, an end-user can claim an attestation through this portal.
+   */
+  function getAttester() public view override returns (address) {
     return msg.sender;
   }
 }

src/example/IncorrectModule.sol

@@ -1,4 +1,11 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.21;
 
-contract IncorrectModule {}
+/**
+ * @title Incorrect Module
+ * @author Consensys
+ * @notice This contract illustrates an invalid Module that doesn't follow the AbstractModule interface
+ */
+contract IncorrectModule {
+
+}

src/example/MsgSenderModule.sol

@@ -25,7 +25,8 @@ contract MsgSenderModule is AbstractModule {
   }
 
   /**
-   * @notice The main method for the module, running the check
+   * @inheritdoc AbstractModule
+   * @notice If the transaction sender is not the expected address, an error is thrown
    */
   function run(
     AttestationPayload memory /*_attestationPayload*/,

src/example/PayableModule.sol

@@ -32,7 +32,8 @@ contract PayableModule is Ownable, AbstractModule {
   }
 
   /**
-   * @notice The main method for the module, running the check
+   * @inheritdoc AbstractModule
+   * @notice If the paid value is below the expected amount, an error is thrown
    * @param _value The value sent for the attestation
    */
   function run(

src/interface/AbstractModule.sol

@@ -4,7 +4,19 @@ pragma solidity 0.8.21;
 import { AttestationPayload } from "../types/Structs.sol";
 import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/IERC165.sol";
 
+/**
+ * @title Abstract Module
+ * @author Consensys
+ * @notice Defines the minimal Module interface
+ */
 abstract contract AbstractModule is IERC165 {
+  /**
+   * @notice Executes the module's custom logic.
+   * @param attestationPayload The incoming attestation data.
+   * @param validationPayload Additional data required for verification.
+   * @param txSender The transaction sender's address.
+   * @param value The transaction value.
+   */
   function run(
     AttestationPayload memory attestationPayload,
     bytes memory validationPayload,
@@ -13,7 +25,9 @@ abstract contract AbstractModule is IERC165 {
   ) public virtual;
 
   /**
-   * @notice To check this contract implements the Module interface
+   * @notice Checks if the contract implements the Module interface.
+   * @param interfaceID The ID of the interface to check.
+   * @return A boolean indicating interface support.
    */
   function supportsInterface(bytes4 interfaceID) public pure virtual override returns (bool) {
     return interfaceID == type(AbstractModule).interfaceId || interfaceID == type(IERC165).interfaceId;

src/interface/AbstractPortal.sol

@@ -40,7 +40,7 @@ abstract contract AbstractPortal is IERC165 {
   function withdraw(address payable to, uint256 amount) external virtual;
 
   /**
-   * @notice attest the schema with given attestationPayload and validationPayload
+   * @notice Attest the schema with given attestationPayload and validationPayload
    * @param attestationPayload the payload to attest
    * @param validationPayloads the payloads to validate via the modules to issue the attestations
    * @dev Runs all modules for the portal and registers the attestation using AttestationRegistry
@@ -50,7 +50,7 @@ abstract contract AbstractPortal is IERC165 {
 
     _onAttest(attestationPayload);
 
-    attestationRegistry.attest(attestationPayload, _getAttester());
+    attestationRegistry.attest(attestationPayload, getAttester());
   }
 
   /**
@@ -66,7 +66,7 @@ abstract contract AbstractPortal is IERC165 {
 
     _onBulkAttest(attestationsPayloads, validationPayloads);
 
-    attestationRegistry.bulkAttest(attestationsPayloads, _getAttester());
+    attestationRegistry.bulkAttest(attestationsPayloads, getAttester());
   }
 
   /**
@@ -85,7 +85,7 @@ abstract contract AbstractPortal is IERC165 {
 
     _onReplace(attestationId, attestationPayload);
 
-    attestationRegistry.replace(attestationId, attestationPayload, _getAttester());
+    attestationRegistry.replace(attestationId, attestationPayload, getAttester());
   }
 
   /**
@@ -103,7 +103,7 @@ abstract contract AbstractPortal is IERC165 {
 
     _onBulkReplace(attestationIds, attestationsPayloads, validationPayloads);
 
-    attestationRegistry.bulkReplace(attestationIds, attestationsPayloads, _getAttester());
+    attestationRegistry.bulkReplace(attestationIds, attestationsPayloads, getAttester());
   }
 
   /**
@@ -149,7 +149,7 @@ abstract contract AbstractPortal is IERC165 {
    * @notice Defines the address of the entity issuing attestations to the subject
    * @dev We strongly encourage a reflection when overriding this rule: who should be set as the attester?
    */
-  function _getAttester() public view virtual returns (address) {
+  function getAttester() public view virtual returns (address) {
     return msg.sender;
   }