From 34c41757185787ba6f5a36fc5acc75448381eca3 Mon Sep 17 00:00:00 2001 From: Alexander Soong Date: Wed, 11 Jul 2018 19:30:22 -0700 Subject: [PATCH] Taker Wallet integration with IssuanceOrder including all tests. Removed some IssuanceOrder logic not relevant to taker wallets --- .../exchange-wrappers/TakerWalletWrapper.sol | 55 ++++---- contracts/core/extensions/CoreAccounting.sol | 82 +++++++++-- .../core/extensions/CoreIssuanceOrder.sol | 45 ++++-- contracts/core/interfaces/ICoreAccounting.sol | 43 ++++++ contracts/core/interfaces/IExchange.sol | 9 +- contracts/core/lib/ExchangeHandler.sol | 10 +- contracts/lib/ERC20Wrapper.sol | 20 ++- contracts/mocks/lib/ERC20WrapperMock.sol | 11 ++ package.json | 2 +- .../takerWalletWrapper.spec.ts | 48 +++---- .../core/extensions/coreIssuanceOrder.spec.ts | 131 ++++++++++-------- test/contracts/lib/authorizable.spec.ts | 2 +- test/contracts/lib/erc20Wrapper.spec.ts | 43 +++++- test/utils/erc20Wrapper.ts | 15 ++ test/utils/exchangeWrapper.ts | 3 - test/utils/orderWrapper.ts | 31 ++++- yarn.lock | 6 +- 17 files changed, 392 insertions(+), 164 deletions(-) create mode 100644 contracts/core/interfaces/ICoreAccounting.sol diff --git a/contracts/core/exchange-wrappers/TakerWalletWrapper.sol b/contracts/core/exchange-wrappers/TakerWalletWrapper.sol index cd654351f..46bdcc0ce 100644 --- a/contracts/core/exchange-wrappers/TakerWalletWrapper.sol +++ b/contracts/core/exchange-wrappers/TakerWalletWrapper.sol @@ -21,7 +21,7 @@ import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; import { Authorizable } from "../../lib/Authorizable.sol"; import { LibBytes } from "../../external/0x/LibBytes.sol"; import { ITransferProxy } from "../interfaces/ITransferProxy.sol"; -import { IVault } from "../interfaces/IVault.sol"; +import { ERC20Wrapper } from "../../lib/ERC20Wrapper.sol"; /** @@ -38,7 +38,6 @@ contract TakerWalletWrapper is /* ============ State Variables ============ */ address public transferProxy; - address public vault; /* ============ Constants ============ */ @@ -47,25 +46,27 @@ contract TakerWalletWrapper is /* ============ Constructor ============ */ constructor( - address _transferProxy, - address _vault + address _transferProxy ) public { transferProxy = _transferProxy; - vault = _vault; } /* ============ Public Functions ============ */ function exchange( - address _tradeOriginator, address _taker, + uint _orderCount, bytes _orderData ) public onlyAuthorized + returns(address[], uint256[]) { + address[] memory takerTokens = new address[](_orderCount); + uint256[] memory takerTokenAmounts = new uint256[](_orderCount); + uint256 scannedBytes = 32; while (scannedBytes < _orderData.length) { @@ -77,39 +78,31 @@ contract TakerWalletWrapper is takerTokenAmount := mload(add(_orderData, add(scannedBytes, 32))) } - executeTransfer( + // Transfer from taker's wallet to this wrapper + ITransferProxy(transferProxy).transfer( + takerToken, + takerTokenAmount, _taker, - _tradeOriginator, + address(this) + ); + + // Ensure allowance of transfer from this wrapper to TransferProxy + ERC20Wrapper.ensureAllowance( takerToken, + address(this), + transferProxy, takerTokenAmount ); + // Record taker token and amount to return values + uint256 orderCount = scannedBytes >> 6; + takerTokens[orderCount] = takerToken; + takerTokenAmounts[orderCount] = takerTokenAmount; + // Update scanned bytes with header and body lengths scannedBytes = scannedBytes.add(TRANSFER_REQUEST_LENGTH); } - } - /* ============ Private ============ */ - - function executeTransfer( - address _taker, - address _tradeOriginator, - address _takerToken, - uint256 _takerTokenAmount - ) - private - { - ITransferProxy(transferProxy).transfer( - _takerToken, - _takerTokenAmount, - _taker, - vault - ); - - IVault(vault).incrementTokenOwner( - _tradeOriginator, - _takerToken, - _takerTokenAmount - ); + return (takerTokens, takerTokenAmounts); } } diff --git a/contracts/core/extensions/CoreAccounting.sol b/contracts/core/extensions/CoreAccounting.sol index c396e6782..c7131f9f0 100644 --- a/contracts/core/extensions/CoreAccounting.sol +++ b/contracts/core/extensions/CoreAccounting.sol @@ -84,13 +84,13 @@ contract CoreAccounting is external isValidBatchTransaction(_tokenAddresses, _quantities) { - // For each token and quantity pair, run deposit function - for (uint i = 0; i < _tokenAddresses.length; i++) { - deposit( - _tokenAddresses[i], - _quantities[i] - ); - } + // Call internal batch deposit function + batchDepositInternal( + msg.sender, + msg.sender, + _tokenAddresses, + _quantities + ); } /** @@ -130,15 +130,8 @@ contract CoreAccounting is isPositiveQuantity(_quantity) { // Call TransferProxy contract to transfer user tokens to Vault - ITransferProxy(state.transferProxyAddress).transfer( - _tokenAddress, - _quantity, + depositInternal( msg.sender, - state.vaultAddress - ); - - // Call Vault contract to attribute deposited tokens to user - IVault(state.vaultAddress).incrementTokenOwner( msg.sender, _tokenAddress, _quantity @@ -171,4 +164,63 @@ contract CoreAccounting is _quantity ); } + + /* ============ Internal Functions ============ */ + + /** + * Deposit any quantity of tokens into the vault. + * + * @param _tokenAddress The address of the ERC20 token + * @param _quantity The number of tokens to deposit + */ + function depositInternal( + address _from, + address _to, + address _tokenAddress, + uint _quantity + ) + public + { + // Call TransferProxy contract to transfer user tokens to Vault + ITransferProxy(state.transferProxyAddress).transfer( + _tokenAddress, + _quantity, + _from, + state.vaultAddress + ); + + // Call Vault contract to attribute deposited tokens to user + IVault(state.vaultAddress).incrementTokenOwner( + _to, + _tokenAddress, + _quantity + ); + } + + /** + * Deposit multiple tokens to the vault. Quantities should be in the + * order of the addresses of the tokens being deposited. + * + * @param _tokenAddresses Array of the addresses of the ERC20 tokens + * @param _quantities Array of the number of tokens to deposit + */ + function batchDepositInternal( + address _from, + address _to, + address[] _tokenAddresses, + uint[] _quantities + ) + internal + isValidBatchTransaction(_tokenAddresses, _quantities) + { + // For each token and quantity pair, run deposit function + for (uint i = 0; i < _tokenAddresses.length; i++) { + depositInternal( + _from, + _to, + _tokenAddresses[i], + _quantities[i] + ); + } + } } diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 15c833b3e..e43d0dbd5 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -23,6 +23,7 @@ import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; import { CoreModifiers } from "../lib/CoreSharedModifiers.sol"; import { CoreState } from "../lib/CoreState.sol"; import { ExchangeHandler } from "../lib/ExchangeHandler.sol"; +import { ICoreAccounting } from "../interfaces/ICoreAccounting.sol"; import { ICoreIssuance } from "../interfaces/ICoreIssuance.sol"; import { IExchange } from "../interfaces/IExchange.sol"; import { ITransferProxy } from "../interfaces/ITransferProxy.sol"; @@ -42,6 +43,7 @@ import { OrderLibrary } from "../lib/OrderLibrary.sol"; */ contract CoreIssuanceOrder is ICoreIssuance, + ICoreAccounting, CoreState, CoreModifiers { @@ -50,7 +52,7 @@ contract CoreIssuanceOrder is /* ============ Constants ============ */ - uint256 constant EXCHANGE_HEADER_LENGTH = 128; + uint256 constant EXCHANGE_HEADER_LENGTH = 160; string constant INVALID_CANCEL_ORDER = "Only maker can cancel order."; string constant INVALID_EXCHANGE = "Exchange does not exist."; @@ -151,11 +153,11 @@ contract CoreIssuanceOrder is settleOrder(order, _fillQuantity, _orderData); //Issue Set - // issueInternal( - // order.makerAddress, - // order.setAddress, - // _fillQuantity - // ); + issueInternal( + order.makerAddress, + order.setAddress, + _fillQuantity + ); } /** @@ -264,7 +266,7 @@ contract CoreIssuanceOrder is // Read the order body based on header order length info uint256 exchangeDataLength = header.totalOrdersLength.add(EXCHANGE_HEADER_LENGTH); - bytes memory orderBody = LibBytes.slice( + bytes memory bodyData = LibBytes.slice( _orderData, scannedBytes.add(EXCHANGE_HEADER_LENGTH), scannedBytes.add(exchangeDataLength) @@ -278,13 +280,28 @@ contract CoreIssuanceOrder is exchange ); - //Call Exchange - //IExchange(header.exchange).exchange(orderBody); + // Call Exchange + address[] memory componentFillTokens = new address[](header.orderCount); + uint[] memory componentFillAmounts = new uint[](header.orderCount); + (componentFillTokens, componentFillAmounts) = IExchange(exchange).exchange( + msg.sender, + header.orderCount, + bodyData + ); + + // Transfer component tokens from wrapper to vault + batchDepositInternal( + exchange, + _makerAddress, + componentFillTokens, + componentFillAmounts + ); // Update scanned bytes with header and body lengths scannedBytes = scannedBytes.add(exchangeDataLength); makerTokenUsed += header.makerTokenAmount; } + return makerTokenUsed; } @@ -415,7 +432,6 @@ contract CoreIssuanceOrder is // Execute exchange orders uint makerTokenAmountUsed = executeExchangeOrders(_orderData, _order.makerAddress); - require(makerTokenAmountUsed <= requiredMakerTokenAmount); // Check that maker's component tokens in Vault have been incremented correctly for (i = 0; i < _order.requiredComponents.length; i++) { @@ -423,10 +439,15 @@ contract CoreIssuanceOrder is _order.makerAddress, _order.requiredComponents[i] ); - //require(currentBal >= requiredBalances[i]); + require(currentBal >= requiredBalances[i]); } - settleAccounts(_order, _fillQuantity, requiredMakerTokenAmount, makerTokenAmountUsed); + settleAccounts( + _order, + _fillQuantity, + requiredMakerTokenAmount, + makerTokenAmountUsed + ); // Tally fill in orderFills mapping state.orderFills[_order.orderHash] = state.orderFills[_order.orderHash].add(_fillQuantity); diff --git a/contracts/core/interfaces/ICoreAccounting.sol b/contracts/core/interfaces/ICoreAccounting.sol new file mode 100644 index 000000000..7a7ddbb83 --- /dev/null +++ b/contracts/core/interfaces/ICoreAccounting.sol @@ -0,0 +1,43 @@ +/* + Copyright 2018 Set Labs Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +pragma solidity 0.4.24; + + +/** + * @title ICoreIssuance + * @author Set Protocol + * + * The ICoreIssuance Contract defines all the functions exposed in the CoreIssuance + * extension. + */ +contract ICoreAccounting { + + /** + * Deposit multiple tokens to the vault. Quantities should be in the + * order of the addresses of the tokens being deposited. + * + * @param _tokenAddresses Array of the addresses of the ERC20 tokens + * @param _quantities Array of the number of tokens to deposit + */ + function batchDepositInternal( + address _from, + address _to, + address[] _tokenAddresses, + uint[] _quantities + ) + internal; +} \ No newline at end of file diff --git a/contracts/core/interfaces/IExchange.sol b/contracts/core/interfaces/IExchange.sol index 1e227b0ba..dead4e5eb 100644 --- a/contracts/core/interfaces/IExchange.sol +++ b/contracts/core/interfaces/IExchange.sol @@ -28,12 +28,15 @@ interface IExchange { /** * Exchange some amount of takerToken for makerToken. * - * @param _orderData Arbitrary bytes data for any information to pass to the exchange - * @return uint256 The amount of makerToken received + * @param _taker Issuance order filler + * @param _orderCount Expected number of orders to execute + * @param _orderData Arbitrary bytes data for any information to pass to the exchange */ function exchange( + address _taker, + uint _orderCount, bytes _orderData ) external - returns (uint256); + returns (address[], uint256[]); } diff --git a/contracts/core/lib/ExchangeHandler.sol b/contracts/core/lib/ExchangeHandler.sol index 1208aff21..50a14f534 100644 --- a/contracts/core/lib/ExchangeHandler.sol +++ b/contracts/core/lib/ExchangeHandler.sol @@ -30,6 +30,7 @@ library ExchangeHandler { struct ExchangeHeader { uint8 exchange; + uint8 orderCount; address makerTokenAddress; uint256 makerTokenAmount; uint256 totalOrdersLength; @@ -53,10 +54,11 @@ library ExchangeHandler { ExchangeHeader memory header; assembly { - mstore(header, mload(add(_headerData, 32))) // exchange - mstore(add(header, 32), mload(add(_headerData, 64))) // makerTokenAddress - mstore(add(header, 64), mload(add(_headerData, 96))) // makerTokenAmount - mstore(add(header, 96), mload(add(_headerData, 128))) // totalOrdersLength + mstore(header, mload(add(_headerData, 32))) // exchange + mstore(add(header, 32), mload(add(_headerData, 64))) // orderCount + mstore(add(header, 64), mload(add(_headerData, 96))) // makerTokenAddress + mstore(add(header, 96), mload(add(_headerData, 128))) // makerTokenAmount + mstore(add(header, 128), mload(add(_headerData, 160))) // totalOrdersLength } return header; diff --git a/contracts/lib/ERC20Wrapper.sol b/contracts/lib/ERC20Wrapper.sol index 465b81d29..fa902094d 100644 --- a/contracts/lib/ERC20Wrapper.sol +++ b/contracts/lib/ERC20Wrapper.sol @@ -100,13 +100,31 @@ library ERC20Wrapper { internal { IERC20(_tokenAddress).approve(_spender, _quantity); - + require( checkSuccess(), INVALID_RETURN_APPROVE ); } + function ensureAllowance( + address _token, + address _owner, + address _spender, + uint256 _quantity + ) + internal + { + uint currentAllowance = allowance(_token, _owner, _spender); + if (currentAllowance < _quantity) { + approve( + _token, + _spender, + CommonMath.maxUInt256() + ); + } + } + // ============ Private Functions ============ /** diff --git a/contracts/mocks/lib/ERC20WrapperMock.sol b/contracts/mocks/lib/ERC20WrapperMock.sol index bc71f3e22..ce52ac6e6 100644 --- a/contracts/mocks/lib/ERC20WrapperMock.sol +++ b/contracts/mocks/lib/ERC20WrapperMock.sol @@ -25,4 +25,15 @@ contract ERC20WrapperMock { { ERC20Wrapper.approve(_token, _spender, _quantity); } + + function ensureAllowance( + address _token, + address _owner, + address _spender, + uint256 _quantity + ) + public + { + ERC20Wrapper.ensureAllowance(_token, _owner, _spender, _quantity); + } } diff --git a/package.json b/package.json index 4e7bd8429..50ffa29a1 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "json-stable-stringify": "^1.0.1", "lodash": "^4.17.4", "solc": "0.4.24", - "solidity-coverage": "^0.5.4", + "solidity-coverage": "^0.5.5", "solidity-sha3": "^0.4.1", "truffle": "4.1.11", "tslint": "^5.8.0", diff --git a/test/contracts/core/exchange-wrappers/takerWalletWrapper.spec.ts b/test/contracts/core/exchange-wrappers/takerWalletWrapper.spec.ts index bb14f4382..1f4e9ecee 100644 --- a/test/contracts/core/exchange-wrappers/takerWalletWrapper.spec.ts +++ b/test/contracts/core/exchange-wrappers/takerWalletWrapper.spec.ts @@ -12,7 +12,6 @@ import { Address, Bytes, Log, UInt } from "../../../../types/common.js"; 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"; // Wrappers import { CoreWrapper } from "../../../utils/coreWrapper"; @@ -30,6 +29,7 @@ const { expect, assert } = chai; import { DEFAULT_GAS, DEPLOYED_TOKEN_QUANTITY, + UNLIMITED_ALLOWANCE_IN_BASE_UNITS, ZERO, } from "../../../utils/constants"; @@ -37,7 +37,7 @@ import { assertTokenBalance, expectRevertError, } from "../../../utils/tokenAssertions"; - + contract("TakerWalletWrapper", (accounts) => { const [ @@ -53,7 +53,6 @@ contract("TakerWalletWrapper", (accounts) => { const exchangeWrapper = new ExchangeWrapper(deployerAccount); let transferProxy: TransferProxyContract; - let vault: VaultContract; let takerWalletWrapper: TakerWalletWrapperContract; let components: StandardTokenMockContract[] = []; @@ -61,13 +60,10 @@ contract("TakerWalletWrapper", (accounts) => { beforeEach(async () => { - vault = await coreWrapper.deployVaultAsync(); transferProxy = await coreWrapper.deployTransferProxyAsync(); - takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(vault, transferProxy); + takerWalletWrapper = await exchangeWrapper.deployTakerWalletExchangeWrapper(transferProxy); await coreWrapper.addAuthorizationAsync(takerWalletWrapper, authorizedAddress); - - await coreWrapper.addAuthorizationAsync(vault, takerWalletWrapper.address); await coreWrapper.addAuthorizationAsync(transferProxy, takerWalletWrapper.address); components = await erc20Wrapper.deployTokensAsync(componentCount, takerAccount); @@ -76,6 +72,7 @@ contract("TakerWalletWrapper", (accounts) => { describe("#exchange", async () => { let subjectCaller: Address; + let subjectOrderCount: BigNumber; let subjectTakerOrdersData: Bytes; let componentToken: StandardTokenMockContract; @@ -87,13 +84,14 @@ contract("TakerWalletWrapper", (accounts) => { const transferAmounts = _.map(components, (token) => transferAmount); subjectCaller = authorizedAddress; + subjectOrderCount = new BigNumber(componentAddresses.length); subjectTakerOrdersData = generateTakerWalletOrders(componentAddresses, transferAmounts); }); async function subject(): Promise { return takerWalletWrapper.exchange.sendTransactionAsync( - issuerAccount, takerAccount, + subjectOrderCount, subjectTakerOrdersData, { from: subjectCaller, gas: DEFAULT_GAS }, ); @@ -109,24 +107,24 @@ contract("TakerWalletWrapper", (accounts) => { expect(newBalance).to.be.bignumber.equal(expectedNewBalance); }); - it("transfers the token to the vault", async () => { - const existingBalance = await componentToken.balanceOf.callAsync(vault.address); + it("transfers the token to the wrapper contract", async () => { + const existingBalance = await componentToken.balanceOf.callAsync(takerWalletWrapper.address); await subject(); const expectedNewBalance = existingBalance.add(transferAmount); - const newBalance = await componentToken.balanceOf.callAsync(vault.address); + const newBalance = await componentToken.balanceOf.callAsync(takerWalletWrapper.address); expect(newBalance).to.be.bignumber.equal(expectedNewBalance); }); - it("makes the issuser the owner of the component in the vault", async () => { - const existingVaultBalance = await vault.balances.callAsync(componentToken.address, issuerAccount); + it("approves the tokens for transfer to the transferProxy with unlimited allowance", async () => { + const existingAllowance = await componentToken.allowance.callAsync(takerWalletWrapper.address, transferProxy.address); await subject(); - const expectedNewVaultBalance = existingVaultBalance.add(transferAmount); - const newVaultBalance = await vault.balances.callAsync(componentToken.address, issuerAccount); - expect(newVaultBalance).to.be.bignumber.equal(expectedNewVaultBalance); + const expectedNewAllowance = existingAllowance.add(UNLIMITED_ALLOWANCE_IN_BASE_UNITS); + const newBalance = await componentToken.allowance.callAsync(takerWalletWrapper.address, transferProxy.address); + expect(newBalance).to.be.bignumber.equal(expectedNewAllowance); }); describe("when the caller is not authorized", async () => { @@ -144,7 +142,7 @@ contract("TakerWalletWrapper", (accounts) => { componentCount = 3; }); - it("transfers the token from the taker", async () => { + it("transfers the tokens from the taker", async () => { const existingTokenBalances = await erc20Wrapper.getTokenBalances(components, takerAccount); await subject(); @@ -154,24 +152,24 @@ contract("TakerWalletWrapper", (accounts) => { expect(newTokenBalances).to.eql(expectedNewBalances); }); - it("transfers the token to the vault", async () => { - const existingTokenBalances = await erc20Wrapper.getTokenBalances(components, vault.address); + it("transfers the token to the wrapper contract", async () => { + const existingTokenBalances = await erc20Wrapper.getTokenBalances(components, takerWalletWrapper.address); await subject(); const expectedNewBalances = _.map(existingTokenBalances, (balance) => balance.add(transferAmount)); - const newTokenBalances = await erc20Wrapper.getTokenBalances(components, vault.address); + const newTokenBalances = await erc20Wrapper.getTokenBalances(components, takerWalletWrapper.address); expect(newTokenBalances).to.eql(expectedNewBalances); }); - it("makes the issuser the owner of the component in the vault", async () => { - const existingVaultBalances = await coreWrapper.getVaultBalancesForTokensForOwner(components, vault, issuerAccount); + it("approves the tokens for transfer to the transferProxy with unlimited allowance", async () => { + const existingAllowances = await erc20Wrapper.getTokenAllowances(components, takerWalletWrapper.address, transferProxy.address); await subject(); - const expectedVaultBalances = _.map(existingVaultBalances, (balance) => balance.add(transferAmount)); - const newVaultBalances = await coreWrapper.getVaultBalancesForTokensForOwner(components, vault, issuerAccount); - expect(newVaultBalances).to.eql(expectedVaultBalances); + const expectedNewAllowances = _.map(existingAllowances, (allowance) => allowance.add(UNLIMITED_ALLOWANCE_IN_BASE_UNITS)); + const newAllowances = await erc20Wrapper.getTokenAllowances(components, takerWalletWrapper.address, transferProxy.address); + expect(newAllowances).to.eql(expectedNewAllowances); }); }); }); diff --git a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts index adfaa2e30..04f11bca1 100644 --- a/test/contracts/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrder.spec.ts @@ -13,6 +13,7 @@ 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"; @@ -22,10 +23,12 @@ 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 @@ -59,8 +62,11 @@ import { DEPLOYED_TOKEN_QUANTITY, ZERO, NULL_ADDRESS, + DEFAULT_GAS, + EXCHANGES, } from "../../../utils/constants"; + contract("CoreIssuanceOrder", (accounts) => { const [ ownerAccount, @@ -76,9 +82,11 @@ contract("CoreIssuanceOrder", (accounts) => { 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); @@ -93,6 +101,12 @@ contract("CoreIssuanceOrder", (accounts) => { 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); }); @@ -121,13 +135,12 @@ contract("CoreIssuanceOrder", (accounts) => { let relayerTokenAmount: BigNumber = ether(1); let timeToExpiration: number; - let orderCount: number; - let orderMakerTokenAmounts: number[]; + let takerAmountsToTransfer: BigNumber[]; let issuanceOrderParams: any; beforeEach(async () => { - deployedTokens = await erc20Wrapper.deployTokensAsync(4, ownerAccount); + deployedTokens = await erc20Wrapper.deployTokensAsync(4, ownerAccount); // Taker Account await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, ownerAccount); await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, signerAccount); await erc20Wrapper.approveTransfersAsync(deployedTokens, transferProxy.address, takerAccount); @@ -137,8 +150,9 @@ contract("CoreIssuanceOrder", (accounts) => { // Give maker their maker and relayer tokens await erc20Wrapper.transferTokensAsync(deployedTokens.slice(2,4), signerAccount, DEPLOYED_TOKEN_QUANTITY.div(2), ownerAccount); - componentAddresses = _.map(deployedTokens.slice(0, 2), (token) => token.address); - componentUnits = _.map(deployedTokens.slice(0, 2), () => ether(4)); // Multiple of naturalUnit + const componentTokens = deployedTokens.slice(0, 2); + componentAddresses = _.map(componentTokens, (token) => token.address); + componentUnits = _.map(componentTokens, () => ether(4)); // Multiple of naturalUnit setToken = await coreWrapper.createSetTokenAsync( core, setTokenFactory.address, @@ -147,9 +161,9 @@ contract("CoreIssuanceOrder", (accounts) => { naturalUnit, ); - defaultComponentAmounts = _.map(componentUnits, (unit) => unit.mul(orderQuantity || ether(4))); + defaultComponentAmounts = _.map(componentUnits, (unit) => unit.mul(orderQuantity || ether(4)).div(naturalUnit)); - await coreWrapper.registerDefaultExchanges(core); + await coreWrapper.registerExchange(core, EXCHANGES.TAKER_WALLET, takerWalletWrapper.address); relayerAddress = relayerAccount; makerToken = deployedTokens[2]; relayerToken = deployedTokens[3]; @@ -168,11 +182,17 @@ contract("CoreIssuanceOrder", (accounts) => { timeToExpiration || 10, ); - subjectExchangeOrdersData = generateOrdersDataForOrderCount( - orderCount || 3, + const defaultTakerAmountsToTransfer = _.map(componentTokens, (balance, idx) => { + const units = componentUnits[idx]; + return ether(4).div(naturalUnit).mul(units); + }); + + subjectExchangeOrdersData = generateOrdersDataWithTakerOrders( makerToken.address, - orderMakerTokenAmounts || [3, 3, 3], + componentAddresses, + takerAmountsToTransfer || defaultTakerAmountsToTransfer, ); + subjectCaller = takerAccount; subjectQuantityToIssue = ether(4); }); @@ -193,47 +213,44 @@ contract("CoreIssuanceOrder", (accounts) => { it("transfers the full maker token amount from the maker", async () => { const existingBalance = await makerToken.balanceOf.callAsync(signerAccount); - assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), 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); - assertTokenBalance(makerToken, expectedNewBalance, signerAccount); + await assertTokenBalance(makerToken, expectedNewBalance, signerAccount); }); it("transfers the remaining maker tokens to the taker", async () => { const existingBalance = await makerToken.balanceOf.callAsync(subjectCaller); - assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), subjectCaller); + await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), subjectCaller); await subject(); - const testMakerTokenAmount = ether(10); // makerTokenAmount - const sumTestOrderMakerTokenAmounts = ether(9); // Sum orderMakerTokenAmounts - - const netMakerToTaker = testMakerTokenAmount.sub(sumTestOrderMakerTokenAmounts); + const netMakerToTaker = ether(10); const expectedNewBalance = existingBalance.plus(netMakerToTaker); - assertTokenBalance(makerToken, expectedNewBalance, subjectCaller); + await assertTokenBalance(makerToken, expectedNewBalance, subjectCaller); }); it("transfers the fees to the relayer", async () => { const existingBalance = await relayerToken.balanceOf.callAsync(relayerAddress); - assertTokenBalance(relayerToken, ZERO, relayerAddress); + await assertTokenBalance(relayerToken, ZERO, relayerAddress); await subject(); const expectedNewBalance = relayerTokenAmount.mul(2); - assertTokenBalance(relayerToken, expectedNewBalance, relayerAddress); + await assertTokenBalance(relayerToken, expectedNewBalance, relayerAddress); }); - // it("mints the correct quantity of the set for the maker", async () => { - // const existingBalance = await setToken.balanceOf.callAsync(signerAccount); + it("mints the correct quantity of the set for the maker", async () => { + const existingBalance = await setToken.balanceOf.callAsync(signerAccount); - // await subject(); + await subject(); - // assertTokenBalance(setToken, existingBalance.add(subjectQuantityToIssue), signerAccount); - // }); + 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); @@ -257,7 +274,7 @@ contract("CoreIssuanceOrder", (accounts) => { relayerAddress, relayerToken.address, subjectQuantityToIssue, - ether(1), + ether(10), ether(2), issuanceOrderParams.orderHash, core.address @@ -267,62 +284,50 @@ contract("CoreIssuanceOrder", (accounts) => { }); describe("when the fill size is less than the order quantity", async () => { - before(async () => { - orderMakerTokenAmounts = [1, 1, 1]; - }); - beforeEach(async () => { subjectQuantityToIssue = ether(2); }); - after(async () => { - orderMakerTokenAmounts = undefined; - }); - it("transfers the partial maker token amount from the maker", async () => { const existingBalance = await makerToken.balanceOf.callAsync(signerAccount); - assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), signerAccount); + await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), signerAccount); await subject(); const partialMakerTokenAmount = ether(10).mul(subjectQuantityToIssue).div(ether(4)); const newBalance = await makerToken.balanceOf.callAsync(signerAccount); const expectedNewBalance = existingBalance.sub(partialMakerTokenAmount); - assertTokenBalance(makerToken, expectedNewBalance, signerAccount); + await assertTokenBalance(makerToken, expectedNewBalance, signerAccount); }); it("transfers the remaining maker tokens to the taker", async () => { const existingBalance = await makerToken.balanceOf.callAsync(subjectCaller); - assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), subjectCaller); + await assertTokenBalance(makerToken, DEPLOYED_TOKEN_QUANTITY.div(2), subjectCaller); await subject(); - const testMakerTokenAmount = ether(10); // MakerTokenAmount - const sumTestOrderMakerTokenAmounts = ether(3); // Sum orderMakerTokenAmounts - - const partialMakerTokenAmount = testMakerTokenAmount.mul(subjectQuantityToIssue).div(ether(4)) - const netMakerToTaker = partialMakerTokenAmount.sub(sumTestOrderMakerTokenAmounts); + const netMakerToTaker = ether(10).mul(subjectQuantityToIssue).div(ether(4)); const expectedNewBalance = existingBalance.plus(netMakerToTaker); - assertTokenBalance(makerToken, expectedNewBalance, subjectCaller); + await assertTokenBalance(makerToken, expectedNewBalance, subjectCaller); }); it("transfers the partial fees to the relayer", async () => { const existingBalance = await relayerToken.balanceOf.callAsync(relayerAddress); - assertTokenBalance(relayerToken, ZERO, relayerAddress); + await assertTokenBalance(relayerToken, ZERO, relayerAddress); await subject(); const expectedNewBalance = relayerTokenAmount.mul(2).mul(subjectQuantityToIssue).div(ether(4)); - assertTokenBalance(relayerToken, expectedNewBalance, relayerAddress); + await assertTokenBalance(relayerToken, expectedNewBalance, relayerAddress); }); - // it("mints the correct quantity of the set for the user", async () => { - // const existingBalance = await setToken.balanceOf.callAsync(signerAccount); + it("mints the correct quantity of the set for the user", async () => { + const existingBalance = await setToken.balanceOf.callAsync(signerAccount); - // await subject(); + await subject(); - // assertTokenBalance(setToken, existingBalance.add(subjectQuantityToIssue), signerAccount); - // }); + 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); @@ -346,7 +351,7 @@ contract("CoreIssuanceOrder", (accounts) => { relayerAddress, relayerToken.address, subjectQuantityToIssue, - ether(2), + ether(5), ether(1), issuanceOrderParams.orderHash, core.address @@ -356,16 +361,6 @@ contract("CoreIssuanceOrder", (accounts) => { }); }); - describe("when submitted exchange orders use more maker tokens than alloted for trades", async () => { - beforeEach(async () => { - subjectQuantityToIssue = ether(2); - }); - - it("should revert", async () => { - await expectRevertError(subject()); - }); - }); - describe("when the full fill size has been taken", async () => { beforeEach(async () => { const quantityToCancel = ether(4); @@ -525,6 +520,20 @@ contract("CoreIssuanceOrder", (accounts) => { await expectRevertError(subject()); }); }); + + describe("when takerAmountsToTransfer is less than requiredTokenAmounts", async () => { + before(async () => { + takerAmountsToTransfer = [ether(1), ether(1)]; + }); + + after(async () => { + takerAmountsToTransfer = undefined; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); }); describe("#cancelOrder", async () => { diff --git a/test/contracts/lib/authorizable.spec.ts b/test/contracts/lib/authorizable.spec.ts index 8608d17e2..34c2b5be7 100644 --- a/test/contracts/lib/authorizable.spec.ts +++ b/test/contracts/lib/authorizable.spec.ts @@ -23,7 +23,7 @@ BigNumberSetup.configure(); ChaiSetup.configure(); const { expect, assert } = chai; -import { +import { assertLogEquivalence, getFormattedLogsFromTxHash } from "../../utils/logs"; diff --git a/test/contracts/lib/erc20Wrapper.spec.ts b/test/contracts/lib/erc20Wrapper.spec.ts index 31798c7cf..1cd881730 100644 --- a/test/contracts/lib/erc20Wrapper.spec.ts +++ b/test/contracts/lib/erc20Wrapper.spec.ts @@ -3,6 +3,7 @@ import * as _ from "lodash"; import * as ABIDecoder from "abi-decoder"; import { BigNumber } from "bignumber.js"; +import { ether } from "../../utils/units"; // Types import { Address } from "../../../types/common.js"; @@ -116,9 +117,49 @@ contract("ERC20WrapperMock", (accounts) => { ); } - it("approves the spender on behalf of the calling contract", async () => { + it("returns the allowance of the spending contract", async () => { const allowance = await subject(); expect(allowance).to.bignumber.equal(ZERO); }); }); + + describe("#ensureAllowance", async () => { + let token: StandardTokenMockContract; + + beforeEach(async () => { + token = await erc20Wrapper.deployTokenAsync(ownerAccount); + erc20WrapperLibrary = await libraryMockWrapper.deployERC20WrapperLibraryAsync(); + }); + + async function subject(): Promise { + return erc20WrapperLibrary.ensureAllowance.sendTransactionAsync( + token.address, + erc20WrapperLibrary.address, + spenderAccount, + ether(10), + { from: ownerAccount }, + ); + } + + it("approves the spender on behalf of the calling contract", async () => { + await subject(); + + const allowance = await erc20WrapperLibrary.allowance.callAsync(token.address, erc20WrapperLibrary.address, spenderAccount, { from: ownerAccount }); + expect(allowance).to.bignumber.equal(UNLIMITED_ALLOWANCE_IN_BASE_UNITS); + }); + + describe("#when the token has already been approved", async () => { + + beforeEach(async () => { + await erc20WrapperLibrary.approve.sendTransactionAsync(token.address, spenderAccount, ether(11), {from: ownerAccount}) + }); + + it("should not alter the allowance", async () => { + await subject(); + + const allowance = await erc20WrapperLibrary.allowance.callAsync(token.address, erc20WrapperLibrary.address, spenderAccount, { from: ownerAccount }); + expect(allowance).to.bignumber.equal(ether(11)); + }); + }); + }); }); diff --git a/test/utils/erc20Wrapper.ts b/test/utils/erc20Wrapper.ts index 9298c960f..606ceb636 100644 --- a/test/utils/erc20Wrapper.ts +++ b/test/utils/erc20Wrapper.ts @@ -253,4 +253,19 @@ export class ERC20Wrapper { return balances; } + + public async getTokenAllowances( + tokens: StandardTokenMockContract[], + owner: Address, + spender: Address, + ): Promise { + const allowancePromises = _.map(tokens, (token) => token.allowance.callAsync(owner, spender)); + + let allowances: BigNumber[]; + await Promise.all(allowancePromises).then((fetchedAllowances) => { + allowances = fetchedAllowances; + }); + + return allowances; + } } diff --git a/test/utils/exchangeWrapper.ts b/test/utils/exchangeWrapper.ts index 2f5dd9aff..ccb9dd81f 100644 --- a/test/utils/exchangeWrapper.ts +++ b/test/utils/exchangeWrapper.ts @@ -2,7 +2,6 @@ import * as _ from "lodash"; import { TakerWalletWrapperContract } from "../../types/generated/taker_wallet_wrapper"; import { TransferProxyContract } from "../../types/generated/transfer_proxy"; -import { VaultContract } from "../../types/generated/vault"; import { BigNumber } from "bignumber.js"; import { Address } from "../../types/common.js"; @@ -21,13 +20,11 @@ export class ExchangeWrapper { /* ============ Deployment ============ */ public async deployTakerWalletExchangeWrapper( - vault: VaultContract, transferProxy: TransferProxyContract, from: Address = this._contractOwnerAddress ): Promise { const takerWalletWrapperInstance = await TakerWalletWrapper.new( transferProxy.address, - vault.address, { from, gas: DEFAULT_GAS }, ); diff --git a/test/utils/orderWrapper.ts b/test/utils/orderWrapper.ts index e2542c019..9fc9bb05a 100644 --- a/test/utils/orderWrapper.ts +++ b/test/utils/orderWrapper.ts @@ -40,12 +40,10 @@ export function generateOrdersDataForOrderCount( _.times(orderCount, (index) => { const exchange = _.sample(EXCHANGES); exchangeOrderDatum.push(paddedBufferForData(exchange)); - exchangeOrderDatum.push(paddedBufferForData(makerTokenAddress)); - exchangeOrderDatum.push(bufferAndLPad32BigNumber(ether(makerTokenAmounts[index]))); - const totalOrdersLength = _.random(200, 250); + const totalOrdersLength = _.random(200, 250); // Fake order data exchangeOrderDatum.push(paddedBufferForData(totalOrdersLength)); exchangeOrderDatum.push(randomBufferOfLength(totalOrdersLength)); }); @@ -53,6 +51,33 @@ export function generateOrdersDataForOrderCount( return ethUtil.bufferToHex(Buffer.concat(exchangeOrderDatum)); } +export function generateOrdersDataWithTakerOrders( + makerTokenAddress: Address, + takerTokenAddresses: Address[], + takerTokenAmounts: BigNumber[], +): Bytes32 { + // Header for entire ordersData + const exchangeOrderDatum: Buffer[] = [ + paddedBufferForData(EXCHANGES.TAKER_WALLET), + paddedBufferForData(takerTokenAddresses.length), // Include the number of orders as part of header + paddedBufferForData(makerTokenAddress), + paddedBufferForData(0), // Taker wallet orders do not take any maker token to execute + ]; + + // Body for takers orders + const takerOrdersData: Buffer[] = []; + _.each(takerTokenAmounts, (amount, idx) => { + takerOrdersData.push(paddedBufferForData(takerTokenAddresses[idx])); + takerOrdersData.push(paddedBufferForData(web3.toHex(amount))); + }); + const ordersBuffer = Buffer.concat(takerOrdersData); + + exchangeOrderDatum.push(paddedBufferForData(ordersBuffer.length)); + exchangeOrderDatum.push(ordersBuffer); + + return ethUtil.bufferToHex(Buffer.concat(exchangeOrderDatum)); +} + export function generateTakerWalletOrders( takerTokenAddress: Address[], takerTokenAmount: BigNumber[], diff --git a/yarn.lock b/yarn.lock index b032f6801..88525c1b2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5746,9 +5746,9 @@ sol-explore@^1.6.2: version "1.6.2" resolved "https://registry.yarnpkg.com/sol-explore/-/sol-explore-1.6.2.tgz#43ae8c419fd3ac056a05f8a9d1fb1022cd41ecc2" -sol-trace-set@^0.0.1: - version "0.0.1" - resolved "https://registry.yarnpkg.com/sol-trace-set/-/sol-trace-set-0.0.1.tgz#f362ede6a465b52337ae4d2446f79b26e501af50" +sol-trace-set@0.0.2: + version "0.0.2" + resolved "https://registry.yarnpkg.com/sol-trace-set/-/sol-trace-set-0.0.2.tgz#d325fb59758661cdb3bd6848fc417cb723525507" dependencies: ethereumjs-util "^5.2.0" glob "^7.1.2"