refactor: streamline interfaces (#150)

* refactor: internal mappings * refactor: streamline interface * refactor: reorganise interfaces
Open-Attestation · Mar 13, 2024 · e0b2219 · e0b2219
1 parent 67aab11
commit e0b2219
Show file tree

Hide file tree

Showing 5 changed files with 192 additions and 153 deletions.
diff --git a/src/BaseDocumentStore.sol b/src/BaseDocumentStore.sol
@@ -3,14 +3,24 @@
 pragma solidity >=0.8.23 <0.9.0;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
 
-import {IDocumentStore} from "./interfaces/IDocumentStore.sol";
+import "./interfaces/IDocumentStore.sol";
+import "./interfaces/IDocumentStoreBatchable.sol";
+import "./base/DocumentStoreAccessControl.sol";
 
 /**
  * @title BaseDocumentStore
  * @notice A base contract for storing and revoking documents
  */
-abstract contract BaseDocumentStore is Initializable, IDocumentStore {
+abstract contract BaseDocumentStore is
+  Initializable,
+  IDocumentStoreBatchable,
+  IDocumentStore,
+  DocumentStoreAccessControl
+{
+  using MerkleProof for bytes32[];
+
   /**
    * @notice The name of the contract
    */
@@ -19,52 +29,160 @@ abstract contract BaseDocumentStore is Initializable, IDocumentStore {
   /**
    * @notice A mapping of the document hash to the block number that was issued
    */
-  mapping(bytes32 => uint256) public documentIssued;
+  mapping(bytes32 => uint256) internal documentIssued;
 
   /**
    * @notice A mapping of the hash of the claim being revoked to the revocation block number
    */
-  mapping(bytes32 => uint256) public documentRevoked;
+  mapping(bytes32 => uint256) internal documentRevoked;
 
   /**
    * @notice Initialises the contract with a name
    * @param _name The name of the contract
    */
-  function __BaseDocumentStore_init(string memory _name) internal onlyInitializing {
+  function __BaseDocumentStore_init(string memory _name, address initAdmin) internal onlyInitializing {
+    __DocumentStoreAccessControl_init(initAdmin);
     name = _name;
   }
 
   /**
    * @notice Issues a document
-   * @param document The hash of the document to issue
+   * @param documentRoot The hash of the document to issue
    */
-  function _issue(bytes32 document) internal {
-    documentIssued[document] = block.number;
+  function issue(bytes32 documentRoot) external onlyRole(ISSUER_ROLE) {
+    _issue(documentRoot);
   }
 
   /**
-   * @notice Checks if a document has been issued
-   * @param document The hash of the document to check
-   * @return A boolean indicating whether the document has been issued
+   * @notice Issues multiple documents
+   * @param documentRoots The hashes of the documents to issue
    */
-  function _isIssued(bytes32 document) internal view returns (bool) {
-    return (documentIssued[document] != 0);
+  function bulkIssue(bytes32[] memory documentRoots) external onlyRole(ISSUER_ROLE) {
+    for (uint256 i = 0; i < documentRoots.length; i++) {
+      _issue(documentRoots[i]);
+    }
+  }
+
+  function revoke(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) external onlyRole(REVOKER_ROLE) {
+    _revoke(documentRoot, document, proof);
   }
 
   /**
    * @notice Revokes a document
-   * @param document The hash of the document to revoke
+   * @param documentRoot The hash of the document to revoke
    */
-  function _revoke(bytes32 document) internal {
-    documentRevoked[document] = block.number;
+  function revoke(bytes32 documentRoot) external onlyRole(REVOKER_ROLE) {
+    _revoke(documentRoot, documentRoot, new bytes32[](0));
+  }
+
+  /**
+   * @notice Revokes documents in bulk
+   * @param documentRoots The hashes of the documents to revoke
+   */
+  function bulkRevoke(
+    bytes32[] memory documentRoots,
+    bytes32[] memory documents,
+    bytes32[][] memory proofs
+  ) external onlyRole(REVOKER_ROLE) {
+    for (uint256 i = 0; i < documentRoots.length; i++) {
+      _revoke(documentRoots[i], documents[i], proofs[i]);
+    }
+  }
+
+  function isIssued(
+    bytes32 documentRoot,
+    bytes32 document,
+    bytes32[] memory proof
+  ) public view onlyValidDocument(documentRoot, document, proof) returns (bool) {
+    if (documentRoot == document && proof.length == 0) {
+      return documentIssued[document] != 0;
+    }
+    return documentIssued[documentRoot] != 0;
+  }
+
+  function isIssued(bytes32 documentRoot) public view returns (bool) {
+    return isIssued(documentRoot, documentRoot, new bytes32[](0));
+  }
+
+  function isRevoked(
+    bytes32 documentRoot,
+    bytes32 document,
+    bytes32[] memory proof
+  ) public view onlyValidDocument(documentRoot, document, proof) returns (bool) {
+    if (!isIssued(documentRoot, document, proof)) {
+      revert DocumentNotIssued(documentRoot, document);
+    }
+    return _isRevoked(documentRoot, document, proof);
   }
 
   /**
    * @notice Checks if a document has been revoked
-   * @param document The hash of the document to check
+   * @param documentRoot The hash of the document to check
    * @return A boolean indicating whether the document has been revoked
    */
-  function _isRevoked(bytes32 document) internal view returns (bool) {
-    return documentRevoked[document] != 0;
+  function isRevoked(bytes32 documentRoot) public view returns (bool) {
+    return isRevoked(documentRoot, documentRoot, new bytes32[](0));
+  }
+
+  function isActive(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) public view returns (bool) {
+    if (!isIssued(documentRoot, document, proof)) {
+      revert DocumentNotIssued(documentRoot, document);
+    }
+    return !_isRevoked(documentRoot, document, proof);
+  }
+
+  function isActive(bytes32 documentRoot) public view returns (bool) {
+    return isActive(documentRoot, documentRoot, new bytes32[](0));
+  }
+
+  function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
+    return
+      interfaceId == type(IDocumentStore).interfaceId ||
+      interfaceId == type(IDocumentStoreBatchable).interfaceId ||
+      super.supportsInterface(interfaceId);
+  }
+
+  /**
+   * @notice Issues a document
+   * @param documentRoot The hash of the document to issue
+   */
+  function _issue(bytes32 documentRoot) internal {
+    if (isIssued(documentRoot)) {
+      revert DocumentExists(documentRoot);
+    }
+
+    documentIssued[documentRoot] = block.number;
+
+    emit DocumentIssued(documentRoot);
+  }
+
+  function _revoke(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) internal {
+    bool active = isActive(documentRoot, document, proof);
+    if (!active) {
+      revert InactiveDocument(documentRoot, document);
+    }
+    documentRevoked[document] = block.number;
+    emit DocumentRevoked(documentRoot, document);
+  }
+
+  function _isRevoked(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) internal view returns (bool) {
+    if (documentRoot == document && proof.length == 0) {
+      return documentRevoked[document] != 0;
+    }
+    return documentRevoked[documentRoot] != 0 || documentRevoked[document] != 0;
+  }
+
+  modifier onlyValidDocument(
+    bytes32 documentRoot,
+    bytes32 document,
+    bytes32[] memory proof
+  ) {
+    if (document == 0x0 || documentRoot == 0x0) {
+      revert ZeroDocument();
+    }
+    if (!proof.verify(documentRoot, document)) {
+      revert InvalidDocument(documentRoot, document);
+    }
+    _;
   }
 }
diff --git a/src/DocumentStore.sol b/src/DocumentStore.sol
@@ -6,148 +6,28 @@ import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
 
 import "./BaseDocumentStore.sol";
 import "./base/DocumentStoreAccessControl.sol";
+import "./interfaces/IDocumentStoreBatchable.sol";
 
 /**
  * @title DocumentStore
  * @notice A contract for storing and revoking documents with access control
  */
-contract DocumentStore is DocumentStoreAccessControl, BaseDocumentStore {
-  using MerkleProof for bytes32[];
-
+contract DocumentStore is BaseDocumentStore {
   /**
-   * @notice Initialises the contract with a name and owner
+   * @notice Initialises the contract with a name and initial admin
    * @param _name The name of the contract
-   * @param owner The owner of the contract
+   * @param initAdmin The initial admin of the contract
    */
-  constructor(string memory _name, address owner) {
-    initialize(_name, owner);
+  constructor(string memory _name, address initAdmin) {
+    initialize(_name, initAdmin);
   }
 
   /**
    * @notice Internally initialises the contract with a name and owner
    * @param _name The name of the contract
-   * @param owner The owner of the contract
-   */
-  function initialize(string memory _name, address owner) internal initializer {
-    __DocumentStoreAccessControl_init(owner);
-    __BaseDocumentStore_init(_name);
-  }
-
-  /**
-   * @notice Issues a document
-   * @param documentRoot The hash of the document to issue
+   * @param initAdmin The owner of the contract
    */
-  function issue(bytes32 documentRoot) public onlyRole(ISSUER_ROLE) {
-    if (isIssued(documentRoot)) {
-      revert DocumentExists(documentRoot);
-    }
-
-    _issue(documentRoot);
-
-    emit DocumentIssued(documentRoot);
-  }
-
-  /**
-   * @notice Issues multiple documents
-   * @param documentRoots The hashes of the documents to issue
-   */
-  function bulkIssue(bytes32[] memory documentRoots) public {
-    for (uint256 i = 0; i < documentRoots.length; i++) {
-      issue(documentRoots[i]);
-    }
-  }
-
-  /**
-   * @notice Revokes a document
-   * @param documentRoot The hash of the document to revoke
-   */
-  function revoke(bytes32 documentRoot) public onlyRole(REVOKER_ROLE) {
-    revoke(documentRoot, documentRoot, new bytes32[](0));
-  }
-
-  function revoke(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) public onlyRole(REVOKER_ROLE) {
-    bool active = isActive(documentRoot, document, proof);
-    if (!active) {
-      revert InactiveDocument(documentRoot, document);
-    }
-    _revoke(document);
-    emit DocumentRevoked(documentRoot, document);
-  }
-
-  /**
-   * @notice Revokes documents in bulk
-   * @param documentRoots The hashes of the documents to revoke
-   */
-  function bulkRevoke(
-    bytes32[] memory documentRoots,
-    bytes32[] memory documents,
-    bytes32[][] memory proofs
-  ) public onlyRole(REVOKER_ROLE) {
-    for (uint256 i = 0; i < documentRoots.length; i++) {
-      revoke(documentRoots[i], documents[i], proofs[i]);
-    }
-  }
-
-  function isIssued(
-    bytes32 documentRoot,
-    bytes32 document,
-    bytes32[] memory proof
-  ) public view onlyValidDocument(documentRoot, document, proof) returns (bool) {
-    if (documentRoot == document && proof.length == 0) {
-      return _isIssued(document);
-    }
-    return _isIssued(documentRoot);
-  }
-
-  function isIssued(bytes32 documentRoot) public view returns (bool) {
-    return isIssued(documentRoot, documentRoot, new bytes32[](0));
-  }
-
-  function isRevoked(
-    bytes32 documentRoot,
-    bytes32 document,
-    bytes32[] memory proof
-  ) public view onlyValidDocument(documentRoot, document, proof) returns (bool) {
-    if (!isIssued(documentRoot, document, proof)) {
-      revert InvalidDocument(documentRoot, document);
-    }
-    return _isRevoked(documentRoot, document, proof);
-  }
-
-  function _isRevoked(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) internal view returns (bool) {
-    if (documentRoot == document && proof.length == 0) {
-      return _isRevoked(document);
-    }
-    return (_isRevoked(documentRoot) || _isRevoked(document));
-  }
-
-  /**
-   * @notice Checks if a document has been revoked
-   * @param documentRoot The hash of the document to check
-   * @return A boolean indicating whether the document has been revoked
-   */
-  function isRevoked(bytes32 documentRoot) public view returns (bool) {
-    return isRevoked(documentRoot, documentRoot, new bytes32[](0));
-  }
-
-  function isActive(bytes32 documentRoot, bytes32 document, bytes32[] memory proof) public view returns (bool) {
-    if (!isIssued(documentRoot, document, proof)) {
-      revert InvalidDocument(documentRoot, document);
-    }
-    return !_isRevoked(documentRoot, document, proof);
-  }
-
-  modifier onlyValidDocument(
-    bytes32 documentRoot,
-    bytes32 document,
-    bytes32[] memory proof
-  ) {
-    if (document == 0x0 || documentRoot == 0x0) {
-      revert ZeroDocument();
-    }
-    if (!proof.verify(documentRoot, document)) {
-      revert InvalidDocument(documentRoot, document);
-    }
-    _;
+  function initialize(string memory _name, address initAdmin) internal initializer {
+    __BaseDocumentStore_init(_name, initAdmin);
   }
 }
diff --git a/src/interfaces/IDocumentStore.sol b/src/interfaces/IDocumentStore.sol
@@ -3,10 +3,15 @@ pragma solidity >=0.8.23 <0.9.0;
 
 interface IDocumentStore {
   error InactiveDocument(bytes32 documentRoot, bytes32 document);
+
   error DocumentExists(bytes32 document);
+
   error ZeroDocument();
+
   error InvalidDocument(bytes32 documentRoot, bytes32 document);
 
+  error DocumentNotIssued(bytes32 documentRoot, bytes32 document);
+
   /**
    * @notice Emitted when a document is issued
    * @param document The hash of the issued document
@@ -18,4 +23,16 @@ interface IDocumentStore {
    * @param document The hash of the revoked document
    */
   event DocumentRevoked(bytes32 indexed documentRoot, bytes32 indexed document);
+
+  function name() external view returns (string memory);
+
+  function issue(bytes32 documentRoot) external;
+
+  function revoke(bytes32 documentRoot) external;
+
+  function isIssued(bytes32 documentRoot) external view returns (bool);
+
+  function isRevoked(bytes32 documentRoot) external view returns (bool);
+
+  function isActive(bytes32 documentRoot) external view returns (bool);
 }