From 2920233e7071baa3ad8052425cca1194348ce2d7 Mon Sep 17 00:00:00 2001 From: bweick Date: Sat, 14 Jul 2018 23:25:48 -0700 Subject: [PATCH 01/12] Added CoreIssuanceOrder scenario specs. --- .../coreIssuanceOrderScenarios.spec.ts | 303 ++++++++++++++++++ .../extensions/coreIssuanceOrderScenarios.ts | 47 +++ 2 files changed, 350 insertions(+) create mode 100644 test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts create mode 100644 test/contracts/core/extensions/coreIssuanceOrderScenarios.ts diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts new file mode 100644 index 000000000..f1149809d --- /dev/null +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -0,0 +1,303 @@ +import * as chai from "chai"; +import * as _ from "lodash"; + +import * as ABIDecoder from "abi-decoder"; +import { BigNumber } from "bignumber.js"; +import { ether } from "../../../utils/units"; + +// Types +import { Address, Bytes32, IssuanceOrder } from "../../../../types/common.js"; + +// Contract types +import { CoreContract } from "../../../../types/generated/core"; +import { SetTokenContract } from "../../../../types/generated/set_token"; +import { SetTokenFactoryContract } from "../../../../types/generated/set_token_factory"; +import { StandardTokenMockContract } from "../../../../types/generated/standard_token_mock"; +import { TakerWalletWrapperContract } from "../../../../types/generated/taker_wallet_wrapper"; +import { TransferProxyContract } from "../../../../types/generated/transfer_proxy"; +import { VaultContract } from "../../../../types/generated/vault"; + +// Artifacts +const Core = artifacts.require("Core"); + +// Core wrapper +import { CoreWrapper } from "../../../utils/coreWrapper"; +import { ERC20Wrapper } from "../../../utils/erc20Wrapper"; +import { ExchangeWrapper } from "../../../utils/exchangeWrapper"; +import { + generateFillOrderParameters, + generateOrdersDataForOrderCount, + generateOrdersDataWithIncorrectExchange, + generateOrdersDataWithTakerOrders, +} from "../../../utils/orderWrapper"; + +// Log Testing Tools +import { + assertLogEquivalence, + getFormattedLogsFromTxHash +} from "../../../utils/logs"; + +import { + getExpectedFillLog, + getExpectedCancelLog, +} from "../../../utils/contract_logs/coreIssuanceOrder"; + +// Testing Set up +import { BigNumberSetup } from "../../../utils/bigNumberSetup"; +import ChaiSetup from "../../../utils/chaiSetup"; +BigNumberSetup.configure(); +ChaiSetup.configure(); +const { expect, assert } = chai; + +import { + IssuanceComponentDeposited, +} from "../../../utils/contract_logs/core"; + +import { + assertTokenBalance, + expectRevertError, +} from "../../../utils/tokenAssertions"; + +import { + DEPLOYED_TOKEN_QUANTITY, + ZERO, + NULL_ADDRESS, + DEFAULT_GAS, + EXCHANGES, +} from "../../../utils/constants"; + +import { SCENARIOS } from "./coreIssuanceOrderScenarios"; + +contract("CoreIssuanceOrder::Scenarios", (accounts) => { + const [ + ownerAccount, + takerAccount, + makerAccount, + signerAccount, + relayerAccount, + mockSetTokenAccount, + mockTokenAccount + ] = accounts; + + let core: CoreContract; + let transferProxy: TransferProxyContract; + let vault: VaultContract; + let setTokenFactory: SetTokenFactoryContract; + let takerWalletWrapper: TakerWalletWrapperContract; + + const coreWrapper = new CoreWrapper(ownerAccount, ownerAccount); + const erc20Wrapper = new ERC20Wrapper(ownerAccount); + const exchangeWrapper = new ExchangeWrapper(ownerAccount); + + before(async () => { + ABIDecoder.addABI(Core.abi); + }); + + after(async () => { + ABIDecoder.removeABI(Core.abi); + }); + + beforeEach(async () => { + core = await coreWrapper.deployCoreAsync(); + vault = await coreWrapper.deployVaultAsync(); + transferProxy = await coreWrapper.deployTransferProxyAsync(); + setTokenFactory = await coreWrapper.deploySetTokenFactoryAsync(); + takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(transferProxy); + + // TODO: Move these authorizations into setDefaultStateAndAuthrorizations + await coreWrapper.addAuthorizationAsync(takerWalletWrapper, core.address);; + await coreWrapper.addAuthorizationAsync(transferProxy, takerWalletWrapper.address); + + await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); + }); + + describe("#fillOrder", async () => { + SCENARIOS.forEach(async (scenario) => { + describe(scenario.description, async () => { + let subjectCaller: Address; + let subjectQuantityToIssue: BigNumber; + let subjectExchangeOrdersData: Bytes32; + + const naturalUnit: BigNumber = ether(2); + let deployedTokens: StandardTokenMockContract[] = []; + let componentUnits: BigNumber[]; + let setToken: SetTokenContract; + + let setAddress: Address; + let makerAddress: Address; + let signerAddress: Address; + let relayerAddress: Address; + let componentAddresses: Address[]; + let defaultComponentAmounts: BigNumber[]; + let requiredComponents: Address[]; + let requiredComponentAmounts: BigNumber[]; + let makerToken: StandardTokenMockContract; + let relayerToken: StandardTokenMockContract; + let makerTokenAmount: BigNumber; + let relayerTokenAmount: BigNumber = ether(1); + let timeToExpiration: number; + + let issuanceOrderParams: any; + + beforeEach(async () => { + deployedTokens = await erc20Wrapper.deployTokensAsync(scenario.tokenState.numberOfComponents + 2, ownerAccount); + await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, ownerAccount); + await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, signerAccount); + await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, takerAccount); + + // Give taker its Set component tokens + scenario.tokenState.takerAmounts.forEach(async (amount, idx) => { + await erc20Wrapper.transferTokenAsync(deployedTokens[idx], takerAccount, amount, ownerAccount); + }); + + // Give maker its Set component tokens + scenario.tokenState.makerAmounts.forEach(async (amount, idx) => { + await erc20Wrapper.transferTokenAsync(deployedTokens[idx], takerAccount, amount, ownerAccount); + }); + + //Deposit maker tokens in Vault + + // Give maker and taker their maker and relayer tokens + await erc20Wrapper.transferTokensAsync(deployedTokens.slice(-2), signerAccount, DEPLOYED_TOKEN_QUANTITY.div(2), ownerAccount); + await erc20Wrapper.transferTokensAsync(deployedTokens.slice(-2), takerAccount, DEPLOYED_TOKEN_QUANTITY.div(2), ownerAccount); + + const componentTokens = deployedTokens.slice(0, scenario.tokenState.numberOfComponents); + componentAddresses = _.map(componentTokens, (token) => token.address); + componentUnits = _.map(componentTokens, () => ether(4)); // Multiple of naturalUnit + setToken = await coreWrapper.createSetTokenAsync( + core, + setTokenFactory.address, + componentAddresses, + componentUnits, + naturalUnit, + ); + + requiredComponentAmounts = _.map(componentUnits, (unit, idx) => + unit.mul(scenario.exchangeOrders.orderQuantity) + .mul(scenario.issuanceOrderParams.takerWeightsToTransfer[idx]).div(naturalUnit)); + + await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); + + makerAddress = signerAccount; + relayerAddress = relayerAccount; + makerToken = deployedTokens.slice(-2, -1)[0]; + relayerToken = deployedTokens.slice(-1)[0]; + timeToExpiration = 10; + + issuanceOrderParams = await generateFillOrderParameters( + setToken.address, + signerAccount, + makerAddress, + componentAddresses, + requiredComponentAmounts, + makerToken.address, + relayerAddress, + relayerToken.address, + scenario.exchangeOrders.orderQuantity, + scenario.exchangeOrders.makerTokenAmount, + timeToExpiration, + ); + + const takerAmountsToTransfer = _.map(componentUnits, (unit, idx) => + unit.mul(scenario.exchangeOrders.orderQuantity) + .mul(scenario.exchangeOrders.requiredComponentWeighting[idx]).div(naturalUnit)); + + subjectExchangeOrdersData = generateOrdersDataWithTakerOrders( + makerToken.address, + componentAddresses, + takerAmountsToTransfer, + ); + + subjectCaller = takerAccount; + subjectQuantityToIssue = ether(4); + }); + + async function subject(): Promise { + return core.fillOrder.sendTransactionAsync( + issuanceOrderParams.addresses, + issuanceOrderParams.values, + issuanceOrderParams.requiredComponents, + issuanceOrderParams.requiredComponentAmounts, + subjectQuantityToIssue, + issuanceOrderParams.signature.v, + [issuanceOrderParams.signature.r, issuanceOrderParams.signature.s], + subjectExchangeOrdersData, + { from: subjectCaller }, + ); + } + + it("transfers the full maker token amount from the maker", async () => { + const existingBalance = await makerToken.balanceOf.callAsync(signerAccount); + await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), signerAccount); + + await subject(); + + const fullMakerTokenAmount = ether(10); + const newBalance = await makerToken.balanceOf.callAsync(signerAccount); + const expectedNewBalance = existingBalance.sub(fullMakerTokenAmount); + await assertTokenBalance(makerToken, expectedNewBalance, signerAccount); + }); + + it("transfers the remaining maker tokens to the taker", async () => { + const existingBalance = await makerToken.balanceOf.callAsync(subjectCaller); + await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), subjectCaller); + + await subject(); + + const netMakerToTaker = ether(10); + const expectedNewBalance = existingBalance.plus(netMakerToTaker); + await assertTokenBalance(makerToken, expectedNewBalance, subjectCaller); + }); + + it("transfers the fees to the relayer", async () => { + const existingBalance = await relayerToken.balanceOf.callAsync(relayerAddress); + await assertTokenBalance(relayerToken, ZERO, relayerAddress); + + await subject(); + + const expectedNewBalance = relayerTokenAmount.mul(2); + await assertTokenBalance(relayerToken, expectedNewBalance, relayerAddress); + }); + + it("mints the correct quantity of the set for the maker", async () => { + const existingBalance = await setToken.balanceOf.callAsync(signerAccount); + + await subject(); + + await assertTokenBalance(setToken, existingBalance.add(subjectQuantityToIssue), signerAccount); + }); + + it("marks the correct amount as filled in orderFills mapping", async () => { + const preFilled = await core.orderFills.callAsync(issuanceOrderParams.orderHash); + expect(preFilled).to.be.bignumber.equal(ZERO); + + await subject(); + + const filled = await core.orderFills.callAsync(issuanceOrderParams.orderHash); + expect(filled).to.be.bignumber.equal(subjectQuantityToIssue); + }); + + it("emits correct LogFill event", async () => { + const txHash = await subject(); + + const formattedLogs = await getFormattedLogsFromTxHash(txHash); + const expectedLogs = getExpectedFillLog( + setToken.address, + signerAccount, + subjectCaller, + makerToken.address, + relayerAddress, + relayerToken.address, + subjectQuantityToIssue, + ether(10), + ether(2), + issuanceOrderParams.orderHash, + core.address + ); + + await assertLogEquivalence(expectedLogs, formattedLogs); + }); + }); + }); + }); +}); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts new file mode 100644 index 000000000..0dcd278bd --- /dev/null +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -0,0 +1,47 @@ +import { + DEPLOYED_TOKEN_QUANTITY, + ZERO, + NULL_ADDRESS, + EXCHANGES, +} from "../../../utils/constants"; + +import { + ether, +} from "../../../utils/units"; + +export const SCENARIOS = [ +{ + description: "Base Case", + tokenState: { + numberOfComponents: 2, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [ZERO, ZERO], + vault: [ZERO, ZERO], + }, + exchangeOrders: { + requiredComponentWeighting: [1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, + issuanceOrderParams: { + takerWeightsToTransfer: [1, 1], + }, +}, +{ + description: "Three Set Components Case", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [ZERO, ZERO, ZERO], + vault: [ZERO, ZERO, ZERO], + }, + exchangeOrders: { + requiredComponentWeighting: [1, 1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, + issuanceOrderParams: { + takerWeightsToTransfer: [1, 1, 1], + }, +}, +] \ No newline at end of file From c41fb78c4c72ef13e13b2cf252293d31fadf7a9e Mon Sep 17 00:00:00 2001 From: bweick Date: Sat, 14 Jul 2018 23:45:10 -0700 Subject: [PATCH 02/12] Clarified naming on scenario objects. --- .../extensions/coreIssuanceOrderScenarios.spec.ts | 14 +++++++------- .../core/extensions/coreIssuanceOrderScenarios.ts | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index f1149809d..f17eeb220 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -111,7 +111,7 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe("#fillOrder", async () => { + describe.only("#fillOrder", async () => { SCENARIOS.forEach(async (scenario) => { describe(scenario.description, async () => { let subjectCaller: Address; @@ -173,8 +173,8 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { ); requiredComponentAmounts = _.map(componentUnits, (unit, idx) => - unit.mul(scenario.exchangeOrders.orderQuantity) - .mul(scenario.issuanceOrderParams.takerWeightsToTransfer[idx]).div(naturalUnit)); + unit.mul(scenario.issuanceOrderParams.orderQuantity) + .mul(scenario.issuanceOrderParams.requiredComponentWeighting[idx]).div(naturalUnit)); await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); @@ -193,14 +193,14 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { makerToken.address, relayerAddress, relayerToken.address, - scenario.exchangeOrders.orderQuantity, - scenario.exchangeOrders.makerTokenAmount, + scenario.issuanceOrderParams.orderQuantity, + scenario.issuanceOrderParams.makerTokenAmount, timeToExpiration, ); const takerAmountsToTransfer = _.map(componentUnits, (unit, idx) => - unit.mul(scenario.exchangeOrders.orderQuantity) - .mul(scenario.exchangeOrders.requiredComponentWeighting[idx]).div(naturalUnit)); + unit.mul(scenario.issuanceOrderParams.orderQuantity) + .mul(scenario.exchangeOrders.takerWeightsToTransfer[idx]).div(naturalUnit)); subjectExchangeOrdersData = generateOrdersDataWithTakerOrders( makerToken.address, diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index 0dcd278bd..91dab1b8d 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -19,13 +19,13 @@ export const SCENARIOS = [ vault: [ZERO, ZERO], }, exchangeOrders: { + takerWeightsToTransfer: [1, 1], + }, + issuanceOrderParams: { requiredComponentWeighting: [1, 1], orderQuantity: ether(4), makerTokenAmount: ether(10), }, - issuanceOrderParams: { - takerWeightsToTransfer: [1, 1], - }, }, { description: "Three Set Components Case", @@ -36,12 +36,12 @@ export const SCENARIOS = [ vault: [ZERO, ZERO, ZERO], }, exchangeOrders: { + takerWeightsToTransfer: [1, 1, 1], + }, + issuanceOrderParams: { requiredComponentWeighting: [1, 1, 1], orderQuantity: ether(4), makerTokenAmount: ether(10), }, - issuanceOrderParams: { - takerWeightsToTransfer: [1, 1, 1], - }, }, ] \ No newline at end of file From fb2d6ec464f3d88ffb629c0059796fadd3f87c10 Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 16 Jul 2018 12:13:33 -0700 Subject: [PATCH 03/12] Removed unneccesary modifiers blocking issuanceOrders. --- contracts/core/Vault.sol | 1 - contracts/core/extensions/CoreAccounting.sol | 1 - contracts/core/extensions/CoreIssuance.sol | 16 ++++++++-------- .../coreIssuanceOrderScenarios.spec.ts | 7 +++++-- .../extensions/coreIssuanceOrderScenarios.ts | 17 +++++++++++++++++ 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/contracts/core/Vault.sol b/contracts/core/Vault.sol index 2e9d95d47..7e5a73eb6 100644 --- a/contracts/core/Vault.sol +++ b/contracts/core/Vault.sol @@ -127,7 +127,6 @@ contract Vault is ) external onlyAuthorized - isNonZero(_quantity) { // Increment balances state variable adding _quantity to user's token amount balances[_tokenAddress][_owner] = balances[_tokenAddress][_owner].add(_quantity); diff --git a/contracts/core/extensions/CoreAccounting.sol b/contracts/core/extensions/CoreAccounting.sol index 6487db0ea..6c01ead41 100644 --- a/contracts/core/extensions/CoreAccounting.sol +++ b/contracts/core/extensions/CoreAccounting.sol @@ -211,7 +211,6 @@ contract CoreAccounting is uint[] _quantities ) internal - isValidBatchTransaction(_tokenAddresses, _quantities) { // For each token and quantity pair, run deposit function for (uint i = 0; i < _tokenAddresses.length; i++) { diff --git a/contracts/core/extensions/CoreIssuance.sol b/contracts/core/extensions/CoreIssuance.sol index 6dbe69c48..593e0eeb2 100644 --- a/contracts/core/extensions/CoreIssuance.sol +++ b/contracts/core/extensions/CoreIssuance.sol @@ -259,14 +259,14 @@ contract CoreIssuance is uint amountToDeposit = requiredComponentQuantity.sub(vaultBalance); // Transfer the remainder component quantity required to vault - ITransferProxy(state.transferProxyAddress).transfer( - component, - requiredComponentQuantity.sub(vaultBalance), - _owner, - state.vaultAddress - ); - - // Log transfer of component from issuer waller + // ITransferProxy(state.transferProxyAddress).transfer( + // component, + // requiredComponentQuantity.sub(vaultBalance), + // _owner, + // state.vaultAddress + // ); + + // Log transfer of component from issuer wallet emit IssuanceComponentDeposited( _setAddress, component, diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index f17eeb220..8fcf06791 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -68,6 +68,9 @@ import { import { SCENARIOS } from "./coreIssuanceOrderScenarios"; +// import { injectInTruffle } from "sol-trace-set"; +// injectInTruffle(web3, artifacts); + contract("CoreIssuanceOrder::Scenarios", (accounts) => { const [ ownerAccount, @@ -104,8 +107,8 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { setTokenFactory = await coreWrapper.deploySetTokenFactoryAsync(); takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(transferProxy); - // TODO: Move these authorizations into setDefaultStateAndAuthrorizations - await coreWrapper.addAuthorizationAsync(takerWalletWrapper, core.address);; + // TODO: Move these authorizations into setDefaultStateAndAuthorizations + await coreWrapper.addAuthorizationAsync(takerWalletWrapper, core.address); await coreWrapper.addAuthorizationAsync(transferProxy, takerWalletWrapper.address); await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index 91dab1b8d..5d2fadfc8 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -44,4 +44,21 @@ export const SCENARIOS = [ makerTokenAmount: ether(10), }, }, +// { +// description: "Maker Has One Component in Wallet", +// tokenState: { +// numberOfComponents: 3, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], +// vault: [ZERO, ZERO, ZERO], +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [0, 1, 1], +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [0, 1, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, ] \ No newline at end of file From 7a35322287d314d6ddcca4b8689dae38d0d3d12f Mon Sep 17 00:00:00 2001 From: bweick Date: Wed, 18 Jul 2018 09:45:32 -0700 Subject: [PATCH 04/12] Refactored scenario tests. --- contracts/core/extensions/CoreIssuance.sol | 12 +- .../core/extensions/CoreIssuanceOrder.sol | 2 +- .../coreIssuanceOrderScenarios.spec.ts | 125 +++++++----------- .../extensions/coreIssuanceOrderScenarios.ts | 37 +++--- 4 files changed, 75 insertions(+), 101 deletions(-) diff --git a/contracts/core/extensions/CoreIssuance.sol b/contracts/core/extensions/CoreIssuance.sol index 593e0eeb2..7944daabd 100644 --- a/contracts/core/extensions/CoreIssuance.sol +++ b/contracts/core/extensions/CoreIssuance.sol @@ -259,12 +259,12 @@ contract CoreIssuance is uint amountToDeposit = requiredComponentQuantity.sub(vaultBalance); // Transfer the remainder component quantity required to vault - // ITransferProxy(state.transferProxyAddress).transfer( - // component, - // requiredComponentQuantity.sub(vaultBalance), - // _owner, - // state.vaultAddress - // ); + ITransferProxy(state.transferProxyAddress).transfer( + component, + requiredComponentQuantity.sub(vaultBalance), + _owner, + state.vaultAddress + ); // Log transfer of component from issuer wallet emit IssuanceComponentDeposited( diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index e43d0dbd5..5f3a58a6a 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -152,7 +152,7 @@ contract CoreIssuanceOrder is // Settle Order settleOrder(order, _fillQuantity, _orderData); - //Issue Set + //Issue Settle issueInternal( order.makerAddress, order.setAddress, diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index 8fcf06791..9d9611c7f 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -117,33 +117,28 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { describe.only("#fillOrder", async () => { SCENARIOS.forEach(async (scenario) => { describe(scenario.description, async () => { - let subjectCaller: Address; - let subjectQuantityToIssue: BigNumber; + let subjectCaller: Address = takerAccount; + let subjectQuantityToIssue: BigNumber = scenario.exchangeOrders.subjectQuantityToIssue; let subjectExchangeOrdersData: Bytes32; const naturalUnit: BigNumber = ether(2); - let deployedTokens: StandardTokenMockContract[] = []; - let componentUnits: BigNumber[]; - let setToken: SetTokenContract; - let setAddress: Address; - let makerAddress: Address; - let signerAddress: Address; - let relayerAddress: Address; - let componentAddresses: Address[]; - let defaultComponentAmounts: BigNumber[]; - let requiredComponents: Address[]; - let requiredComponentAmounts: BigNumber[]; + let makerAddress: Address = signerAccount; + let relayerAddress: Address = relayerAccount; + + let setToken: SetTokenContract; let makerToken: StandardTokenMockContract; let relayerToken: StandardTokenMockContract; - let makerTokenAmount: BigNumber; + + let makerTokenAmount: BigNumber = scenario.issuanceOrderParams.makerTokenAmount; let relayerTokenAmount: BigNumber = ether(1); - let timeToExpiration: number; + let orderQuantity: BigNumber = scenario.issuanceOrderParams.orderQuantity; + let fillPercentage: BigNumber = subjectQuantityToIssue.div(orderQuantity); let issuanceOrderParams: any; beforeEach(async () => { - deployedTokens = await erc20Wrapper.deployTokensAsync(scenario.tokenState.numberOfComponents + 2, ownerAccount); + const deployedTokens = await erc20Wrapper.deployTokensAsync(scenario.tokenState.numberOfComponents + 2, ownerAccount); await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, ownerAccount); await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, signerAccount); await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, takerAccount); @@ -164,9 +159,10 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await erc20Wrapper.transferTokensAsync(deployedTokens.slice(-2), signerAccount, DEPLOYED_TOKEN_QUANTITY.div(2), ownerAccount); await erc20Wrapper.transferTokensAsync(deployedTokens.slice(-2), takerAccount, DEPLOYED_TOKEN_QUANTITY.div(2), ownerAccount); + // Set up and create SetToken const componentTokens = deployedTokens.slice(0, scenario.tokenState.numberOfComponents); - componentAddresses = _.map(componentTokens, (token) => token.address); - componentUnits = _.map(componentTokens, () => ether(4)); // Multiple of naturalUnit + const componentAddresses = _.map(componentTokens, (token) => token.address); + const componentUnits = _.map(componentTokens, () => ether(4)); // Multiple of naturalUnit setToken = await coreWrapper.createSetTokenAsync( core, setTokenFactory.address, @@ -175,17 +171,15 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { naturalUnit, ); - requiredComponentAmounts = _.map(componentUnits, (unit, idx) => - unit.mul(scenario.issuanceOrderParams.orderQuantity) - .mul(scenario.issuanceOrderParams.requiredComponentWeighting[idx]).div(naturalUnit)); - - await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); - - makerAddress = signerAccount; - relayerAddress = relayerAccount; + // Define other tokens in test makerToken = deployedTokens.slice(-2, -1)[0]; relayerToken = deployedTokens.slice(-1)[0]; - timeToExpiration = 10; + + // Define rest of params for issuanceOrder and create issuanceOrder object + const requiredComponentAmounts = _.map(componentUnits, (unit, idx) => + unit.mul(scenario.issuanceOrderParams.orderQuantity) + .mul(scenario.issuanceOrderParams.requiredComponentWeighting[idx]).div(naturalUnit)); + const timeToExpiration = 10; issuanceOrderParams = await generateFillOrderParameters( setToken.address, @@ -201,6 +195,10 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { timeToExpiration, ); + // Register exchange with core + await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); + + // Create parameters for exchange orders and generate exchange order data const takerAmountsToTransfer = _.map(componentUnits, (unit, idx) => unit.mul(scenario.issuanceOrderParams.orderQuantity) .mul(scenario.exchangeOrders.takerWeightsToTransfer[idx]).div(naturalUnit)); @@ -210,9 +208,6 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { componentAddresses, takerAmountsToTransfer, ); - - subjectCaller = takerAccount; - subjectQuantityToIssue = ether(4); }); async function subject(): Promise { @@ -230,54 +225,30 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { } it("transfers the full maker token amount from the maker", async () => { - const existingBalance = await makerToken.balanceOf.callAsync(signerAccount); - await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), signerAccount); - - await subject(); - - const fullMakerTokenAmount = ether(10); - const newBalance = await makerToken.balanceOf.callAsync(signerAccount); - const expectedNewBalance = existingBalance.sub(fullMakerTokenAmount); - await assertTokenBalance(makerToken, expectedNewBalance, signerAccount); - }); - - it("transfers the remaining maker tokens to the taker", async () => { - const existingBalance = await makerToken.balanceOf.callAsync(subjectCaller); - await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), subjectCaller); - - await subject(); - - const netMakerToTaker = ether(10); - const expectedNewBalance = existingBalance.plus(netMakerToTaker); - await assertTokenBalance(makerToken, expectedNewBalance, subjectCaller); - }); - - it("transfers the fees to the relayer", async () => { - const existingBalance = await relayerToken.balanceOf.callAsync(relayerAddress); - await assertTokenBalance(relayerToken, ZERO, relayerAddress); - - await subject(); - - const expectedNewBalance = relayerTokenAmount.mul(2); - await assertTokenBalance(relayerToken, expectedNewBalance, relayerAddress); - }); - - it("mints the correct quantity of the set for the maker", async () => { - const existingBalance = await setToken.balanceOf.callAsync(signerAccount); - - await subject(); - - await assertTokenBalance(setToken, existingBalance.add(subjectQuantityToIssue), signerAccount); - }); - - it("marks the correct amount as filled in orderFills mapping", async () => { - const preFilled = await core.orderFills.callAsync(issuanceOrderParams.orderHash); - expect(preFilled).to.be.bignumber.equal(ZERO); + // Get pre-run balances + const makerMakerTokenPreBalance = await makerToken.balanceOf.callAsync(signerAccount); + const takerMakerTokenPreBalance = await makerToken.balanceOf.callAsync(subjectCaller); + const relayerRelayerTokenPreBalance = await relayerToken.balanceOf.callAsync(relayerAddress); + const makerSetTokenPreBalance = await setToken.balanceOf.callAsync(signerAccount); + const preFillOrderBalance = await core.orderFills.callAsync(issuanceOrderParams.orderHash); await subject(); - const filled = await core.orderFills.callAsync(issuanceOrderParams.orderHash); - expect(filled).to.be.bignumber.equal(subjectQuantityToIssue); + // Expected token balances + const makerMakerTokenExpectedBalance = makerMakerTokenPreBalance.sub(makerTokenAmount.mul(fillPercentage)); + const takerMakerTokenExpectedBalance = takerMakerTokenPreBalance.add(makerTokenAmount.mul(fillPercentage)); + const relayerRelayerTokenExpectedBalance = relayerRelayerTokenPreBalance.add(relayerTokenAmount.mul(2).mul(fillPercentage)); + const makerSetTokenExpectedBalance = makerSetTokenPreBalance.add(subjectQuantityToIssue); + const expectedFillOrderBalance = preFillOrderBalance.add(subjectQuantityToIssue); + + // Assert token balance equal what we expect + await assertTokenBalance(makerToken, makerMakerTokenExpectedBalance, signerAccount); + await assertTokenBalance(makerToken, takerMakerTokenExpectedBalance, subjectCaller); + await assertTokenBalance(relayerToken, relayerRelayerTokenExpectedBalance, relayerAddress); + await assertTokenBalance(setToken, makerSetTokenExpectedBalance, signerAccount); + + const postFillOrderBalance = await core.orderFills.callAsync(issuanceOrderParams.orderHash); + expect(expectedFillOrderBalance).to.be.bignumber.equal(postFillOrderBalance); }); it("emits correct LogFill event", async () => { @@ -292,8 +263,8 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { relayerAddress, relayerToken.address, subjectQuantityToIssue, - ether(10), - ether(2), + makerTokenAmount.mul(fillPercentage), + relayerTokenAmount.mul(2).mul(fillPercentage), issuanceOrderParams.orderHash, core.address ); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index 5d2fadfc8..70c88996e 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -20,6 +20,7 @@ export const SCENARIOS = [ }, exchangeOrders: { takerWeightsToTransfer: [1, 1], + subjectQuantityToIssue: ether(4), }, issuanceOrderParams: { requiredComponentWeighting: [1, 1], @@ -37,6 +38,7 @@ export const SCENARIOS = [ }, exchangeOrders: { takerWeightsToTransfer: [1, 1, 1], + subjectQuantityToIssue: ether(4), }, issuanceOrderParams: { requiredComponentWeighting: [1, 1, 1], @@ -44,21 +46,22 @@ export const SCENARIOS = [ makerTokenAmount: ether(10), }, }, -// { -// description: "Maker Has One Component in Wallet", -// tokenState: { -// numberOfComponents: 3, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], -// vault: [ZERO, ZERO, ZERO], -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [0, 1, 1], -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [0, 1, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, +{ + description: "Maker Has One Component in Wallet", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], + vault: [ZERO, ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [0, 1, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [0, 1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, ] \ No newline at end of file From f877b08be43e8a6875e3d7fd5e3a44dd4e028a1b Mon Sep 17 00:00:00 2001 From: bweick Date: Wed, 18 Jul 2018 12:33:23 -0700 Subject: [PATCH 05/12] Fixed error in set up that wasn't transferring expected tokens to the maker. --- .../core/extensions/coreIssuanceOrderScenarios.spec.ts | 4 ++-- utils/logs.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index 9d9611c7f..88d0fb4f5 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -150,7 +150,7 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { // Give maker its Set component tokens scenario.tokenState.makerAmounts.forEach(async (amount, idx) => { - await erc20Wrapper.transferTokenAsync(deployedTokens[idx], takerAccount, amount, ownerAccount); + await erc20Wrapper.transferTokenAsync(deployedTokens[idx], signerAccount, amount, ownerAccount); }); //Deposit maker tokens in Vault @@ -269,7 +269,7 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { core.address ); - await assertLogEquivalence(expectedLogs, formattedLogs); + await assertLogEquivalence(expectedLogs, [formattedLogs[0]]); }); }); }); diff --git a/utils/logs.ts b/utils/logs.ts index c2ea46d61..749c1d74b 100644 --- a/utils/logs.ts +++ b/utils/logs.ts @@ -73,5 +73,5 @@ export function formatLogEntry(logs: ABIDecoder.DecodedLog): Log { } export async function assertLogEquivalence(expected: Log[], actual: Log[]) { - expect(JSON.stringify(expected)).to.eql(JSON.stringify(actual)); + expect(JSON.stringify(actual)).to.eql(JSON.stringify(expected)); } From 7f6b3314ab3d24d9edad7e6e3014236ccf7951e2 Mon Sep 17 00:00:00 2001 From: bweick Date: Fri, 20 Jul 2018 17:31:09 -0700 Subject: [PATCH 06/12] Added scenarios, changed tests to handle solidity rounding errors, added function in contract to revert txn if rounding error too large. --- .../core/extensions/CoreIssuanceOrder.sol | 28 +++- .../coreIssuanceOrderScenarios.spec.ts | 27 +++- .../extensions/coreIssuanceOrderScenarios.ts | 150 +++++++++++++----- 3 files changed, 154 insertions(+), 51 deletions(-) diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 5f3a58a6a..30b2eff54 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -61,6 +61,7 @@ contract CoreIssuanceOrder is string constant INVALID_SIGNATURE = "Invalid order signature."; string constant POSITIVE_AMOUNT_REQUIRED = "Quantity should be greater than 0."; string constant ORDER_EXPIRED = "This order has expired."; + string constant ROUNDING_ERROR_TOO_LARGE = "Rounding error too large."; /* ============ Events ============ */ @@ -364,7 +365,7 @@ contract CoreIssuanceOrder is ); // Calculate fees required - uint requiredFees = _order.relayerTokenAmount.mul(_fillQuantity).div(_order.quantity); + uint requiredFees = getPartialAmount(_order.relayerTokenAmount, _fillQuantity, _order.quantity); //Send fees to relayer ITransferProxy(state.transferProxyAddress).transfer( @@ -412,8 +413,7 @@ contract CoreIssuanceOrder is uint[] memory requiredBalances = new uint[](_order.requiredComponents.length); // Calculate amount of maker token required - // Look into rounding errors - uint requiredMakerTokenAmount = _order.makerTokenAmount.mul(_fillQuantity).div(_order.quantity); + uint requiredMakerTokenAmount = getPartialAmount(_order.makerTokenAmount, _fillQuantity, _order.quantity); // Calculate amount of component tokens required to issue for (uint16 i = 0; i < _order.requiredComponents.length; i++) { @@ -424,7 +424,7 @@ contract CoreIssuanceOrder is ); // Amount of component tokens to be added to Vault - uint requiredAddition = _order.requiredComponentAmounts[i].mul(_fillQuantity).div(_order.quantity); + uint requiredAddition = getPartialAmount(_order.requiredComponentAmounts[i], _fillQuantity, _order.quantity); // Required vault balances after exchange order executed requiredBalances[i] = tokenBalance.add(requiredAddition); @@ -452,4 +452,24 @@ contract CoreIssuanceOrder is // Tally fill in orderFills mapping state.orderFills[_order.orderHash] = state.orderFills[_order.orderHash].add(_fillQuantity); } + + function getPartialAmount( + uint principal, + uint numerator, + uint denominator + ) + internal + returns (uint256) + { + assert(denominator != 0); + uint remainder = mulmod(principal, numerator, denominator); + if (remainder == 0) { + return principal.mul(numerator).div(denominator); + } + + uint errPercentageTimes1000000 = remainder.mul(1000000).div(numerator.mul(principal)); + + require(errPercentageTimes1000000 < 1000, ROUNDING_ERROR_TOO_LARGE); + return principal.mul(numerator).div(denominator); + } } diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index 88d0fb4f5..290e26a59 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -68,8 +68,6 @@ import { import { SCENARIOS } from "./coreIssuanceOrderScenarios"; -// import { injectInTruffle } from "sol-trace-set"; -// injectInTruffle(web3, artifacts); contract("CoreIssuanceOrder::Scenarios", (accounts) => { const [ @@ -153,7 +151,16 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await erc20Wrapper.transferTokenAsync(deployedTokens[idx], signerAccount, amount, ownerAccount); }); - //Deposit maker tokens in Vault + // Deposit maker tokens in Vault + scenario.tokenState.vault.forEach(async (amount, idx) => { + if (amount.greaterThan(new BigNumber(0))) { + await core.deposit.sendTransactionAsync( + deployedTokens[idx].address, + amount, + { from: signerAccount }, + ); + }; + }); // Give maker and taker their maker and relayer tokens await erc20Wrapper.transferTokensAsync(deployedTokens.slice(-2), signerAccount, DEPLOYED_TOKEN_QUANTITY.div(2), ownerAccount); @@ -235,16 +242,20 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await subject(); // Expected token balances - const makerMakerTokenExpectedBalance = makerMakerTokenPreBalance.sub(makerTokenAmount.mul(fillPercentage)); - const takerMakerTokenExpectedBalance = takerMakerTokenPreBalance.add(makerTokenAmount.mul(fillPercentage)); - const relayerRelayerTokenExpectedBalance = relayerRelayerTokenPreBalance.add(relayerTokenAmount.mul(2).mul(fillPercentage)); + const makerMakerTokenExpectedBalance = makerMakerTokenPreBalance.sub((makerTokenAmount.mul(fillPercentage)).round(0,3)); + const takerMakerTokenExpectedBalance = takerMakerTokenPreBalance.add((makerTokenAmount.mul(fillPercentage)).round(0,3)); + const relayerRelayerTokenExpectedBalance = relayerRelayerTokenPreBalance.add((relayerTokenAmount.mul(fillPercentage)).round(0,3).mul(2)); const makerSetTokenExpectedBalance = makerSetTokenPreBalance.add(subjectQuantityToIssue); const expectedFillOrderBalance = preFillOrderBalance.add(subjectQuantityToIssue); // Assert token balance equal what we expect + console.log("Expected maker token amount taken from maker."); await assertTokenBalance(makerToken, makerMakerTokenExpectedBalance, signerAccount); + console.log("Expected maker token amount given to taker."); await assertTokenBalance(makerToken, takerMakerTokenExpectedBalance, subjectCaller); + console.log("Expected relayer token amount given to relayer."); await assertTokenBalance(relayerToken, relayerRelayerTokenExpectedBalance, relayerAddress); + console.log("Expected set token amount minted for maker."); await assertTokenBalance(setToken, makerSetTokenExpectedBalance, signerAccount); const postFillOrderBalance = await core.orderFills.callAsync(issuanceOrderParams.orderHash); @@ -263,8 +274,8 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { relayerAddress, relayerToken.address, subjectQuantityToIssue, - makerTokenAmount.mul(fillPercentage), - relayerTokenAmount.mul(2).mul(fillPercentage), + (makerTokenAmount.mul(fillPercentage)).round(0,3), + (relayerTokenAmount.mul(fillPercentage)).round(0,3).mul(2), issuanceOrderParams.orderHash, core.address ); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index 70c88996e..380a0774a 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -10,8 +10,116 @@ import { } from "../../../utils/units"; export const SCENARIOS = [ +// { +// description: "Base Case", +// tokenState: { +// numberOfComponents: 2, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [ZERO, ZERO], +// vault: [ZERO, ZERO], +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [1, 1], +// subjectQuantityToIssue: ether(4), +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [1, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, +// { +// description: "Three Set Components Case", +// tokenState: { +// numberOfComponents: 3, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [ZERO, ZERO, ZERO], +// vault: [ZERO, ZERO, ZERO], +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [1, 1, 1], +// subjectQuantityToIssue: ether(4), +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [1, 1, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, +// { +// description: "Maker Has One Component in Wallet", +// tokenState: { +// numberOfComponents: 3, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], +// vault: [ZERO, ZERO, ZERO], +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [0, 1, 1], +// subjectQuantityToIssue: ether(4), +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [0, 1, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, +// { +// description: "Maker Has Two Components in Wallet", +// tokenState: { +// numberOfComponents: 3, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], +// vault: [ZERO, ZERO, ZERO], +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [0, 0, 1], +// subjectQuantityToIssue: ether(4), +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [0, 0, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, +// { +// description: "Maker Has One Component in Wallet, One in Vault", +// tokenState: { +// numberOfComponents: 3, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], +// vault: [ZERO, DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], // Must havevault amount declared in makerAmounts as well to make xfer go through +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [0, 0, 1], +// subjectQuantityToIssue: ether(4), +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [0, 0, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, +// { +// description: "Maker Has One Component in Wallet, One in Vault, Taker takes half", +// tokenState: { +// numberOfComponents: 3, +// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], +// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], +// vault: [ZERO, DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], // Must havevault amount declared in makerAmounts as well to make xfer go through +// }, +// exchangeOrders: { +// takerWeightsToTransfer: [0, 0, .5], +// subjectQuantityToIssue: ether(2), +// }, +// issuanceOrderParams: { +// requiredComponentWeighting: [0, 0, 1], +// orderQuantity: ether(4), +// makerTokenAmount: ether(10), +// }, +// }, { - description: "Base Case", + description: "Base Case with rounding errors", tokenState: { numberOfComponents: 2, takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], @@ -24,44 +132,8 @@ export const SCENARIOS = [ }, issuanceOrderParams: { requiredComponentWeighting: [1, 1], - orderQuantity: ether(4), + orderQuantity: ether(6), makerTokenAmount: ether(10), }, }, -{ - description: "Three Set Components Case", - tokenState: { - numberOfComponents: 3, - takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], - makerAmounts: [ZERO, ZERO, ZERO], - vault: [ZERO, ZERO, ZERO], - }, - exchangeOrders: { - takerWeightsToTransfer: [1, 1, 1], - subjectQuantityToIssue: ether(4), - }, - issuanceOrderParams: { - requiredComponentWeighting: [1, 1, 1], - orderQuantity: ether(4), - makerTokenAmount: ether(10), - }, -}, -{ - description: "Maker Has One Component in Wallet", - tokenState: { - numberOfComponents: 3, - takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], - makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], - vault: [ZERO, ZERO, ZERO], - }, - exchangeOrders: { - takerWeightsToTransfer: [0, 1, 1], - subjectQuantityToIssue: ether(4), - }, - issuanceOrderParams: { - requiredComponentWeighting: [0, 1, 1], - orderQuantity: ether(4), - makerTokenAmount: ether(10), - }, -}, -] \ No newline at end of file +] From fafc30f9bb7b477bf0648181b99471ddd6530400 Mon Sep 17 00:00:00 2001 From: bweick Date: Sat, 21 Jul 2018 13:48:45 -0700 Subject: [PATCH 07/12] Uncommented scenarios. Added console log message. --- .../coreIssuanceOrderScenarios.spec.ts | 3 +- .../extensions/coreIssuanceOrderScenarios.ts | 216 +++++++++--------- 2 files changed, 110 insertions(+), 109 deletions(-) diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index 290e26a59..2ceadfd68 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -112,7 +112,7 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe.only("#fillOrder", async () => { + describe("#fillOrder", async () => { SCENARIOS.forEach(async (scenario) => { describe(scenario.description, async () => { let subjectCaller: Address = takerAccount; @@ -259,6 +259,7 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await assertTokenBalance(setToken, makerSetTokenExpectedBalance, signerAccount); const postFillOrderBalance = await core.orderFills.callAsync(issuanceOrderParams.orderHash); + console.log("Expected fill amount marked in mapping.") expect(expectedFillOrderBalance).to.be.bignumber.equal(postFillOrderBalance); }); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index 380a0774a..a07370cec 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -10,114 +10,114 @@ import { } from "../../../utils/units"; export const SCENARIOS = [ -// { -// description: "Base Case", -// tokenState: { -// numberOfComponents: 2, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [ZERO, ZERO], -// vault: [ZERO, ZERO], -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [1, 1], -// subjectQuantityToIssue: ether(4), -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [1, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, -// { -// description: "Three Set Components Case", -// tokenState: { -// numberOfComponents: 3, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [ZERO, ZERO, ZERO], -// vault: [ZERO, ZERO, ZERO], -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [1, 1, 1], -// subjectQuantityToIssue: ether(4), -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [1, 1, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, -// { -// description: "Maker Has One Component in Wallet", -// tokenState: { -// numberOfComponents: 3, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], -// vault: [ZERO, ZERO, ZERO], -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [0, 1, 1], -// subjectQuantityToIssue: ether(4), -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [0, 1, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, -// { -// description: "Maker Has Two Components in Wallet", -// tokenState: { -// numberOfComponents: 3, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], -// vault: [ZERO, ZERO, ZERO], -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [0, 0, 1], -// subjectQuantityToIssue: ether(4), -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [0, 0, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, -// { -// description: "Maker Has One Component in Wallet, One in Vault", -// tokenState: { -// numberOfComponents: 3, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], -// vault: [ZERO, DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], // Must havevault amount declared in makerAmounts as well to make xfer go through -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [0, 0, 1], -// subjectQuantityToIssue: ether(4), -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [0, 0, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, -// { -// description: "Maker Has One Component in Wallet, One in Vault, Taker takes half", -// tokenState: { -// numberOfComponents: 3, -// takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], -// makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], -// vault: [ZERO, DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], // Must havevault amount declared in makerAmounts as well to make xfer go through -// }, -// exchangeOrders: { -// takerWeightsToTransfer: [0, 0, .5], -// subjectQuantityToIssue: ether(2), -// }, -// issuanceOrderParams: { -// requiredComponentWeighting: [0, 0, 1], -// orderQuantity: ether(4), -// makerTokenAmount: ether(10), -// }, -// }, +{ + description: "Base Case", + tokenState: { + numberOfComponents: 2, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [ZERO, ZERO], + vault: [ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [1, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, +{ + description: "Three Set Components Case", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [ZERO, ZERO, ZERO], + vault: [ZERO, ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [1, 1, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [1, 1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, +{ + description: "Maker Has One Component in Wallet", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), ZERO, ZERO], + vault: [ZERO, ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [0, 1, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [0, 1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, +{ + description: "Maker Has Two Components in Wallet", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], + vault: [ZERO, ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [0, 0, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [0, 0, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, +{ + description: "Maker Has One Component in Wallet, One in Vault", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], + vault: [ZERO, DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], // Must havevault amount declared in makerAmounts as well to make xfer go through + }, + exchangeOrders: { + takerWeightsToTransfer: [0, 0, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [0, 0, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, +{ + description: "Maker Has One Component in Wallet, One in Vault, Taker takes half", + tokenState: { + numberOfComponents: 3, + takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], + makerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], + vault: [ZERO, DEPLOYED_TOKEN_QUANTITY.div(2), ZERO], // Must havevault amount declared in makerAmounts as well to make xfer go through + }, + exchangeOrders: { + takerWeightsToTransfer: [0, 0, .5], + subjectQuantityToIssue: ether(2), + }, + issuanceOrderParams: { + requiredComponentWeighting: [0, 0, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, { description: "Base Case with rounding errors", tokenState: { From 24001129e68d6366a15f6fbf3ea2160d557c1086 Mon Sep 17 00:00:00 2001 From: bweick Date: Sat, 21 Jul 2018 19:49:44 -0700 Subject: [PATCH 08/12] Replaced some modifiers. Moved get PartialAmount to library. Added test for getPartialAmount. --- contracts/core/Vault.sol | 1 + contracts/core/extensions/CoreAccounting.sol | 1 + .../core/extensions/CoreIssuanceOrder.sol | 27 +++---------------- contracts/core/lib/OrderLibrary.sol | 25 +++++++++++++++++ .../core/extensions/coreIssuanceOrder.spec.ts | 20 ++++++++++++++ 5 files changed, 50 insertions(+), 24 deletions(-) diff --git a/contracts/core/Vault.sol b/contracts/core/Vault.sol index 7e5a73eb6..2e9d95d47 100644 --- a/contracts/core/Vault.sol +++ b/contracts/core/Vault.sol @@ -127,6 +127,7 @@ contract Vault is ) external onlyAuthorized + isNonZero(_quantity) { // Increment balances state variable adding _quantity to user's token amount balances[_tokenAddress][_owner] = balances[_tokenAddress][_owner].add(_quantity); diff --git a/contracts/core/extensions/CoreAccounting.sol b/contracts/core/extensions/CoreAccounting.sol index 6c01ead41..6487db0ea 100644 --- a/contracts/core/extensions/CoreAccounting.sol +++ b/contracts/core/extensions/CoreAccounting.sol @@ -211,6 +211,7 @@ contract CoreAccounting is uint[] _quantities ) internal + isValidBatchTransaction(_tokenAddresses, _quantities) { // For each token and quantity pair, run deposit function for (uint i = 0; i < _tokenAddresses.length; i++) { diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 30b2eff54..43b40b24a 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -61,7 +61,6 @@ contract CoreIssuanceOrder is string constant INVALID_SIGNATURE = "Invalid order signature."; string constant POSITIVE_AMOUNT_REQUIRED = "Quantity should be greater than 0."; string constant ORDER_EXPIRED = "This order has expired."; - string constant ROUNDING_ERROR_TOO_LARGE = "Rounding error too large."; /* ============ Events ============ */ @@ -365,7 +364,7 @@ contract CoreIssuanceOrder is ); // Calculate fees required - uint requiredFees = getPartialAmount(_order.relayerTokenAmount, _fillQuantity, _order.quantity); + uint requiredFees = OrderLibrary.getPartialAmount(_order.relayerTokenAmount, _fillQuantity, _order.quantity); //Send fees to relayer ITransferProxy(state.transferProxyAddress).transfer( @@ -413,7 +412,7 @@ contract CoreIssuanceOrder is uint[] memory requiredBalances = new uint[](_order.requiredComponents.length); // Calculate amount of maker token required - uint requiredMakerTokenAmount = getPartialAmount(_order.makerTokenAmount, _fillQuantity, _order.quantity); + uint requiredMakerTokenAmount = OrderLibrary.getPartialAmount(_order.makerTokenAmount, _fillQuantity, _order.quantity); // Calculate amount of component tokens required to issue for (uint16 i = 0; i < _order.requiredComponents.length; i++) { @@ -424,7 +423,7 @@ contract CoreIssuanceOrder is ); // Amount of component tokens to be added to Vault - uint requiredAddition = getPartialAmount(_order.requiredComponentAmounts[i], _fillQuantity, _order.quantity); + uint requiredAddition = OrderLibrary.getPartialAmount(_order.requiredComponentAmounts[i], _fillQuantity, _order.quantity); // Required vault balances after exchange order executed requiredBalances[i] = tokenBalance.add(requiredAddition); @@ -452,24 +451,4 @@ contract CoreIssuanceOrder is // Tally fill in orderFills mapping state.orderFills[_order.orderHash] = state.orderFills[_order.orderHash].add(_fillQuantity); } - - function getPartialAmount( - uint principal, - uint numerator, - uint denominator - ) - internal - returns (uint256) - { - assert(denominator != 0); - uint remainder = mulmod(principal, numerator, denominator); - if (remainder == 0) { - return principal.mul(numerator).div(denominator); - } - - uint errPercentageTimes1000000 = remainder.mul(1000000).div(numerator.mul(principal)); - - require(errPercentageTimes1000000 < 1000, ROUNDING_ERROR_TOO_LARGE); - return principal.mul(numerator).div(denominator); - } } diff --git a/contracts/core/lib/OrderLibrary.sol b/contracts/core/lib/OrderLibrary.sol index 104daaee3..72ecc2302 100644 --- a/contracts/core/lib/OrderLibrary.sol +++ b/contracts/core/lib/OrderLibrary.sol @@ -16,6 +16,8 @@ pragma solidity 0.4.24; +import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; + /** * @title OrderLibrary @@ -26,6 +28,10 @@ pragma solidity 0.4.24; */ library OrderLibrary { + using SafeMath for uint256; + + /* ============ Constants ============ */ + string constant ROUNDING_ERROR_TOO_LARGE = "Rounding error too large."; /* ============ Structs ============ */ @@ -137,4 +143,23 @@ library OrderLibrary { return recAddress == _signerAddress; } + + function getPartialAmount( + uint principal, + uint numerator, + uint denominator + ) + internal + returns (uint256) + { + uint remainder = mulmod(principal, numerator, denominator); + if (remainder == 0) { + return principal.mul(numerator).div(denominator); + } + + uint errPercentageTimes1000000 = remainder.mul(1000000).div(numerator.mul(principal)); + + require(errPercentageTimes1000000 < 1000, ROUNDING_ERROR_TOO_LARGE); + return principal.mul(numerator).div(denominator); + } } diff --git a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts index 945794dc0..b5418f17b 100644 --- a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts @@ -529,6 +529,26 @@ contract('CoreIssuanceOrder', accounts => { await expectRevertError(subject()); }); }); + + describe("when rounding error is too large", async () => { + before(async () => { + orderQuantity = ether(900); + makerTokenAmount = ether(50); + }); + + beforeEach(async () => { + subjectQuantityToIssue = ether(20); + }); + + after(async () => { + orderQuantity = undefined; + makerTokenAmount = undefined; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); }); describe('#cancelOrder', async () => { From 8c110794ba3b3d19b80694e3d74287bb0f37c59e Mon Sep 17 00:00:00 2001 From: bweick Date: Sun, 22 Jul 2018 14:49:23 -0700 Subject: [PATCH 09/12] Redid scenarios to include description parameter. Also changed how requiredComponentAmounts and Addresses arrays are set up. --- .../coreIssuanceOrderScenarios.spec.ts | 37 ++++++++++------ .../extensions/coreIssuanceOrderScenarios.ts | 44 ++++++++++++++++--- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index 2ceadfd68..ab2ed37d7 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -114,13 +114,11 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { describe("#fillOrder", async () => { SCENARIOS.forEach(async (scenario) => { - describe(scenario.description, async () => { + describe(scenario.title, async () => { let subjectCaller: Address = takerAccount; let subjectQuantityToIssue: BigNumber = scenario.exchangeOrders.subjectQuantityToIssue; let subjectExchangeOrdersData: Bytes32; - const naturalUnit: BigNumber = ether(2); - let makerAddress: Address = signerAccount; let relayerAddress: Address = relayerAccount; @@ -169,13 +167,13 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { // Set up and create SetToken const componentTokens = deployedTokens.slice(0, scenario.tokenState.numberOfComponents); const componentAddresses = _.map(componentTokens, (token) => token.address); - const componentUnits = _.map(componentTokens, () => ether(4)); // Multiple of naturalUnit + const componentUnits = _.map(componentTokens, () => scenario.componentUnit); // Multiple of naturalUnit setToken = await coreWrapper.createSetTokenAsync( core, setTokenFactory.address, componentAddresses, componentUnits, - naturalUnit, + scenario.naturalUnit, ); // Define other tokens in test @@ -183,16 +181,22 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { relayerToken = deployedTokens.slice(-1)[0]; // Define rest of params for issuanceOrder and create issuanceOrder object - const requiredComponentAmounts = _.map(componentUnits, (unit, idx) => - unit.mul(scenario.issuanceOrderParams.orderQuantity) - .mul(scenario.issuanceOrderParams.requiredComponentWeighting[idx]).div(naturalUnit)); + let requiredComponentAmounts: BigNumber[] = []; + let requiredComponents: Address[] = []; + scenario.issuanceOrderParams.requiredComponentWeighting.forEach((weight, idx) => { + if (weight != 0) { + requiredComponents.push(componentAddresses[idx]); + const requiredAmount = orderQuantity.mul(weight).mul(componentUnits[idx]).div(scenario.naturalUnit); + requiredComponentAmounts.push(requiredAmount); + } + }); const timeToExpiration = 10; issuanceOrderParams = await generateFillOrderParameters( setToken.address, signerAccount, makerAddress, - componentAddresses, + requiredComponents, requiredComponentAmounts, makerToken.address, relayerAddress, @@ -206,13 +210,19 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); // Create parameters for exchange orders and generate exchange order data - const takerAmountsToTransfer = _.map(componentUnits, (unit, idx) => - unit.mul(scenario.issuanceOrderParams.orderQuantity) - .mul(scenario.exchangeOrders.takerWeightsToTransfer[idx]).div(naturalUnit)); + let takerAmountsToTransfer: BigNumber[] = []; + let takerComponents: Address[] = []; + scenario.exchangeOrders.takerWeightsToTransfer.forEach((weight, idx) => { + if (weight != 0) { + takerComponents.push(componentAddresses[idx]); + const takerAmount = orderQuantity.mul(weight).mul(componentUnits[idx]).div(scenario.naturalUnit); + takerAmountsToTransfer.push(takerAmount); + } + }); subjectExchangeOrdersData = generateOrdersDataWithTakerOrders( makerToken.address, - componentAddresses, + takerComponents, takerAmountsToTransfer, ); }); @@ -232,6 +242,7 @@ contract("CoreIssuanceOrder::Scenarios", (accounts) => { } it("transfers the full maker token amount from the maker", async () => { + console.log(scenario.description); // Get pre-run balances const makerMakerTokenPreBalance = await makerToken.balanceOf.callAsync(signerAccount); const takerMakerTokenPreBalance = await makerToken.balanceOf.callAsync(subjectCaller); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index a07370cec..ca8f28681 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -11,7 +11,11 @@ import { export const SCENARIOS = [ { - description: "Base Case", + title: "Base Case", + description: "Maker pays for all components in the Set and receives Set in return.\ + Taker receives maker tokens and gives up component units.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 2, takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], @@ -29,7 +33,11 @@ export const SCENARIOS = [ }, }, { - description: "Three Set Components Case", + title: "Three Set Components Case", + description: "Maker pays for all components in the Set and receives Set in return.\ + Taker receives maker tokens and gives up component units.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 3, takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], @@ -47,7 +55,11 @@ export const SCENARIOS = [ }, }, { - description: "Maker Has One Component in Wallet", + title: "Maker Has One Component in Wallet", + description: "Maker pays for two components in the Set and uses one of their own tokens to mint Set.\ + Taker receives maker tokens and gives up two component units.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 3, takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], @@ -65,7 +77,11 @@ export const SCENARIOS = [ }, }, { - description: "Maker Has Two Components in Wallet", + title: "Maker Has Two Components in Wallet", + description: "Maker pays for one component in the Set and uses two of their own tokens to mint Set.\ + Taker receives maker tokens and gives up one component unit.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 3, takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], @@ -83,7 +99,11 @@ export const SCENARIOS = [ }, }, { - description: "Maker Has One Component in Wallet, One in Vault", + title: "Maker Has One Component in Wallet, One in Vault", + description: "Maker pays for one component in the Set and uses two of their own tokens to mint Set\ + (one in wallet and one in Vault). Taker receives maker tokens and gives up one component unit.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 3, takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], @@ -101,7 +121,12 @@ export const SCENARIOS = [ }, }, { - description: "Maker Has One Component in Wallet, One in Vault, Taker takes half", + title: "Maker Has One Component in Wallet, One in Vault, Taker takes half", + description: "Maker pays for one component in the Set and uses two of their own tokens to mint Set\ + (one in wallet and one in Vault). Taker receives maker tokens and gives up one component unit. Only\ + half of order is filled.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 3, takerAmounts: [DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY.div(2), DEPLOYED_TOKEN_QUANTITY], @@ -119,7 +144,12 @@ export const SCENARIOS = [ }, }, { - description: "Base Case with rounding errors", + title: "Base Case with rounding errors", + description: "Maker pays for all components in the Set and receives Set in return.\ + Taker receives maker tokens and gives up component units. Due to rounding errors amounts\ + received are not exact.", + naturalUnit: ether(2), + componentUnit: ether(4), tokenState: { numberOfComponents: 2, takerAmounts: [DEPLOYED_TOKEN_QUANTITY, DEPLOYED_TOKEN_QUANTITY], From 627a74729dcd31bb78b41effcb1f75358a3c5959 Mon Sep 17 00:00:00 2001 From: bweick Date: Sun, 22 Jul 2018 15:09:51 -0700 Subject: [PATCH 10/12] Fixed comment --- contracts/core/extensions/CoreIssuanceOrder.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 43b40b24a..b8f3ad4b8 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -152,7 +152,7 @@ contract CoreIssuanceOrder is // Settle Order settleOrder(order, _fillQuantity, _orderData); - //Issue Settle + // Issue Set issueInternal( order.makerAddress, order.setAddress, From 4c18fe97fe43dc8c3950ab47e602a690a5fb71fd Mon Sep 17 00:00:00 2001 From: bweick Date: Sun, 22 Jul 2018 16:20:58 -0700 Subject: [PATCH 11/12] Edited dependency paths for new file structure. Changed inputs to rounding error test to get full coverage. --- .../core/extensions/coreIssuanceOrder.spec.ts | 8 +++---- .../coreIssuanceOrderScenarios.spec.ts | 24 +++++++++---------- .../extensions/coreIssuanceOrderScenarios.ts | 4 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts index b5418f17b..8e2f96f14 100644 --- a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts @@ -102,7 +102,7 @@ contract('CoreIssuanceOrder', accounts => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe('#fillOrder', async () => { + describe.only('#fillOrder', async () => { let subjectCaller: Address; let subjectQuantityToIssue: BigNumber; let subjectExchangeOrdersData: Bytes32; @@ -532,12 +532,12 @@ contract('CoreIssuanceOrder', accounts => { describe("when rounding error is too large", async () => { before(async () => { - orderQuantity = ether(900); - makerTokenAmount = ether(50); + orderQuantity = ether(6); + makerTokenAmount = new BigNumber(10); }); beforeEach(async () => { - subjectQuantityToIssue = ether(20); + subjectQuantityToIssue = ether(4); }); after(async () => { diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index ab2ed37d7..1f50bc9bc 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -3,7 +3,7 @@ import * as _ from "lodash"; import * as ABIDecoder from "abi-decoder"; import { BigNumber } from "bignumber.js"; -import { ether } from "../../../utils/units"; +import { ether } from "../../../../utils/units"; // Types import { Address, Bytes32, IssuanceOrder } from "../../../../types/common.js"; @@ -21,42 +21,42 @@ import { VaultContract } from "../../../../types/generated/vault"; const Core = artifacts.require("Core"); // Core wrapper -import { CoreWrapper } from "../../../utils/coreWrapper"; -import { ERC20Wrapper } from "../../../utils/erc20Wrapper"; -import { ExchangeWrapper } from "../../../utils/exchangeWrapper"; +import { CoreWrapper } from "../../../../utils/coreWrapper"; +import { ERC20Wrapper } from "../../../../utils/erc20Wrapper"; +import { ExchangeWrapper } from "../../../../utils/exchangeWrapper"; import { generateFillOrderParameters, generateOrdersDataForOrderCount, generateOrdersDataWithIncorrectExchange, generateOrdersDataWithTakerOrders, -} from "../../../utils/orderWrapper"; +} from "../../../../utils/orderWrapper"; // Log Testing Tools import { assertLogEquivalence, getFormattedLogsFromTxHash -} from "../../../utils/logs"; +} from "../../../../utils/logs"; import { getExpectedFillLog, getExpectedCancelLog, -} from "../../../utils/contract_logs/coreIssuanceOrder"; +} from "../../../../utils/contract_logs/coreIssuanceOrder"; // Testing Set up -import { BigNumberSetup } from "../../../utils/bigNumberSetup"; -import ChaiSetup from "../../../utils/chaiSetup"; +import { BigNumberSetup } from "../../../../utils/bigNumberSetup"; +import ChaiSetup from "../../../../utils/chaiSetup"; BigNumberSetup.configure(); ChaiSetup.configure(); const { expect, assert } = chai; import { IssuanceComponentDeposited, -} from "../../../utils/contract_logs/core"; +} from "../../../../utils/contract_logs/core"; import { assertTokenBalance, expectRevertError, -} from "../../../utils/tokenAssertions"; +} from "../../../../utils/tokenAssertions"; import { DEPLOYED_TOKEN_QUANTITY, @@ -64,7 +64,7 @@ import { NULL_ADDRESS, DEFAULT_GAS, EXCHANGES, -} from "../../../utils/constants"; +} from "../../../../utils/constants"; import { SCENARIOS } from "./coreIssuanceOrderScenarios"; diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts index ca8f28681..6974c58bf 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -3,11 +3,11 @@ import { ZERO, NULL_ADDRESS, EXCHANGES, -} from "../../../utils/constants"; +} from "../../../../utils/constants"; import { ether, -} from "../../../utils/units"; +} from "../../../../utils/units"; export const SCENARIOS = [ { From ad50866a8c07c37ef50c6cb3c4c3c536853b9215 Mon Sep 17 00:00:00 2001 From: bweick Date: Sun, 22 Jul 2018 16:32:09 -0700 Subject: [PATCH 12/12] Removed .only --- test/contracts/core/extensions/coreIssuanceOrder.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts index 8e2f96f14..eb9d48eb8 100644 --- a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts @@ -102,7 +102,7 @@ contract('CoreIssuanceOrder', accounts => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe.only('#fillOrder', async () => { + describe('#fillOrder', async () => { let subjectCaller: Address; let subjectQuantityToIssue: BigNumber; let subjectExchangeOrdersData: Bytes32;