diff --git a/contracts/Collector.sol b/contracts/Collector.sol index d9057c7f..c1c399c5 100644 --- a/contracts/Collector.sol +++ b/contracts/Collector.sol @@ -74,9 +74,19 @@ contract Collector is ICollector{ noBalanceToShare { uint256 balance = token.balanceOf(address(this)); + address tokenAddr = address(token); if(balance != 0) { - token.transfer(remainderAddress, balance); + // solhint-disable-next-line avoid-low-level-calls + (bool success, bytes memory ret ) = tokenAddr.call{gas: 200000}(abi.encodeWithSelector( + hex"a9059cbb", + remainderAddress, + balance)); + + require( + success && (ret.length == 0 || abi.decode(ret, (bool))), + "Unable to transfer remainder" + ); } // solhint-disable-next-line @@ -98,8 +108,20 @@ contract Collector is ICollector{ uint256 balance = token.balanceOf(address(this)); require(balance >= partners.length, "Not enough balance to split"); - for(uint256 i = 0; i < partners.length; i++) - token.transfer(partners[i].beneficiary, SafeMath.div(SafeMath.mul(balance, partners[i].share), 100)); + address tokenAddr = address(token); + + for(uint256 i = 0; i < partners.length; i++){ + // solhint-disable-next-line avoid-low-level-calls + (bool success, bytes memory ret ) = tokenAddr.call(abi.encodeWithSelector( + hex"a9059cbb", + partners[i].beneficiary, + SafeMath.div(SafeMath.mul(balance, partners[i].share), 100))); + + require( + success && (ret.length == 0 || abi.decode(ret, (bool))), + "Unable to withdraw" + ); + } } function transferOwnership(address _owner) diff --git a/test/collector.test.ts b/test/collector.test.ts index 5e9b040d..8e1b35c9 100644 --- a/test/collector.test.ts +++ b/test/collector.test.ts @@ -1,11 +1,13 @@ import { expect, use } from 'chai'; import Collector from '../build/contracts/Collector.json'; import TestToken from '../build/contracts/TestToken.json'; +import IERC20 from '../build/contracts/IERC20.json'; import { deployContract, loadFixture, MockProvider, - solidity + solidity, + deployMockContract } from 'ethereum-waffle'; import { Wallet } from '@ethersproject/wallet'; @@ -285,6 +287,41 @@ describe('Collector', () => { ) ).to.be.revertedWith('Only owner can call this'); }); + + it('Should fail if the transfer returns false', async () => { + const [ + owner, + oldRemainder, + newRemainder, + partner1, + partner2, + partner3, + partner4 + ] = new MockProvider().getWallets(); + + const mockERC20 = await deployMockContract(owner, IERC20.abi); + await mockERC20.mock.transfer.returns(false); + await mockERC20.mock.balanceOf.returns(2); + + const partners = await buildPartners( + [partner1, partner2, partner3, partner4], + PARTNER_SHARES + ); + + const collector = await deployContract(owner, Collector, [ + owner.address, + mockERC20.address, + partners, + oldRemainder.address + ]); + + await expect( + collector.updateRemainderAddress(newRemainder.address, { + from: owner.address, + gasLimit: 100000 + }) + ).to.be.revertedWith('Unable to transfer remainder'); + }); }); describe('getBalance', () => { @@ -377,6 +414,31 @@ describe('Collector', () => { externallyLinkedCollector.withdraw() ).to.be.revertedWith('Only owner can call this'); }); + + it('Should fail if the transfer returns false', async () => { + const [owner, remainder, partner1, partner2, partner3, partner4] = + new MockProvider().getWallets(); + + const mockERC20 = await deployMockContract(owner, IERC20.abi); + await mockERC20.mock.transfer.returns(false); + await mockERC20.mock.balanceOf.returns(100); + + const partners = await buildPartners( + [partner1, partner2, partner3, partner4], + PARTNER_SHARES + ); + + const collector = await deployContract(owner, Collector, [ + owner.address, + mockERC20.address, + partners, + remainder.address + ]); + + await expect( + collector.withdraw({ from: owner.address, gasLimit: 1000000 }) + ).to.be.revertedWith('Unable to withdraw'); + }); }); describe('transferOwnership', () => {