From a272a8046992a15d338b095f2012bb84e9ef18f6 Mon Sep 17 00:00:00 2001 From: Justin Chen Date: Sat, 16 Jun 2018 17:02:07 -0700 Subject: [PATCH 1/5] Unskip SetToken.sol for coverage checks --- .solcover.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/.solcover.js b/.solcover.js index ed0eaea4a..aed5cfdb7 100644 --- a/.solcover.js +++ b/.solcover.js @@ -4,7 +4,5 @@ module.exports = { skipFiles: [ 'Migrations.sol', 'test', - // Temporarily skipped - 'core/SetToken.sol', ], }; From 2ac370b3ea8b5b10ce210d523626e3412e3f4029 Mon Sep 17 00:00:00 2001 From: Justin Chen Date: Sat, 16 Jun 2018 17:24:42 -0700 Subject: [PATCH 2/5] Remove unused modifier --- contracts/core/SetToken.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/contracts/core/SetToken.sol b/contracts/core/SetToken.sol index 6e1f2ae41..f28823a81 100644 --- a/contracts/core/SetToken.sol +++ b/contracts/core/SetToken.sol @@ -67,11 +67,6 @@ contract SetToken is /* ============ Modifiers ============ */ - modifier isMultipleOfNaturalUnit(uint _quantity) { - require((_quantity % naturalUnit) == 0); - _; - } - modifier isCore() { require( msg.sender == ISetFactory(factory).core(), From d191c334b4849a6eeb96d6108378ed96be8749a4 Mon Sep 17 00:00:00 2001 From: Justin Chen Date: Sat, 16 Jun 2018 18:52:44 -0700 Subject: [PATCH 3/5] Fix styling of operators --- contracts/core/SetToken.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/core/SetToken.sol b/contracts/core/SetToken.sol index f28823a81..212279e28 100644 --- a/contracts/core/SetToken.sol +++ b/contracts/core/SetToken.sol @@ -174,7 +174,7 @@ contract SetToken is // This is the minimum natural unit possible for a Set with these components. require( - _naturalUnit >= uint(10)**(18 - minDecimals), + _naturalUnit >= uint(10) ** (18 - minDecimals), "Set naturalUnit must be greater than minimum of component decimals" ); From cdf85eb7b72f59225e6e781fa9bcc7cfb7a04a60 Mon Sep 17 00:00:00 2001 From: Justin Chen Date: Sun, 17 Jun 2018 01:10:06 -0700 Subject: [PATCH 4/5] Add some more specs for SetToken --- test/setToken.spec.ts | 147 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/test/setToken.spec.ts b/test/setToken.spec.ts index 289bd66ff..ee0972e00 100644 --- a/test/setToken.spec.ts +++ b/test/setToken.spec.ts @@ -399,4 +399,151 @@ contract("SetToken", (accounts) => { }); }); }); + + describe("#transfer", async () => { + let subjectCaller: Address; + let subjectTokenReceiver: Address; + let subjectQuantityToTransfer: BigNumber; + + beforeEach(async () => { + const quantityToMint = ether(5); + components = await coreWrapper.deployTokensAsync(3, deployerAccount); + factory = await coreWrapper.deploySetTokenFactoryAsync(); + await coreWrapper.setCoreAddress(factory, coreAccount); + + const componentAddresses = _.map(components, (token) => token.address); + const componentUnits = _.map(components, () => ether(randomIntegerLessThan(4))); + setToken = await coreWrapper.deploySetTokenAsync( + factory.address, + componentAddresses, + componentUnits, + STANDARD_NATURAL_UNIT, + "Set Token", + "SET", + ); + + await setToken.mint.sendTransactionAsync( + deployerAccount, + quantityToMint, + { from: coreAccount }, + ); + + subjectCaller = deployerAccount; + subjectTokenReceiver = otherAccount; + subjectQuantityToTransfer = STANDARD_NATURAL_UNIT; + }); + + async function subject(): Promise { + return setToken.transfer.sendTransactionAsync( + subjectTokenReceiver, + subjectQuantityToTransfer, + { from: subjectCaller }, + ); + } + + it("transfers the tokens to the right receiver", async () => { + const existingReceiverBalance = await setToken.balanceOf.callAsync(subjectTokenReceiver); + + await subject(); + + const newReceiverBalance = await setToken.balanceOf.callAsync(subjectTokenReceiver); + expect(newReceiverBalance).to.be.bignumber.equal(existingReceiverBalance.add(subjectQuantityToTransfer)); + }); + + describe("when the destination is null address", async () => { + beforeEach(async () => { + subjectTokenReceiver = NULL_ADDRESS; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + + describe("when the destination is set token address", async () => { + beforeEach(async () => { + subjectTokenReceiver = setToken.address; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + }); + + describe("#transferFrom", async () => { + let subjectCaller: Address; + let subjectTokenReceiver: Address; + let subjectTokenSender: Address; + let subjectQuantityToTransfer: BigNumber; + + beforeEach(async () => { + const quantityToMint = ether(5); + components = await coreWrapper.deployTokensAsync(3, deployerAccount); + factory = await coreWrapper.deploySetTokenFactoryAsync(); + await coreWrapper.setCoreAddress(factory, coreAccount); + + const componentAddresses = _.map(components, (token) => token.address); + const componentUnits = _.map(components, () => ether(randomIntegerLessThan(4))); + setToken = await coreWrapper.deploySetTokenAsync( + factory.address, + componentAddresses, + componentUnits, + STANDARD_NATURAL_UNIT, + "Set Token", + "SET", + ); + + await setToken.mint.sendTransactionAsync( + deployerAccount, + quantityToMint, + { from: coreAccount }, + ); + + await coreWrapper.approveTransferAsync(setToken, deployerAccount, deployerAccount); + + subjectCaller = deployerAccount; + subjectTokenReceiver = otherAccount; + subjectTokenSender = deployerAccount + subjectQuantityToTransfer = STANDARD_NATURAL_UNIT; + }); + + async function subject(): Promise { + return setToken.transferFrom.sendTransactionAsync( + subjectTokenSender, + subjectTokenReceiver, + subjectQuantityToTransfer, + { from: subjectCaller }, + ); + } + + it("transfers the tokens to the right receiver", async () => { + const existingReceiverBalance = await setToken.balanceOf.callAsync(subjectTokenReceiver); + + await subject(); + + const newReceiverBalance = await setToken.balanceOf.callAsync(subjectTokenReceiver); + expect(newReceiverBalance).to.be.bignumber.equal(existingReceiverBalance.add(subjectQuantityToTransfer)); + }); + + describe("when the destination is null address", async () => { + beforeEach(async () => { + subjectTokenReceiver = NULL_ADDRESS; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + + describe("when the destination is set token address", async () => { + beforeEach(async () => { + subjectTokenReceiver = setToken.address; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + }); }); From 60902546f3d1fc0022890d03069572533d4bf3ce Mon Sep 17 00:00:00 2001 From: Justin Chen Date: Sun, 17 Jun 2018 08:56:40 -0700 Subject: [PATCH 5/5] Add in NoDecimalTokenMock and associated test with SetToken. Get rid of AddressArrayUtils contract --- contracts/core/SetToken.sol | 4 +--- contracts/lib/AddressArrayUtils.sol | 21 --------------------- contracts/test/NoDecimalTokenMock.sol | 25 +++++++++++++++++++++++++ test/setToken.spec.ts | 15 +++++++++++++++ test/utils/coreWrapper.ts | 24 ++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 24 deletions(-) delete mode 100644 contracts/lib/AddressArrayUtils.sol create mode 100644 contracts/test/NoDecimalTokenMock.sol diff --git a/contracts/core/SetToken.sol b/contracts/core/SetToken.sol index 212279e28..636d35d66 100644 --- a/contracts/core/SetToken.sol +++ b/contracts/core/SetToken.sol @@ -22,7 +22,6 @@ import { ERC20 } from "zeppelin-solidity/contracts/token/ERC20/ERC20.sol"; import { StandardToken } from "zeppelin-solidity/contracts/token/ERC20/StandardToken.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; import { ISetFactory } from "./interfaces/ISetFactory.sol"; -import "../lib/AddressArrayUtils.sol"; /** @@ -37,7 +36,6 @@ contract SetToken is DetailedERC20 { using SafeMath for uint256; - using AddressArrayUtils for address[]; /* ============ Constants ============ */ @@ -175,7 +173,7 @@ contract SetToken is // This is the minimum natural unit possible for a Set with these components. require( _naturalUnit >= uint(10) ** (18 - minDecimals), - "Set naturalUnit must be greater than minimum of component decimals" + "Set naturalUnit does not work with underlying component decimals" ); factory = _factory; diff --git a/contracts/lib/AddressArrayUtils.sol b/contracts/lib/AddressArrayUtils.sol deleted file mode 100644 index c91505286..000000000 --- a/contracts/lib/AddressArrayUtils.sol +++ /dev/null @@ -1,21 +0,0 @@ -pragma solidity 0.4.24; - -/** - * @title AddressArrayUtil - */ -library AddressArrayUtils { - function hasValue(address[] addresses, address value) internal pure returns (bool) { - for (uint i = 0; i < addresses.length; i++) { - if (addresses[i] == value) { - return true; - } - } - - return false; - } - - function removeByIndex(address[] storage a, uint256 index) internal returns (uint256) { - a[index] = a[a.length - 1]; - a.length -= 1; - } -} \ No newline at end of file diff --git a/contracts/test/NoDecimalTokenMock.sol b/contracts/test/NoDecimalTokenMock.sol new file mode 100644 index 000000000..1fa638a9c --- /dev/null +++ b/contracts/test/NoDecimalTokenMock.sol @@ -0,0 +1,25 @@ +pragma solidity 0.4.24; + + +import "zeppelin-solidity/contracts/token/ERC20/StandardToken.sol"; + + +contract NoDecimalTokenMock is StandardToken { + string public name; + string public symbol; + uint256 public totalSupply; + + constructor( + address initialAccount, + uint256 initialBalance, + string _name, + string _symbol) + public + { + balances[initialAccount] = initialBalance; + totalSupply = initialBalance; + name = _name; + symbol = _symbol; + } + +} diff --git a/test/setToken.spec.ts b/test/setToken.spec.ts index ee0972e00..5ab0e3152 100644 --- a/test/setToken.spec.ts +++ b/test/setToken.spec.ts @@ -246,6 +246,21 @@ contract("SetToken", (accounts) => { await expectRevertError(subject()); }); }); + + describe("when a component does not implement decimals() and natural unit lower", async () => { + beforeEach(async () => { + const minNaturalUnit = 10 ** 18; + const noDecimalToken = await coreWrapper.deployTokenWithNoDecimalAsync(deployerAccount); + subjectComponentAddresses.push(noDecimalToken.address); + subjectComponentUnits.push(STANDARD_COMPONENT_UNIT); + + subjectNaturalUnit = new BigNumber(minNaturalUnit).sub(1); + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); }); describe("#mint", async () => { diff --git a/test/utils/coreWrapper.ts b/test/utils/coreWrapper.ts index 7044b0813..28ee6142c 100644 --- a/test/utils/coreWrapper.ts +++ b/test/utils/coreWrapper.ts @@ -6,6 +6,7 @@ import { CoreContract } from "../../types/generated/core"; import { StandardTokenContract } from "../../types/generated/standard_token"; import { StandardTokenMockContract } from "../../types/generated/standard_token_mock"; import { StandardTokenWithFeeMockContract } from "../../types/generated/standard_token_with_fee_mock"; +import { NoDecimalTokenMockContract } from "../../types/generated/no_decimal_token_mock"; import { TransferProxyContract } from "../../types/generated/transfer_proxy"; import { VaultContract } from "../../types/generated/vault"; @@ -32,6 +33,7 @@ const TransferProxy = artifacts.require("TransferProxy"); const SetTokenFactory = artifacts.require("SetTokenFactory"); const StandardTokenMock = artifacts.require("StandardTokenMock"); const StandardTokenWithFeeMock = artifacts.require("StandardTokenWithFeeMock"); +const NoDecimalTokenMock = artifacts.require("NoDecimalTokenMock"); const Vault = artifacts.require("Vault"); const SetToken = artifacts.require("SetToken"); @@ -97,6 +99,28 @@ export class CoreWrapper { ); } + public async deployTokenWithNoDecimalAsync( + initialAccount: Address, + from: Address = this._tokenOwnerAddress + ): Promise { + const truffleMockToken = await NoDecimalTokenMock.new( + initialAccount, + DEPLOYED_TOKEN_QUANTITY, + "No Decimal Token", + "NDT", + { from, gas: DEFAULT_GAS }, + ); + + const mockTokenWeb3Contract = web3.eth + .contract(truffleMockToken.abi) + .at(truffleMockToken.address); + + return new NoDecimalTokenMockContract( + mockTokenWeb3Contract, + { from }, + ); + } + public async deployTokensAsync( tokenCount: number, initialAccount: Address,