diff --git a/contracts/core/extensions/CoreIssuance.sol b/contracts/core/extensions/CoreIssuance.sol index 6dbe69c48..7944daabd 100644 --- a/contracts/core/extensions/CoreIssuance.sol +++ b/contracts/core/extensions/CoreIssuance.sol @@ -266,7 +266,7 @@ contract CoreIssuance is state.vaultAddress ); - // Log transfer of component from issuer waller + // Log transfer of component from issuer wallet emit IssuanceComponentDeposited( _setAddress, component, diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index e43d0dbd5..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 Set + // Issue Set issueInternal( order.makerAddress, order.setAddress, @@ -364,7 +364,7 @@ contract CoreIssuanceOrder is ); // Calculate fees required - uint requiredFees = _order.relayerTokenAmount.mul(_fillQuantity).div(_order.quantity); + uint requiredFees = OrderLibrary.getPartialAmount(_order.relayerTokenAmount, _fillQuantity, _order.quantity); //Send fees to relayer ITransferProxy(state.transferProxyAddress).transfer( @@ -412,8 +412,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 = 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 = _order.requiredComponentAmounts[i].mul(_fillQuantity).div(_order.quantity); + uint requiredAddition = OrderLibrary.getPartialAmount(_order.requiredComponentAmounts[i], _fillQuantity, _order.quantity); // Required vault balances after exchange order executed requiredBalances[i] = tokenBalance.add(requiredAddition); 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..eb9d48eb8 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(6); + makerTokenAmount = new BigNumber(10); + }); + + beforeEach(async () => { + subjectQuantityToIssue = ether(4); + }); + + after(async () => { + orderQuantity = undefined; + makerTokenAmount = undefined; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); }); describe('#cancelOrder', async () => { diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts new file mode 100644 index 000000000..1f50bc9bc --- /dev/null +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -0,0 +1,300 @@ +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 setDefaultStateAndAuthorizations + 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.title, async () => { + let subjectCaller: Address = takerAccount; + let subjectQuantityToIssue: BigNumber = scenario.exchangeOrders.subjectQuantityToIssue; + let subjectExchangeOrdersData: Bytes32; + + let makerAddress: Address = signerAccount; + let relayerAddress: Address = relayerAccount; + + let setToken: SetTokenContract; + let makerToken: StandardTokenMockContract; + let relayerToken: StandardTokenMockContract; + + let makerTokenAmount: BigNumber = scenario.issuanceOrderParams.makerTokenAmount; + let relayerTokenAmount: BigNumber = ether(1); + let orderQuantity: BigNumber = scenario.issuanceOrderParams.orderQuantity; + let fillPercentage: BigNumber = subjectQuantityToIssue.div(orderQuantity); + + let issuanceOrderParams: any; + + beforeEach(async () => { + 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); + + // 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], signerAccount, amount, ownerAccount); + }); + + // 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); + 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); + const componentAddresses = _.map(componentTokens, (token) => token.address); + const componentUnits = _.map(componentTokens, () => scenario.componentUnit); // Multiple of naturalUnit + setToken = await coreWrapper.createSetTokenAsync( + core, + setTokenFactory.address, + componentAddresses, + componentUnits, + scenario.naturalUnit, + ); + + // Define other tokens in test + makerToken = deployedTokens.slice(-2, -1)[0]; + relayerToken = deployedTokens.slice(-1)[0]; + + // Define rest of params for issuanceOrder and create issuanceOrder object + 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, + requiredComponents, + requiredComponentAmounts, + makerToken.address, + relayerAddress, + relayerToken.address, + scenario.issuanceOrderParams.orderQuantity, + scenario.issuanceOrderParams.makerTokenAmount, + timeToExpiration, + ); + + // Register exchange with core + await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); + + // Create parameters for exchange orders and generate exchange order data + 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, + takerComponents, + takerAmountsToTransfer, + ); + }); + + 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 () => { + console.log(scenario.description); + // 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(); + + // Expected token balances + 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); + console.log("Expected fill amount marked in mapping.") + expect(expectedFillOrderBalance).to.be.bignumber.equal(postFillOrderBalance); + }); + + 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, + (makerTokenAmount.mul(fillPercentage)).round(0,3), + (relayerTokenAmount.mul(fillPercentage)).round(0,3).mul(2), + issuanceOrderParams.orderHash, + core.address + ); + + await assertLogEquivalence(expectedLogs, [formattedLogs[0]]); + }); + }); + }); + }); +}); diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts new file mode 100644 index 000000000..6974c58bf --- /dev/null +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.ts @@ -0,0 +1,169 @@ +import { + DEPLOYED_TOKEN_QUANTITY, + ZERO, + NULL_ADDRESS, + EXCHANGES, +} from "../../../../utils/constants"; + +import { + ether, +} from "../../../../utils/units"; + +export const SCENARIOS = [ +{ + 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], + makerAmounts: [ZERO, ZERO], + vault: [ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [1, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [1, 1], + orderQuantity: ether(4), + makerTokenAmount: ether(10), + }, +}, +{ + 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], + 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), + }, +}, +{ + 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], + 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), + }, +}, +{ + 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], + 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), + }, +}, +{ + 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], + 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), + }, +}, +{ + 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], + 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), + }, +}, +{ + 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], + makerAmounts: [ZERO, ZERO], + vault: [ZERO, ZERO], + }, + exchangeOrders: { + takerWeightsToTransfer: [1, 1], + subjectQuantityToIssue: ether(4), + }, + issuanceOrderParams: { + requiredComponentWeighting: [1, 1], + orderQuantity: ether(6), + makerTokenAmount: ether(10), + }, +}, +] 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)); }