From 9d168730f2e988128dfc6906a04a12e4d7520173 Mon Sep 17 00:00:00 2001 From: Drygin Date: Tue, 26 Jul 2022 20:07:04 +0300 Subject: [PATCH] TC-127 add proxy contracts --- README.md | 6 +- config.js | 2 +- contracts/AdminUpgradeableProxy.sol | 24 ++++ contracts/InstanceFactory.sol | 28 +++-- contracts/InstanceFactoryWithRegistry.sol | 31 +++-- contracts/MultipleInstanceFactory.sol | 7 -- scripts/deployInstanceFactoryWithRegistry.js | 8 +- scripts/deployMultipleInstanceFactory.js | 8 +- src/generateAddresses.js | 126 ++++++++++++------- test/factory.with.registry.test.js | 17 ++- test/multiple.instance.factory.test.js | 15 ++- 11 files changed, 188 insertions(+), 84 deletions(-) create mode 100644 contracts/AdminUpgradeableProxy.sol diff --git a/README.md b/README.md index b96b07d..80f5d1f 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,10 @@ Check config.js for actual values. With `salt` = `0x0000000000000000000000000000000000000000000000000000000047941987` address must be: -1. `MultipleInstanceFactory` - `0x49Be4c1dF49268066f97eD15fe4b4B90cc68d1A6` -2. `InstanceFactoryWithRegistry` - `0x7D4a76AF4b2d765D1D62Ac87Cc2aea0b7ECF6102` +1. `MultipleInstanceFactory` - `0x8a1BCFc608DdF6c4715277a24310B9E8ecc4c110` +2. `MultipleInstanceFactory proxy` - `0x0F9AE1d1ABbDd441B813ada0B038df130b2a6A86` +3. `InstanceFactoryWithRegistry` - `0x557fB18B66088728Ea975136de46714983aa4f2E` +4. `InstanceFactoryWithRegistry proxy` - `0x4D1b1c294dA4D14aC0e0Eed7BcD4Db3fe2bDe4C3` Check addresses with current config: diff --git a/config.js b/config.js index 58fed34..3b274e4 100644 --- a/config.js +++ b/config.js @@ -14,7 +14,7 @@ module.exports = { compWhale: '0xF977814e90dA44bFA03b6295A0616a897441aceC', creationFee: '200000000000000000000', // 200 TORN deployGasLimit: 7000000, - owner: '0xBAE5aBfa98466Dbe68836763B087f2d189f4D28f', + admin: '0xBAE5aBfa98466Dbe68836763B087f2d189f4D28f', // TODO WETH: '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2', UniswapV3Factory: '0x1F98431c8aD98523631AE4a59f267346ea31F984', TWAPSlotsMin: 50, diff --git a/contracts/AdminUpgradeableProxy.sol b/contracts/AdminUpgradeableProxy.sol new file mode 100644 index 0000000..caad021 --- /dev/null +++ b/contracts/AdminUpgradeableProxy.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.7.6; + +import "@openzeppelin/contracts/proxy/TransparentUpgradeableProxy.sol"; + +/** + * @dev TransparentUpgradeableProxy where admin is allowed to call implementation methods. + */ +contract AdminUpgradeableProxy is TransparentUpgradeableProxy { + /** + * @dev Initializes an upgradeable proxy backed by the implementation at `_logic`. + */ + constructor( + address _logic, + address _admin, + bytes memory _data + ) payable TransparentUpgradeableProxy(_logic, _admin, _data) {} + + /** + * @dev Override to allow admin access the fallback function. + */ + function _beforeFallback() internal override {} +} diff --git a/contracts/InstanceFactory.sol b/contracts/InstanceFactory.sol index 03ac6d1..e2c1c60 100644 --- a/contracts/InstanceFactory.sol +++ b/contracts/InstanceFactory.sol @@ -3,15 +3,16 @@ pragma solidity 0.7.6; pragma abicoder v2; -import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { Address } from "@openzeppelin/contracts/utils/Address.sol"; +import { Initializable } from "@openzeppelin/contracts/proxy/Initializable.sol"; import "@openzeppelin/contracts/proxy/Clones.sol"; import "./ERC20TornadoCloneable.sol"; -contract InstanceFactory is Ownable { +contract InstanceFactory is Initializable { using Clones for address; using Address for address; + address public admin; address public implementation; address public verifier; address public hasher; @@ -21,20 +22,29 @@ contract InstanceFactory is Ownable { event NewImplementationSet(address indexed newImplemenentation, address verifier, address hasher); event NewInstanceCloneCreated(address indexed clone); - constructor( + modifier onlyAdmin() { + require(admin == msg.sender, "IF: caller is not the admin"); + _; + } + + /** + * @notice initialize function for upgradeability + * @dev this contract will be deployed behind a proxy and should not assign values at logic address, + * params left out because self explainable + * */ + function initialize( address _verifier, address _hasher, uint32 _merkleTreeHeight, - address _owner - ) { + address _admin + ) public initializer { verifier = _verifier; hasher = _hasher; merkleTreeHeight = _merkleTreeHeight; + admin = _admin; ERC20TornadoCloneable implContract = new ERC20TornadoCloneable(_verifier, _hasher); implementation = address(implContract); - - transferOwnership(_owner); } /** @@ -59,12 +69,12 @@ contract InstanceFactory is Ownable { return implementation.predictDeterministicAddress(salt); } - function setMerkleTreeHeight(uint32 _merkleTreeHeight) external onlyOwner { + function setMerkleTreeHeight(uint32 _merkleTreeHeight) external onlyAdmin { merkleTreeHeight = _merkleTreeHeight; emit NewTreeHeightSet(merkleTreeHeight); } - function generateNewImplementation(address _verifier, address _hasher) external onlyOwner { + function generateNewImplementation(address _verifier, address _hasher) external onlyAdmin { verifier = _verifier; hasher = _hasher; implementation = address(new ERC20TornadoCloneable(_verifier, _hasher)); diff --git a/contracts/InstanceFactoryWithRegistry.sol b/contracts/InstanceFactoryWithRegistry.sol index 967b46d..42a4271 100644 --- a/contracts/InstanceFactoryWithRegistry.sol +++ b/contracts/InstanceFactoryWithRegistry.sol @@ -30,27 +30,38 @@ contract InstanceFactoryWithRegistry is InstanceFactory { * @dev Throws if called by any account other than the Governance. */ modifier onlyGovernance() { - require(owner() == _msgSender(), "Caller is not the Governance"); + require(governance == msg.sender, "IF: caller is not the Governance"); _; } constructor( - address _verifier, - address _hasher, - uint32 _merkleTreeHeight, address _governance, address _instanceRegistry, address _torn, address _UniswapV3Factory, - address _WETH, - uint16 _TWAPSlotsMin, - uint256 _creationFee - ) InstanceFactory(_verifier, _hasher, _merkleTreeHeight, _governance) { + address _WETH + ) { governance = _governance; instanceRegistry = _instanceRegistry; torn = _torn; UniswapV3Factory = IUniswapV3Factory(_UniswapV3Factory); WETH = _WETH; + } + + /** + * @notice initialize function for upgradeability + * @dev this contract will be deployed behind a proxy and should not assign values at logic address, + * params left out because self explainable + * */ + function initialize( + address _verifier, + address _hasher, + uint32 _merkleTreeHeight, + address _governance, + uint16 _TWAPSlotsMin, + uint256 _creationFee + ) external initializer { + initialize(_verifier, _hasher, _merkleTreeHeight, _governance); TWAPSlotsMin = _TWAPSlotsMin; creationFee = _creationFee; } @@ -140,12 +151,12 @@ contract InstanceFactoryWithRegistry is InstanceFactory { return proposal; } - function setCreationFee(uint256 _creationFee) external onlyGovernance { + function setCreationFee(uint256 _creationFee) external onlyAdmin { creationFee = _creationFee; emit NewCreationFeeSet(_creationFee); } - function setTWAPSlotsMin(uint16 _TWAPSlotsMin) external onlyGovernance { + function setTWAPSlotsMin(uint16 _TWAPSlotsMin) external onlyAdmin { TWAPSlotsMin = _TWAPSlotsMin; emit NewTWAPSlotsMinSet(_TWAPSlotsMin); } diff --git a/contracts/MultipleInstanceFactory.sol b/contracts/MultipleInstanceFactory.sol index 04307e6..965ffb5 100644 --- a/contracts/MultipleInstanceFactory.sol +++ b/contracts/MultipleInstanceFactory.sol @@ -5,13 +5,6 @@ pragma solidity 0.7.6; import "./InstanceFactory.sol"; contract MultipleInstanceFactory is InstanceFactory { - constructor( - address _verifier, - address _hasher, - uint32 _merkleTreeHeight, - address _owner - ) InstanceFactory(_verifier, _hasher, _merkleTreeHeight, _owner) {} - /** * @dev Creates new Tornado instances. * @param _token address of ERC20 token for a new instance diff --git a/scripts/deployInstanceFactoryWithRegistry.js b/scripts/deployInstanceFactoryWithRegistry.js index bc24b52..bd30e8a 100644 --- a/scripts/deployInstanceFactoryWithRegistry.js +++ b/scripts/deployInstanceFactoryWithRegistry.js @@ -14,9 +14,13 @@ async function deploy({ address, bytecode, singletonFactory }) { async function main() { const singletonFactory = await ethers.getContractAt('SingletonFactory', config.singletonFactory) const contracts = await generate() - await deploy({ ...contracts.factoryWithRegistryContract, singletonFactory }) + await deploy({ ...contracts.factoryWithRegistryContract.implementation, singletonFactory }) + await deploy({ ...contracts.factoryWithRegistryContract.proxy, singletonFactory }) console.log( - `Instance factory with registry contract have been deployed on ${contracts.factoryWithRegistryContract.address} address`, + `Instance factory with registry contract have been deployed on ${contracts.factoryWithRegistryContract.implementation.address} address`, + ) + console.log( + `Instance factory with registry proxy contract have been deployed on ${contracts.factoryWithRegistryContract.proxy.address} address`, ) } diff --git a/scripts/deployMultipleInstanceFactory.js b/scripts/deployMultipleInstanceFactory.js index 1b4dcb7..fb293ac 100644 --- a/scripts/deployMultipleInstanceFactory.js +++ b/scripts/deployMultipleInstanceFactory.js @@ -14,9 +14,13 @@ async function deploy({ address, bytecode, singletonFactory }) { async function main() { const singletonFactory = await ethers.getContractAt('SingletonFactory', config.singletonFactory) const contracts = await generate() - await deploy({ ...contracts.factoryContract, singletonFactory }) + await deploy({ ...contracts.factoryContract.implementation, singletonFactory }) + await deploy({ ...contracts.factoryContract.proxy, singletonFactory }) console.log( - `MultipleInstanceFactory contract have been deployed on ${contracts.factoryContract.address} address`, + `MultipleInstanceFactory contract have been deployed on ${contracts.factoryContract.implementation.address} address`, + ) + console.log( + `MultipleInstanceFactory proxy contract have been deployed on ${contracts.factoryContract.proxy.address} address`, ) } diff --git a/src/generateAddresses.js b/src/generateAddresses.js index 17d7ca8..37600d4 100644 --- a/src/generateAddresses.js +++ b/src/generateAddresses.js @@ -1,55 +1,87 @@ const { ethers } = require('hardhat') const defaultConfig = require('../config') +async function upgradableContract({ contractName, implConstructorArgs, proxyConstructorArgs, salt }) { + const Implementation = await ethers.getContractFactory(contractName) + + const implementationBytecode = + Implementation.bytecode + Implementation.interface.encodeDeploy(implConstructorArgs).slice(2) + + const implementationAddress = ethers.utils.getCreate2Address( + defaultConfig.singletonFactory, + salt, + ethers.utils.keccak256(implementationBytecode), + ) + + const AdminUpgradeableProxy = await ethers.getContractFactory('AdminUpgradeableProxy') + const proxyConst = [implementationAddress, ...proxyConstructorArgs] + const proxyBytecode = + AdminUpgradeableProxy.bytecode + AdminUpgradeableProxy.interface.encodeDeploy(proxyConst).slice(2) + + const proxyAddress = ethers.utils.getCreate2Address( + defaultConfig.singletonFactory, + salt, + ethers.utils.keccak256(proxyBytecode), + ) + + return { + implementation: { + address: implementationAddress, + bytecode: implementationBytecode, + args: implConstructorArgs, + }, + proxy: { address: proxyAddress, bytecode: proxyBytecode, args: proxyConst }, + isProxy: true, + } +} + async function generate(config = defaultConfig) { + // factory contract ----------------------------------------------- const FactoryFactory = await ethers.getContractFactory('MultipleInstanceFactory') - const deploymentBytecodeFactory = - FactoryFactory.bytecode + - FactoryFactory.interface - .encodeDeploy([config.verifier, config.hasher, config.merkleTreeHeight, config.owner]) - .slice(2) + const FactoryInitData = FactoryFactory.interface.encodeFunctionData('initialize', [ + config.verifier, + config.hasher, + config.merkleTreeHeight, + config.admin, + ]) - const factoryAddress = ethers.utils.getCreate2Address( - config.singletonFactory, - config.salt, - ethers.utils.keccak256(deploymentBytecodeFactory), - ) + const factoryContract = await upgradableContract({ + contractName: 'MultipleInstanceFactory', + implConstructorArgs: [], + proxyConstructorArgs: [config.admin, FactoryInitData], + salt: config.salt, + }) + // factory with registry contract --------------------------------- const FactoryWithRegistryFactory = await ethers.getContractFactory('InstanceFactoryWithRegistry') - const deploymentBytecodeFactoryWithRegistry = - FactoryWithRegistryFactory.bytecode + - FactoryWithRegistryFactory.interface - .encodeDeploy([ - config.verifier, - config.hasher, - config.merkleTreeHeight, - config.governance, - config.instanceRegistry, - config.TORN, - config.UniswapV3Factory, - config.WETH, - config.TWAPSlotsMin, - config.creationFee, - ]) - .slice(2) - - const factoryWithRegistryAddress = ethers.utils.getCreate2Address( - config.singletonFactory, - config.salt, - ethers.utils.keccak256(deploymentBytecodeFactoryWithRegistry), + const FactoryWithRegistryInitData = FactoryWithRegistryFactory.interface.encodeFunctionData( + 'initialize(address,address,uint32,address,uint16,uint256)', + [ + config.verifier, + config.hasher, + config.merkleTreeHeight, + config.governance, + config.TWAPSlotsMin, + config.creationFee, + ], ) + const factoryWithRegistryContract = await upgradableContract({ + contractName: 'InstanceFactoryWithRegistry', + implConstructorArgs: [ + config.governance, + config.instanceRegistry, + config.TORN, + config.UniswapV3Factory, + config.WETH, + ], + proxyConstructorArgs: [config.governance, FactoryWithRegistryInitData], + salt: config.salt, + }) + const result = { - factoryContract: { - address: factoryAddress, - bytecode: deploymentBytecodeFactory, - isProxy: false, - }, - factoryWithRegistryContract: { - address: factoryWithRegistryAddress, - bytecode: deploymentBytecodeFactoryWithRegistry, - isProxy: false, - }, + factoryContract, + factoryWithRegistryContract, } return result @@ -57,8 +89,16 @@ async function generate(config = defaultConfig) { async function generateWithLog() { const contracts = await generate() - console.log('MultipleInstanceFactory contract: ', contracts.factoryContract.address) - console.log('Instance factory with registry contract: ', contracts.factoryWithRegistryContract.address) + console.log('MultipleInstanceFactory contract: ', contracts.factoryContract.implementation.address) + console.log('MultipleInstanceFactory proxy contract: ', contracts.factoryContract.proxy.address) + console.log( + 'Instance factory with registry contract: ', + contracts.factoryWithRegistryContract.implementation.address, + ) + console.log( + 'Instance factory with registry proxy contract: ', + contracts.factoryWithRegistryContract.proxy.address, + ) return contracts } diff --git a/test/factory.with.registry.test.js b/test/factory.with.registry.test.js index cf14716..fe7818f 100644 --- a/test/factory.with.registry.test.js +++ b/test/factory.with.registry.test.js @@ -57,14 +57,25 @@ describe('Instance Factory With Registry Tests', () => { config.singletonFactoryVerboseWrapper, ) const contracts = await generate() - if ((await ethers.provider.getCode(contracts.factoryWithRegistryContract.address)) == '0x') { - await singletonFactory.deploy(contracts.factoryWithRegistryContract.bytecode, config.salt, { + if ( + (await ethers.provider.getCode(contracts.factoryWithRegistryContract.implementation.address)) == '0x' + ) { + await singletonFactory.deploy( + contracts.factoryWithRegistryContract.implementation.bytecode, + config.salt, + { + gasLimit: config.deployGasLimit, + }, + ) + } + if ((await ethers.provider.getCode(contracts.factoryWithRegistryContract.proxy.address)) == '0x') { + await singletonFactory.deploy(contracts.factoryWithRegistryContract.proxy.bytecode, config.salt, { gasLimit: config.deployGasLimit, }) } const instanceFactory = await ethers.getContractAt( 'InstanceFactoryWithRegistry', - contracts.factoryWithRegistryContract.address, + contracts.factoryWithRegistryContract.proxy.address, ) return { diff --git a/test/multiple.instance.factory.test.js b/test/multiple.instance.factory.test.js index 78f3be3..8acd878 100644 --- a/test/multiple.instance.factory.test.js +++ b/test/multiple.instance.factory.test.js @@ -14,10 +14,10 @@ describe('Multiple Instance Factory Tests', () => { async function fixture() { const [sender, deployer] = await ethers.getSigners() - const owner = await getSignerFromAddress(config.owner) + const owner = await getSignerFromAddress(config.admin) await sender.sendTransaction({ - to: config.owner, + to: config.admin, value: ethers.utils.parseEther('1'), }) @@ -34,14 +34,19 @@ describe('Multiple Instance Factory Tests', () => { config.singletonFactoryVerboseWrapper, ) const contracts = await generate() - if ((await ethers.provider.getCode(contracts.factoryContract.address)) == '0x') { - await singletonFactory.deploy(contracts.factoryContract.bytecode, config.salt, { + if ((await ethers.provider.getCode(contracts.factoryContract.implementation.address)) == '0x') { + await singletonFactory.deploy(contracts.factoryContract.implementation.bytecode, config.salt, { + gasLimit: config.deployGasLimit, + }) + } + if ((await ethers.provider.getCode(contracts.factoryContract.proxy.address)) == '0x') { + await singletonFactory.deploy(contracts.factoryContract.proxy.bytecode, config.salt, { gasLimit: config.deployGasLimit, }) } const instanceFactory = await ethers.getContractAt( 'MultipleInstanceFactory', - contracts.factoryContract.address, + contracts.factoryContract.proxy.address, ) return {