From 7493fdda96c7a8454648bb4f3b8b5e034b0d8a07 Mon Sep 17 00:00:00 2001 From: superical Date: Fri, 23 Dec 2022 14:32:11 +0800 Subject: [PATCH] fix: title escrow factory create call (#153) --- contracts/TitleEscrowFactory.sol | 6 +- .../interfaces/TitleEscrowFactoryErrors.sol | 6 ++ contracts/mocks/DummyContractMock.sol | 9 +++ test/TitleEscrowFactory.test.ts | 80 +++++++++++++------ 4 files changed, 77 insertions(+), 24 deletions(-) create mode 100644 contracts/interfaces/TitleEscrowFactoryErrors.sol create mode 100644 contracts/mocks/DummyContractMock.sol diff --git a/contracts/TitleEscrowFactory.sol b/contracts/TitleEscrowFactory.sol index 220af2bf..9e952535 100644 --- a/contracts/TitleEscrowFactory.sol +++ b/contracts/TitleEscrowFactory.sol @@ -4,8 +4,9 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/proxy/Clones.sol"; import "./TitleEscrow.sol"; import "./interfaces/ITitleEscrowFactory.sol"; +import "./interfaces/TitleEscrowFactoryErrors.sol"; -contract TitleEscrowFactory is ITitleEscrowFactory { +contract TitleEscrowFactory is ITitleEscrowFactory, TitleEscrowFactoryErrors { address public override implementation; constructor() { @@ -13,6 +14,9 @@ contract TitleEscrowFactory is ITitleEscrowFactory { } function create(uint256 tokenId) external override returns (address) { + if (msg.sender.code.length == 0) { + revert CreateCallerNotContract(); + } bytes32 salt = keccak256(abi.encodePacked(msg.sender, tokenId)); address titleEscrow = Clones.cloneDeterministic(implementation, salt); TitleEscrow(titleEscrow).initialize(msg.sender, tokenId); diff --git a/contracts/interfaces/TitleEscrowFactoryErrors.sol b/contracts/interfaces/TitleEscrowFactoryErrors.sol new file mode 100644 index 00000000..218aa623 --- /dev/null +++ b/contracts/interfaces/TitleEscrowFactoryErrors.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +interface TitleEscrowFactoryErrors { + error CreateCallerNotContract(); +} diff --git a/contracts/mocks/DummyContractMock.sol b/contracts/mocks/DummyContractMock.sol new file mode 100644 index 00000000..7faccf02 --- /dev/null +++ b/contracts/mocks/DummyContractMock.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +contract DummyContractMock { + + string public name = "Dummy"; + + constructor() {} +} diff --git a/test/TitleEscrowFactory.test.ts b/test/TitleEscrowFactory.test.ts index 972d1737..ec8265c8 100644 --- a/test/TitleEscrowFactory.test.ts +++ b/test/TitleEscrowFactory.test.ts @@ -1,15 +1,14 @@ import { ethers } from "hardhat"; import { loadFixture } from "@nomicfoundation/hardhat-network-helpers"; import faker from "faker"; -import { ContractTransaction } from "ethers"; -import { TitleEscrow, TitleEscrowFactory } from "@tradetrust/contracts"; +import { ContractTransaction, Signer } from "ethers"; +import { DummyContractMock, TitleEscrow, TitleEscrowFactory } from "@tradetrust/contracts"; import { TitleEscrowCreatedEvent } from "@tradetrust/contracts/contracts/TitleEscrowFactory"; -import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers"; import { expect } from "."; import { deployEscrowFactoryFixture } from "./fixtures"; import { computeTitleEscrowAddress, getEventFromReceipt } from "../src/utils"; import { contractInterfaceId, defaultAddress } from "../src/constants"; -import { createDeployFixtureRunner, getTestUsers, TestUsers } from "./helpers"; +import { createDeployFixtureRunner, getTestUsers, impersonateAccount, TestUsers } from "./helpers"; describe("TitleEscrowFactory", async () => { let users: TestUsers; @@ -85,35 +84,70 @@ describe("TitleEscrowFactory", async () => { }); describe("Create Title Escrow Contract", () => { - let fakeRegistrySigner: SignerWithAddress; - let titleEscrowFactoryCreateTx: ContractTransaction; - let titleEscrowContract: TitleEscrow; let tokenId: string; beforeEach(async () => { tokenId = faker.datatype.hexaDecimal(64); - fakeRegistrySigner = users.others[faker.datatype.number(users.others.length - 1)]; - titleEscrowFactoryCreateTx = await titleEscrowFactory.connect(fakeRegistrySigner).create(tokenId); + }); - const receipt = await titleEscrowFactoryCreateTx.wait(); - const titleEscrowAddress = getEventFromReceipt( - receipt, - titleEscrowFactory.interface.getEventTopic("TitleEscrowCreated") - ).args.titleEscrow; + describe("Create Caller", () => { + it("should revert when calls create from an EOA", async () => { + const eoa = users.others[faker.datatype.number(users.others.length - 1)]; - titleEscrowContract = (await ethers.getContractFactory("TitleEscrow")).attach(titleEscrowAddress) as TitleEscrow; - }); + const tx = titleEscrowFactory.connect(eoa).create(tokenId); - it("should create with the correct token registry address", async () => { - const registryAddress = await titleEscrowContract.registry(); + await expect(tx).to.be.revertedWithCustomError(titleEscrowFactory, "CreateCallerNotContract"); + }); - expect(registryAddress).to.equal(fakeRegistrySigner.address); + it("should call create successfully from a contract", async () => { + const dummyContractMock = (await ( + await ethers.getContractFactory("DummyContractMock") + ).deploy()) as DummyContractMock; + const mockContractSigner = await impersonateAccount({ address: dummyContractMock.address }); + + const tx = titleEscrowFactory.connect(mockContractSigner).create(tokenId); + + await expect(tx).to.not.be.reverted; + }); }); - it("should emit TitleEscrowCreated event", async () => { - expect(titleEscrowFactoryCreateTx) - .to.emit(titleEscrowFactory, "TitleEscrowCreated") - .withArgs(titleEscrowContract.address, fakeRegistrySigner.address, tokenId); + describe("Create Title Escrow Behaviours", () => { + let mockContractSigner: Signer; + let titleEscrowFactoryCreateTx: ContractTransaction; + let titleEscrowContract: TitleEscrow; + + beforeEach(async () => { + const dummyContractMock = (await ( + await ethers.getContractFactory("DummyContractMock") + ).deploy()) as DummyContractMock; + mockContractSigner = await impersonateAccount({ address: dummyContractMock.address }); + titleEscrowFactoryCreateTx = await titleEscrowFactory.connect(mockContractSigner).create(tokenId); + + const receipt = await titleEscrowFactoryCreateTx.wait(); + const titleEscrowAddress = getEventFromReceipt( + receipt, + titleEscrowFactory.interface.getEventTopic("TitleEscrowCreated") + ).args.titleEscrow; + + titleEscrowContract = (await ethers.getContractFactory("TitleEscrow")).attach( + titleEscrowAddress + ) as TitleEscrow; + }); + + it("should create with the correct token registry address", async () => { + const registryAddress = await titleEscrowContract.registry(); + const signerAddress = await mockContractSigner.getAddress(); + + expect(registryAddress).to.equal(signerAddress); + }); + + it("should emit TitleEscrowCreated event", async () => { + const signerAddress = await mockContractSigner.getAddress(); + + expect(titleEscrowFactoryCreateTx) + .to.emit(titleEscrowFactory, "TitleEscrowCreated") + .withArgs(titleEscrowContract.address, signerAddress, tokenId); + }); }); });