chore: prevent direct calls to implementation contract (#194)

* feat: disable initializers and set paused state for implementation contracts

* test: add deploy guarantee tests for interaction with implementation contracts

* chore: add buildId and implementationAddress to deployStore script

* test: reset initialized state on store of deployed contract for foundry tests

* test: optmize deployUninitialized function

* test: test if is possible to mint in implementation contract
This commit is contained in:
Felipe Mendes 2023-04-04 16:19:06 -03:00 committed by GitHub
parent 9e202f0a1f
commit 2225b301ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 138 additions and 2 deletions

View File

@ -53,6 +53,18 @@ contract FleekERC721 is
mapping(uint256 => address) private _tokenVerifier;
mapping(uint256 => bool) private _tokenVerified;
/**
* @dev This constructor sets the state of implementation contract to paused
* and disable initializers, not allowing interactions with the implementation
* contracts.
*/
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_setPausable(true);
_pause();
_disableInitializers();
}
/**
* @dev Initializes the contract by setting a `name` and a `symbol` to the token collection.
*/

View File

@ -1,3 +1,4 @@
const { getImplementationAddress } = require('@openzeppelin/upgrades-core');
const { writeFile } = require('./file');
const { existsSync } = require('fs');
const path = require('path');
@ -38,9 +39,16 @@ const deployStore = async (network, contractName, contract) => {
const { buildId, solcInput, abi, bytecode, metadata, storageLayout } =
await getBuildData(contractName);
const implementationAddress = await getImplementationAddress(
hre.network.provider,
contract.address
);
const data = {
buildId,
timestamp: new Date().toLocaleString('en-US'),
address: contract.address,
implementationAddress,
transactionHash: contract.deployTransaction.hash,
args: contract.deployTransaction.args,
gasPrice: contract.deployTransaction.gasPrice.toNumber(),

View File

@ -23,7 +23,7 @@ contract Test_FleekERC721_Deploy is Test_FleekERC721_Base {
}
function testFuzz_nameAndSymbol(string memory _name, string memory _symbol) public {
CuT = new FleekERC721();
CuT = deployUninitialized();
CuT.initialize(_name, _symbol, new uint256[](0));
assertEq(CuT.name(), _name);

View File

@ -48,8 +48,15 @@ abstract contract Test_FleekERC721_Base is Test, Test_FleekERC721_Assertions {
FleekERC721 internal CuT; // Contract Under Test
address internal deployer;
function deployUninitialized() internal returns (FleekERC721) {
FleekERC721 _contract = new FleekERC721();
vm.store(address(_contract), bytes32(0), bytes32(0)); // Overrides `_initialized` and `_initializing` states
return _contract;
}
function baseSetUp() internal {
CuT = new FleekERC721();
vm.prank(address(CuT));
CuT = deployUninitialized();
CuT.initialize("Test Contract", "FLKAPS", new uint256[](0));
deployer = address(this);
}

View File

@ -0,0 +1,109 @@
import { expect } from 'chai';
import * as hre from 'hardhat';
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
import deploy from '../../../scripts/deploy';
import { getImplementationAddress } from '@openzeppelin/upgrades-core';
import { Contract } from 'ethers';
import { loadFixture } from '@nomicfoundation/hardhat-network-helpers';
import { Errors, TestConstants } from '../contracts/FleekERC721/helpers';
const taskArgs = {
newProxyInstance: false,
name: 'FleekNFAs',
symbol: 'FLKNFA',
billing: [],
};
const getImplementationContract = async (
proxyAddress: string
): Promise<Contract> => {
const implementationAddress = await getImplementationAddress(
hre.network.provider,
proxyAddress
);
return hre.ethers.getContractAt('FleekERC721', implementationAddress);
};
const deployFixture = async () => {
const [owner] = await hre.ethers.getSigners();
const proxy = (await deploy(taskArgs, hre)) as Contract;
const implementation = await getImplementationContract(proxy.address);
return { proxy, implementation, owner };
};
describe('Deploy', () => {
let fixture: Awaited<ReturnType<typeof deployFixture>>;
// Suppress console.log
const logger = console.log;
before(() => {
console.log = () => undefined;
});
after(() => {
console.log = logger;
});
// --------------------
beforeEach(async () => {
fixture = await loadFixture(deployFixture);
});
it('should deploy the contract', async () => {
const { proxy, implementation } = fixture;
expect(proxy.address).to.be.a('string');
expect(implementation.address).to.be.a('string');
expect(proxy.address).to.be.not.equal(implementation.address);
});
it('should have proxy unpaused and implementation paused', async () => {
const { proxy, implementation } = fixture;
expect(await proxy.isPaused()).to.be.false;
expect(await implementation.isPaused()).to.be.true;
});
it('should not allow initialize implementation contract', async () => {
const { implementation } = fixture;
await expect(
implementation.initialize(
taskArgs.name,
taskArgs.symbol,
taskArgs.billing
)
).to.be.revertedWith('Initializable: contract is already initialized');
});
it('should have owner on proxy but not on implementation', async () => {
const { proxy, implementation, owner } = fixture;
expect(await proxy.hasCollectionRole(0, owner.address)).to.be.true;
expect(await implementation.hasCollectionRole(0, owner.address)).to.be
.false;
});
it('should not allow mint on implementation contract', async () => {
const { implementation, owner } = fixture;
await expect(
implementation.mint(
owner.address,
TestConstants.MintParams.name,
TestConstants.MintParams.description,
TestConstants.MintParams.externalUrl,
TestConstants.MintParams.ens,
TestConstants.MintParams.commitHash,
TestConstants.MintParams.gitRepository,
TestConstants.MintParams.logo,
TestConstants.MintParams.color,
TestConstants.MintParams.accessPointAutoApprovalSettings,
owner.address
)
).to.be.revertedWithCustomError(implementation, Errors.ContractIsPaused);
});
});