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.

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

feat: Implement _disableInitializers in constructor #74

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions contracts/Rentals.sol
Expand Up @@ -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.
Expand Down
32 changes: 32 additions & 0 deletions 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
}
}
}
25 changes: 21 additions & 4 deletions test/Rentals.spec.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -168,6 +172,7 @@ describe('Rentals', () => {

afterEach(async () => {
await network.provider.send('evm_revert', [snapshotId])
await network.provider.send('evm_setAutomine', [true])
})

describe('initialize', () => {
Expand All @@ -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'

Expand All @@ -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', () => {
Expand Down