diff --git a/.gitignore b/.gitignore index c48502e8..60b33fb0 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,5 @@ artifacts /contracts-exposed .vscode +revenue-sharing-addresses.json +deploy-collector.input.json diff --git a/contracts/Collector.sol b/contracts/Collector.sol index 12fd8b3b..4b31161f 100644 --- a/contracts/Collector.sol +++ b/contracts/Collector.sol @@ -9,20 +9,9 @@ import "@openzeppelin/contracts/math/SafeMath.sol"; contract Collector is ICollector { address private _remainderAddress; RevenuePartner[] private _partners; - IERC20 public token; + IERC20[] private _tokens; address public owner; - - modifier validShares(RevenuePartner[] memory partners) { - uint256 totalShares; - - for (uint256 i = 0; i < partners.length; i++) { - require(partners[i].share > 0, "0 is not a valid share"); - totalShares = totalShares + partners[i].share; - } - - require(totalShares == 100, "Shares must add up to 100%"); - _; - } + mapping(IERC20 => bool) public tokenMap; modifier onlyOwner() { require(msg.sender == owner, "Only owner can call this"); @@ -30,24 +19,39 @@ contract Collector is ICollector { } modifier noBalanceToShare() { - require( - token.balanceOf(address(this)) < _partners.length, - "There is balance to share" - ); + for (uint256 i = 0; i < _tokens.length; i++) { + require( + _tokens[i].balanceOf(address(this)) < _partners.length, + "There is balance to share" + ); + } _; } + modifier updateValidShares(RevenuePartner[] memory partners) { + _; + uint256 totalShares; + for (uint256 i = 0; i < partners.length; i++) { + require(partners[i].share > 0, "0 is not a valid share"); + totalShares = totalShares + partners[i].share; + _partners.push(partners[i]); + } + require(totalShares == 100, "Shares must add up to 100%"); + } + constructor( address _owner, - IERC20 _token, + IERC20[] memory tokens, RevenuePartner[] memory partners, address remainderAddress - ) public validShares(partners) { + ) public updateValidShares(partners) { owner = _owner; - token = _token; _remainderAddress = remainderAddress; - for (uint256 i = 0; i < partners.length; i++) - _partners.push(partners[i]); + + for (uint i = 0; i < tokens.length; i++) { + _tokens.push(tokens[i]); + tokenMap[tokens[i]] = true; + } } function getPartners() external view returns (RevenuePartner[] memory) { @@ -56,11 +60,8 @@ contract Collector is ICollector { function updateShares( RevenuePartner[] memory partners - ) external validShares(partners) onlyOwner noBalanceToShare { + ) external onlyOwner noBalanceToShare updateValidShares(partners) { delete _partners; - - for (uint256 i = 0; i < partners.length; i++) - _partners.push(partners[i]); } //@notice Withdraw the actual remainder and then update the remainder's address @@ -68,34 +69,58 @@ contract Collector is ICollector { function updateRemainderAddress( address remainderAddress ) external onlyOwner noBalanceToShare { - uint256 balance = token.balanceOf(address(this)); - address tokenAddr = address(token); + for (uint256 i = 0; i < _tokens.length; i++) { + IERC20 token = _tokens[i]; + uint256 balance = token.balanceOf(address(this)); + + if (balance != 0) { + // solhint-disable-next-line avoid-low-level-calls + (bool success, bytes memory ret) = address(token).call( + abi.encodeWithSelector( + hex"a9059cbb", + _remainderAddress, + balance + ) + ); + + require( + success && (ret.length == 0 || abi.decode(ret, (bool))), + "Unable to transfer remainder" + ); + } + } + _remainderAddress = remainderAddress; + } - if (balance != 0) { - // solhint-disable-next-line avoid-low-level-calls - (bool success, bytes memory ret) = tokenAddr.call{gas: 200000}( - abi.encodeWithSelector( - hex"a9059cbb", - _remainderAddress, - balance - ) - ); + function addToken(IERC20 token) external onlyOwner { + require(tokenMap[token] == false, "Token is already accepted"); + tokenMap[token] = true; + _tokens.push(token); + } - require( - success && (ret.length == 0 || abi.decode(ret, (bool))), - "Unable to transfer remainder" - ); - } + function getTokens() external view returns (IERC20[] memory) { + return _tokens; + } - // solhint-disable-next-line - _remainderAddress = remainderAddress; + function removeToken(IERC20 token, uint256 tokenIndex) external onlyOwner { + require(tokenMap[token] == true, "Token is not accepted"); + require(_tokens[tokenIndex] == token, "Incorrect token"); + require( + _tokens[tokenIndex].balanceOf(address(this)) == 0, + "There is balance to share" + ); + + delete tokenMap[token]; + _tokens[tokenIndex] = _tokens[_tokens.length - 1]; + _tokens.pop(); } - function getBalance() external view returns (uint256) { - return token.balanceOf(address(this)); + function getRemainderAddress() external view returns (address) { + return _remainderAddress; } - function withdraw() external override onlyOwner { + function withdrawToken(IERC20 token) public onlyOwner { + require(tokenMap[token], "Token is not accepted"); uint256 balance = token.balanceOf(address(this)); require(balance >= _partners.length, "Not enough balance to split"); @@ -118,8 +143,14 @@ contract Collector is ICollector { } } + function withdraw() external override onlyOwner { + for (uint256 i = 0; i < _tokens.length; i++) { + withdrawToken(_tokens[i]); + } + } + function transferOwnership(address _owner) external override onlyOwner { - require(_owner != address(0), "New owner is the zero address"); + require(_owner != address(0), "Owner cannot be zero address"); owner = _owner; } } diff --git a/deploy-collector.input.sample.json b/deploy-collector.input.sample.json index b52797a5..a6108074 100644 --- a/deploy-collector.input.sample.json +++ b/deploy-collector.input.sample.json @@ -18,6 +18,8 @@ "share": 32 } ], - "tokenAddress": "0xb824784A5bF2Bc7139d1786639444e4Da259934B", + "tokenAddresses": [ + "0xb824784A5bF2Bc7139d1786639444e4Da259934B" + ], "remainderAddress": "0xc354D97642FAa06781b76Ffb6786f72cd7746C97" } \ No newline at end of file diff --git a/hardhat.config.ts b/hardhat.config.ts index e534c368..4b276631 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -79,8 +79,8 @@ const config: HardhatUserConfig = { }, tdd: { tasks: [ - // 'clean', - // { command: 'compile', params: { quiet: true } }, + 'clean', + { command: 'compile', params: { quiet: true } }, { command: 'test', params: { @@ -89,7 +89,7 @@ const config: HardhatUserConfig = { }, }, ], - files: ['./test/**/*.ts'], + files: ['./contracts/*.sol', './test/**/*.ts', './tasks/**/*.ts'], verbose: true, clearOnStart: true, }, diff --git a/package-lock.json b/package-lock.json index f996d950..5dd42979 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@rsksmart/rif-relay-contracts", - "version": "2.0.0-beta.0", + "version": "2.0.0-beta.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@rsksmart/rif-relay-contracts", - "version": "2.0.0-beta.0", + "version": "2.0.0-beta.1", "license": "MIT", "dependencies": { "@metamask/eth-sig-util": "^4.0.1", diff --git a/package.json b/package.json index 4b5024f9..70285556 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@rsksmart/rif-relay-contracts", - "version": "2.0.0-beta.0", + "version": "2.0.0-beta.1", "private": false, "description": "This project contains all the contracts needed for the rif relay system.", "license": "MIT", diff --git a/tasks/deployCollector.ts b/tasks/deployCollector.ts index 09816bb3..bceed853 100644 --- a/tasks/deployCollector.ts +++ b/tasks/deployCollector.ts @@ -11,12 +11,12 @@ export interface Partner { share: number; } -interface CollectorConfig { +export type CollectorConfig = { collectorOwner: string; partners: Partner[]; - tokenAddress: string; + tokenAddresses: string[]; remainderAddress: string; -} +}; export type DeployCollectorArg = { configFileName?: string; @@ -54,13 +54,13 @@ export const deployCollector = async ( fs.readFileSync(configFileName, { encoding: 'utf-8' }) ) as CollectorConfig; - const { collectorOwner, partners, tokenAddress, remainderAddress } = + const { collectorOwner, partners, tokenAddresses, remainderAddress } = inputConfig; const collectorFactory = await ethers.getContractFactory('Collector'); const collector = await collectorFactory.deploy( collectorOwner, - tokenAddress, + tokenAddresses, partners, remainderAddress ); @@ -79,11 +79,20 @@ export const deployCollector = async ( }, {}); + const tokenPrintings = (await collector.getTokens()).reduce< + Record + >((accumulator, token, i) => { + return { + ...accumulator, + [`Collector Token ${i}`]: token, + }; + }, {}); + const objToPrint = { 'Collector Contract': collector.address, 'Collector Owner': await collector.owner(), - 'Collector Token': await collector.token(), 'Collector Remainder': remainderAddress, + ...tokenPrintings, ...partnerPrintings, }; console.table(objToPrint); @@ -106,7 +115,7 @@ export const deployCollector = async ( jsonConfig[networkId] = { collectorContract: collector.address, collectorOwner: await collector.owner(), - tokenAddress: await collector.token(), + tokenAddresses: await collector.getTokens(), remainderAddress: remainderAddress, partners, }; diff --git a/test/Collector.test.ts b/test/Collector.test.ts new file mode 100644 index 00000000..6335d4e2 --- /dev/null +++ b/test/Collector.test.ts @@ -0,0 +1,757 @@ +import { + FakeContract, + MockContract, + MockContractFactory, + smock, +} from '@defi-wonderland/smock'; +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; +import chai from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +import { ethers } from 'hardhat'; +import { Wallet, BigNumber } from 'ethers'; +import { ERC20, Collector, Collector__factory } from '../typechain-types'; + +chai.use(chaiAsPromised); +chai.use(smock.matchers); + +const expect = chai.expect; + +const PARTNER_SHARES = [20, 30, 40, 10]; + +type Partner = { + beneficiary: string; + share: number; +}; + +const buildPartners = async ( + partnerWallets: SignerWithAddress[], + shares: number[] +) => { + return Promise.all( + partnerWallets.map(async (wallet, index) => { + const partner: Partner = { + beneficiary: await wallet.getAddress(), + share: shares[index], + }; + + return partner; + }) + ); +}; + +type CollectorDeployParams = { + ownerAddr?: string; + tokens?: string[]; + partners?: Partner[]; + remainderDestinationAddr?: string; +}; + +type CollectorDeployFunction = ( + collectorParams: CollectorDeployParams +) => ReturnType['deploy']>; + +const prepareDefaultDeployer = + ( + collectorFactory: MockContractFactory, + defaultOwnerAddr: string, + defaultTokens: string[], + defaultPartners: Partner[], + defaultRemainderDestinationAddr: string + ): CollectorDeployFunction => + ({ + ownerAddr = defaultOwnerAddr, + tokens = defaultTokens, + partners = defaultPartners, + remainderDestinationAddr = defaultRemainderDestinationAddr, + }: CollectorDeployParams) => + collectorFactory.deploy( + ownerAddr, + tokens, + partners, + remainderDestinationAddr + ); + +function checkPartnerBalanceForToken( + partners: Partner[], + tokenToWithdraw: FakeContract +) { + for (let partnerIndex = 0; partnerIndex < partners.length; partnerIndex++) { + expect( + tokenToWithdraw.transfer, + `Partner[${partnerIndex}] balance for token[${tokenToWithdraw.address}]` + ).to.have.been.calledWith( + partners[partnerIndex].beneficiary, + PARTNER_SHARES[partnerIndex] + ); + } +} + +describe('Collector', function () { + let owner: SignerWithAddress; + let collectorFactory: MockContractFactory; + let fakeERC20Tokens: FakeContract[]; + let partners: Partner[]; + let remainderDestination: SignerWithAddress; + let partner1: SignerWithAddress; + let partner2: SignerWithAddress; + let partner3: SignerWithAddress; + let partner4: SignerWithAddress; + + beforeEach(async function () { + [owner, partner1, partner2, partner3, partner4, remainderDestination] = + await ethers.getSigners(); + partners = await buildPartners( + [partner1, partner2, partner3, partner4], + PARTNER_SHARES + ); + + fakeERC20Tokens = await Promise.all( + Array.from(Array(2)).map(async (fakeToken: FakeContract) => { + fakeToken = await smock.fake('ERC20'); + fakeToken.transfer.returns(true); + + return fakeToken; + }) + ); + + collectorFactory = await smock.mock('Collector'); + }); + + describe('constructor()', function () { + it('Should deploy a Collector without tokens', async function () { + partners = await buildPartners( + [partner1, partner2, partner3, partner4], + PARTNER_SHARES + ); + const expectedOwnerAddress = owner.address; + + const collector: MockContract = await collectorFactory.deploy( + expectedOwnerAddress, + [], + partners, + remainderDestination.address + ); + + const actualOwnerAddress = await collector.owner(); + const actualTokens = await collector.getTokens(); + + expect(actualOwnerAddress, 'Failed to set the owner').to.equal( + expectedOwnerAddress + ); + expect(actualTokens, 'Failed to set tokens').to.deep.equal([]); + }); + + it('Should deploy a Collector with single token', async function () { + partners = await buildPartners( + [partner1, partner2, partner3, partner4], + PARTNER_SHARES + ); + const expectedTokens = fakeERC20Tokens.map(({ address }) => address)[0]; + const expectedOwnerAddress = owner.address; + + const collector: MockContract = await collectorFactory.deploy( + expectedOwnerAddress, + [expectedTokens], + partners, + remainderDestination.address + ); + + const actualOwnerAddress = await collector.owner(); + const actualTokens = await collector.getTokens(); + + expect(actualOwnerAddress, 'Failed to set the owner').to.equal( + expectedOwnerAddress + ); + expect(actualTokens, 'Failed to set tokens').to.deep.equal([ + expectedTokens, + ]); + }); + + it('Should deploy a Collector with multiple tokens', async function () { + partners = await buildPartners( + [partner1, partner2, partner3, partner4], + PARTNER_SHARES + ); + const expectedTokens = fakeERC20Tokens.map(({ address }) => address); + const expectedOwnerAddress = owner.address; + + const collector: MockContract = await collectorFactory.deploy( + owner.address, + expectedTokens, + partners, + remainderDestination.address + ); + const actualOwnerAddress = await collector.owner(); + const actualTokens = await collector.getTokens(); + + expect(actualOwnerAddress, 'Failed to set the owner').to.equal( + expectedOwnerAddress + ); + expect(actualTokens, 'Failed to set tokens').to.deep.equal( + expectedTokens + ); + }); + + it('Should not deploy with invalid shares', async function () { + const partners = await buildPartners( + [partner1, partner2, partner3, partner4], + [25, 25, 25, 26] + ); + + await expect( + collectorFactory.deploy( + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ) + ).to.be.rejectedWith('Shares must add up to 100%'); + }); + + it('Should not deploy if the array of partners is empty', async function () { + const partners: Partner[] = []; + + await expect( + collectorFactory.deploy( + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ) + ).to.be.rejectedWith('Shares must add up to 100%'); + }); + + it('Should not deploy if a share is 0', async function () { + const partners = await buildPartners( + [partner1, partner2, partner3, partner4], + [30, 30, 0, 40] + ); + + await expect( + collectorFactory.deploy( + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ) + ).to.be.rejectedWith('0 is not a valid share'); + }); + }); + + describe('addToken', function () { + let deployCollector: CollectorDeployFunction; + + beforeEach(function () { + deployCollector = prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ); + }); + + it('should reject if called by non-owner', async function () { + const collector = await deployCollector({}); + const nonOwner = (await ethers.getSigners()).at(6) as SignerWithAddress; + + await expect( + collector.connect(nonOwner).addToken(Wallet.createRandom().address) + ).to.have.been.rejectedWith('Only owner can call this'); + }); + + it('should reject if token is already added', async function () { + const collector = await deployCollector({}); + const { address: tokenAddress } = await smock.fake('ERC20'); + await collector.addToken(tokenAddress); + await expect(collector.addToken(tokenAddress)).to.be.rejectedWith( + 'Token is already accepted' + ); + }); + + it('should add token to the list', async function () { + const collector = await deployCollector({}); + const { address: expectedTokenAddress } = await smock.fake( + 'ERC20' + ); + await collector.addToken(expectedTokenAddress); + + const actualTokenAddresses = await collector.getTokens(); + + expect(actualTokenAddresses).to.include.members([expectedTokenAddress]); + }); + }); + + describe('removeToken', function () { + let deployCollector: CollectorDeployFunction; + + beforeEach(function () { + deployCollector = prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ); + }); + + it('should reject if called by non-owner', async function () { + const collector = await deployCollector({}); + const nonOwner = (await ethers.getSigners()).at(6) as SignerWithAddress; + const randomAddress = Wallet.createRandom().address; + + await expect( + collector.connect(nonOwner).removeToken(randomAddress, 0) + ).to.have.been.rejectedWith('Only owner can call this'); + }); + + it('should check token balance of the collector', async function () { + const collector = await deployCollector({}); + const tokenIndexToRemove = fakeERC20Tokens.length - 1; + await collector.removeToken( + fakeERC20Tokens[tokenIndexToRemove].address, + tokenIndexToRemove + ); + + expect( + fakeERC20Tokens[tokenIndexToRemove].balanceOf + ).to.have.been.calledWith(collector.address); + }); + + it(`should reject if collector balance is non-zero`, async function () { + const balance = 1; + const tokenIndexToRemove = fakeERC20Tokens.length - 1; + fakeERC20Tokens[tokenIndexToRemove].balanceOf.returns(balance); + const collector = await deployCollector({}); + + await expect( + collector.removeToken( + fakeERC20Tokens[tokenIndexToRemove].address, + tokenIndexToRemove + ) + ).to.have.rejectedWith('There is balance to share'); + }); + + it('should be rejected if token is not accepted', async function () { + const tokenIndexToRemove = fakeERC20Tokens.length - 1; + const collector = await deployCollector({}); + + const randomAddress = Wallet.createRandom().address; + + await expect( + collector.removeToken(randomAddress, tokenIndexToRemove) + ).to.be.rejectedWith('Token is not accepted'); + }); + + it('should remove token', async function () { + const tokenIndexToRemove = fakeERC20Tokens.length - 1; + const collector = await deployCollector({}); + await collector.removeToken( + fakeERC20Tokens[tokenIndexToRemove].address, + tokenIndexToRemove + ); + + expect( + await collector.getTokens(), + 'Before removal' + ).to.not.include.members([fakeERC20Tokens[tokenIndexToRemove].address]); + }); + }); + + describe('updateShares', function () { + let newPartners: Partner[]; + let deployCollector: CollectorDeployFunction; + + beforeEach(async function () { + newPartners = await buildPartners( + (await ethers.getSigners()).slice(6, 10), + PARTNER_SHARES + ); + + deployCollector = prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ); + }); + + it('Should update shares and partners when any token has zero balance', async function () { + const collector = await deployCollector({}); + + const expectedPartnersShares = newPartners.map( + ({ beneficiary, share }) => [beneficiary, share] + ); + + expect(await collector.getPartners()).to.not.have.deep.members( + expectedPartnersShares + ); // ensure the state is initially different + + await collector.updateShares(newPartners); + const actualPartnersShares = (await collector.getPartners()).map( + (partnerShare) => [partnerShare.beneficiary, partnerShare.share] + ); + + expect(actualPartnersShares).to.have.deep.members(expectedPartnersShares); + }); + + it('Should update shares and partners when all token balances are less than the number of partners', async function () { + const tokens = fakeERC20Tokens.map((token) => { + token.balanceOf.returns(3); + + return token.address; + }); + const collector = await deployCollector({ + tokens, + }); + const expectedPartnersShares = newPartners.map( + ({ beneficiary, share }) => [beneficiary, share] + ); + + expect(await collector.getPartners()).to.not.have.deep.members( + expectedPartnersShares + ); // make sure the state is initially different + + await collector.updateShares(newPartners); + const actualPartnersShares = (await collector.getPartners()).map( + (partnerShare) => [partnerShare.beneficiary, partnerShare.share] + ); + + expect(actualPartnersShares).to.have.deep.members(expectedPartnersShares); + }); + + it('Should fail when any token balance > number of partners', async function () { + fakeERC20Tokens[1].balanceOf.returns(5); + + const collector = await deployCollector({ + tokens: fakeERC20Tokens.map(({ address }) => address), + }); + + await expect(collector.updateShares(newPartners)).to.be.rejectedWith( + 'There is balance to share' + ); + }); + + it('Should fail when it is called by an account different than the owner', async function () { + const collector = await deployCollector({}); + const notTheOwner = (await ethers.getSigners()).at( + 11 + ) as SignerWithAddress; + + await expect( + collector.connect(notTheOwner).updateShares(newPartners) + ).to.eventually.be.rejectedWith('Only owner can call this'); + }); + + it('Should fail if the shares does not sum up to 100', async function () { + const collector = await deployCollector({}); + newPartners[0].share = 25; + newPartners[1].share = 25; + newPartners[2].share = 25; + newPartners[3].share = 26; + + await expect(collector.updateShares(newPartners)).to.be.rejectedWith( + 'Shares must add up to 100%' + ); + }); + + it('Should fail if any one of the shares is 0', async function () { + const collector = await deployCollector({}); + newPartners[3].share = 0; + + await expect(collector.updateShares(newPartners)).to.be.rejectedWith( + '0 is not a valid share' + ); + }); + }); + + describe('updateRemainderAddress', function () { + let deployCollector: CollectorDeployFunction; + + beforeEach(function () { + deployCollector = prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ); + }); + + it('Should update remainder address when any token balance is zero', async function () { + const collector = await deployCollector({}); + const expectedAddress = Wallet.createRandom().address; + await collector.updateRemainderAddress(expectedAddress); + const actualAddress = await collector.getRemainderAddress(); + + expect(actualAddress).eql(expectedAddress); + }); + + it(`Should withdraw all tokens' non-zero balances to the previous remainder address`, async function () { + const tokens: string[] = fakeERC20Tokens.map((token) => { + token.balanceOf.returns(3); + + return token.address; + }); + const collector = await deployCollector({ + tokens, + }); + const previousRemainderAddress = remainderDestination.address; + const newRemainderAddress = Wallet.createRandom().address; + await collector.updateRemainderAddress(newRemainderAddress); + + await Promise.allSettled( + fakeERC20Tokens.map(async (token, i) => { + const tokenBalance: BigNumber = await token.balanceOf( + previousRemainderAddress + ); + + expect( + token.transfer, + `Token #${i} @${ + token.address + } didn't transfer ${tokenBalance.toString()}` + ).to.have.been.calledWith(remainderDestination.address, tokenBalance); + }) + ); + }); + + it('Should update remainder address when any token balance is non-zero', async function () { + fakeERC20Tokens[0].balanceOf.returns(1); + const collector = await deployCollector({ + tokens: fakeERC20Tokens.map(({ address }) => address), + }); + const expectedAddress = Wallet.createRandom().address; + await collector.updateRemainderAddress(expectedAddress); + const actualAddress = await collector.getRemainderAddress(); + + expect(actualAddress).eql(expectedAddress); + }); + + it('Should fail when any token balance >= remainder', async function () { + const tokens = fakeERC20Tokens.map((token) => { + token.balanceOf.returns(5); + + return token.address; + }); + const collector = await deployCollector({ tokens }); + + await expect( + collector.updateRemainderAddress(Wallet.createRandom().address) + ).to.be.rejectedWith('There is balance to share'); + }); + + it('Should reject non-owner calls', async function () { + const collector = await deployCollector({}); + + const [newRemainderDestination, notTheOwner] = ( + await ethers.getSigners() + ).slice(11, 13); + + await expect( + collector + .connect(notTheOwner) + .updateRemainderAddress(newRemainderDestination.address) + ).to.be.rejectedWith('Only owner can call this'); + }); + }); + + describe('withdraw', function () { + let deployCollector: CollectorDeployFunction; + + beforeEach(function () { + deployCollector = prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ); + }); + + it('Should withdraw for each partner', async function () { + const tokens = fakeERC20Tokens.map((token) => { + token.balanceOf.returns(100); + + return token.address; + }); + const collector = await deployCollector({ tokens }); + await collector.withdraw(); + + for (let tokenIndex = 0; tokenIndex < tokens.length; tokenIndex++) { + checkPartnerBalanceForToken(partners, fakeERC20Tokens[tokenIndex]); + } + }); + + it('Should not fail if the revenue to share is equal to the number of partners', async function () { + const expectedTransferCount = partners.length; + const token = fakeERC20Tokens[0]; + token.balanceOf.returns(expectedTransferCount); + const collector = await deployCollector({ tokens: [token.address] }); + + await collector.withdraw(); + + expect(token.transfer).to.have.callCount(expectedTransferCount); + }); + + it('Should fail when not enough revenue to share in any of the tokens', async function () { + const lastTokenIndex = fakeERC20Tokens.length - 1; + const tokens = fakeERC20Tokens.map((token, i) => { + token.balanceOf.returns( + partners.length - (i === lastTokenIndex ? 1 : 0) + ); + + return token.address; + }); + const collector = await deployCollector({ tokens }); + + await expect(collector.withdraw()).to.be.revertedWith( + 'Not enough balance to split' + ); + }); + + it('Should fail if called by non-owner', async function () { + const collector = await deployCollector({}); + + const notTheOwner = (await ethers.getSigners()).at( + 11 + ) as SignerWithAddress; + + await expect( + collector.connect(notTheOwner).withdraw() + ).to.be.revertedWith('Only owner can call this'); + }); + + it('Should fail if the transfer returns false', async function () { + const token = fakeERC20Tokens[0]; + token.balanceOf.returns(100); + token.transfer.returns(false); + const collector = await deployCollector({ tokens: [token.address] }); + + await expect(collector.withdraw()).to.be.revertedWith( + 'Unable to withdraw' + ); + }); + }); + + describe('withdrawToken', function () { + let deployCollector: CollectorDeployFunction; + + beforeEach(function () { + deployCollector = prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + ); + }); + + it('Should withdraw for each partner', async function () { + const tokens = fakeERC20Tokens.map((token) => { + token.balanceOf.returns(100); + + return token.address; + }); + + const tokenToWithdraw = fakeERC20Tokens[0]; + const collector = await deployCollector({ tokens }); + await collector.withdrawToken(tokenToWithdraw.address); + + checkPartnerBalanceForToken(partners, tokenToWithdraw); + }); + + it('Should not fail if the revenue to share is equal to the number of partners', async function () { + const expectedTransferCount = partners.length; + const token = fakeERC20Tokens[0]; + token.balanceOf.returns(expectedTransferCount); + const collector = await deployCollector({ tokens: [token.address] }); + + await collector.withdrawToken(token.address); + + expect(token.transfer).to.have.callCount(expectedTransferCount); + }); + + it('Should fail when not enough revenue to share', async function () { + const tokens = fakeERC20Tokens.map(({ address }) => address); + const token = fakeERC20Tokens[0]; + token.balanceOf.returns(partners.length - 1); + const collector = await deployCollector({ tokens }); + + await expect(collector.withdrawToken(token.address)).to.be.revertedWith( + 'Not enough balance to split' + ); + }); + + it("should raise an error if the token isn't accepted", async function () { + const token = fakeERC20Tokens[0]; + const notAllowedToken = fakeERC20Tokens[1]; + [token, notAllowedToken].map((token) => token.balanceOf.returns(100)); + const collector = await deployCollector({ tokens: [token.address] }); + + await expect( + collector.withdrawToken(notAllowedToken.address) + ).to.be.revertedWith('Token is not accepted'); + }); + + it('Should fail if the transfer returns false', async function () { + const token = fakeERC20Tokens[0]; + token.balanceOf.returns(100); + token.transfer.returns(false); + const collector = await deployCollector({ tokens: [token.address] }); + + await expect(collector.withdrawToken(token.address)).to.be.revertedWith( + 'Unable to withdraw' + ); + }); + + it('Should fail if called by non-owner', async function () { + const collector = await deployCollector({}); + const token = fakeERC20Tokens[0]; + + const notTheOwner = (await ethers.getSigners()).at( + 11 + ) as SignerWithAddress; + + await expect( + collector.connect(notTheOwner).withdrawToken(token.address) + ).to.be.revertedWith('Only owner can call this'); + }); + }); + + describe('transferOwnership', function () { + let collector: MockContract; + + beforeEach(async function () { + collector = await prepareDefaultDeployer( + collectorFactory, + owner.address, + fakeERC20Tokens.map(({ address }) => address), + partners, + remainderDestination.address + )({}); + }); + + it('Should transfer ownership', async function () { + const { address: newOwnerAddress } = (await ethers.getSigners()).at( + 6 + ) as SignerWithAddress; + + await collector.transferOwnership(newOwnerAddress); + + expect(await collector.owner()).to.be.equal(newOwnerAddress); + }); + + it('Should fail when called by an address that is not the owner', async function () { + const [newOwner, notCurrentOwner] = (await ethers.getSigners()).slice( + 6, + 8 + ); + + await expect( + collector.connect(notCurrentOwner).transferOwnership(newOwner.address) + ).to.be.revertedWith('Only owner can call this'); + }); + }); +}); diff --git a/test/collector.test.ts b/test/collector.test.ts deleted file mode 100644 index 0e7144c9..00000000 --- a/test/collector.test.ts +++ /dev/null @@ -1,434 +0,0 @@ -import { - smock, - FakeContract, - MockContractFactory, - MockContract, -} from '@defi-wonderland/smock'; -import chaiAsPromised from 'chai-as-promised'; -import chai from 'chai'; -import { ethers as hardhat } from 'hardhat'; -import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; -import { Collector, Collector__factory } from '../typechain-types'; - -chai.use(smock.matchers); -chai.use(chaiAsPromised); - -const expect = chai.expect; - -const PARTNER_SHARES = [20, 30, 40, 10]; -const NUMBER_OF_PARTNERS = PARTNER_SHARES.length; - -type Partner = { - beneficiary: string; - share: number; -}; - -async function buildPartners( - partnerWallets: SignerWithAddress[], - shares: number[] -) { - return await Promise.all( - partnerWallets.map(async (wallet, index) => { - const partner: Partner = { - beneficiary: await wallet.getAddress(), - share: shares[index], - }; - - return partner; - }) - ); -} - -describe('Collector', function () { - let owner: SignerWithAddress; - let collectorFactory: MockContractFactory; - let fakeToken: FakeContract; - let partners: Partner[]; - let remainderDestination: SignerWithAddress; - let partner1: SignerWithAddress; - let partner2: SignerWithAddress; - let partner3: SignerWithAddress; - let partner4: SignerWithAddress; - let collector: MockContract; - - async function deployCollector() { - partners = await buildPartners( - [partner1, partner2, partner3, partner4], - PARTNER_SHARES - ); - - collector = await collectorFactory.deploy( - owner.address, - fakeToken.address, - partners, - remainderDestination.address - ); - } - - beforeEach(async function () { - [owner, partner1, partner2, partner3, partner4, remainderDestination] = - await hardhat.getSigners(); - - fakeToken = await smock.fake('ERC20'); - fakeToken['transfer'].returns(true); - - collectorFactory = await smock.mock('Collector'); - }); - - describe('constructor()', function () { - it('Should deploy a Collector', async function () { - partners = await buildPartners( - [partner1, partner2, partner3, partner4], - PARTNER_SHARES - ); - - const collector = await collectorFactory.deploy( - owner.address, - fakeToken.address, - partners, - remainderDestination.address - ); - - expect(await collector.owner(), 'Failed to set the owner').to.be.a - .properAddress; - expect(await collector.token(), 'Failed to set the token').not.to.be.null; - }); - - it('Should not let deploy with invalid shares', async function () { - const partners = await buildPartners( - [partner1, partner2, partner3, partner4], - [25, 25, 25, 26] - ); - - await expect( - collectorFactory.deploy( - owner.address, - fakeToken.address, - partners, - remainderDestination.address - ) - ).to.be.revertedWith('Shares must add up to 100%'); - }); - - it('Should not let deploy if the array of partners is empty', async function () { - const partners: Partner[] = []; - - await expect( - collectorFactory.deploy( - owner.address, - fakeToken.address, - partners, - remainderDestination.address - ) - ).to.be.revertedWith('Shares must add up to 100%'); - }); - - it('Should not let deploy if a share is 0', async function () { - const partners = await buildPartners( - [partner1, partner2, partner3, partner4], - [30, 30, 0, 40] - ); - - await expect( - collectorFactory.deploy( - owner.address, - fakeToken.address, - partners, - remainderDestination.address - ) - ).to.be.revertedWith('0 is not a valid share'); - }); - }); - - describe('updateShares', function () { - beforeEach(async function () { - await deployCollector(); - }); - - it('Should update shares and partners when token balance is zero', async function () { - const [, , , , , , newPartner1, newPartner2, newPartner3, newPartner4] = - await hardhat.getSigners(); - - const newPartners = await buildPartners( - [newPartner1, newPartner2, newPartner3, newPartner4], - PARTNER_SHARES - ); - - await collector.updateShares(newPartners); - - expect(collector.updateShares).to.have.been.called; - }); - - it('Should update shares and partners when token balance is a remainder amount', async function () { - fakeToken.balanceOf.returns(3); - - const [, , , , , , newPartner1, newPartner2, newPartner3, newPartner4] = - await hardhat.getSigners(); - - const newPartners = await buildPartners( - [newPartner1, newPartner2, newPartner3, newPartner4], - PARTNER_SHARES - ); - - await collector.updateShares(newPartners); - - expect(collector.updateShares).to.have.been.called; - }); - - it('Should fail when token balance is grater than a remainder', async function () { - fakeToken.balanceOf.returns(5); - - const [, , , , , , newPartner1, newPartner2, newPartner3, newPartner4] = - await hardhat.getSigners(); - - const newPartners = await buildPartners( - [newPartner1, newPartner2, newPartner3, newPartner4], - PARTNER_SHARES - ); - - await expect(collector.updateShares(newPartners)).to.be.revertedWith( - 'There is balance to share' - ); - }); - - it('Should fail when is called by an account different than the owner', async function () { - const [ - , - , - , - , - , - , - newPartner1, - newPartner2, - newPartner3, - newPartner4, - notTheOwner, - ] = await hardhat.getSigners(); - - const newPartners = await buildPartners( - [newPartner1, newPartner2, newPartner3, newPartner4], - PARTNER_SHARES - ); - - await expect( - collector.connect(notTheOwner).updateShares(newPartners) - ).to.be.revertedWith('Only owner can call this'); - }); - - it('Should fail if the shares does not sum up to 100', async function () { - const [, , , , , , newPartner1, newPartner2, newPartner3, newPartner4] = - await hardhat.getSigners(); - - const newPartners = await buildPartners( - [newPartner1, newPartner2, newPartner3, newPartner4], - [25, 25, 25, 26] - ); - - await expect(collector.updateShares(newPartners)).to.be.revertedWith( - 'Shares must add up to 100%' - ); - }); - - it('Should fail if a share is 0', async function () { - const [, , , , , , newPartner1, newPartner2, newPartner3, newPartner4] = - await hardhat.getSigners(); - - const newPartners = await buildPartners( - [newPartner1, newPartner2, newPartner3, newPartner4], - [30, 30, 40, 0] - ); - - await expect(collector.updateShares(newPartners)).to.be.revertedWith( - '0 is not a valid share' - ); - }); - }); - - describe('updateRemainderAddress', function () { - beforeEach(async function () { - await deployCollector(); - }); - - it('Should update remainder address when token balance is zero', async function () { - const [, , , , , , newRemainderDestination] = await hardhat.getSigners(); - - await collector.updateRemainderAddress(newRemainderDestination.address); - - expect(collector.updateRemainderAddress).to.have.been.called; - }); - - it('Should update remainder when token balance is a remainder and should withdraw remainder', async function () { - fakeToken.balanceOf.returns(3); - - const [, , , , , , newRemainderDestination] = await hardhat.getSigners(); - - await collector.updateRemainderAddress(newRemainderDestination.address); - - expect(fakeToken.transfer).to.have.been.calledWith( - remainderDestination.address, - 3 - ); - }); - - it('Should withdraw when the remainders address sent is the same as the current one', async function () { - fakeToken.balanceOf.returns(3); - - await collector.updateRemainderAddress(remainderDestination.address); - - expect(fakeToken.transfer).to.have.been.calledWith( - remainderDestination.address, - 3 - ); - }); - - it('Should fail when token balance > = remainder', async function () { - fakeToken.balanceOf.returns(5); - - const [, , , , , , newRemainderDestination] = await hardhat.getSigners(); - - await expect( - collector.updateRemainderAddress(newRemainderDestination.address) - ).to.be.revertedWith('There is balance to share'); - }); - - it('Should fail when is called by an address that is not the owner', async function () { - fakeToken.balanceOf.returns(5); - - const [, , , , , , newRemainderDestination, notTheOwner] = - await hardhat.getSigners(); - - await expect( - collector - .connect(notTheOwner) - .updateRemainderAddress(newRemainderDestination.address) - ).to.be.revertedWith('Only owner can call this'); - }); - }); - - describe('getBalance()', function () { - beforeEach(async function () { - await deployCollector(); - }); - - it('Should return 0 if the contract has been just deployed', async function () { - fakeToken.balanceOf.returns(0); - - expect(await collector.getBalance()).to.equal(0); - }); - - describe('updateRemainderAddress', function () { - it('Should fail if the transfer returns false', async function () { - const [owner, , newRemainder] = await hardhat.getSigners(); - - fakeToken['transfer'].returns(false); - fakeToken['balanceOf'].returns(2); - - await expect( - collector.updateRemainderAddress(newRemainder.address, { - from: owner.address, - gasLimit: 100000, - }) - ).to.be.revertedWith('Unable to transfer remainder'); - }); - - it('Should return 100 after that value has been minted', async function () { - fakeToken.balanceOf.returns(100); - - expect(await collector.getBalance()).to.equal(100); - }); - }); - - describe('withdraw', function () { - beforeEach(async function () { - await deployCollector(); - }); - - it('Should withdraw', async function () { - fakeToken.balanceOf.returns(100); - await collector.withdraw(); - - expect( - fakeToken.transfer, - 'Partner[0] balance' - ).to.have.been.calledWith(partners[0].beneficiary, PARTNER_SHARES[0]); - - expect( - fakeToken.transfer, - 'Partner[1] balance' - ).to.have.been.calledWith(partners[1].beneficiary, PARTNER_SHARES[1]); - - expect( - fakeToken.transfer, - 'Partner[2] balance' - ).to.have.been.calledWith(partners[2].beneficiary, PARTNER_SHARES[2]); - - expect( - fakeToken.transfer, - 'Partner[3] balance' - ).to.have.been.calledWith(partners[3].beneficiary, PARTNER_SHARES[3]); - }); - - it('Should not fail if the revenue to share is equal to the number of partners', async function () { - // We assume the current balance of the collector to be - // equal to the number of partners - fakeToken.balanceOf.returns(NUMBER_OF_PARTNERS); - - await collector.withdraw(); - - expect(fakeToken.transfer).to.have.callCount(4); - }); - - it('Should fail when no revenue to share', async function () { - fakeToken.balanceOf.returns(NUMBER_OF_PARTNERS - 1); - - await expect(collector.withdraw()).to.be.revertedWith( - 'Not enough balance to split' - ); - }); - - it('Should fail when is called by an address that is not the owner', async function () { - fakeToken.balanceOf.returns(100); - - const [, , , , , , notTheOwner] = await hardhat.getSigners(); - - await expect( - collector.connect(notTheOwner).withdraw() - ).to.be.revertedWith('Only owner can call this'); - }); - - it('Should fail if the transfer returns false', async function () { - const [owner] = await hardhat.getSigners(); - - fakeToken['transfer'].returns(false); - fakeToken['balanceOf'].returns(100); - - await expect( - collector.withdraw({ from: owner.address, gasLimit: 1000000 }) - ).to.be.revertedWith('Unable to withdraw'); - }); - }); - - describe('transferOwnership', function () { - beforeEach(async function () { - await deployCollector(); - }); - - it('Should transfer ownership', async function () { - const [, , , , , , newOwner] = await hardhat.getSigners(); - - await collector.transferOwnership(newOwner.address); - - expect(await collector.owner()).to.be.equal(newOwner.address); - }); - - it('Should fail when is called by an address that is not the owner', async function () { - const [, , , , , , newOwner, notTheOwner] = await hardhat.getSigners(); - - await expect( - collector.connect(notTheOwner).transferOwnership(newOwner.address) - ).to.be.revertedWith('Only owner can call this'); - }); - }); - }); -}); diff --git a/test/tasks/deployCollector.test.ts b/test/tasks/deployCollector.test.ts index 4c288f65..2c4d31c9 100644 --- a/test/tasks/deployCollector.test.ts +++ b/test/tasks/deployCollector.test.ts @@ -8,6 +8,7 @@ import { ethers } from 'hardhat'; import sinon, { SinonStub, SinonStubbedInstance } from 'sinon'; import { Collector, Collector__factory } from 'typechain-types'; import { + CollectorConfig, DEFAULT_CONFIG_FILE_NAME, DEFAULT_OUTPUT_FILE_NAME, deployCollector, @@ -19,7 +20,7 @@ use(smock.matchers); use(chaiAsPromised); describe('Deploy Script', function () { - const collectorConfiguration = { + const collectorConfiguration: CollectorConfig = { collectorOwner: '0x4E28f372BCe2d0Bf1B129b6A278F582558BF08a7', partners: [ { @@ -31,7 +32,7 @@ describe('Deploy Script', function () { share: 80, }, ], - tokenAddress: '0x726ECC75d5D51356AA4d0a5B648790cC345985ED', + tokenAddresses: ['0x726ECC75d5D51356AA4d0a5B648790cC345985ED'], remainderAddress: '0xc354D97642FAa06781b76Ffb6786f72cd7746C97', }; let configurationFileContent: string; @@ -47,7 +48,7 @@ describe('Deploy Script', function () { ); collector = await collectorFactoryMock.deploy( collectorConfiguration.collectorOwner, - collectorConfiguration.tokenAddress, + collectorConfiguration.tokenAddresses, collectorConfiguration.partners, collectorConfiguration.remainderAddress ); @@ -111,6 +112,7 @@ describe('Deploy Script', function () { }); it('deploy a collector', async function () { + //FIXME: Mismatch of test case and test itself const configFileName = 'inputFile.json'; const taskArgs: DeployCollectorArg = { configFileName, @@ -120,16 +122,17 @@ describe('Deploy Script', function () { expect(await collector.owner()).to.be.eq( collectorConfiguration.collectorOwner ); - expect(await collector.token()).to.be.eq( - collectorConfiguration.tokenAddress + expect(await collector.getTokens()).to.have.members( + collectorConfiguration.tokenAddresses ); const partners = await collector.getPartners(); - partners.forEach(([beneficiary, share]) => { - expect(collectorConfiguration.partners).to.deep.include({ + + expect(partners).to.have.deep.members( + collectorConfiguration.partners.map(({ beneficiary, share }) => [ beneficiary, share, - }); - }); + ]) + ); }); describe('', function () { @@ -146,7 +149,7 @@ describe('Deploy Script', function () { [chainId.toString()]: { collectorContract: collector.address, collectorOwner: await collector.owner(), - tokenAddress: await collector.token(), + tokenAddresses: await collector.getTokens(), remainderAddress: collectorConfiguration.remainderAddress, partners: collectorConfiguration.partners, }, diff --git a/tsconfig.json b/tsconfig.json index 729ea8e8..88fa65a6 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,6 +8,13 @@ * due to import of types from typechain-types. */ "importsNotUsedAsValues": "remove", + /* + * Adding `undefined` to array item type makes less sense + * in testing, where items of arrays in runtime are + * generally guaranteed to exist, thus we overwrite + * `noUncheckedIndexedAccess` to false. + */ + "noUncheckedIndexedAccess": false, "resolveJsonModule": true, "outDir": "dist",