Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GMS-1362] Remove setOperatorAllowlistRegistry from preset nft contracts #162

Merged
merged 2 commits into from
Jan 16, 2024
Merged

[GMS-1362] Remove setOperatorAllowlistRegistry from preset nft contracts #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69cd677
wip
allan-almeida-imtbl Jan 16, 2024
2efe0e0
pass tests
allan-almeida-imtbl Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions clients/erc721-mint-by-id.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
}
15 changes: 0 additions & 15 deletions clients/erc721.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
}
8 changes: 0 additions & 8 deletions contracts/allowlist/OperatorAllowlistEnforced.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 0 additions & 33 deletions test/allowlist/AllowlistERC721TransfersApprovals.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
33 changes: 0 additions & 33 deletions test/allowlist/HybridApproval.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 0 additions & 5 deletions test/royalty-enforcement/RoyaltyMarketplace.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading