fix: add parameter ('provider') to the custom error ProviderAlreadyEx…

…ists - added test cases to check that these errors are thrown - adjust test case for GitcoinPassportDecoder to redeploy contract before every test
passportxyz · Nov 9, 2023 · 30c315a · 30c315a
1 parent 9abea2d
commit 30c315a
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 8 deletions.
diff --git a/contracts/GitcoinPassportDecoder.sol b/contracts/GitcoinPassportDecoder.sol
@@ -40,7 +40,7 @@ contract GitcoinPassportDecoder is
   bytes32 public schemaUID;
 
   // Errors
-  error ProviderAlreadyExists();
+  error ProviderAlreadyExists(string provider);
 
   function initialize() public initializer {
     __Ownable_init();
@@ -88,10 +88,10 @@ contract GitcoinPassportDecoder is
   function addProviders(string[] memory providers) public onlyOwner {
     for (uint256 i = 0; i < providers.length; ) {
       if (savedProviders[providers[i]] == 1) {
-        revert ProviderAlreadyExists();
+        revert ProviderAlreadyExists(providers[i]);
       }
 
-      providerVersions[currentVersion].push(providers[i]);  
+      providerVersions[currentVersion].push(providers[i]);
       savedProviders[providers[i]] = 1;
 
       unchecked {
@@ -226,4 +226,3 @@ contract GitcoinPassportDecoder is
     return passportMemoryArray;
   }
 }
-
diff --git a/test/GitcoinPassportDecoder.ts b/test/GitcoinPassportDecoder.ts
@@ -83,8 +83,9 @@ const easEncodeInvalidStamp = () => {
   return encodedData;
 };
 
-describe.only("GitcoinPassportDecoder", function () {
-  this.beforeAll(async function () {
+describe("GitcoinPassportDecoder", function () {
+  this.beforeEach(async function () {
+    // this.beforeAll(async function () {
     const [ownerAccount, iamAcct, recipientAccount, otherAccount] =
       await ethers.getSigners();
 
@@ -233,9 +234,7 @@ describe.only("GitcoinPassportDecoder", function () {
     await this.gitcoinPassportDecoder.setEASAddress(EAS_CONTRACT_ADDRESS);
     await this.gitcoinPassportDecoder.setGitcoinResolver(this.resolverAddress);
     await this.gitcoinPassportDecoder.setSchemaUID(this.passportSchemaUID);
-  });
 
-  this.beforeEach(async function () {
     this.passport.nonce = await this.gitcoinVerifier.recipientNonces(
       this.passport.multiAttestationRequest[0].data[0].recipient
     );
@@ -270,6 +269,42 @@ describe.only("GitcoinPassportDecoder", function () {
           .addProviders(["NewStamp3"])
       ).to.be.revertedWith("Ownable: caller is not the owner");
     });
+
+    it("should throw an error when trying to add the same provider twice in different function calls", async function () {
+      const providersForCall1 = ["NewStamp1", "NewStamp2"];
+      const providersForCall2 = ["NewStamp3", "NewStamp2"];
+
+      await this.gitcoinPassportDecoder
+        .connect(this.owner)
+        .addProviders(providersForCall1);
+
+      await expect(
+        this.gitcoinPassportDecoder
+          .connect(this.owner)
+          .addProviders(providersForCall2)
+      ).to.be.revertedWithCustomError(
+        this.gitcoinPassportDecoder,
+        "ProviderAlreadyExists"
+      );
+    });
+
+    it("should throw an error when trying to add the same provider twice in the same function call", async function () {
+      const providersForCall1 = [
+        "NewStamp1",
+        "NewStamp2",
+        "NewStamp3",
+        "NewStamp2",
+      ];
+
+      await expect(
+        this.gitcoinPassportDecoder
+          .connect(this.owner)
+          .addProviders(providersForCall1)
+      ).to.be.revertedWithCustomError(
+        this.gitcoinPassportDecoder,
+        "ProviderAlreadyExists"
+      );
+    });
   });
 
   describe("Decoding Passports", async function () {
@@ -284,6 +319,10 @@ describe.only("GitcoinPassportDecoder", function () {
 
       const { v, r, s } = ethers.Signature.from(signature);
 
+      await this.gitcoinPassportDecoder
+        .connect(this.owner)
+        .addProviders(providers);
+
       // Submit attestations
       const verifiedPassport = await this.gitcoinVerifier.verifyAndAttest(
         this.passport,
@@ -320,6 +359,10 @@ describe.only("GitcoinPassportDecoder", function () {
 
       const { v, r, s } = ethers.Signature.from(signature);
 
+      await this.gitcoinPassportDecoder
+        .connect(this.owner)
+        .addProviders(providers);
+
       // Submit attestations
       const verifiedPassport = await this.gitcoinVerifier.verifyAndAttest(
         this.passport,