feat: title escrow init owners on receive token (Open-Attestation#97)

contracts/TitleEscrow.sol

@@ -46,17 +46,10 @@ contract TitleEscrow is IERC165, ITitleEscrow, Initializable {
     _;
   }
 
-  function initialize(
-    address _registry,
-    address _beneficiary,
-    address _holder,
-    uint256 _tokenId
-  ) public initializer {
+  function initialize(address _registry, uint256 _tokenId) public initializer {
     registry = _registry;
     tokenId = _tokenId;
     active = true;
-    _setBeneficiary(_beneficiary);
-    _setHolder(_holder);
   }
 
   function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
@@ -67,21 +60,29 @@ contract TitleEscrow is IERC165, ITitleEscrow, Initializable {
     address, /* operator */
     address, /* from */
     uint256 _tokenId,
-    bytes calldata /* data */
+    bytes calldata data
   ) external override whenNotPaused whenActive returns (bytes4) {
     require(tokenId == _tokenId, "TE: Invalid token");
     require(msg.sender == address(registry), "TE: Wrong registry");
-
-    emit TokenReceived(registry, tokenId);
+    bool isMinting = false;
+    if (beneficiary == address(0) || holder == address(0)) {
+      require(data.length > 0, "TE: Empty data");
+      (address _beneficiary, address _holder) = abi.decode(data, (address, address));
+      _setBeneficiary(_beneficiary);
+      _setHolder(_holder);
+      isMinting = true;
+    }
+
+    emit TokenReceived(beneficiary, holder, isMinting, registry, tokenId);
     return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
   }
 
   function _nominateBeneficiary(address _beneficiaryNominee)
     internal
     whenNotPaused
+    whenActive
     onlyBeneficiary
     whenHoldingToken
-    whenActive
   {
     require(beneficiary != _beneficiaryNominee, "TE: Nominee is beneficiary");
     require(beneficiaryNominee != _beneficiaryNominee, "TE: Already beneficiary nominee");
@@ -97,9 +98,9 @@ contract TitleEscrow is IERC165, ITitleEscrow, Initializable {
     public
     override
     whenNotPaused
+    whenActive
     onlyHolder
     whenHoldingToken
-    whenActive
   {
     require(_beneficiaryNominee != address(0), "TE: Endorsing zero");
     require(beneficiary == holder || (beneficiaryNominee == _beneficiaryNominee), "TE: Recipient is non-nominee");
@@ -108,7 +109,7 @@ contract TitleEscrow is IERC165, ITitleEscrow, Initializable {
     beneficiaryNominee = address(0);
   }
 
-  function transferHolder(address newHolder) public override whenNotPaused onlyHolder whenHoldingToken whenActive {
+  function transferHolder(address newHolder) public override whenNotPaused whenActive onlyHolder whenHoldingToken {
     require(newHolder != address(0), "TE: Transfer to zero");
     require(holder != newHolder, "TE: Already holder");
 
@@ -136,7 +137,7 @@ contract TitleEscrow is IERC165, ITitleEscrow, Initializable {
     transferHolder(newHolder);
   }
 
-  function surrender() external override onlyBeneficiary onlyHolder whenNotPaused whenHoldingToken whenActive {
+  function surrender() external override whenNotPaused whenActive onlyBeneficiary onlyHolder whenHoldingToken {
     beneficiaryNominee = address(0);
     ITradeTrustERC721(registry).safeTransferFrom(address(this), registry, tokenId);
 

contracts/TitleEscrowFactory.sol

@@ -12,16 +12,12 @@ contract TitleEscrowFactory is ITitleEscrowFactory {
     implementation = address(new TitleEscrow());
   }
 
-  function create(
-    address beneficiary,
-    address holder,
-    uint256 tokenId
-  ) external override returns (address) {
+  function create(uint256 tokenId) external override returns (address) {
     bytes32 salt = keccak256(abi.encodePacked(msg.sender, tokenId));
     address titleEscrow = Clones.cloneDeterministic(implementation, salt);
-    TitleEscrow(titleEscrow).initialize(msg.sender, beneficiary, holder, tokenId);
+    TitleEscrow(titleEscrow).initialize(msg.sender, tokenId);
 
-    emit TitleEscrowCreated(titleEscrow, msg.sender, tokenId, beneficiary, holder);
+    emit TitleEscrowCreated(titleEscrow, msg.sender, tokenId);
 
     return titleEscrow;
   }

contracts/TradeTrustERC721Base.sol

@@ -102,8 +102,8 @@ abstract contract TradeTrustERC721Base is ITradeTrustERC721, RegistryAccess, Pau
     address holder,
     uint256 tokenId
   ) internal virtual returns (address) {
-    address newTitleEscrow = titleEscrowFactory().create(beneficiary, holder, tokenId);
-    _safeMint(newTitleEscrow, tokenId);
+    address newTitleEscrow = titleEscrowFactory().create(tokenId);
+    _safeMint(newTitleEscrow, tokenId, abi.encode(beneficiary, holder));
 
     return newTitleEscrow;
   }

contracts/interfaces/ITitleEscrow.sol

@@ -5,7 +5,7 @@ import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
 
 /// @title Title Escrow for Transferable Records
 interface ITitleEscrow is IERC721Receiver {
-  event TokenReceived(address indexed tokenRegistry, uint256 indexed tokenId);
+  event TokenReceived(address indexed beneficiary, address indexed holder, bool indexed isMinting, address tokenRegistry, uint256 tokenId);
   event BeneficiaryNomination(
     address indexed prevNominee,
     address indexed nominee,
@@ -18,12 +18,7 @@ interface ITitleEscrow is IERC721Receiver {
     address tokenRegistry,
     uint256 tokenId
   );
-  event HolderTransfer(
-    address indexed fromHolder,
-    address indexed toHolder,
-    address tokenRegistry,
-    uint256 tokenId
-  );
+  event HolderTransfer(address indexed fromHolder, address indexed toHolder, address tokenRegistry, uint256 tokenId);
   event Surrender(address indexed surrenderer, address tokenRegistry, uint256 tokenId);
   event Shred(address tokenRegistry, uint256 tokenId);
 

contracts/interfaces/ITitleEscrowFactory.sol

@@ -2,21 +2,11 @@
 pragma solidity ^0.8.0;
 
 interface ITitleEscrowFactory {
-  event TitleEscrowCreated(
-    address indexed titleEscrow,
-    address indexed tokenRegistry,
-    uint256 indexed tokenId,
-    address beneficiary,
-    address holder
-  );
+  event TitleEscrowCreated(address indexed titleEscrow, address indexed tokenRegistry, uint256 indexed tokenId);
 
   function implementation() external view returns (address);
 
-  function create(
-    address beneficiary,
-    address holder,
-    uint256 tokenId
-  ) external returns (address);
+  function create(uint256 tokenId) external returns (address);
 
   function getAddress(address tokenRegistry, uint256 tokenId) external view returns (address);
 }

test/TitleEscrow.test.ts

@@ -53,7 +53,7 @@ describe("Title Escrow", async () => {
     });
 
     it("should initialise implementation", async () => {
-      const tx = implContract.initialize(defaultAddress.Zero, users.beneficiary.address, users.holder.address, tokenId);
+      const tx = implContract.initialize(defaultAddress.Zero, tokenId);
 
       await expect(tx).to.be.revertedWith("Initializable: contract is already initialized");
     });
@@ -64,24 +64,23 @@ describe("Title Escrow", async () => {
       beforeEach(async () => {
         fakeRegistryAddress = ethers.utils.getAddress(faker.finance.ethereumAddress());
 
-        await titleEscrowContract.initialize(
-          fakeRegistryAddress,
-          users.beneficiary.address,
-          users.holder.address,
-          tokenId
-        );
+        await titleEscrowContract.initialize(fakeRegistryAddress, tokenId);
       });
 
       it("should be initialised with the correct registry address", async () => {
         expect(await titleEscrowContract.registry()).to.equal(fakeRegistryAddress);
       });
 
-      it("should initialise with the correct beneficiary address", async () => {
-        expect(await titleEscrowContract.beneficiary()).to.equal(users.beneficiary.address);
+      it("should set active as true", async () => {
+        expect(await titleEscrowContract.active()).to.be.true;
+      });
+
+      it("should keep beneficiary intact", async () => {
+        expect(await titleEscrowContract.beneficiary()).to.equal(defaultAddress.Zero);
       });
 
-      it("should initialise with the correct holder address", async () => {
-        expect(await titleEscrowContract.holder()).to.equal(users.holder.address);
+      it("should keep holder intact", async () => {
+        expect(await titleEscrowContract.holder()).to.equal(defaultAddress.Zero);
       });
 
       it("should initialise with the correct token ID", async () => {
@@ -99,15 +98,9 @@ describe("Title Escrow", async () => {
 
       beforeEach(async () => {
         fakeRegistry = (await smock.fake("TradeTrustERC721")) as FakeContract<TradeTrustERC721>;
-        fakeRegistry.ownerOf.returns(titleEscrowContract.address);
         fakeAddress = ethers.utils.getAddress(faker.finance.ethereumAddress());
 
-        await titleEscrowContract.initialize(
-          fakeRegistry.address,
-          users.beneficiary.address,
-          users.holder.address,
-          tokenId
-        );
+        await titleEscrowContract.initialize(fakeRegistry.address, tokenId);
       });
 
       it("should only be able to receive designated token ID", async () => {
@@ -128,17 +121,124 @@ describe("Title Escrow", async () => {
         await expect(tx).to.be.revertedWith("TE: Wrong registry");
       });
 
-      it("should emit TokenReceived event when successfully receiving token", async () => {
-        await users.carrier.sendTransaction({
-          to: fakeRegistry.address,
-          value: ethers.utils.parseEther("0.1"),
-        });
+      describe("onERC721Received Data", () => {
+        let data: string;
 
-        const tx = await titleEscrowContract
-          .connect(fakeRegistry.wallet)
-          .onERC721Received(fakeAddress, fakeAddress, tokenId, "0x00");
+        beforeEach(async () => {
+          data = new ethers.utils.AbiCoder().encode(
+            ["address", "address"],
+            [users.beneficiary.address, users.holder.address]
+          );
 
-        expect(tx).to.emit(titleEscrowContract, "TokenReceived").withArgs(fakeRegistry.address, tokenId);
+          await users.carrier.sendTransaction({
+            to: fakeRegistry.address,
+            value: ethers.utils.parseEther("0.1"),
+          });
+        });
+
+        describe("Minting Token Receive", () => {
+          it("should initialise beneficiary correctly on minting token receive", async () => {
+            await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+            const beneficiary = await titleEscrowContract.beneficiary();
+
+            expect(beneficiary).to.equal(users.beneficiary.address);
+          });
+
+          it("should initialise holder correctly on minting token receive", async () => {
+            await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+            const holder = await titleEscrowContract.holder();
+
+            expect(holder).to.equal(users.holder.address);
+          });
+
+          it("should emit TokenReceived event with correct values", async () => {
+            const tx = await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+
+            expect(tx)
+              .to.emit(titleEscrowContract, "TokenReceived")
+              .withArgs(users.beneficiary.address, users.holder.address, true, fakeRegistry.address, tokenId);
+          });
+
+          describe("When minting token receive is sent without data", () => {
+            it("should revert: Empty data", async () => {
+              const tx = titleEscrowContract
+                .connect(fakeRegistry.wallet)
+                .onERC721Received(fakeAddress, fakeAddress, tokenId, "0x");
+
+              await expect(tx).to.be.revertedWith("TE: Empty data");
+            });
+
+            it("should revert: Missing data", async () => {
+              const tx = titleEscrowContract
+                .connect(fakeRegistry.wallet)
+                .onERC721Received(fakeAddress, fakeAddress, tokenId, "");
+
+              await expect(tx).to.be.reverted;
+            });
+
+            it("should revert: Invalid data", async () => {
+              const tx = titleEscrowContract
+                .connect(fakeRegistry.wallet)
+                .onERC721Received(fakeAddress, fakeAddress, tokenId, "0xabcd");
+
+              await expect(tx).to.be.reverted;
+            });
+          });
+        });
+
+        describe("After Minting Token Receive", () => {
+          it("should return successfully without data after minting token receive", async () => {
+            await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+            const tx = titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, "0x");
+
+            await expect(tx).to.not.be.reverted;
+          });
+
+          it("should emit TokenReceived event with correct values", async () => {
+            await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+            const tx = await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, "0x");
+
+            expect(tx)
+              .to.emit(titleEscrowContract, "TokenReceived")
+              .withArgs(users.beneficiary.address, users.holder.address, false, fakeRegistry.address, tokenId);
+          });
+        });
+
+        describe("Beneficiary and Holder Transfer Events", () => {
+          it("should emit BeneficiaryTransfer event", async () => {
+            const tx = await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+
+            expect(tx)
+              .to.emit(titleEscrowContract, "BeneficiaryTransfer")
+              .withArgs(defaultAddress.Zero, users.beneficiary.address, fakeRegistry.address, tokenId);
+          });
+
+          it("should emit HolderTransfer event", async () => {
+            const tx = await titleEscrowContract
+              .connect(fakeRegistry.wallet)
+              .onERC721Received(fakeAddress, fakeAddress, tokenId, data);
+
+            expect(tx)
+              .to.emit(titleEscrowContract, "HolderTransfer")
+              .withArgs(defaultAddress.Zero, users.holder.address, fakeRegistry.address, tokenId);
+          });
+        });
       });
     });
 
@@ -186,12 +286,7 @@ describe("Title Escrow", async () => {
       it("should return true after being initialised", async () => {
         const fakeRegistry = (await smock.fake("TradeTrustERC721")) as FakeContract<TradeTrustERC721>;
         fakeRegistry.ownerOf.returns(titleEscrowContract.address);
-        await titleEscrowContract.initialize(
-          fakeRegistry.address,
-          users.beneficiary.address,
-          users.holder.address,
-          tokenId
-        );
+        await titleEscrowContract.initialize(fakeRegistry.address, tokenId);
 
         const res = await titleEscrowContract.active();
 
@@ -211,12 +306,7 @@ describe("Title Escrow", async () => {
           ).deploy()) as unknown as MockContract<TitleEscrow>;
           await mockTitleEscrowContract.setVariable("_initialized", false);
 
-          await mockTitleEscrowContract.initialize(
-            fakeRegistry.address,
-            users.beneficiary.address,
-            users.beneficiary.address,
-            tokenId
-          );
+          await mockTitleEscrowContract.initialize(fakeRegistry.address, tokenId);
 
           await mockTitleEscrowContract.setVariable("active", false);
           fakeRegistry.ownerOf.returns(mockTitleEscrowContract.address);