diff --git a/contracts/GitcoinPassportDecoder.sol b/contracts/GitcoinPassportDecoder.sol index 52fae9e..28f4e68 100644 --- 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 index 10260b1..e277ffa 100644 --- 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,