From 65a01c3ce844ccd98dde5b02d24cf6852afbbbe4 Mon Sep 17 00:00:00 2001 From: Drygin Date: Tue, 15 Mar 2022 18:20:36 +0300 Subject: [PATCH] max 4 instances --- README.md | 2 +- contracts/AddInstanceProposal.sol | 13 +++-- contracts/InstanceFactory.sol | 26 +++------- test/factory.test.js | 8 ++- test/factory.with.registry.test.js | 81 +++++++++++++----------------- 5 files changed, 54 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index 3dc2b65..69b7584 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ Anyone can create governance proposal for the addition of a new ERC20 instance b ### InstanceFactoryWithRegistry -1. `max number of new instances in one proposal` - the current version supports the addition of a maximum of 3 instances at once. +1. `max number of new instances in one proposal` - the current version supports the addition of a maximum of 4 instances at once. 2. `proposal creation fee` - this fee is charged from creator of proposal during `createProposalApprove/createProposalPermit` factory method execution. It can be changed by governance. Default value is stored in `config.js`. ## Warnings diff --git a/contracts/AddInstanceProposal.sol b/contracts/AddInstanceProposal.sol index acfb386..876f4f0 100644 --- a/contracts/AddInstanceProposal.sol +++ b/contracts/AddInstanceProposal.sol @@ -16,9 +16,11 @@ contract AddInstanceProposal { uint256 internal immutable denomination0; uint256 internal immutable denomination1; uint256 internal immutable denomination2; + uint256 internal immutable denomination3; uint32 internal immutable protocolFee0; uint32 internal immutable protocolFee1; uint32 internal immutable protocolFee2; + uint32 internal immutable protocolFee3; event AddInstanceForRegistry(address instance, address token, uint256 denomination); @@ -35,18 +37,19 @@ contract AddInstanceProposal { token = _token; uniswapPoolSwappingFee = _uniswapPoolSwappingFee; - require(_denominations.length == _protocolFees.length); + require(_denominations.length == _protocolFees.length, "denominations length != protocolFees length"); uint256 _numInstances = _denominations.length; - require(_numInstances > 0); - require(_numInstances < 4); + require(_numInstances > 0 && _numInstances <= 4, "incorrect instances number"); numInstances = _numInstances; denomination0 = _numInstances > 0 ? _denominations[0] : 0; denomination1 = _numInstances > 1 ? _denominations[1] : 0; denomination2 = _numInstances > 2 ? _denominations[2] : 0; + denomination3 = _numInstances > 3 ? _denominations[3] : 0; protocolFee0 = _numInstances > 0 ? _protocolFees[0] : 0; protocolFee1 = _numInstances > 1 ? _protocolFees[1] : 0; protocolFee2 = _numInstances > 2 ? _protocolFees[2] : 0; + protocolFee3 = _numInstances > 3 ? _protocolFees[3] : 0; } function executeProposal() external { @@ -76,6 +79,8 @@ contract AddInstanceProposal { return denomination1; } else if (_index == 2) { return denomination2; + } else if (_index == 3) { + return denomination3; } else { revert("Invalid instance index"); } @@ -88,6 +93,8 @@ contract AddInstanceProposal { return protocolFee1; } else if (_index == 2) { return protocolFee2; + } else if (_index == 3) { + return protocolFee3; } else { revert("Invalid instance index"); } diff --git a/contracts/InstanceFactory.sol b/contracts/InstanceFactory.sol index 4c4331a..f3e5a80 100644 --- a/contracts/InstanceFactory.sol +++ b/contracts/InstanceFactory.sol @@ -17,10 +17,8 @@ contract InstanceFactory is Ownable { address public hasher; uint32 public merkleTreeHeight; - event NewVerifierSet(address indexed newVerifier); - event NewHasherSet(address indexed newHasher); event NewTreeHeightSet(uint32 indexed newTreeHeight); - event NewImplementationSet(address indexed newImplemenentation); + event NewImplementationSet(address indexed newImplemenentation, address verifier, address hasher); event NewInstanceCloneCreated(address indexed clone); constructor( @@ -61,27 +59,15 @@ contract InstanceFactory is Ownable { return implementation.predictDeterministicAddress(salt); } - function setVerifier(address _verifier) external onlyOwner { - verifier = _verifier; - emit NewVerifierSet(verifier); - } - - function setHasher(address _hasher) external onlyOwner { - hasher = _hasher; - emit NewHasherSet(hasher); - } - function setMerkleTreeHeight(uint32 _merkleTreeHeight) external onlyOwner { merkleTreeHeight = _merkleTreeHeight; emit NewTreeHeightSet(merkleTreeHeight); } - function setImplementation(address _newImplementation) external onlyOwner { - implementation = _newImplementation; - emit NewImplementationSet(implementation); - } - - function generateNewImplementation() external onlyOwner { - implementation = address(new ERC20TornadoCloneable(verifier, hasher)); + function generateNewImplementation(address _verifier, address _hasher) external onlyOwner { + verifier = _verifier; + hasher = _hasher; + implementation = address(new ERC20TornadoCloneable(_verifier, _hasher)); + emit NewImplementationSet(implementation, _verifier, _hasher); } } diff --git a/test/factory.test.js b/test/factory.test.js index c588ab8..0201c63 100644 --- a/test/factory.test.js +++ b/test/factory.test.js @@ -70,20 +70,18 @@ describe('Instance Factory Tests', () => { it('Governance should be able to set factory params', async function () { let { instanceFactory, owner } = await loadFixture(fixture) - await expect(instanceFactory.setVerifier(addressZero)).to.be.reverted + await expect(instanceFactory.setMerkleTreeHeight(1)).to.be.reverted instanceFactory = await instanceFactory.connect(owner) - await instanceFactory.setVerifier(addressZero) - await instanceFactory.setHasher(addressZero) + await instanceFactory.generateNewImplementation(addressZero, addressZero) await instanceFactory.setMerkleTreeHeight(1) expect(await instanceFactory.verifier()).to.be.equal(addressZero) expect(await instanceFactory.hasher()).to.be.equal(addressZero) expect(await instanceFactory.merkleTreeHeight()).to.be.equal(1) - await instanceFactory.setVerifier(config.verifier) - await instanceFactory.setHasher(config.hasher) + await instanceFactory.generateNewImplementation(config.verifier, config.hasher) await instanceFactory.setMerkleTreeHeight(config.merkleTreeHeight) expect(await instanceFactory.verifier()).to.be.equal(config.verifier) diff --git a/test/factory.with.registry.test.js b/test/factory.with.registry.test.js index 271c0ef..8927dc2 100644 --- a/test/factory.with.registry.test.js +++ b/test/factory.with.registry.test.js @@ -106,13 +106,12 @@ describe('Instance Factory With Registry Tests', () => { it('Governance should be able to set factory params', async function () { let { instanceFactory, gov } = await loadFixture(fixture) - await expect(instanceFactory.setVerifier(addressZero)).to.be.reverted + await expect(instanceFactory.setMerkleTreeHeight(1)).to.be.reverted const govSigner = await getSignerFromAddress(gov.address) instanceFactory = await instanceFactory.connect(govSigner) - await instanceFactory.setVerifier(addressZero) - await instanceFactory.setHasher(addressZero) + await instanceFactory.generateNewImplementation(addressZero, addressZero) await instanceFactory.setMerkleTreeHeight(1) await instanceFactory.setCreationFee(0) @@ -121,8 +120,7 @@ describe('Instance Factory With Registry Tests', () => { expect(await instanceFactory.merkleTreeHeight()).to.be.equal(1) expect(await instanceFactory.creationFee()).to.be.equal(0) - await instanceFactory.setVerifier(config.verifier) - await instanceFactory.setHasher(config.hasher) + await instanceFactory.generateNewImplementation(config.verifier, config.hasher) await instanceFactory.setMerkleTreeHeight(config.merkleTreeHeight) await instanceFactory.setCreationFee(config.creationFee) @@ -221,19 +219,22 @@ describe('Instance Factory With Registry Tests', () => { it('Should successfully deploy/propose/execute proposal - add instances', async function () { let { sender, instanceFactory, gov, instanceRegistry, tornWhale, tornToken } = await loadFixture(fixture) + const denominations = [ + ethers.utils.parseEther('1'), + ethers.utils.parseEther('10'), + ethers.utils.parseEther('100'), + ethers.utils.parseEther('1000'), + ] + const numInstances = denominations.length + + const protocolFees = [30, 30, 30, 30] + // deploy proposal ---------------------------------------------- await tornToken.connect(tornWhale).transfer(sender.address, config.creationFee) await tornToken.approve(instanceFactory.address, config.creationFee) await expect(() => - instanceFactory - .connect(sender) - .createProposalApprove( - config.COMP, - 3000, - [ethers.utils.parseEther('100'), ethers.utils.parseEther('1000')], - [30, 30], - ), + instanceFactory.connect(sender).createProposalApprove(config.COMP, 3000, denominations, protocolFees), ).to.changeTokenBalances( tornToken, [sender, gov], @@ -250,11 +251,11 @@ describe('Instance Factory With Registry Tests', () => { expect(await proposal.instanceRegistry()).to.be.equal(instanceRegistry.address) expect(await proposal.token()).to.be.equal(config.COMP) expect(await proposal.uniswapPoolSwappingFee()).to.be.equal(3000) - expect(await proposal.numInstances()).to.be.equal(2) - expect(await proposal.protocolFeeByIndex(0)).to.be.equal(30) - expect(await proposal.protocolFeeByIndex(1)).to.be.equal(30) - expect(await proposal.denominationByIndex(0)).to.be.equal(ethers.utils.parseEther('100')) - expect(await proposal.denominationByIndex(1)).to.be.equal(ethers.utils.parseEther('1000')) + expect(await proposal.numInstances()).to.be.equal(numInstances) + for (let i = 0; i < numInstances; i++) { + expect(await proposal.protocolFeeByIndex(i)).to.be.equal(protocolFees[i]) + expect(await proposal.denominationByIndex(i)).to.be.equal(denominations[i]) + } // propose proposal --------------------------------------------- let response, id, state @@ -294,37 +295,23 @@ describe('Instance Factory With Registry Tests', () => { // check instances initialization ------------------------------- logs = await instanceFactory.queryFilter('NewInstanceCloneCreated') - let instanceAddr = '0x' + logs[logs.length - 2].topics[1].slice(-40) - let instance = await ethers.getContractAt('ERC20TornadoCloneable', instanceAddr) + for (let i = 0; i < numInstances; i++) { + let instanceAddr = '0x' + logs[logs.length - numInstances + i].topics[1].slice(-40) + let instance = await ethers.getContractAt('ERC20TornadoCloneable', instanceAddr) - expect(await instance.token()).to.be.equal(config.COMP) - expect(await instance.verifier()).to.be.equal(config.verifier) - expect(await instance.hasher()).to.be.equal(config.hasher) - expect(await instance.levels()).to.be.equal(config.merkleTreeHeight) - expect(await instance.denomination()).to.equal(ethers.utils.parseEther('100')) + expect(await instance.token()).to.be.equal(config.COMP) + expect(await instance.verifier()).to.be.equal(config.verifier) + expect(await instance.hasher()).to.be.equal(config.hasher) + expect(await instance.levels()).to.be.equal(config.merkleTreeHeight) + expect(await instance.denomination()).to.equal(denominations[i]) - let instanceData = await instanceRegistry.instances(instance.address) - expect(instanceData.isERC20).to.be.equal(true) - expect(instanceData.token).to.be.equal(config.COMP) - expect(instanceData.state).to.be.equal(1) - expect(instanceData.uniswapPoolSwappingFee).to.be.equal(3000) - expect(instanceData.protocolFeePercentage).to.be.equal(30) - - instanceAddr = '0x' + logs[logs.length - 1].topics[1].slice(-40) - instance = await ethers.getContractAt('ERC20TornadoCloneable', instanceAddr) - - expect(await instance.token()).to.be.equal(config.COMP) - expect(await instance.verifier()).to.be.equal(config.verifier) - expect(await instance.hasher()).to.be.equal(config.hasher) - expect(await instance.levels()).to.be.equal(config.merkleTreeHeight) - expect(await instance.denomination()).to.equal(ethers.utils.parseEther('1000')) - - instanceData = await instanceRegistry.instances(instance.address) - expect(instanceData.isERC20).to.be.equal(true) - expect(instanceData.token).to.be.equal(config.COMP) - expect(instanceData.state).to.be.equal(1) - expect(instanceData.uniswapPoolSwappingFee).to.be.equal(3000) - expect(instanceData.protocolFeePercentage).to.be.equal(30) + let instanceData = await instanceRegistry.instances(instance.address) + expect(instanceData.isERC20).to.be.equal(true) + expect(instanceData.token).to.be.equal(config.COMP) + expect(instanceData.state).to.be.equal(1) + expect(instanceData.uniswapPoolSwappingFee).to.be.equal(3000) + expect(instanceData.protocolFeePercentage).to.be.equal(protocolFees[i]) + } }) it('Should successfully deploy proposal with permit', async function () {