feat: Implement _disableInitializers in constructor (#74)

* feat: Use _disableInitializers in constructor

* feat: Add RentalsProxy for testing purposes

* feat: Add proxy in tests

* chore: Automine true in afterEach

* fix: Tests

* chore: Add test to check that the implementation cannot be initialized

* chore: Add comment

* chore: Update test

contracts/Rentals.sol

@@ -130,6 +130,12 @@ contract Rentals is
         bytes _signature
     );
 
+    constructor() {
+        // Prevents the implementation to be initialized.
+        // Initialization can only be done through a Proxy.
+        _disableInitializers();
+    }
+
     /// @notice Initialize the contract.
     /// @dev This method should be called as soon as the contract is deployed.
     /// Using this method in favor of a constructor allows the implementation of various kinds of proxies.

contracts/mocks/RentalsProxy.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.7;
+
+import "@openzeppelin/contracts/proxy/Proxy.sol";
+
+/// @dev Contract used to test Rentals behind a proxy
+/// Implementation based on https://eips.ethereum.org/EIPS/eip-1967
+contract RentalsProxy is Proxy {
+    /// @dev This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1
+    bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
+
+    constructor(address _impl) {
+        StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = _impl;
+    }
+
+    function _implementation() internal view virtual override returns (address) {
+        return StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value;
+    }
+}
+
+library StorageSlot {
+    struct AddressSlot {
+        address value;
+    }
+
+    function getAddressSlot(bytes32 slot) internal pure returns (AddressSlot storage r) {
+        assembly {
+            r.slot := slot
+        }
+    }
+}

test/Rentals.spec.ts

@@ -2,7 +2,7 @@ import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
 import { expect } from 'chai'
 import { BigNumber, BigNumberish } from 'ethers'
 import { ethers, network } from 'hardhat'
-import { EstateRegistry, ExtendedRentals, LANDRegistry, MANAToken, ReentrantERC721, Rentals } from '../typechain-types'
+import { EstateRegistry, ExtendedRentals__factory, LANDRegistry, MANAToken, ReentrantERC721, Rentals, Rentals__factory } from '../typechain-types'
 import {
   daysToSeconds,
   ether,
@@ -63,7 +63,11 @@ describe('Rentals', () => {
 
     // Deploy Rentals contract
     const RentalsFactory = await ethers.getContractFactory('Rentals')
-    rentals = await RentalsFactory.connect(deployer).deploy()
+    const rentalsImpl = await RentalsFactory.connect(deployer).deploy()
+    const RentalsProxyFactory = await ethers.getContractFactory('RentalsProxy')
+    const rentalsProxy = await RentalsProxyFactory.connect(deployer).deploy(rentalsImpl.address)
+
+    rentals = await ethers.getContractAt('Rentals', rentalsProxy.address)
 
     // Deploy and Prepare LANDRegistry
     const LANDRegistryFactory = await ethers.getContractFactory('LANDRegistry')
@@ -168,6 +172,7 @@ describe('Rentals', () => {
 
   afterEach(async () => {
     await network.provider.send('evm_revert', [snapshotId])
+    await network.provider.send('evm_setAutomine', [true])
   })
 
   describe('initialize', () => {
@@ -192,8 +197,11 @@ describe('Rentals', () => {
     })
 
     it('should set eip712 name and version hashes', async () => {
-      const ExtendedRentals = await ethers.getContractFactory('ExtendedRentals')
-      const rentals: ExtendedRentals = await ExtendedRentals.connect(deployer).deploy()
+      const ExtendedRentalsFactory = await ethers.getContractFactory('ExtendedRentals')
+      const rentalsImpl = await ExtendedRentalsFactory.connect(deployer).deploy()
+      const RentalsProxyFactory = await ethers.getContractFactory('RentalsProxy')
+      const rentalsProxy = await RentalsProxyFactory.connect(deployer).deploy(rentalsImpl.address)
+      const rentals = await ethers.getContractAt('ExtendedRentals', rentalsProxy.address)
 
       const zeroBytes32 = '0x0000000000000000000000000000000000000000000000000000000000000000'
 
@@ -215,6 +223,15 @@ describe('Rentals', () => {
         'Initializable: contract is already initialized'
       )
     })
+
+    it('should revert when trying to initialize the implementation', async () => {
+      const RentalsFactory = await ethers.getContractFactory('Rentals')
+      const rentalsImpl = await RentalsFactory.connect(deployer).deploy()
+
+      await expect(rentalsImpl.initialize(owner.address, mana.address, collector.address, fee)).to.be.revertedWith(
+        'Initializable: contract is already initialized'
+      )
+    })
   })
 
   describe('setFeeCollector', () => {