refactor: document store errors

(cherry picked from commit efd9dce6c45647e21275b6939c46dcf9380ebc8f)
Open-Attestation · Mar 14, 2024 · 7db518e · 7db518e
1 parent 2742c66
commit 7db518e
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 47 deletions.
diff --git a/src/base/BaseDocumentStore.sol b/src/base/BaseDocumentStore.sol
@@ -8,6 +8,7 @@ import "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol";
 
 import "../interfaces/IDocumentStoreBatchable.sol";
 import "./DocumentStoreAccessControl.sol";
+import "../interfaces/IDocumentStoreErrors.sol";
 
 /**
  * @title BaseDocumentStore
@@ -17,6 +18,7 @@ abstract contract BaseDocumentStore is
   Initializable,
   MulticallUpgradeable,
   DocumentStoreAccessControl,
+  IDocumentStoreErrors,
   IDocumentStoreBatchable
 {
   using MerkleProof for bytes32[];

diff --git a/src/interfaces/IDocumentStore.sol b/src/interfaces/IDocumentStore.sol
@@ -2,17 +2,7 @@
 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);
-
-  error DocumentIsRevoked(bytes32 documentRoot, bytes32 document);
 
   /**
    * @notice Emitted when a document is issued

diff --git a/src/interfaces/IDocumentStoreErrors.sol b/src/interfaces/IDocumentStoreErrors.sol
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity >=0.8.23 <0.9.0;
+
+interface IDocumentStoreErrors {
+  error InactiveDocument(bytes32 documentRoot, bytes32 document);
+
+  error DocumentExists(bytes32 document);
+
+  error ZeroDocument();
+
+  error InvalidDocument(bytes32 documentRoot, bytes32 document);
+
+  error DocumentNotIssued(bytes32 documentRoot, bytes32 document);
+
+  error DocumentIsRevoked(bytes32 documentRoot, bytes32 document);
+}
diff --git a/test/CommonTest.t.sol b/test/CommonTest.t.sol
@@ -5,10 +5,9 @@ import "forge-std/Test.sol";
 
 import "../src/DocumentStore.sol";
 import "./fixtures/DocumentStoreFixture.sol";
+import "../src/OwnableDocumentStore.sol";
 
 abstract contract CommonTest is Test {
-  string public storeName = "DocumentStore Test";
-
   address public owner = vm.addr(1);
   address public issuer = vm.addr(2);
   address public revoker = vm.addr(3);
@@ -19,6 +18,8 @@ abstract contract CommonTest is Test {
 abstract contract DocumentStoreCommonTest is CommonTest {
   DocumentStore public documentStore;
 
+  string public storeName = "DocumentStore Test";
+
   function setUp() public virtual override {
     super.setUp();
 
@@ -151,7 +152,7 @@ abstract contract DocumentStore_multicall_revoke_Base is DocumentStoreCommonTest
     bulkRevokeData[1] = abi.encodeCall(IDocumentStoreBatchable.revoke, (docRoots()[0], documents()[0], proofs()[0]));
 
     // It should revert that document0 is already inactive
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InactiveDocument.selector, docRoots()[0], documents()[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InactiveDocument.selector, docRoots()[0], documents()[0]));
 
     vm.prank(revoker);
     documentStore.multicall(bulkRevokeData);
@@ -231,3 +232,24 @@ abstract contract DocumentStore_multicall_revoke_Initializer is DocumentStore_mu
     documentStore.multicall(issueData);
   }
 }
+
+abstract contract OwnableDocumentStoreCommonTest is CommonTest {
+  OwnableDocumentStore public documentStore;
+
+  string public storeName = "OwnableDocumentStore Test";
+  string public storeSymbol = "XYZ";
+
+  address public recipient = vm.addr(4);
+
+  function setUp() public virtual override {
+    super.setUp();
+
+    vm.startPrank(owner);
+
+    documentStore = new OwnableDocumentStore(storeName, storeSymbol, owner);
+    documentStore.grantRole(documentStore.ISSUER_ROLE(), issuer);
+    documentStore.grantRole(documentStore.REVOKER_ROLE(), revoker);
+
+    vm.stopPrank();
+  }
+}
diff --git a/test/DocumentStore.t.sol b/test/DocumentStore.t.sol
@@ -96,7 +96,7 @@ contract DocumentStore_issue_Test is DocumentStoreCommonTest {
     vm.startPrank(issuer);
     documentStore.issue(docHash);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentExists.selector, bytes32(docHash)));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.DocumentExists.selector, bytes32(docHash)));
 
     documentStore.issue(docHash);
     vm.stopPrank();
@@ -110,14 +110,14 @@ contract DocumentStore_issue_Test is DocumentStoreCommonTest {
     documentStore.revoke(docHash);
     vm.stopPrank();
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentExists.selector, bytes32(docHash)));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.DocumentExists.selector, bytes32(docHash)));
 
     vm.prank(issuer);
     documentStore.issue(docHash);
   }
 
   function testIssueZeroDocument() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
 
     vm.prank(issuer);
     documentStore.issue(0x0);
@@ -173,7 +173,7 @@ contract DocumentStore_multicall_Issue_Test is DocumentStoreCommonTest {
     docHashes[1] = docHashes[0];
     bulkIssueData[1] = abi.encodeCall(IDocumentStoreBatchable.issue, (docHashes[0]));
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentExists.selector, bytes32(docHashes[1])));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.DocumentExists.selector, bytes32(docHashes[1])));
 
     vm.prank(issuer);
     documentStore.multicall(bulkIssueData);
@@ -186,7 +186,7 @@ contract DocumentStore_isIssued_Test is DocumentStoreBatchable_Initializer {
   }
 
   function testIsRootIssuedWithZeroRoot() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
 
     documentStore.isIssued(0x0);
   }
@@ -202,22 +202,22 @@ contract DocumentStore_isIssued_Test is DocumentStoreBatchable_Initializer {
   }
 
   function testIsIssuedWithInvalidProof() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[1]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[1]));
 
     documentStore.isIssued(docRoot, documents[1], proofs[0]);
   }
 
   function testIsIssuedWithInvalidEmptyProof() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[1]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[1]));
 
     documentStore.isIssued(docRoot, documents[1], new bytes32[](0));
   }
 
   function testIsIssuedWithZeroDocument() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isIssued(0x0, documents[0], proofs[0]);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isIssued(docRoot, 0x0, proofs[0]);
   }
 }
@@ -280,7 +280,7 @@ contract DocumentStore_revoke_Test is DocumentStore_Initializer {
   }
 
   function testRevokeWithZeroRoot() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
 
     vm.prank(revoker);
     documentStore.revoke(0x0);
@@ -290,7 +290,7 @@ contract DocumentStore_revoke_Test is DocumentStore_Initializer {
     vm.startPrank(revoker);
     documentStore.revoke(targetDoc);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InactiveDocument.selector, targetDoc, targetDoc));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InactiveDocument.selector, targetDoc, targetDoc));
 
     documentStore.revoke(targetDoc);
     vm.stopPrank();
@@ -299,7 +299,9 @@ contract DocumentStore_revoke_Test is DocumentStore_Initializer {
   function testRevokeNotIssuedRootRevert() public {
     bytes32 nonIssuedRoot = "0x1234";
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentNotIssued.selector, nonIssuedRoot, nonIssuedRoot));
+    vm.expectRevert(
+      abi.encodeWithSelector(IDocumentStoreErrors.DocumentNotIssued.selector, nonIssuedRoot, nonIssuedRoot)
+    );
 
     vm.prank(revoker);
     documentStore.revoke(nonIssuedRoot);
@@ -338,15 +340,17 @@ contract DocumentStore_isRevoked_Test is DocumentStore_Initializer {
   }
 
   function testIsRevokedWithZeroDocumentRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
 
     documentStore.isRevoked(0x0);
   }
 
   function testIsRevokedWithNotIssuedDocumentRevert() public {
     bytes32 notIssuedRoot = "0x1234";
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentNotIssued.selector, notIssuedRoot, notIssuedRoot));
+    vm.expectRevert(
+      abi.encodeWithSelector(IDocumentStoreErrors.DocumentNotIssued.selector, notIssuedRoot, notIssuedRoot)
+    );
 
     documentStore.isRevoked(notIssuedRoot);
   }
@@ -369,14 +373,16 @@ contract DocumentStore_isActive_Test is DocumentStore_Initializer {
   }
 
   function testIsActiveWithZeroDocumentRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isActive(0x0);
   }
 
   function testIsActiveWithNotIssuedDocumentRevert() public {
     bytes32 notIssuedDoc = "0x1234";
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentNotIssued.selector, notIssuedDoc, notIssuedDoc));
+    vm.expectRevert(
+      abi.encodeWithSelector(IDocumentStoreErrors.DocumentNotIssued.selector, notIssuedDoc, notIssuedDoc)
+    );
 
     documentStore.isActive(notIssuedDoc);
   }

diff --git a/test/DocumentStoreBatchable.t.sol b/test/DocumentStoreBatchable.t.sol
@@ -60,14 +60,14 @@ contract DocumentStoreBatchable_revoke_Test is DocumentStoreBatchable_Initialize
   }
 
   function testRevokeWithInvalidProofRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[0]));
 
     vm.prank(revoker);
     documentStore.revoke(docRoot, documents[0], proofs[1]);
   }
 
   function testRevokeWithEmptyProofRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[0]));
 
     vm.prank(revoker);
     documentStore.revoke(docRoot, documents[0], new bytes32[](0));
@@ -76,10 +76,10 @@ contract DocumentStoreBatchable_revoke_Test is DocumentStoreBatchable_Initialize
   function testRevokeWithZeroDocument() public {
     vm.startPrank(revoker);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.revoke(0x0, documents[0], proofs[0]);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.revoke(docRoot, 0x0, proofs[0]);
 
     vm.stopPrank();
@@ -90,7 +90,7 @@ contract DocumentStoreBatchable_revoke_Test is DocumentStoreBatchable_Initialize
 
     documentStore.revoke(docRoot, documents[0], proofs[0]);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InactiveDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InactiveDocument.selector, docRoot, documents[0]));
 
     documentStore.revoke(docRoot, documents[0], proofs[0]);
 
@@ -102,7 +102,7 @@ contract DocumentStoreBatchable_revoke_Test is DocumentStoreBatchable_Initialize
 
     documentStore.revoke(docRoot);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InactiveDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InactiveDocument.selector, docRoot, documents[0]));
 
     documentStore.revoke(docRoot, documents[0], proofs[0]);
 
@@ -112,7 +112,9 @@ contract DocumentStoreBatchable_revoke_Test is DocumentStoreBatchable_Initialize
   function testRevokeNotIssuedDocumentRevert() public {
     bytes32 nonIssuedRoot = "0x1234";
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentNotIssued.selector, nonIssuedRoot, nonIssuedRoot));
+    vm.expectRevert(
+      abi.encodeWithSelector(IDocumentStoreErrors.DocumentNotIssued.selector, nonIssuedRoot, nonIssuedRoot)
+    );
 
     vm.prank(revoker);
     documentStore.revoke(nonIssuedRoot);
@@ -154,29 +156,29 @@ contract DocumentStoreBatchable_isRevoked_Test is DocumentStoreBatchable_Initial
   }
 
   function testIsRevokedWithInvalidProofRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[0]));
 
     documentStore.isRevoked(docRoot, documents[0], proofs[1]);
   }
 
   function testIsRevokedWithEmptyProofRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[0]));
 
     documentStore.isRevoked(docRoot, documents[0], new bytes32[](0));
   }
 
   function testIsRevokedWithZeroDocumentRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isRevoked(docRoot, 0x0, proofs[0]);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isRevoked(0x0, documents[0], proofs[0]);
   }
 
   function testIsRevokedWithNotIssuedDocumentRevert() public {
     bytes32 notIssuedDoc = "0x1234";
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, notIssuedDoc));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, notIssuedDoc));
 
     documentStore.isRevoked(docRoot, notIssuedDoc, proofs[0]);
   }
@@ -199,29 +201,31 @@ contract DocumentStoreBatchable_isActive_Test is DocumentStoreBatchable_Initiali
   }
 
   function testIsActiveWithInvalidProofRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[0]));
 
     documentStore.isActive(docRoot, documents[0], proofs[1]);
   }
 
   function testIsActiveWithEmptyProofRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.InvalidDocument.selector, docRoot, documents[0]));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.InvalidDocument.selector, docRoot, documents[0]));
 
     documentStore.isActive(docRoot, documents[0], new bytes32[](0));
   }
 
   function testIsActiveWithZeroDocumentRevert() public {
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isActive(docRoot, 0x0, proofs[0]);
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.ZeroDocument.selector));
+    vm.expectRevert(abi.encodeWithSelector(IDocumentStoreErrors.ZeroDocument.selector));
     documentStore.isActive(0x0, documents[0], proofs[0]);
   }
 
   function testIsActiveWithNotIssuedDocumentRevert(bytes32 notIssuedDoc) public {
     vm.assume(notIssuedDoc != docRoot && notIssuedDoc != bytes32(0));
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentNotIssued.selector, notIssuedDoc, notIssuedDoc));
+    vm.expectRevert(
+      abi.encodeWithSelector(IDocumentStoreErrors.DocumentNotIssued.selector, notIssuedDoc, notIssuedDoc)
+    );
 
     documentStore.isActive(notIssuedDoc, notIssuedDoc, new bytes32[](0));
   }
@@ -233,7 +237,9 @@ contract DocumentStoreBatchable_isActive_Test is DocumentStoreBatchable_Initiali
     bytes32[] memory proofs = new bytes32[](1);
     proofs[0] = 0x9800b3feae3c44fe4263f6cbb2d8dd529c26c3a1c3ca7208a30cfa5efbc362e7;
 
-    vm.expectRevert(abi.encodeWithSelector(IDocumentStore.DocumentNotIssued.selector, notIssuedRoot, notIssuedDoc));
+    vm.expectRevert(
+      abi.encodeWithSelector(IDocumentStoreErrors.DocumentNotIssued.selector, notIssuedRoot, notIssuedDoc)
+    );
 
     documentStore.isActive(notIssuedRoot, notIssuedDoc, proofs);
   }