[GMS-1362] Remove setOperatorAllowlistRegistry from preset nft contra…

…cts (#162) * wip * pass tests
immutable · Jan 16, 2024 · 70debe5 · 70debe5
1 parent 1cb96e9
commit 70debe5
Show file tree

Hide file tree

Showing 6 changed files with 0 additions and 109 deletions.
diff --git a/clients/erc721-mint-by-id.ts b/clients/erc721-mint-by-id.ts
@@ -542,19 +542,4 @@ export class ERC721MintByIDClient {
       ...overrides,
     });
   }
-
-  /**
-   * @returns a populated transaction for the setOperatorAllowlistRegistry contract function
-   */
-  public async populateSetOperatorAllowlistRegistry(
-    _operatorAllowlist: PromiseOrValue<string>,
-    overrides: Overrides & {
-      from?: PromiseOrValue<string>;
-    } = {}
-  ): Promise<PopulatedTransaction> {
-    return await this.contract.populateTransaction.setOperatorAllowlistRegistry(_operatorAllowlist, {
-      ...defaultGasOverrides,
-      ...overrides,
-    });
-  }
 }
diff --git a/clients/erc721.ts b/clients/erc721.ts
@@ -655,19 +655,4 @@ export class ERC721Client {
       ...overrides,
     });
   }
-
-  /**
-   * @returns a populated transaction for the setOperatorAllowlistRegistry contract function
-   */
-  public async populateSetOperatorAllowlistRegistry(
-    _operatorAllowlist: PromiseOrValue<string>,
-    overrides: Overrides & {
-      from?: PromiseOrValue<string>;
-    } = {}
-  ): Promise<PopulatedTransaction> {
-    return await this.contract.populateTransaction.setOperatorAllowlistRegistry(_operatorAllowlist, {
-      ...defaultGasOverrides,
-      ...overrides,
-    });
-  }
 }
diff --git a/contracts/allowlist/OperatorAllowlistEnforced.sol b/contracts/allowlist/OperatorAllowlistEnforced.sol
@@ -87,14 +87,6 @@ abstract contract OperatorAllowlistEnforced is AccessControlEnumerable, Operator
 
     ///     =====  External functions  =====
 
-    /**
-     * @notice Allows admin to set the operator allowlist the calling contract will interface with
-     * @param _operatorAllowlist the address of the Allowlist registry
-     */
-    function setOperatorAllowlistRegistry(address _operatorAllowlist) public onlyRole(DEFAULT_ADMIN_ROLE) {
-        _setOperatorAllowlistRegistry(_operatorAllowlist);
-    }
-
     /**
      * @notice ERC-165 interface support
      * @param interfaceId The interface identifier, which is a 4-byte selector.

diff --git a/test/allowlist/AllowlistERC721TransfersApprovals.test.ts b/test/allowlist/AllowlistERC721TransfersApprovals.test.ts
@@ -45,33 +45,6 @@ describe("Allowlisted ERC721 Transfers", function () {
     it("Should have operatorAllowlist set upon deployment", async function () {
       expect(await erc721.operatorAllowlist()).to.equal(operatorAllowlist.address);
     });
-
-    it("Should not allow contracts that do not implement the IOperatorAllowlist to be set", async function () {
-      // Deploy another contract that implements IERC165, but not IOperatorAllowlist
-      const factory = await ethers.getContractFactory("ImmutableERC721MintByID");
-      const erc721Two = await factory.deploy(
-        owner.address,
-        "",
-        "",
-        "",
-        "",
-        operatorAllowlist.address,
-        owner.address,
-        0
-      );
-
-      await expect(erc721.connect(owner).setOperatorAllowlistRegistry(erc721Two.address)).to.be.revertedWith(
-        "AllowlistDoesNotImplementIOperatorAllowlist"
-      );
-    });
-
-    it("Should not allow a non-admin to access the function to update the registry", async function () {
-      await expect(
-        erc721.connect(registrar).setOperatorAllowlistRegistry(operatorAllowlist.address)
-      ).to.be.revertedWith(
-        "AccessControl: account 0x3c44cdddb6a900fa2b585dd299e03d12fa4293bc is missing role 0x0000000000000000000000000000000000000000000000000000000000000000"
-      );
-    });
   });
 
   describe("Approvals", function () {
@@ -128,9 +101,6 @@ describe("Allowlisted ERC721 Transfers", function () {
   });
 
   describe("Transfers", function () {
-    beforeEach(async function () {
-      await erc721.connect(owner).setOperatorAllowlistRegistry(operatorAllowlist.address);
-    });
     it("Should freely allow transfers between EOAs", async function () {
       await erc721.connect(owner).grantMinterRole(accs[0].address);
       await erc721.connect(owner).grantMinterRole(accs[1].address);
@@ -214,9 +184,6 @@ describe("Allowlisted ERC721 Transfers", function () {
   });
 
   describe("Malicious Contracts", function () {
-    beforeEach(async function () {
-      await erc721.connect(owner).setOperatorAllowlistRegistry(operatorAllowlist.address);
-    });
     // The EOA disguise attack vector is a where a pre-computed CREATE2 deterministic address is disguised as an EOA.
     // By virtue of this, approvals and transfers to this address will pass. We need to catch actions from this address
     // once it is deployed.

diff --git a/test/allowlist/HybridApproval.test.ts b/test/allowlist/HybridApproval.test.ts
@@ -43,33 +43,6 @@ describe("Royalty Checks with Hybrid ERC721", function () {
     it("Should have operatorAllowlist set upon deployment", async function () {
       expect(await erc721.operatorAllowlist()).to.equal(operatorAllowlist.address);
     });
-
-    it("Should not allow contracts that do not implement the IOperatorAllowlist to be set", async function () {
-      // Deploy another contract that implements IERC165, but not IOperatorAllowlist
-      const factory = await ethers.getContractFactory("ImmutableERC721");
-      const erc721Two = await factory.deploy(
-        owner.address,
-        "",
-        "",
-        "",
-        "",
-        operatorAllowlist.address,
-        owner.address,
-        0
-      );
-
-      await expect(erc721.connect(owner).setOperatorAllowlistRegistry(erc721Two.address)).to.be.revertedWith(
-        "AllowlistDoesNotImplementIOperatorAllowlist"
-      );
-    });
-
-    it("Should not allow a non-admin to access the function to update the registry", async function () {
-      await expect(
-        erc721.connect(registrar).setOperatorAllowlistRegistry(operatorAllowlist.address)
-      ).to.be.revertedWith(
-        "AccessControl: account 0x3c44cdddb6a900fa2b585dd299e03d12fa4293bc is missing role 0x0000000000000000000000000000000000000000000000000000000000000000"
-      );
-    });
   });
 
   describe("Approvals", function () {
@@ -124,9 +97,6 @@ describe("Royalty Checks with Hybrid ERC721", function () {
   });
 
   describe("Transfers", function () {
-    beforeEach(async function () {
-      await erc721.connect(owner).setOperatorAllowlistRegistry(operatorAllowlist.address);
-    });
     it("Should freely allow transfers between EOAs", async function () {
       const first = await erc721.mintBatchByQuantityThreshold();
       await erc721.connect(minter).mintByQuantity(accs[0].address, 1);
@@ -214,9 +184,6 @@ describe("Royalty Checks with Hybrid ERC721", function () {
   });
 
   describe("Malicious Contracts", function () {
-    beforeEach(async function () {
-      await erc721.connect(owner).setOperatorAllowlistRegistry(operatorAllowlist.address);
-    });
     // The EOA disguise attack vector is a where a pre-computed CREATE2 deterministic address is disguised as an EOA.
     // By virtue of this, approvals and transfers to this address will pass. We need to catch actions from this address
     // once it is deployed.

diff --git a/test/royalty-enforcement/RoyaltyMarketplace.test.ts b/test/royalty-enforcement/RoyaltyMarketplace.test.ts
@@ -64,11 +64,6 @@ describe("Marketplace Royalty Enforcement", function () {
   });
 
   describe("Royalties", function () {
-    it("Should set a valid OperatorAllowlist registry", async function () {
-      await erc721.connect(owner).setOperatorAllowlistRegistry(operatorAllowlist.address);
-      expect(await erc721.operatorAllowlist()).to.be.equal(operatorAllowlist.address);
-    });
-
     it("Should allow a marketplace contract to be Allowlisted", async function () {
       await operatorAllowlist.connect(registrar).addAddressToAllowlist([mockMarketplace.address]);
       expect(await operatorAllowlist.isAllowlisted(mockMarketplace.address)).to.be.equal(true);