From cde01697338e3d3817d18a86968373af6fcc4e46 Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Wed, 9 Oct 2019 15:42:50 -0700 Subject: [PATCH 1/3] Update BlockchainBalanceStore to not rely on erc*_wrappers + better balance equality assertions --- contracts/exchange/package.json | 1 + contracts/exchange/src/index.ts | 1 + .../assertion_wrappers/fill_order_wrapper.ts | 93 ++++------ .../test/balance_stores/balance_store.ts | 175 +++++++++++------- .../blockchain_balance_store.ts | 144 +++++++++++--- .../exchange/test/balance_stores/index.ts | 4 + .../balance_stores/local_balance_store.ts | 70 ++++--- .../exchange/test/balance_stores/types.ts | 51 +++++ contracts/exchange/test/core.ts | 19 +- contracts/test-utils/src/index.ts | 1 + contracts/test-utils/src/types.ts | 20 +- 11 files changed, 398 insertions(+), 181 deletions(-) create mode 100644 contracts/exchange/test/balance_stores/index.ts create mode 100644 contracts/exchange/test/balance_stores/types.ts diff --git a/contracts/exchange/package.json b/contracts/exchange/package.json index 6f09054ea8..c954a8165e 100644 --- a/contracts/exchange/package.json +++ b/contracts/exchange/package.json @@ -63,6 +63,7 @@ "chai-as-promised": "^7.1.0", "chai-bignumber": "^3.0.0", "dirty-chai": "^2.0.1", + "js-combinatorics": "^0.5.3", "make-promises-safe": "^1.1.0", "mocha": "^6.2.0", "npm-run-all": "^4.1.2", diff --git a/contracts/exchange/src/index.ts b/contracts/exchange/src/index.ts index ea1294353f..4b64e3df2b 100644 --- a/contracts/exchange/src/index.ts +++ b/contracts/exchange/src/index.ts @@ -1,4 +1,5 @@ export * from './artifacts'; export * from './wrappers'; +export * from '../test/balance_stores'; export * from '../test/utils'; export * from './wrapper_interfaces'; diff --git a/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts b/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts index e675019145..d5f17d6dc1 100644 --- a/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts +++ b/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts @@ -1,49 +1,43 @@ -import { - artifacts as proxyArtifacts, - ERC1155ProxyWrapper, - ERC20Wrapper, - ERC721Wrapper, -} from '@0x/contracts-asset-proxy'; -import { artifacts as erc20Artifacts } from '@0x/contracts-erc20'; -import { artifacts as erc721Artifacts } from '@0x/contracts-erc721'; import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs'; import { constants, expect, FillEventArgs, filterLogsToArguments, - LogDecoder, OrderStatus, orderUtils, - Web3ProviderEngine, } from '@0x/contracts-test-utils'; import { orderHashUtils } from '@0x/order-utils'; import { FillResults, SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; -import { Web3Wrapper } from '@0x/web3-wrapper'; -import { TransactionReceiptWithDecodedLogs, ZeroExProvider } from 'ethereum-types'; +import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; -import { artifacts, ExchangeContract } from '../../src'; -import { BalanceStore } from '../balance_stores/balance_store'; -import { BlockchainBalanceStore } from '../balance_stores/blockchain_balance_store'; -import { LocalBalanceStore } from '../balance_stores/local_balance_store'; +import { + BalanceStore, + BlockchainBalanceStore, + ExchangeContract, + LocalBalanceStore, + TokenContractsByName, + TokenIds, + TokenOwnersByName, +} from '../../src'; export class FillOrderWrapper { private readonly _exchange: ExchangeContract; private readonly _blockchainBalanceStore: BlockchainBalanceStore; - private readonly _web3Wrapper: Web3Wrapper; /** - * Simulates matching two orders by transferring amounts defined in - * `transferAmounts` and returns the results. - * @param orders The orders being matched and their filled states. + * Locally simulates filling an order. + * @param txReceipt Transaction receipt from the actual fill, needed to update eth balance + * @param signedOrder The order being filled. * @param takerAddress Address of taker (the address who matched the two orders) - * @param tokenBalances Current token balances. - * @param transferAmounts Amounts to transfer during the simulation. - * @return The new account balances and fill events that occurred during the match. + * @param opts Optionally specifies the amount to fill. + * @param initBalanceStore Account balances prior to the fill. + * @return The expected account balances, fill results, and fill events. */ public static simulateFillOrder( + txReceipt: TransactionReceiptWithDecodedLogs, signedOrder: SignedOrder, takerAddress: string, opts: { takerAssetFillAmount?: BigNumber } = {}, @@ -88,6 +82,7 @@ export class FillOrderWrapper { fillResults.makerFeePaid, signedOrder.makerFeeAssetData, ); + balanceStore.burnGas(txReceipt.from, constants.DEFAULT_GAS_PRICE * txReceipt.gasUsed); return [fillResults, fillEvent, balanceStore]; } @@ -126,22 +121,19 @@ export class FillOrderWrapper { /** * Constructor. - * @param exchangeContract Insstance of the deployed exchange contract - * @param erc20Wrapper The ERC20 Wrapper used to interface with deployed erc20 tokens. - * @param erc721Wrapper The ERC721 Wrapper used to interface with deployed erc20 tokens. - * @param erc1155ProxyWrapper The ERC1155 Proxy Wrapper used to interface with deployed erc20 tokens. - * @param provider Web3 provider to be used by a `Web3Wrapper` instance + * @param exchangeContract Instance of the deployed exchange contract. + * @param tokenOwnersByName The addresses of token owners to assert the balances of. + * @param tokenContractsByName The contracts of tokens to assert the balances of. + * @param tokenIds The tokenIds of ERC721 and ERC1155 assets to assert the balances of. */ public constructor( exchangeContract: ExchangeContract, - erc20Wrapper: ERC20Wrapper, - erc721Wrapper: ERC721Wrapper, - erc1155ProxyWrapper: ERC1155ProxyWrapper, - provider: Web3ProviderEngine | ZeroExProvider, + tokenOwnersByName: TokenOwnersByName, + tokenContractsByName: Partial, + tokenIds: Partial, ) { this._exchange = exchangeContract; - this._blockchainBalanceStore = new BlockchainBalanceStore(erc20Wrapper, erc721Wrapper, erc1155ProxyWrapper); - this._web3Wrapper = new Web3Wrapper(provider); + this._blockchainBalanceStore = new BlockchainBalanceStore(tokenOwnersByName, tokenContractsByName, tokenIds); } /** @@ -171,31 +163,32 @@ export class FillOrderWrapper { // Assert init state of exchange await this._assertOrderStateAsync(signedOrder, initTakerAssetFilledAmount); // Simulate and execute fill then assert outputs + const [fillResults, fillEvent, txReceipt] = await this._fillOrderAsync(signedOrder, from, opts); const [ simulatedFillResults, simulatedFillEvent, simulatedFinalBalanceStore, - ] = FillOrderWrapper.simulateFillOrder(signedOrder, from, opts, this._blockchainBalanceStore); - const [fillResults, fillEvent] = await this._fillOrderAsync(signedOrder, from, opts); + ] = FillOrderWrapper.simulateFillOrder(txReceipt, signedOrder, from, opts, this._blockchainBalanceStore); // Assert state transition expect(simulatedFillResults, 'Fill Results').to.be.deep.equal(fillResults); expect(simulatedFillEvent, 'Fill Events').to.be.deep.equal(fillEvent); - const areBalancesEqual = BalanceStore.isEqual(simulatedFinalBalanceStore, this._blockchainBalanceStore); - expect(areBalancesEqual, 'Balances After Fill').to.be.true(); + + await this._blockchainBalanceStore.updateBalancesAsync(); + this._blockchainBalanceStore.assertEquals(simulatedFinalBalanceStore); + // Assert end state of exchange const finalTakerAssetFilledAmount = initTakerAssetFilledAmount.plus(fillResults.takerAssetFilledAmount); await this._assertOrderStateAsync(signedOrder, finalTakerAssetFilledAmount); } /** - * Fills an order on-chain. As an optimization this function auto-updates the blockchain balance store - * used by this contract. + * Fills an order on-chain. */ protected async _fillOrderAsync( signedOrder: SignedOrder, from: string, opts: { takerAssetFillAmount?: BigNumber } = {}, - ): Promise<[FillResults, FillEventArgs]> { + ): Promise<[FillResults, FillEventArgs, TransactionReceiptWithDecodedLogs]> { const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); const fillResults = await this._exchange.fillOrder.callAsync( params.order, @@ -203,33 +196,21 @@ export class FillOrderWrapper { params.signature, { from }, ); - // @TODO: Replace with `awaitTransactionAsync` once `development` is merged into `3.0` branch - const txHash = await this._exchange.fillOrder.sendTransactionAsync( + const txReceipt = await this._exchange.fillOrder.awaitTransactionSuccessAsync( params.order, params.takerAssetFillAmount, params.signature, { from }, ); - const logDecoder = new LogDecoder(this._web3Wrapper, { - ...artifacts, - ...proxyArtifacts, - ...erc20Artifacts, - ...erc721Artifacts, - }); - const txReceipt = await logDecoder.getTxWithDecodedLogsAsync(txHash); const fillEvent = FillOrderWrapper._extractFillEventsfromReceipt(txReceipt)[0]; - await this._blockchainBalanceStore.updateBalancesAsync(); - return [fillResults, fillEvent]; + return [fillResults, fillEvent, txReceipt]; } /** * Asserts that the provided order's fill amount and order status * are the expected values. * @param order The order to verify for a correct state. - * @param expectedFilledAmount The amount that the order should - * have been filled. - * @param side The side that the provided order should be matched on. - * @param exchangeWrapper The ExchangeWrapper instance. + * @param expectedFilledAmount The amount that the order should have been filled. */ private async _assertOrderStateAsync( order: SignedOrder, diff --git a/contracts/exchange/test/balance_stores/balance_store.ts b/contracts/exchange/test/balance_stores/balance_store.ts index 1e518b2d10..08ed22d0bc 100644 --- a/contracts/exchange/test/balance_stores/balance_store.ts +++ b/contracts/exchange/test/balance_stores/balance_store.ts @@ -1,92 +1,131 @@ -import { ERC1155Holdings, ERC1155HoldingsByOwner, TokenBalances } from '@0x/contracts-test-utils'; +import { BaseContract } from '@0x/base-contract'; +import { expect, TokenBalances } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; -/** - * Note - this should live in `@0x/contracts-test-utils` but that would create a circular - * dependency in `BlockchainBlanceStore`. We should be able to mvoe this once we can rely - * solely on auto-generated wrappers opposed to the existing ERC20Wrapper, ERC721Wrapper, - * and ERC1155Wrapper. - */ +import { TokenAddresses, TokenContractsByName, TokenOwnersByName } from './types'; + export class BalanceStore { protected _balances: TokenBalances; + protected _tokenAddresses: TokenAddresses; + protected _ownerAddresses: string[]; + private _addressNames: { + [address: string]: string; + }; /** - * Returns true iff balance stores do have the same entries. - * @param lhs First balance store to compare - * @param rhs Second balance store to compare + * Constructor. + * @param tokenOwnersByName Addresses of token owners to track balances of. + * @param tokenContractsByName Contracts of tokens to track balances of. */ - public static isEqual(lhs: BalanceStore, rhs: BalanceStore): boolean { - return _.isEqual(lhs.getRawBalances(), rhs.getRawBalances()); + public constructor(tokenOwnersByName: TokenOwnersByName, tokenContractsByName: Partial) { + this._balances = { erc20: {}, erc721: {}, erc1155: {}, eth: {} }; + this._ownerAddresses = Object.values(tokenOwnersByName); + + _.defaults(tokenContractsByName, { erc20: {}, erc721: {}, erc1155: {} }); + const tokenAddressesByName = _.mapValues( + { ...tokenContractsByName.erc20, ...tokenContractsByName.erc721, ...tokenContractsByName.erc1155 }, + contract => (contract as BaseContract).address, + ); + this._addressNames = _.invert({ ...tokenOwnersByName, ...tokenAddressesByName }); + + this._tokenAddresses = { + erc20: Object.values(tokenContractsByName.erc20 || {}).map(contract => contract.address), + erc721: Object.values(tokenContractsByName.erc721 || {}).map(contract => contract.address), + erc1155: Object.values(tokenContractsByName.erc1155 || {}).map(contract => contract.address), + }; } /** * Throws iff balance stores do not have the same entries. - * @param lhs First balance store to compare - * @param rhs Second balance store to compare + * @param rhs Balance store to compare to */ - public static assertEqual(lhs: BalanceStore, rhs: BalanceStore): void { - if (!BalanceStore.isEqual(lhs, rhs)) { - throw new Error(`Balance stores are not equal:\n\nLeft:\n${lhs}\n\nRight:\n${rhs}`); - } + public assertEquals(rhs: BalanceStore): void { + this._assertEthBalancesEqual(rhs); + this._assertErc20BalancesEqual(rhs); + this._assertErc721BalancesEqual(rhs); + this._assertErc1155BalancesEqual(rhs); } /** - * Restructures `ERC1155HoldingsByOwner` to be compatible with `TokenBalances.erc1155`. - * @param erc1155HoldingsByOwner Holdings returned by `ERC1155ProxyWrapper.getBalancesAsync()`. + * Copies from an existing balance store. + * @param balanceStore to copy from. */ - protected static _transformERC1155Holdings(erc1155HoldingsByOwner: ERC1155HoldingsByOwner): ERC1155Holdings { - const result = {}; - for (const owner of _.keys(erc1155HoldingsByOwner.fungible)) { - for (const contract of _.keys(erc1155HoldingsByOwner.fungible[owner])) { - _.set(result as any, [owner, contract, 'fungible'], erc1155HoldingsByOwner.fungible[owner][contract]); - } - } - for (const owner of _.keys(erc1155HoldingsByOwner.nonFungible)) { - for (const contract of _.keys(erc1155HoldingsByOwner.nonFungible[owner])) { - const tokenIds = _.flatten(_.values(erc1155HoldingsByOwner.nonFungible[owner][contract])); - _.set(result as any, [owner, contract, 'nonFungible'], _.uniqBy(tokenIds, v => v.toString(10))); - } - } - return result; - } - - /** - * Encodes token balances in a way that they can be compared by lodash. - */ - protected static _encodeTokenBalances(obj: any): any { - if (!_.isPlainObject(obj)) { - if (BigNumber.isBigNumber(obj)) { - return obj.toString(10); - } - if (_.isArray(obj)) { - return _.sortBy(obj, v => BalanceStore._encodeTokenBalances(v)); - } - return obj; - } - const keys = _.keys(obj).sort(); - return _.zip(keys, keys.map(k => BalanceStore._encodeTokenBalances(obj[k]))); - } - - /** - * Constructor. - */ - public constructor() { - this._balances = { erc20: {}, erc721: {}, erc1155: {}, eth: {} }; - } - - /** - * Copies the balance from an existing balance store. - * @param balanceStore to copy balances from. - */ - public copyBalancesFrom(balanceStore: BalanceStore): void { + public cloneFrom(balanceStore: BalanceStore): void { this._balances = _.cloneDeep(balanceStore._balances); + this._tokenAddresses = _.cloneDeep(balanceStore._tokenAddresses); + this._ownerAddresses = _.cloneDeep(balanceStore._ownerAddresses); + this._addressNames = _.cloneDeep(balanceStore._addressNames); + } + + private _readableAddressName(address: string): string { + return this._addressNames[address] || address; + } + + + /** + * Throws iff balance stores do not have the same ETH balances. + * @param rhs Balance store to compare to + */ + private _assertEthBalancesEqual(rhs: BalanceStore): void { + for (const ownerAddress of [...this._ownerAddresses, ...rhs._ownerAddresses]) { + const thisBalance = _.get(this._balances.eth, [ownerAddress], new BigNumber(0)); + const rhsBalance = _.get(rhs._balances.eth, [ownerAddress], new BigNumber(0)); + expect(thisBalance, `${this._readableAddressName(ownerAddress)} ETH balance`).to.bignumber.equal( + rhsBalance, + ); + } + } + + + /** + * Throws iff balance stores do not have the same ERC20 balances. + * @param rhs Balance store to compare to + */ + private _assertErc20BalancesEqual(rhs: BalanceStore): void { + for (const ownerAddress of [...this._ownerAddresses, ...rhs._ownerAddresses]) { + for (const tokenAddress of [...this._tokenAddresses.erc20, ...rhs._tokenAddresses.erc20]) { + const thisBalance = _.get(this._balances.erc20, [ownerAddress, tokenAddress], new BigNumber(0)); + const rhsBalance = _.get(rhs._balances.erc20, [ownerAddress, tokenAddress], new BigNumber(0)); + expect( + thisBalance, + `${this._readableAddressName(ownerAddress)} ${this._readableAddressName(tokenAddress)} balance`, + ).to.bignumber.equal(rhsBalance); + } + } } /** - * Returns the raw `TokenBalances` that this class encapsulates. + * Throws iff balance stores do not have the same ERC721 balances. + * @param rhs Balance store to compare to */ - public getRawBalances(): TokenBalances { - return _.cloneDeep(this._balances); + private _assertErc721BalancesEqual(rhs: BalanceStore): void { + for (const ownerAddress of [...this._ownerAddresses, ...rhs._ownerAddresses]) { + for (const tokenAddress of [...this._tokenAddresses.erc721, ...rhs._tokenAddresses.erc721]) { + const thisBalance = _.get(this._balances.erc721, [ownerAddress, tokenAddress], []); + const rhsBalance = _.get(rhs._balances.erc721, [ownerAddress, tokenAddress], []); + expect( + thisBalance, + `${this._readableAddressName(ownerAddress)} ${this._readableAddressName(tokenAddress)} balance`, + ).to.deep.equal(rhsBalance); + } + } + } + + /** + * Throws iff balance stores do not have the same ERC1155 balances. + * @param rhs Balance store to compare to + */ + private _assertErc1155BalancesEqual(rhs: BalanceStore): void { + for (const ownerAddress of [...this._ownerAddresses, ...rhs._ownerAddresses]) { + for (const tokenAddress of [...this._tokenAddresses.erc1155, ...rhs._tokenAddresses.erc1155]) { + const thisBalance = _.get(this._balances.erc1155, [ownerAddress, tokenAddress], {}); + const rhsBalance = _.get(rhs._balances.erc1155, [ownerAddress, tokenAddress], {}); + expect( + thisBalance, + `${this._readableAddressName(ownerAddress)} ${this._readableAddressName(tokenAddress)} balance`, + ).to.deep.equal(rhsBalance); + } + } } } diff --git a/contracts/exchange/test/balance_stores/blockchain_balance_store.ts b/contracts/exchange/test/balance_stores/blockchain_balance_store.ts index ce994d2f0c..64952296b2 100644 --- a/contracts/exchange/test/balance_stores/blockchain_balance_store.ts +++ b/contracts/exchange/test/balance_stores/blockchain_balance_store.ts @@ -1,40 +1,140 @@ -import { ERC1155ProxyWrapper, ERC20Wrapper, ERC721Wrapper } from '@0x/contracts-asset-proxy'; +import { web3Wrapper } from '@0x/contracts-test-utils'; +import { BigNumber } from '@0x/utils'; +import * as combinatorics from 'js-combinatorics'; +import * as _ from 'lodash'; import { BalanceStore } from './balance_store'; +import { TokenContracts, TokenContractsByName, TokenIds, TokenOwnersByName } from './types'; export class BlockchainBalanceStore extends BalanceStore { - private readonly _erc20Wrapper: ERC20Wrapper; - private readonly _erc721Wrapper: ERC721Wrapper; - private readonly _erc1155ProxyWrapper: ERC1155ProxyWrapper; + private readonly _tokenContracts: TokenContracts; + private readonly _tokenIds: TokenIds; /** * Constructor. - * @param erc20Wrapper The ERC20 Wrapper used to interface with deployed erc20 tokens. - * @param erc721Wrapper The ERC721 Wrapper used to interface with deployed erc20 tokens. - * @param erc1155ProxyWrapper The ERC1155 Proxy Wrapper used to interface with deployed erc20 tokens. + * @param tokenOwnersByName The addresses of token owners whose balances will be tracked. + * @param tokenContractsByName The contracts of tokens to track. + * @param tokenIds The tokenIds of ERC721 and ERC1155 assets to track. */ public constructor( - erc20Wrapper: ERC20Wrapper, - erc721Wrapper: ERC721Wrapper, - erc1155ProxyWrapper: ERC1155ProxyWrapper, + tokenOwnersByName: TokenOwnersByName, + tokenContractsByName: Partial, + tokenIds: Partial, ) { - super(); - this._erc20Wrapper = erc20Wrapper; - this._erc721Wrapper = erc721Wrapper; - this._erc1155ProxyWrapper = erc1155ProxyWrapper; + super(tokenOwnersByName, tokenContractsByName); + this._tokenContracts = { + erc20: Object.values(tokenContractsByName.erc20 || {}), + erc721: Object.values(tokenContractsByName.erc721 || {}), + erc1155: Object.values(tokenContractsByName.erc1155 || {}), + }; + this._tokenIds = { + erc721: tokenIds.erc721 || {}, + erc1155: tokenIds.erc1155 || {}, + }; } /** - * Updates balances by querying on-chain values managed by the erc20, erc721, and erc1155 wrappers. + * Updates balances by querying on-chain values. */ public async updateBalancesAsync(): Promise { - const [erc20, erc721, erc1155] = await Promise.all([ - this._erc20Wrapper.getBalancesAsync(), - this._erc721Wrapper.getBalancesAsync(), - this._erc1155ProxyWrapper.getBalancesAsync(), + await Promise.all([ + this.updateEthBalancesAsync(), + this.updateErc20BalancesAsync(), + this.updateErc721BalancesAsync(), + this.updateErc1155BalancesAsync(), ]); - this._balances.erc20 = erc20; - this._balances.erc721 = erc721; - this._balances.erc1155 = BalanceStore._transformERC1155Holdings(erc1155); + } + + /** + * Updates ETH balances. + */ + public async updateEthBalancesAsync(): Promise { + const ethBalances = _.zipObject( + this._ownerAddresses, + await Promise.all(this._ownerAddresses.map(address => web3Wrapper.getBalanceInWeiAsync(address))), + ); + this._balances.eth = ethBalances; + } + + /** + * Updates ERC20 balances. + */ + public async updateErc20BalancesAsync(): Promise { + const balances = await Promise.all( + this._ownerAddresses.map(async account => + _.zipObject( + this._tokenContracts.erc20.map(token => token.address), + await Promise.all(this._tokenContracts.erc20.map(token => token.balanceOf.callAsync(account))), + ), + ), + ); + this._balances.erc20 = _.zipObject(this._ownerAddresses, balances); + } + + /** + * Updates ERC721 balances. + */ + public async updateErc721BalancesAsync(): Promise { + const erc721ContractsByAddress = _.zipObject( + this._tokenContracts.erc721.map(contract => contract.address), + this._tokenContracts.erc721, + ); + + this._balances.erc721 = {}; + for (const [tokenAddress, tokenIds] of Object.entries(this._tokenIds.erc721)) { + for (const tokenId of tokenIds) { + const tokenOwner = await erc721ContractsByAddress[tokenAddress].ownerOf.callAsync(tokenId); + _.update(this._balances.erc721, [tokenOwner, tokenAddress], nfts => _.union([tokenId], nfts).sort()); + } + } + } + + /** + * Updates ERC1155 balances. + */ + public async updateErc1155BalancesAsync(): Promise { + const erc1155ContractsByAddress = _.zipObject( + this._tokenContracts.erc1155.map(contract => contract.address), + this._tokenContracts.erc1155, + ); + + for (const [tokenAddress, { fungible, nonFungible }] of Object.entries(this._tokenIds.erc1155)) { + const contract = erc1155ContractsByAddress[tokenAddress]; + const tokenIds = [...fungible, ...nonFungible]; + if (this._ownerAddresses.length === 0 || tokenIds.length === 0) { + continue; + } + + const [_tokenIds, _tokenOwners] = _.unzip( + combinatorics.cartesianProduct(tokenIds, this._ownerAddresses).toArray(), + ); + const balances = await contract.balanceOfBatch.callAsync( + _tokenOwners as string[], + _tokenIds as BigNumber[], + ); + + let i = 0; + for (const tokenOwner of this._ownerAddresses) { + // Fungible tokens + _.set(this._balances.erc1155, [tokenOwner, tokenAddress, 'fungible'], {}); + for (const tokenId of fungible) { + _.set( + this._balances.erc1155, + [tokenOwner, tokenAddress, 'fungible', tokenId.toString()], + balances[i++], + ); + } + // Non-fungible tokens + _.set(this._balances.erc1155, [tokenOwner, tokenAddress, 'nonFungible'], []); + for (const tokenId of nonFungible) { + const isOwner = balances[i++]; + if (isOwner.isEqualTo(1)) { + _.update(this._balances.erc1155, [tokenOwner, tokenAddress, 'nonFungible'], nfts => + _.union([tokenId], nfts).sort(), + ); + } + } + } + } } } diff --git a/contracts/exchange/test/balance_stores/index.ts b/contracts/exchange/test/balance_stores/index.ts new file mode 100644 index 0000000000..cb0e290bca --- /dev/null +++ b/contracts/exchange/test/balance_stores/index.ts @@ -0,0 +1,4 @@ +export { BalanceStore } from './balance_store'; +export { LocalBalanceStore } from './local_balance_store'; +export { BlockchainBalanceStore } from './blockchain_balance_store'; +export * from './types'; diff --git a/contracts/exchange/test/balance_stores/local_balance_store.ts b/contracts/exchange/test/balance_stores/local_balance_store.ts index 7d339c4182..6f2a65806d 100644 --- a/contracts/exchange/test/balance_stores/local_balance_store.ts +++ b/contracts/exchange/test/balance_stores/local_balance_store.ts @@ -1,28 +1,42 @@ +import { constants } from '@0x/contracts-test-utils'; import { assetDataUtils } from '@0x/order-utils'; import { AssetProxyId } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; import { BalanceStore } from './balance_store'; +import { TokenContractsByName, TokenOwnersByName } from './types'; export class LocalBalanceStore extends BalanceStore { /** * Creates a new balance store based on an existing one. - * @param balanceStore Existing balance store whose values should be copied. + * @param sourceBalanceStore Existing balance store whose values should be copied. */ public static create(sourceBalanceStore?: BalanceStore): LocalBalanceStore { const localBalanceStore = new LocalBalanceStore(); if (sourceBalanceStore !== undefined) { - localBalanceStore.copyBalancesFrom(sourceBalanceStore); + localBalanceStore.cloneFrom(sourceBalanceStore); } return localBalanceStore; } /** * Constructor. + * Note that parameters are given {} defaults because `LocalBalanceStore`s will typically + * be initialized via `create`. */ - public constructor() { - super(); + public constructor( + tokenOwnersByName: TokenOwnersByName = {}, + tokenContractsByName: Partial = {}, + ) { + super(tokenOwnersByName, tokenContractsByName); + } + + /** + * Decreases the ETH balance of an address to simulate gas usage. + */ + public burnGas(senderAddress: string, amount: BigNumber | number): void { + this._balances.eth[senderAddress] = this._balances.eth[senderAddress].minus(amount); } /** @@ -41,52 +55,66 @@ export class LocalBalanceStore extends BalanceStore { case AssetProxyId.ERC20: { const erc20AssetData = assetDataUtils.decodeERC20AssetData(assetData); const assetAddress = erc20AssetData.tokenAddress; - const fromBalances = this._balances.erc20[fromAddress]; - const toBalances = this._balances.erc20[toAddress]; - fromBalances[assetAddress] = fromBalances[assetAddress].minus(amount); - toBalances[assetAddress] = toBalances[assetAddress].plus(amount); + _.update(this._balances.erc20, [fromAddress, assetAddress], balance => balance.minus(amount)); + _.update(this._balances.erc20, [toAddress, assetAddress], balance => + (balance || constants.ZERO_AMOUNT).plus(amount), + ); break; } case AssetProxyId.ERC721: { const erc721AssetData = assetDataUtils.decodeERC721AssetData(assetData); const assetAddress = erc721AssetData.tokenAddress; const tokenId = erc721AssetData.tokenId; - const fromTokens = this._balances.erc721[fromAddress][assetAddress]; - const toTokens = this._balances.erc721[toAddress][assetAddress]; + const fromTokens = _.get(this._balances.erc721, [fromAddress, assetAddress], []); + const toTokens = _.get(this._balances.erc721, [toAddress, assetAddress], []); if (amount.gte(1)) { - const tokenIndex = _.findIndex(fromTokens, t => t.eq(tokenId)); + const tokenIndex = _.findIndex(fromTokens as BigNumber[], t => t.eq(tokenId)); if (tokenIndex !== -1) { fromTokens.splice(tokenIndex, 1); toTokens.push(tokenId); + toTokens.sort(); } } + _.set(this._balances.erc721, [fromAddress, assetAddress], fromTokens); + _.set(this._balances.erc721, [toAddress, assetAddress], toTokens); break; } case AssetProxyId.ERC1155: { const erc1155AssetData = assetDataUtils.decodeERC1155AssetData(assetData); const assetAddress = erc1155AssetData.tokenAddress; - const fromBalances = this._balances.erc1155[fromAddress][assetAddress]; - const toBalances = this._balances.erc1155[toAddress][assetAddress]; - for (const i of _.times(erc1155AssetData.tokenIds.length)) { - const tokenId = erc1155AssetData.tokenIds[i]; + const fromBalances = { + // tslint:disable-next-line:no-inferred-empty-object-type + fungible: _.get(this._balances.erc1155, [fromAddress, assetAddress, 'fungible'], {}), + nonFungible: _.get(this._balances.erc1155, [fromAddress, assetAddress, 'nonFungible'], []), + }; + const toBalances = { + // tslint:disable-next-line:no-inferred-empty-object-type + fungible: _.get(this._balances.erc1155, [toAddress, assetAddress, 'fungible'], {}), + nonFungible: _.get(this._balances.erc1155, [toAddress, assetAddress, 'nonFungible'], []), + }; + for (const [i, tokenId] of erc1155AssetData.tokenIds.entries()) { const tokenValue = erc1155AssetData.tokenValues[i]; const tokenAmount = amount.times(tokenValue); if (tokenAmount.gt(0)) { - const tokenIndex = _.findIndex(fromBalances.nonFungible, t => t.eq(tokenId)); + const tokenIndex = _.findIndex(fromBalances.nonFungible as BigNumber[], t => t.eq(tokenId)); if (tokenIndex !== -1) { // Transfer a non-fungible. fromBalances.nonFungible.splice(tokenIndex, 1); toBalances.nonFungible.push(tokenId); + // sort NFT's by name + toBalances.nonFungible.sort(); } else { // Transfer a fungible. - const _tokenId = tokenId.toString(10); - fromBalances.fungible[_tokenId] = fromBalances.fungible[_tokenId].minus(tokenAmount); - toBalances.fungible[_tokenId] = toBalances.fungible[_tokenId].plus(tokenAmount); + const _tokenId = tokenId.toString(); + _.update(fromBalances.fungible, [_tokenId], balance => balance.minus(tokenAmount)); + _.update(toBalances.fungible, [_tokenId], balance => + (balance || constants.ZERO_AMOUNT).plus(tokenAmount), + ); } } } - // sort NFT's by name - toBalances.nonFungible.sort(); + _.set(this._balances.erc1155, [fromAddress, assetAddress], fromBalances); + _.set(this._balances.erc1155, [toAddress, assetAddress], toBalances); break; } case AssetProxyId.MultiAsset: { diff --git a/contracts/exchange/test/balance_stores/types.ts b/contracts/exchange/test/balance_stores/types.ts new file mode 100644 index 0000000000..c8090f0e39 --- /dev/null +++ b/contracts/exchange/test/balance_stores/types.ts @@ -0,0 +1,51 @@ +import { ERC1155MintableContract } from '@0x/contracts-erc1155'; +import { DummyERC20TokenContract, DummyNoReturnERC20TokenContract } from '@0x/contracts-erc20'; +import { DummyERC721TokenContract } from '@0x/contracts-erc721'; +import { BigNumber } from '@0x/utils'; + +// alias for clarity +type address = string; + +interface TokenData { + erc20: TERC20; + erc721: TERC721; + erc1155: TERC1155; +} + +export type TokenAddresses = TokenData; + +export type TokenContracts = TokenData< + Array, + DummyERC721TokenContract[], + ERC1155MintableContract[] +>; + +interface Named { + [readableName: string]: T; +} + +export type TokenOwnersByName = Named
; + +export type TokenAddressesByName = TokenData, Named
, Named
>; + +export type TokenContractsByName = TokenData< + Named, + Named, + Named +>; + +interface ERC721TokenIds { + [tokenAddress: string]: BigNumber[]; +} + +interface ERC1155TokenIds { + [tokenAddress: string]: { + fungible: BigNumber[]; + nonFungible: BigNumber[]; + }; +} + +export interface TokenIds { + erc721: ERC721TokenIds; + erc1155: ERC1155TokenIds; +} diff --git a/contracts/exchange/test/core.ts b/contracts/exchange/test/core.ts index 8951bb2627..b34405f084 100644 --- a/contracts/exchange/test/core.ts +++ b/contracts/exchange/test/core.ts @@ -222,7 +222,24 @@ blockchainTests.resets('Exchange core', () => { }; const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; orderFactory = new OrderFactory(privateKey, defaultOrderParams); - fillOrderWrapper = new FillOrderWrapper(exchange, erc20Wrapper, erc721Wrapper, erc1155ProxyWrapper, provider); + fillOrderWrapper = new FillOrderWrapper( + exchange, + { makerAddress, takerAddress, feeRecipientAddress }, + { + erc20: { erc20TokenA, erc20TokenB, feeToken, noReturnErc20Token }, + erc721: { erc721Token }, + erc1155: { erc1155Contract }, + }, + { + erc721: { [erc721Token.address]: [...erc721MakerAssetIds, ...erc721TakerAssetIds] }, + erc1155: { + [erc1155Contract.address]: { + fungible: erc1155FungibleTokens, + nonFungible: [...erc1155NonFungibleTokensOwnedByMaker, ...erc1155NonFungibleTokensOwnedByTaker], + }, + }, + }, + ); }); describe('fillOrder', () => { beforeEach(async () => { diff --git a/contracts/test-utils/src/index.ts b/contracts/test-utils/src/index.ts index 68bc19c5b5..1753c590c4 100644 --- a/contracts/test-utils/src/index.ts +++ b/contracts/test-utils/src/index.ts @@ -48,6 +48,7 @@ export { ERC1155Holdings, ERC1155NonFungibleHoldingsByOwner, ERC721TokenIdsByOwner, + EthBalancesByOwner, FillEventArgs, MarketBuyOrders, MarketSellOrders, diff --git a/contracts/test-utils/src/types.ts b/contracts/test-utils/src/types.ts index 98388eb60c..eb3d745680 100644 --- a/contracts/test-utils/src/types.ts +++ b/contracts/test-utils/src/types.ts @@ -37,6 +37,10 @@ export interface ERC1155HoldingsByOwner { nonFungible: ERC1155NonFungibleHoldingsByOwner; } +export interface EthBalancesByOwner { + [owner: string]: BigNumber; +} + export interface SubmissionContractEventArgs { transactionId: BigNumber; } @@ -150,20 +154,10 @@ export interface ERC1155Holdings { } export interface TokenBalances { - erc20: { - [owner: string]: { - [contract: string]: BigNumber; - }; - }; - erc721: { - [owner: string]: { - [contract: string]: BigNumber[]; - }; - }; + erc20: ERC20BalancesByOwner; + erc721: ERC721TokenIdsByOwner; erc1155: ERC1155Holdings; - eth: { - [owner: string]: BigNumber; - }; + eth: EthBalancesByOwner; } export interface FillEventArgs { From ffac52f42eb7600aa792ba74b38a7c7fcb2a5650 Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Wed, 16 Oct 2019 20:19:04 -0700 Subject: [PATCH 2/3] lint --- contracts/exchange/test/balance_stores/balance_store.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/exchange/test/balance_stores/balance_store.ts b/contracts/exchange/test/balance_stores/balance_store.ts index 08ed22d0bc..a9a1b56941 100644 --- a/contracts/exchange/test/balance_stores/balance_store.ts +++ b/contracts/exchange/test/balance_stores/balance_store.ts @@ -58,11 +58,14 @@ export class BalanceStore { this._addressNames = _.cloneDeep(balanceStore._addressNames); } + /** + * Returns the human-readable name for the given address, if it exists. + * @param address The address to get the name for. + */ private _readableAddressName(address: string): string { return this._addressNames[address] || address; } - /** * Throws iff balance stores do not have the same ETH balances. * @param rhs Balance store to compare to @@ -77,7 +80,6 @@ export class BalanceStore { } } - /** * Throws iff balance stores do not have the same ERC20 balances. * @param rhs Balance store to compare to From b45ec47eee8a9d773a329a893285498b3d0cfe7f Mon Sep 17 00:00:00 2001 From: Michael Zhu Date: Thu, 17 Oct 2019 19:53:43 -0700 Subject: [PATCH 3/3] address comments --- .../assertion_wrappers/fill_order_wrapper.ts | 4 +-- contracts/exchange/test/core.ts | 31 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts b/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts index d5f17d6dc1..242a0ab8e7 100644 --- a/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts +++ b/contracts/exchange/test/assertion_wrappers/fill_order_wrapper.ts @@ -40,8 +40,8 @@ export class FillOrderWrapper { txReceipt: TransactionReceiptWithDecodedLogs, signedOrder: SignedOrder, takerAddress: string, - opts: { takerAssetFillAmount?: BigNumber } = {}, initBalanceStore: BalanceStore, + opts: { takerAssetFillAmount?: BigNumber } = {}, ): [FillResults, FillEventArgs, BalanceStore] { const balanceStore = LocalBalanceStore.create(initBalanceStore); const takerAssetFillAmount = @@ -168,7 +168,7 @@ export class FillOrderWrapper { simulatedFillResults, simulatedFillEvent, simulatedFinalBalanceStore, - ] = FillOrderWrapper.simulateFillOrder(txReceipt, signedOrder, from, opts, this._blockchainBalanceStore); + ] = FillOrderWrapper.simulateFillOrder(txReceipt, signedOrder, from, this._blockchainBalanceStore, opts); // Assert state transition expect(simulatedFillResults, 'Fill Results').to.be.deep.equal(fillResults); expect(simulatedFillEvent, 'Fill Events').to.be.deep.equal(fillEvent); diff --git a/contracts/exchange/test/core.ts b/contracts/exchange/test/core.ts index b34405f084..2ee5a2b5e5 100644 --- a/contracts/exchange/test/core.ts +++ b/contracts/exchange/test/core.ts @@ -222,23 +222,26 @@ blockchainTests.resets('Exchange core', () => { }; const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; orderFactory = new OrderFactory(privateKey, defaultOrderParams); + + const tokenContracts = { + erc20: { erc20TokenA, erc20TokenB, feeToken, noReturnErc20Token }, + erc721: { erc721Token }, + erc1155: { erc1155Contract }, + }; + const tokenIds = { + erc721: { [erc721Token.address]: [...erc721MakerAssetIds, ...erc721TakerAssetIds] }, + erc1155: { + [erc1155Contract.address]: { + fungible: erc1155FungibleTokens, + nonFungible: [...erc1155NonFungibleTokensOwnedByMaker, ...erc1155NonFungibleTokensOwnedByTaker], + }, + }, + }; fillOrderWrapper = new FillOrderWrapper( exchange, { makerAddress, takerAddress, feeRecipientAddress }, - { - erc20: { erc20TokenA, erc20TokenB, feeToken, noReturnErc20Token }, - erc721: { erc721Token }, - erc1155: { erc1155Contract }, - }, - { - erc721: { [erc721Token.address]: [...erc721MakerAssetIds, ...erc721TakerAssetIds] }, - erc1155: { - [erc1155Contract.address]: { - fungible: erc1155FungibleTokens, - nonFungible: [...erc1155NonFungibleTokensOwnedByMaker, ...erc1155NonFungibleTokensOwnedByTaker], - }, - }, - }, + tokenContracts, + tokenIds, ); }); describe('fillOrder', () => {