From 59cb2132f27c0c28a1e6077aaac320e485786e65 Mon Sep 17 00:00:00 2001 From: fragosti Date: Tue, 5 Jun 2018 11:46:05 -0700 Subject: Linter now passes --- packages/contracts/src/utils/coverage.ts | 1 - packages/contracts/src/utils/exchange_wrapper.ts | 2 +- packages/contracts/src/utils/log_decoder.ts | 2 +- packages/contracts/src/utils/match_order_tester.ts | 24 +--------------------- packages/contracts/src/utils/multi_sig_wrapper.ts | 1 - packages/contracts/src/utils/order_factory.ts | 1 - packages/contracts/src/utils/order_utils.ts | 3 +-- .../contracts/src/utils/transaction_factory.ts | 1 - packages/contracts/src/utils/types.ts | 4 ++-- packages/contracts/src/utils/web3_wrapper.ts | 1 - 10 files changed, 6 insertions(+), 34 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/coverage.ts b/packages/contracts/src/utils/coverage.ts index a37939464..41c83f703 100644 --- a/packages/contracts/src/utils/coverage.ts +++ b/packages/contracts/src/utils/coverage.ts @@ -1,6 +1,5 @@ import { devConstants } from '@0xproject/dev-utils'; import { CoverageSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; -import * as fs from 'fs'; import * as _ from 'lodash'; let coverageSubprovider: CoverageSubprovider; diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index dd278e77c..ea8eb47c7 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -1,7 +1,7 @@ import { AssetProxyId, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; -import { LogEntry, Provider, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; +import { Provider, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; import { ExchangeContract } from '../contract_wrappers/generated/exchange'; diff --git a/packages/contracts/src/utils/log_decoder.ts b/packages/contracts/src/utils/log_decoder.ts index 07d10e69d..79d41d2da 100644 --- a/packages/contracts/src/utils/log_decoder.ts +++ b/packages/contracts/src/utils/log_decoder.ts @@ -39,7 +39,7 @@ export class LogDecoder { } public decodeLogOrThrow(log: LogEntry): LogWithDecodedArgs | RawLog { const logWithDecodedArgsOrLog = this._abiDecoder.tryToDecodeLogOrNoop(log); - if (_.isUndefined((logWithDecodedArgsOrLog as LogWithDecodedArgs).args)) { + if (_.isUndefined((logWithDecodedArgsOrLog).args)) { throw new Error(`Unable to decode log: ${JSON.stringify(log)}`); } LogDecoder.wrapLogBigNumbers(logWithDecodedArgsOrLog); diff --git a/packages/contracts/src/utils/match_order_tester.ts b/packages/contracts/src/utils/match_order_tester.ts index 6170188bc..f4f7f965b 100644 --- a/packages/contracts/src/utils/match_order_tester.ts +++ b/packages/contracts/src/utils/match_order_tester.ts @@ -1,38 +1,21 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils, crypto, orderHashUtils } from '@0xproject/order-utils'; +import { assetProxyUtils, orderHashUtils } from '@0xproject/order-utils'; import { AssetProxyId, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; -import { LogWithDecodedArgs } from 'ethereum-types'; -import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../contract_wrappers/generated/dummy_e_r_c20_token'; -import { DummyERC721TokenContract } from '../contract_wrappers/generated/dummy_e_r_c721_token'; -import { ERC20ProxyContract } from '../contract_wrappers/generated/e_r_c20_proxy'; -import { ERC721ProxyContract } from '../contract_wrappers/generated/e_r_c721_proxy'; -import { - CancelContractEventArgs, - ExchangeContract, - FillContractEventArgs, -} from '../contract_wrappers/generated/exchange'; import { chaiSetup } from '../utils/chai_setup'; -import { constants } from '../utils/constants'; import { ERC20Wrapper } from '../utils/erc20_wrapper'; import { ERC721Wrapper } from '../utils/erc721_wrapper'; import { ExchangeWrapper } from '../utils/exchange_wrapper'; -import { OrderFactory } from '../utils/order_factory'; import { - ContractName, ERC20BalancesByOwner, ERC721TokenIdsByOwner, TransferAmountsByMatchOrders as TransferAmounts, } from '../utils/types'; -import { provider, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); export class MatchOrderTester { private _exchangeWrapper: ExchangeWrapper; @@ -112,11 +95,6 @@ export class MatchOrderTester { initialTakerAssetFilledAmountLeft?: BigNumber, initialTakerAssetFilledAmountRight?: BigNumber, ): Promise<[ERC20BalancesByOwner, ERC721TokenIdsByOwner]> { - // Test setup & verify preconditions - const makerAddressLeft = signedOrderLeft.makerAddress; - const makerAddressRight = signedOrderRight.makerAddress; - const feeRecipientAddressLeft = signedOrderLeft.feeRecipientAddress; - const feeRecipientAddressRight = signedOrderRight.feeRecipientAddress; // Verify Left order preconditions const orderTakerAssetFilledAmountLeft = await this._exchangeWrapper.getTakerAssetFilledAmountAsync( orderHashUtils.getOrderHashHex(signedOrderLeft), diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 9971e8f6e..750466ce1 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -6,7 +6,6 @@ import * as _ from 'lodash'; import { AssetProxyOwnerContract } from '../contract_wrappers/generated/asset_proxy_owner'; import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet'; -import { constants } from './constants'; import { LogDecoder } from './log_decoder'; export class MultiSigWrapper { diff --git a/packages/contracts/src/utils/order_factory.ts b/packages/contracts/src/utils/order_factory.ts index af411c01f..6e4c9a883 100644 --- a/packages/contracts/src/utils/order_factory.ts +++ b/packages/contracts/src/utils/order_factory.ts @@ -1,7 +1,6 @@ import { generatePseudoRandomSalt, orderHashUtils } from '@0xproject/order-utils'; import { Order, SignatureType, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; -import * as _ from 'lodash'; import { constants } from './constants'; import { signingUtils } from './signing_utils'; diff --git a/packages/contracts/src/utils/order_utils.ts b/packages/contracts/src/utils/order_utils.ts index 0d0329aa1..2a8791e4c 100644 --- a/packages/contracts/src/utils/order_utils.ts +++ b/packages/contracts/src/utils/order_utils.ts @@ -1,6 +1,5 @@ -import { Order, OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; +import { OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; -import ethUtil = require('ethereumjs-util'); import { CancelOrder, MatchOrder } from './types'; diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts index 434611908..a060263b1 100644 --- a/packages/contracts/src/utils/transaction_factory.ts +++ b/packages/contracts/src/utils/transaction_factory.ts @@ -1,6 +1,5 @@ import { crypto, generatePseudoRandomSalt } from '@0xproject/order-utils'; import { SignatureType } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; import * as ethUtil from 'ethereumjs-util'; import { signingUtils } from './signing_utils'; diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 360e1fdbc..491890fa1 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -1,6 +1,6 @@ -import { Order, OrderWithoutExchangeAddress } from '@0xproject/types'; +import { OrderWithoutExchangeAddress } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; -import { AbiDefinition, ContractAbi } from 'ethereum-types'; +import { AbiDefinition } from 'ethereum-types'; export interface ERC20BalancesByOwner { [ownerAddress: string]: { diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 1049ab967..95fb55753 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -1,7 +1,6 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; import { prependSubprovider } from '@0xproject/subproviders'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; -import { Provider } from 'ethereum-types'; import { coverage } from './coverage'; -- cgit v1.2.3 From cb754ee1253622974e751e4a8d723a424250c878 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Wed, 6 Jun 2018 15:39:38 +0200 Subject: move generated contract wrappers from `contract_wrappers/generated/` to `generated_contract_wrappers` in package with no non-generated contract wrappers --- packages/contracts/src/utils/erc20_wrapper.ts | 4 ++-- packages/contracts/src/utils/erc721_wrapper.ts | 4 ++-- packages/contracts/src/utils/exchange_wrapper.ts | 2 +- packages/contracts/src/utils/match_order_tester.ts | 10 +++++----- packages/contracts/src/utils/multi_sig_wrapper.ts | 4 ++-- packages/contracts/src/utils/token_registry_wrapper.ts | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/erc20_wrapper.ts b/packages/contracts/src/utils/erc20_wrapper.ts index dceeceeea..91c9d50b7 100644 --- a/packages/contracts/src/utils/erc20_wrapper.ts +++ b/packages/contracts/src/utils/erc20_wrapper.ts @@ -3,8 +3,8 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../contract_wrappers/generated/dummy_e_r_c20_token'; -import { ERC20ProxyContract } from '../contract_wrappers/generated/e_r_c20_proxy'; +import { DummyERC20TokenContract } from '../generated_contract_wrappers/dummy_e_r_c20_token'; +import { ERC20ProxyContract } from '../generated_contract_wrappers/e_r_c20_proxy'; import { artifacts } from './artifacts'; import { constants } from './constants'; diff --git a/packages/contracts/src/utils/erc721_wrapper.ts b/packages/contracts/src/utils/erc721_wrapper.ts index 13fdf630e..b20d9acd2 100644 --- a/packages/contracts/src/utils/erc721_wrapper.ts +++ b/packages/contracts/src/utils/erc721_wrapper.ts @@ -4,8 +4,8 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; -import { DummyERC721TokenContract } from '../contract_wrappers/generated/dummy_e_r_c721_token'; -import { ERC721ProxyContract } from '../contract_wrappers/generated/e_r_c721_proxy'; +import { DummyERC721TokenContract } from '../generated_contract_wrappers/dummy_e_r_c721_token'; +import { ERC721ProxyContract } from '../generated_contract_wrappers/e_r_c721_proxy'; import { artifacts } from './artifacts'; import { constants } from './constants'; diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index dd278e77c..ebc16dc00 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -4,7 +4,7 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { LogEntry, Provider, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; -import { ExchangeContract } from '../contract_wrappers/generated/exchange'; +import { ExchangeContract } from '../generated_contract_wrappers/exchange'; import { constants } from './constants'; import { formatters } from './formatters'; diff --git a/packages/contracts/src/utils/match_order_tester.ts b/packages/contracts/src/utils/match_order_tester.ts index 6170188bc..8dafe16f3 100644 --- a/packages/contracts/src/utils/match_order_tester.ts +++ b/packages/contracts/src/utils/match_order_tester.ts @@ -7,15 +7,15 @@ import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../contract_wrappers/generated/dummy_e_r_c20_token'; -import { DummyERC721TokenContract } from '../contract_wrappers/generated/dummy_e_r_c721_token'; -import { ERC20ProxyContract } from '../contract_wrappers/generated/e_r_c20_proxy'; -import { ERC721ProxyContract } from '../contract_wrappers/generated/e_r_c721_proxy'; +import { DummyERC20TokenContract } from '../generated_contract_wrappers/dummy_e_r_c20_token'; +import { DummyERC721TokenContract } from '../generated_contract_wrappers/dummy_e_r_c721_token'; +import { ERC20ProxyContract } from '../generated_contract_wrappers/e_r_c20_proxy'; +import { ERC721ProxyContract } from '../generated_contract_wrappers/e_r_c721_proxy'; import { CancelContractEventArgs, ExchangeContract, FillContractEventArgs, -} from '../contract_wrappers/generated/exchange'; +} from '../generated_contract_wrappers/exchange'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { ERC20Wrapper } from '../utils/erc20_wrapper'; diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 9971e8f6e..bcb31dac2 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -3,8 +3,8 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; -import { AssetProxyOwnerContract } from '../contract_wrappers/generated/asset_proxy_owner'; -import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet'; +import { AssetProxyOwnerContract } from '../generated_contract_wrappers/asset_proxy_owner'; +import { MultiSigWalletContract } from '../generated_contract_wrappers/multi_sig_wallet'; import { constants } from './constants'; import { LogDecoder } from './log_decoder'; diff --git a/packages/contracts/src/utils/token_registry_wrapper.ts b/packages/contracts/src/utils/token_registry_wrapper.ts index 240c06fdc..91895aa59 100644 --- a/packages/contracts/src/utils/token_registry_wrapper.ts +++ b/packages/contracts/src/utils/token_registry_wrapper.ts @@ -1,7 +1,7 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { Provider } from 'ethereum-types'; -import { TokenRegistryContract } from '../contract_wrappers/generated/token_registry'; +import { TokenRegistryContract } from '../generated_contract_wrappers/token_registry'; import { Token } from './types'; -- cgit v1.2.3 From 72fb8460e90237fb7879fc47e95d84b6aa54911b Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 21 May 2018 14:26:25 -0700 Subject: Update code after rebase --- packages/contracts/src/utils/assertions.ts | 20 ++++++++++++++++++++ packages/contracts/src/utils/constants.ts | 1 + packages/contracts/src/utils/web3_wrapper.ts | 8 ++++++-- 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 packages/contracts/src/utils/assertions.ts (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts new file mode 100644 index 000000000..72c2734d8 --- /dev/null +++ b/packages/contracts/src/utils/assertions.ts @@ -0,0 +1,20 @@ +import * as chai from 'chai'; +import * as _ from 'lodash'; + +import { constants } from './constants'; + +const expect = chai.expect; + +// throws if the given promise does not reject with one of two expected error +// messages. +export const expectRevertOrAlwaysFailingTransaction = (p: Promise) => { + return expect(p) + .to.be.rejected() + .then(e => { + expect(e).to.satisfy( + (err: Error) => + _.includes(err.message, constants.REVERT) || + _.includes(err.message, constants.ALWAYS_FAILING_TRANSACTION), + ); + }); +}; diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index 9b0b92545..144e22bc2 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,6 +19,7 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', + ALWAYS_FAILING_TRANSACTION: 'always failing transaction', TESTRPC_NETWORK_ID: 50, AWAIT_TRANSACTION_MINED_MS: 100, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 1049ab967..a89d7e8d0 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -7,13 +7,17 @@ import { coverage } from './coverage'; export const txDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, - gas: devConstants.GAS_LIMIT, + // gas: devConstants.GAS_LIMIT, +}; +const providerConfigs = { + shouldUseInProcessGanache: false, + rpcUrl: 'http://localhost:8501', }; -const providerConfigs = { shouldUseInProcessGanache: true }; export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); prependSubprovider(provider, coverageSubprovider); } + export const web3Wrapper = new Web3Wrapper(provider); -- cgit v1.2.3 From 1cc9d9c0713a56b59717498fcae6dc2720ca4fb0 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 22 May 2018 12:47:37 -0700 Subject: Replace constant.REVERT test assertions with expectRevertOrAlwaysFailingTransaction --- packages/contracts/src/utils/assertions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 72c2734d8..e3f31bf89 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -7,7 +7,7 @@ const expect = chai.expect; // throws if the given promise does not reject with one of two expected error // messages. -export const expectRevertOrAlwaysFailingTransaction = (p: Promise) => { +export function expectRevertOrAlwaysFailingTransaction(p: Promise): PromiseLike { return expect(p) .to.be.rejected() .then(e => { @@ -17,4 +17,4 @@ export const expectRevertOrAlwaysFailingTransaction = (p: Promise) => { _.includes(err.message, constants.ALWAYS_FAILING_TRANSACTION), ); }); -}; +} -- cgit v1.2.3 From 2004c0d7398a5e77d08e3b4d8030c0f22cb09cc8 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Fri, 1 Jun 2018 13:19:36 -0700 Subject: Add ability to quickly switch between Geth and Ganache by changing a const --- packages/contracts/src/utils/web3_wrapper.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index a89d7e8d0..bd582e841 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -5,14 +5,27 @@ import { Provider } from 'ethereum-types'; import { coverage } from './coverage'; -export const txDefaults = { +const useGeth = false; + +const ganacheTxDefaults = { + from: devConstants.TESTRPC_FIRST_ADDRESS, + gas: devConstants.GAS_LIMIT, +}; +const gethTxDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, - // gas: devConstants.GAS_LIMIT, }; -const providerConfigs = { +export const txDefaults = useGeth ? gethTxDefaults : ganacheTxDefaults; + +const gethConfigs = { shouldUseInProcessGanache: false, rpcUrl: 'http://localhost:8501', + shouldUseFakeGasEstimate: false, +}; +const ganacheConfigs = { + shouldUseInProcessGanache: true, }; +const providerConfigs = useGeth ? gethConfigs : ganacheConfigs; + export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); if (isCoverageEnabled) { -- cgit v1.2.3 From 98ffe9931d4fd8886955c45c42eb63b33184ddd2 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Fri, 1 Jun 2018 14:03:12 -0700 Subject: Get LibBytes tests working on both Ganache and Geth --- packages/contracts/src/utils/constants.ts | 5 +++++ packages/contracts/src/utils/web3_wrapper.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index 144e22bc2..a0369c256 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -20,6 +20,11 @@ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', ALWAYS_FAILING_TRANSACTION: 'always failing transaction', + LIB_BYTES_GT_ZERO_LENGTH_REQUIRED: 'Length must be greater than 0.', + LIB_BYTES_GTE_4_LENGTH_REQUIRED: 'Length must be greater than or equal to 4.', + LIB_BYTES_GTE_20_LENGTH_REQUIRED: 'Length must be greater than or equal to 20.', + LIB_BYTES_GTE_32_LENGTH_REQUIRED: 'Length must be greater than or equal to 32.', + LIB_BYTES_INDEX_OUT_OF_BOUNDS: 'Specified array index is out of bounds.', TESTRPC_NETWORK_ID: 50, AWAIT_TRANSACTION_MINED_MS: 100, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index bd582e841..49744dea1 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -5,7 +5,7 @@ import { Provider } from 'ethereum-types'; import { coverage } from './coverage'; -const useGeth = false; +const useGeth = true; const ganacheTxDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, -- cgit v1.2.3 From 2dfc4680941293ca9f4a55f3ca58b9ee68872754 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Fri, 1 Jun 2018 15:59:34 -0700 Subject: Update more tests to pass on Geth --- packages/contracts/src/utils/assertions.ts | 12 ++++++++++++ packages/contracts/src/utils/constants.ts | 2 ++ 2 files changed, 14 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index e3f31bf89..1e8d58b9f 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -18,3 +18,15 @@ export function expectRevertOrAlwaysFailingTransaction(p: Promise): Promis ); }); } + +export function expectInsufficientFunds(p: Promise): PromiseLike { + return expect(p) + .to.be.rejected() + .then(e => { + expect(e).to.satisfy( + (err: Error) => + _.includes(err.message, 'insufficient funds') || + _.includes(err.message, "sender doesn't have enough funds"), + ); + }); +} diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index a0369c256..60f41b51b 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -25,6 +25,8 @@ export const constants = { LIB_BYTES_GTE_20_LENGTH_REQUIRED: 'Length must be greater than or equal to 20.', LIB_BYTES_GTE_32_LENGTH_REQUIRED: 'Length must be greater than or equal to 32.', LIB_BYTES_INDEX_OUT_OF_BOUNDS: 'Specified array index is out of bounds.', + ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', + ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, AWAIT_TRANSACTION_MINED_MS: 100, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, -- cgit v1.2.3 From 5900899c0195a851c8d20ca0d4ad85dbbf4c100f Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 4 Jun 2018 17:47:18 -0700 Subject: Add support for TEST_PROVIDER env var --- packages/contracts/src/utils/web3_wrapper.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 49744dea1..6df8ac073 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -5,7 +5,7 @@ import { Provider } from 'ethereum-types'; import { coverage } from './coverage'; -const useGeth = true; +const testProvider = process.env.TEST_PROVIDER || 'ganache'; const ganacheTxDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, @@ -14,7 +14,7 @@ const ganacheTxDefaults = { const gethTxDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, }; -export const txDefaults = useGeth ? gethTxDefaults : ganacheTxDefaults; +export const txDefaults = testProvider === 'ganache' ? ganacheTxDefaults : gethTxDefaults; const gethConfigs = { shouldUseInProcessGanache: false, @@ -24,7 +24,8 @@ const gethConfigs = { const ganacheConfigs = { shouldUseInProcessGanache: true, }; -const providerConfigs = useGeth ? gethConfigs : ganacheConfigs; + +const providerConfigs = testProvider === 'ganache' ? ganacheConfigs : gethConfigs; export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); -- cgit v1.2.3 From 577a8dd005715ba0fd22a5118d99ccc87af0782c Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 4 Jun 2018 17:51:50 -0700 Subject: Fix some more test cases, especially those that call increaseTime --- packages/contracts/src/utils/increase_time.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 packages/contracts/src/utils/increase_time.ts (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/increase_time.ts b/packages/contracts/src/utils/increase_time.ts new file mode 100644 index 000000000..726a759f3 --- /dev/null +++ b/packages/contracts/src/utils/increase_time.ts @@ -0,0 +1,26 @@ +import * as _ from 'lodash'; + +import { constants } from './constants'; +import { web3Wrapper } from './web3_wrapper'; + +let firstAccount: string | undefined; + +// increases time by the given number of seconds and then mines a block so that +// the current block timestamp has the offset applied. +export async function increaseTimeAndMineBlockAsync(seconds: number): Promise { + if (_.isUndefined(firstAccount)) { + const accounts = await web3Wrapper.getAvailableAddressesAsync(); + firstAccount = accounts[0]; + } + + const offset = await web3Wrapper.increaseTimeAsync(seconds); + // Note: we need to send a transaction after increasing time so + // that a block is actually mined. The contract looks at the + // last mined block for the timestamp. + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ from: firstAccount, to: firstAccount, value: 0 }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + + return offset; +} -- cgit v1.2.3 From bca62c813d2e821c56968916615861366402435b Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 4 Jun 2018 18:10:42 -0700 Subject: Throw in web3-wrapper when rawCallResult is '0x' --- packages/contracts/src/utils/assertions.ts | 12 ++++++++++++ packages/contracts/src/utils/constants.ts | 1 + 2 files changed, 13 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 1e8d58b9f..4fc410363 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -30,3 +30,15 @@ export function expectInsufficientFunds(p: Promise): PromiseLike { ); }); } + +export function expectRevertOrContractCallFailed(p: Promise): PromiseLike { + return expect(p) + .to.be.rejected() + .then(e => { + expect(e).to.satisfy( + (err: Error) => + _.includes(err.message, constants.REVERT) || + _.includes(err.message, constants.CONTRACT_CALL_FAILED), + ); + }); +} diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index 60f41b51b..a21ca29ed 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -20,6 +20,7 @@ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', ALWAYS_FAILING_TRANSACTION: 'always failing transaction', + CONTRACT_CALL_FAILED: 'Contract call failed', LIB_BYTES_GT_ZERO_LENGTH_REQUIRED: 'Length must be greater than 0.', LIB_BYTES_GTE_4_LENGTH_REQUIRED: 'Length must be greater than or equal to 4.', LIB_BYTES_GTE_20_LENGTH_REQUIRED: 'Length must be greater than or equal to 20.', -- cgit v1.2.3 From 36b01fbdcfcda93d185e018e31a919c36f2848ac Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 5 Jun 2018 14:59:20 -0700 Subject: Add additional gas to calls to fillOrderNoThrow --- packages/contracts/src/utils/exchange_wrapper.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index dd278e77c..24c3ba4be 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -60,14 +60,14 @@ export class ExchangeWrapper { public async fillOrderNoThrowAsync( signedOrder: SignedOrder, from: string, - opts: { takerAssetFillAmount?: BigNumber } = {}, + opts: { takerAssetFillAmount?: BigNumber; gas?: number } = {}, ): Promise { const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); const txHash = await this._exchange.fillOrderNoThrow.sendTransactionAsync( params.order, params.takerAssetFillAmount, params.signature, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; @@ -105,14 +105,14 @@ export class ExchangeWrapper { public async batchFillOrdersNoThrowAsync( orders: SignedOrder[], from: string, - opts: { takerAssetFillAmounts?: BigNumber[] } = {}, + opts: { takerAssetFillAmounts?: BigNumber[]; gas?: number } = {}, ): Promise { const params = formatters.createBatchFill(orders, opts.takerAssetFillAmounts); const txHash = await this._exchange.batchFillOrdersNoThrow.sendTransactionAsync( params.orders, params.takerAssetFillAmounts, params.signatures, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; @@ -135,14 +135,14 @@ export class ExchangeWrapper { public async marketSellOrdersNoThrowAsync( orders: SignedOrder[], from: string, - opts: { takerAssetFillAmount: BigNumber }, + opts: { takerAssetFillAmount: BigNumber; gas?: number }, ): Promise { const params = formatters.createMarketSellOrders(orders, opts.takerAssetFillAmount); const txHash = await this._exchange.marketSellOrdersNoThrow.sendTransactionAsync( params.orders, params.takerAssetFillAmount, params.signatures, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; -- cgit v1.2.3 From 63caddea62453863de84a4b53e14fe3e61d3008f Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 5 Jun 2018 15:12:09 -0700 Subject: Small fixes and cleanup --- packages/contracts/src/utils/assertions.ts | 6 ++---- packages/contracts/src/utils/constants.ts | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 4fc410363..1ea071d01 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -13,8 +13,7 @@ export function expectRevertOrAlwaysFailingTransaction(p: Promise): Promis .then(e => { expect(e).to.satisfy( (err: Error) => - _.includes(err.message, constants.REVERT) || - _.includes(err.message, constants.ALWAYS_FAILING_TRANSACTION), + _.includes(err.message, constants.REVERT) || _.includes(err.message, 'always failing transaction'), ); }); } @@ -37,8 +36,7 @@ export function expectRevertOrContractCallFailed(p: Promise): PromiseLike< .then(e => { expect(e).to.satisfy( (err: Error) => - _.includes(err.message, constants.REVERT) || - _.includes(err.message, constants.CONTRACT_CALL_FAILED), + _.includes(err.message, constants.REVERT) || _.includes(err.message, 'Contract call failed'), ); }); } diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index a21ca29ed..fa2a4af3c 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,8 +19,6 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', - ALWAYS_FAILING_TRANSACTION: 'always failing transaction', - CONTRACT_CALL_FAILED: 'Contract call failed', LIB_BYTES_GT_ZERO_LENGTH_REQUIRED: 'Length must be greater than 0.', LIB_BYTES_GTE_4_LENGTH_REQUIRED: 'Length must be greater than or equal to 4.', LIB_BYTES_GTE_20_LENGTH_REQUIRED: 'Length must be greater than or equal to 20.', -- cgit v1.2.3 From d6d7f4e875b161aa7284467a61f67989f76ec89e Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 5 Jun 2018 16:20:38 -0700 Subject: Update more things to work with both Geth and Ganache --- packages/contracts/src/utils/assertions.ts | 36 ++++++++++++------------------ 1 file changed, 14 insertions(+), 22 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 1ea071d01..c08bc7271 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -5,38 +5,30 @@ import { constants } from './constants'; const expect = chai.expect; -// throws if the given promise does not reject with one of two expected error -// messages. -export function expectRevertOrAlwaysFailingTransaction(p: Promise): PromiseLike { +function _expectEitherError(p: Promise, error1: string, error2: string): PromiseLike { return expect(p) .to.be.rejected() .then(e => { expect(e).to.satisfy( - (err: Error) => - _.includes(err.message, constants.REVERT) || _.includes(err.message, 'always failing transaction'), + (err: Error) => _.includes(err.message, error1) || _.includes(err.message, error2), + `expected promise to reject with error message that includes "${error1}" or "${error2}", but got: ` + + `"${e.message}"\n`, ); }); } export function expectInsufficientFunds(p: Promise): PromiseLike { - return expect(p) - .to.be.rejected() - .then(e => { - expect(e).to.satisfy( - (err: Error) => - _.includes(err.message, 'insufficient funds') || - _.includes(err.message, "sender doesn't have enough funds"), - ); - }); + return _expectEitherError(p, 'insufficient funds', "sender doesn't have enough funds"); +} + +export function expectRevertOrOtherError(p: Promise, otherError: string): PromiseLike { + return _expectEitherError(p, constants.REVERT, otherError); +} + +export function expectRevertOrAlwaysFailingTransaction(p: Promise): PromiseLike { + return expectRevertOrOtherError(p, 'always failing transaction'); } export function expectRevertOrContractCallFailed(p: Promise): PromiseLike { - return expect(p) - .to.be.rejected() - .then(e => { - expect(e).to.satisfy( - (err: Error) => - _.includes(err.message, constants.REVERT) || _.includes(err.message, 'Contract call failed'), - ); - }); + return expectRevertOrOtherError(p, 'Contract call failed'); } -- cgit v1.2.3 From ba6806df5d2d4b31c125a0c58cc6cd65bf555933 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 5 Jun 2018 16:45:37 -0700 Subject: Fix linter errors --- packages/contracts/src/utils/assertions.ts | 29 +++++++++++++++++++++++++++ packages/contracts/src/utils/increase_time.ts | 9 +++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index c08bc7271..fc57f93fb 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -17,18 +17,47 @@ function _expectEitherError(p: Promise, error1: string, error2: string): P }); } +/** + * Rejects if the given Promise does not reject with an error indicating + * insufficient funds. + * @param p the Promise which is expected to reject + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ export function expectInsufficientFunds(p: Promise): PromiseLike { return _expectEitherError(p, 'insufficient funds', "sender doesn't have enough funds"); } +/** + * Rejects if the given Promise does not reject with a "revert" error or the + * given otherError. + * @param p the Promise which is expected to reject + * @param otherError the other error which is accepted as a valid reject error. + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ export function expectRevertOrOtherError(p: Promise, otherError: string): PromiseLike { return _expectEitherError(p, constants.REVERT, otherError); } +/** + * Rejects if the given Promise does not reject with a "revert" or "always + * failing transaction" error. + * @param p the Promise which is expected to reject + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ export function expectRevertOrAlwaysFailingTransaction(p: Promise): PromiseLike { return expectRevertOrOtherError(p, 'always failing transaction'); } +/** + * Rejects if the given Promise does not reject with a "revert" or "Contract + * call failed" error. + * @param p the Promise which is expected to reject + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ export function expectRevertOrContractCallFailed(p: Promise): PromiseLike { return expectRevertOrOtherError(p, 'Contract call failed'); } diff --git a/packages/contracts/src/utils/increase_time.ts b/packages/contracts/src/utils/increase_time.ts index 726a759f3..5336a180d 100644 --- a/packages/contracts/src/utils/increase_time.ts +++ b/packages/contracts/src/utils/increase_time.ts @@ -5,8 +5,13 @@ import { web3Wrapper } from './web3_wrapper'; let firstAccount: string | undefined; -// increases time by the given number of seconds and then mines a block so that -// the current block timestamp has the offset applied. +/** + * Increases time by the given number of seconds and then mines a block so that + * the current block timestamp has the offset applied. + * @param seconds the Promise which is expected to reject + * @returns a new Promise which will resolve with the new total time offset or + * reject if the time could not be increased. + */ export async function increaseTimeAndMineBlockAsync(seconds: number): Promise { if (_.isUndefined(firstAccount)) { const accounts = await web3Wrapper.getAvailableAddressesAsync(); -- cgit v1.2.3 From 167a38e27d09af12af6c59f1b486c835420fbac1 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 5 Jun 2018 16:56:16 -0700 Subject: Add Async suffix to relevant assertions --- packages/contracts/src/utils/assertions.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index fc57f93fb..615e648f3 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -5,7 +5,7 @@ import { constants } from './constants'; const expect = chai.expect; -function _expectEitherError(p: Promise, error1: string, error2: string): PromiseLike { +function _expectEitherErrorAsync(p: Promise, error1: string, error2: string): PromiseLike { return expect(p) .to.be.rejected() .then(e => { @@ -24,8 +24,8 @@ function _expectEitherError(p: Promise, error1: string, error2: string): P * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export function expectInsufficientFunds(p: Promise): PromiseLike { - return _expectEitherError(p, 'insufficient funds', "sender doesn't have enough funds"); +export function expectInsufficientFundsAsync(p: Promise): PromiseLike { + return _expectEitherErrorAsync(p, 'insufficient funds', "sender doesn't have enough funds"); } /** @@ -36,8 +36,8 @@ export function expectInsufficientFunds(p: Promise): PromiseLike { * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export function expectRevertOrOtherError(p: Promise, otherError: string): PromiseLike { - return _expectEitherError(p, constants.REVERT, otherError); +export function expectRevertOrOtherErrorAsync(p: Promise, otherError: string): PromiseLike { + return _expectEitherErrorAsync(p, constants.REVERT, otherError); } /** @@ -47,8 +47,8 @@ export function expectRevertOrOtherError(p: Promise, otherError: string): * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export function expectRevertOrAlwaysFailingTransaction(p: Promise): PromiseLike { - return expectRevertOrOtherError(p, 'always failing transaction'); +export function expectRevertOrAlwaysFailingTransactionAsync(p: Promise): PromiseLike { + return expectRevertOrOtherErrorAsync(p, 'always failing transaction'); } /** @@ -58,6 +58,6 @@ export function expectRevertOrAlwaysFailingTransaction(p: Promise): Promis * @returns a new Promise which will reject if the conditions are not met and * otherwise resolve with no value. */ -export function expectRevertOrContractCallFailed(p: Promise): PromiseLike { - return expectRevertOrOtherError(p, 'Contract call failed'); +export function expectRevertOrContractCallFailedAsync(p: Promise): PromiseLike { + return expectRevertOrOtherErrorAsync(p, 'Contract call failed'); } -- cgit v1.2.3 From 5d2f9d7a33c4494b06098f13fca487613fe83c73 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Wed, 6 Jun 2018 10:56:01 -0700 Subject: Use an enum for ProviderType in contracts/src/utils/web3_wrapper --- packages/contracts/src/utils/web3_wrapper.ts | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 6df8ac073..63ce2c8cc 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -5,7 +5,25 @@ import { Provider } from 'ethereum-types'; import { coverage } from './coverage'; -const testProvider = process.env.TEST_PROVIDER || 'ganache'; +enum ProviderType { + Ganache = 'ganache', + Geth = 'geth', +} + +let testProvider: ProviderType; +switch (process.env.TEST_PROVIDER) { + case undefined: + testProvider = ProviderType.Ganache; + break; + case 'ganache': + testProvider = ProviderType.Ganache; + break; + case 'geth': + testProvider = ProviderType.Geth; + break; + default: + throw new Error(`Unknown TEST_PROVIDER: ${process.env.TEST_PROVIDER}`); +} const ganacheTxDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, @@ -14,7 +32,7 @@ const ganacheTxDefaults = { const gethTxDefaults = { from: devConstants.TESTRPC_FIRST_ADDRESS, }; -export const txDefaults = testProvider === 'ganache' ? ganacheTxDefaults : gethTxDefaults; +export const txDefaults = testProvider === ProviderType.Ganache ? ganacheTxDefaults : gethTxDefaults; const gethConfigs = { shouldUseInProcessGanache: false, @@ -24,8 +42,7 @@ const gethConfigs = { const ganacheConfigs = { shouldUseInProcessGanache: true, }; - -const providerConfigs = testProvider === 'ganache' ? ganacheConfigs : gethConfigs; +const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : gethConfigs; export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); -- cgit v1.2.3 From dd8727d3aebf1f4b552f8bad921b92107ad22936 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Wed, 6 Jun 2018 11:43:07 -0700 Subject: Apply various fixes based on PR feedback --- packages/contracts/src/utils/increase_time.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/increase_time.ts b/packages/contracts/src/utils/increase_time.ts index 5336a180d..4565d8dbc 100644 --- a/packages/contracts/src/utils/increase_time.ts +++ b/packages/contracts/src/utils/increase_time.ts @@ -8,7 +8,7 @@ let firstAccount: string | undefined; /** * Increases time by the given number of seconds and then mines a block so that * the current block timestamp has the offset applied. - * @param seconds the Promise which is expected to reject + * @param seconds the number of seconds by which to incrase the time offset. * @returns a new Promise which will resolve with the new total time offset or * reject if the time could not be increased. */ -- cgit v1.2.3 From 3342dd40012f4fca4108b0d7fe474c20d791b60c Mon Sep 17 00:00:00 2001 From: mohoff Date: Wed, 6 Jun 2018 23:21:53 +0200 Subject: typo --- .../src/contracts/current/protocol/Exchange/MixinExchangeCore.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 42128c92a..12b57d99f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -50,7 +50,7 @@ contract MixinExchangeCore is ////// Core exchange functions ////// - /// @dev Cancels all orders reated by sender with a salt less than or equal to the specified salt value. + /// @dev Cancels all orders created by sender with a salt less than or equal to the specified salt value. /// @param salt Orders created with a salt less or equal to this value will be cancelled. function cancelOrdersUpTo(uint256 salt) external -- cgit v1.2.3 From a97d77064aacda3de74b04894f110b05298c5ca8 Mon Sep 17 00:00:00 2001 From: fragosti Date: Wed, 6 Jun 2018 15:26:40 -0700 Subject: Get build and tests to pass --- packages/contracts/src/utils/log_decoder.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/log_decoder.ts b/packages/contracts/src/utils/log_decoder.ts index 79d41d2da..07127ba79 100644 --- a/packages/contracts/src/utils/log_decoder.ts +++ b/packages/contracts/src/utils/log_decoder.ts @@ -39,7 +39,8 @@ export class LogDecoder { } public decodeLogOrThrow(log: LogEntry): LogWithDecodedArgs | RawLog { const logWithDecodedArgsOrLog = this._abiDecoder.tryToDecodeLogOrNoop(log); - if (_.isUndefined((logWithDecodedArgsOrLog).args)) { + // tslint:disable-next-line:no-unnecessary-type-assertion + if (_.isUndefined((logWithDecodedArgsOrLog as LogWithDecodedArgs).args)) { throw new Error(`Unable to decode log: ${JSON.stringify(log)}`); } LogDecoder.wrapLogBigNumbers(logWithDecodedArgsOrLog); -- cgit v1.2.3 From a200eaacaa3975b63f24d8be6cdfc7b0921d91ef Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 7 Jun 2018 18:51:52 +0200 Subject: Fix tslint failure --- packages/contracts/src/utils/multi_sig_wrapper.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 750466ce1..8c4dbcedf 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -44,6 +44,7 @@ export class MultiSigWrapper { txId: BigNumber, from: string, ): Promise { + // tslint:disable-next-line:no-unnecessary-type-assertion const txHash = await (this ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); -- cgit v1.2.3 From 80215ea1818874bcd3661259df6f2d3279cc59f2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 23 May 2018 15:36:35 -0700 Subject: LibMem + TestLibMem + LibAssetProxyDecoder + DummyERC721Receiver --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 24 +-- .../current/protocol/AssetProxy/ERC721Proxy.sol | 28 ++- .../DummyERC721Receiver/DummyERC721Receiver.sol | 62 ++++++ .../current/test/TestLibMem/TestLibMem.sol | 238 +++++++++++++++++++++ .../LibAssetProxyDecoder/LibAssetProxyDecoder.sol | 74 +++++++ .../contracts/current/utils/LibBytes/LibBytes.sol | 76 ++++++- .../src/contracts/current/utils/LibMem/LibMem.sol | 104 +++++++++ 7 files changed, 579 insertions(+), 27 deletions(-) create mode 100644 packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol create mode 100644 packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol create mode 100644 packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol create mode 100644 packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 2c321e134..017f94b1a 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,12 +20,14 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; contract ERC20Proxy is LibBytes, + LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -34,34 +36,32 @@ contract ERC20Proxy is uint8 constant PROXY_ID = 1; /// @dev Internal version of `transferFrom`. - /// @param assetMetadata Encoded byte array. + /// @param proxyData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory assetMetadata, + bytes memory proxyData, address from, address to, uint256 amount ) internal { + // Decode proxy data. + ( + uint8 proxyId, + address token + ) = decodeERC20Data(proxyData); + // Data must be intended for this proxy. uint256 length = assetMetadata.length; require( - length == 21, - LENGTH_21_REQUIRED - ); - // TODO: Is this too inflexible in the future? - require( - uint8(assetMetadata[length - 1]) == PROXY_ID, - ASSET_PROXY_ID_MISMATCH + proxyId == PROXY_ID, + PROXY_ID_MISMATCH ); - // Decode metadata. - address token = readAddress(assetMetadata, 0); - // Transfer tokens. bool success = IERC20Token(token).transferFrom(from, to, amount); require( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 07e01c774..f35e48eee 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -20,12 +20,14 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC721Token/ERC721Token.sol"; contract ERC721Proxy is LibBytes, + LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -33,19 +35,29 @@ contract ERC721Proxy is // Id of this proxy. uint8 constant PROXY_ID = 2; + string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; + /// @dev Internal version of `transferFrom`. - /// @param assetMetadata Encoded byte array. + /// @param proxyData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory assetMetadata, + bytes memory proxyData, address from, address to, uint256 amount ) internal { + // Decode proxy data. + ( + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory data + ) = decodeERC721Data(proxyData); + // Data must be intended for this proxy. uint256 length = assetMetadata.length; @@ -56,8 +68,8 @@ contract ERC721Proxy is // TODO: Is this too inflexible in the future? require( - uint8(assetMetadata[length - 1]) == PROXY_ID, - ASSET_PROXY_ID_MISMATCH + proxyId == PROXY_ID, + PROXY_ID_MISMATCH ); // There exists only 1 of each token. @@ -66,15 +78,9 @@ contract ERC721Proxy is INVALID_AMOUNT ); - // Decode metadata - address token = readAddress(assetMetadata, 0); - uint256 tokenId = readUint256(assetMetadata, 20); - // Transfer token. // Either succeeds or throws. - // @TODO: Call safeTransferFrom if there is additional - // data stored in `assetMetadata`. - ERC721Token(token).transferFrom(from, to, tokenId); + ERC721Token(token).safeTransferFrom(from, to, tokenId, data); } /// @dev Gets the proxy id associated with the proxy address. diff --git a/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol new file mode 100644 index 000000000..1596f3357 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol @@ -0,0 +1,62 @@ +/* +The MIT License (MIT) + +Copyright (c) 2016 Smart Contract Solutions, Inc. + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +"Software"), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be included +in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +*/ + +pragma solidity ^0.4.24; + +import "../../tokens/ERC721Token/IERC721Receiver.sol"; + +contract DummyERC721Receiver is + IERC721Receiver +{ + + event TokenReceived( + address from, + uint256 tokenId, + bytes data + ); + + /** + * @notice Handle the receipt of an NFT + * @dev The ERC721 smart contract calls this function on the recipient + * after a `safetransfer`. This function MAY throw to revert and reject the + * transfer. This function MUST use 50,000 gas or less. Return of other + * than the magic value MUST result in the transaction being reverted. + * Note: the contract address is always the message sender. + * @param _from The sending address + * @param _tokenId The NFT identifier which is being transfered + * @param _data Additional data with no specified format + * @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + */ + function onERC721Received( + address _from, + uint256 _tokenId, + bytes _data) + public + returns (bytes4) + { + emit TokenReceived(_from, _tokenId, _data); + return ERC721_RECEIVED; + } +} diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol new file mode 100644 index 000000000..4cf62bf3a --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -0,0 +1,238 @@ +/* + + Copyright 2018 ZeroEx Intl. + + 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; + +import "../../utils/LibMem/LibMem.sol"; +import "../../utils/LibBytes/LibBytes.sol"; + +contract TestLibMem is + LibMem, + LibBytes +{ + + function test1() + public + pure + { + // Length of array & length to copy + uint256 length = 0; + + // Create source array + bytes memory sourceArray = new bytes(length); + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #1 failed. Array contents are not the same." + ); + } + + function test2() + public + pure + { + // Length of array & length to copy + uint256 length = 1; + + // Create source array + bytes memory sourceArray = new bytes(length); + sourceArray[0] = byte(1); + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #2 failed. Array contents are not the same." + ); + } + + function test3() + public + pure + { + // Length of array & length to copy + uint256 length = 11; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #3 failed. Array contents are not the same." + ); + } + + function test4() + public + pure + { + // Length of array & length to copy + uint256 length = 32; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #4 failed. Array contents are not the same." + ); + } + + function test5() + public + pure + { + // Length of array & length to copy + uint256 length = 72; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #5 failed. Array contents are not the same." + ); + } + + + function test6() + public + pure + { + // Length of arrays + uint256 length1 = 72; + uint256 length2 = 100; + + // The full source array is used for comparisons at the end + bytes memory fullSourceArray = new bytes(length1 + length2); + + // First source array + bytes memory sourceArray1 = new bytes(length1); + for(uint256 i = 0; i < length1; ++i) { + sourceArray1[i] = byte((i % 0xF) + 1); // [1..f] + fullSourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Second source array + bytes memory sourceArray2 = new bytes(length2); + for(uint256 j = 0; i < length2; ++i) { + sourceArray2[j] = byte((j % 0xF) + 1); // [1..f] + fullSourceArray[length1+j] = byte((j % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source arrays + bytes memory destArray = new bytes(length1 + length2); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray1) + 32, // skip copying array length + length1 + ); + memcpy( + getMemAddress(destArray) + 32 + length1, // skip copying array length + sourceArray1 bytes + getMemAddress(sourceArray2) + 32, // skip copying array length + length2 + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(fullSourceArray, destArray), + "Test #6 failed. Array contents are not the same." + ); + } + + function test7() + public + pure + { + // Length of array & length to copy + uint256 length = 72; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length - 8 // Copy all but last byte. + ); + + // Verify contents of source & dest arrays match + // We expect this to fail + require( + areBytesEqual(sourceArray, destArray), + "Test #7 failed. Array contents are not the same." + ); + } +} diff --git a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol new file mode 100644 index 000000000..ba53f2769 --- /dev/null +++ b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol @@ -0,0 +1,74 @@ +/* + + Copyright 2018 ZeroEx Intl. + + 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; +pragma experimental ABIEncoderV2; + +import "../LibBytes/LibBytes.sol"; + +contract LibAssetProxyDecoder is + LibBytes +{ + + string constant INVALID_ERC20_METADATA_LENGTH = "Metadata must have a length of 21."; + string constant INVALID_ERC721_METADATA_LENGTH = "Metadata must have a length of at least 53."; + + /// @dev Decodes ERC721 Asset Proxy data + function decodeERC20Data(bytes memory proxyData) + internal + pure + returns ( + uint8 proxyId, + address token + ) + { + require( + proxyData.length == 21, + INVALID_ERC20_METADATA_LENGTH + ); + proxyId = uint8(proxyData[0]); + token = readAddress(proxyData, 1); + + return (proxyId, token); + } + + /// @dev Decodes ERC721 Asset Proxy data + function decodeERC721Data(bytes memory proxyData) + internal + pure + returns ( + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory data + ) + { + require( + proxyData.length >= 53, + INVALID_ERC721_METADATA_LENGTH + ); + proxyId = uint8(proxyData[0]); + token = readAddress(proxyData, 1); + tokenId = readUint256(proxyData, 21); + if (proxyData.length > 53) { + data = readBytes(proxyData, 53); + } + + return (proxyId, token, tokenId, data); + } +} diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index df2221c93..fb8359462 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -18,7 +18,11 @@ pragma solidity ^0.4.24; -contract LibBytes { +import "../LibMem/LibMem.sol"; + +contract LibBytes is + LibMem +{ // Revert reasons string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0."; @@ -42,7 +46,7 @@ contract LibBytes { // Store last byte. result = b[b.length - 1]; - + assembly { // Decrement length of byte array. let newLen := sub(mload(b), 1) @@ -125,7 +129,7 @@ contract LibBytes { require( b.length >= index + 20, // 20 is length of address GTE_20_LENGTH_REQUIRED - ); + ); // Add offset to index: // 1. Arrays are prefixed by 32-byte length parameter (add 32 to index) @@ -157,7 +161,7 @@ contract LibBytes { require( b.length >= index + 20, // 20 is length of address GTE_20_LENGTH_REQUIRED - ); + ); // Add offset to index: // 1. Arrays are prefixed by 32-byte length parameter (add 32 to index) @@ -264,6 +268,7 @@ contract LibBytes { writeBytes32(b, index, bytes32(input)); } +======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -281,4 +286,67 @@ contract LibBytes { } return result; } + + /// @dev Reads a uint256 value from a position in a byte array. + /// @param b Byte array containing a uint256 value. + /// @param index Index in byte array of uint256 value. + /// @return uint256 value from byte array. + function readBytes( + bytes memory b, + uint256 index + ) + internal + pure + returns (bytes memory result) + { + // Read length of nested bytes + require( + b.length >= index + 32, + GTE_32_LENGTH_REQUIRED + ); + uint256 nestedBytesLength = readUint256(b, index); + + // Assert length of is valid, given + // length of nested bytes + require( + b.length >= index + 32 + nestedBytesLength, + GTE_32_LENGTH_REQUIRED + ); + + // Allocate memory and copy value to result + result = new bytes(nestedBytesLength); + memcpy( + getMemAddress(result) + 32, // +32 skips array length + getMemAddress(b) + index + 32, // +32 skips array length + nestedBytesLength + ); + + return result; + } + + /// @dev Writes a uint256 into a specific position in a byte array. + /// @param b Byte array to insert into. + /// @param index Index in byte array of . + /// @param input uint256 to put into byte array. + function writeBytes( + bytes memory b, + uint256 index, + bytes memory input + ) + internal + pure + { + // Read length of nested bytes + require( + b.length >= index + 32 /* 32 bytes to store length */ + input.length, + GTE_32_LENGTH_REQUIRED + ); + + // Copy into + memcpy( + getMemAddress(b) + index, + getMemAddress(input), + input.length + 32 /* 32 bytes to store length */ + ); + } } diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol new file mode 100644 index 000000000..b07a5da54 --- /dev/null +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -0,0 +1,104 @@ +/* + + Copyright 2018 ZeroEx Intl. + + 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; + +contract LibMem { + + function getMemAddress(bytes memory input) + internal + pure + returns (uint256 address_) + { + assembly { + address_ := input + } + return address_; + } + + /// @dev Writes a uint256 into a specific position in a byte array. + /// @param dest memory adress to copy bytes to + function memcpy( + uint256 dest, + uint256 source, + uint256 length + ) + internal + pure + { + // Base cases + if(length == 0) return; + if(source == dest) return; + + // Copy bytes from source to dest + assembly { + // Compute number of complete words to copy + remaining bytes + let lenFullWords := div(add(length, 0x1F), 0x20) + let remainder := mod(length, 0x20) + if gt(remainder, 0) { + lenFullWords := sub(lenFullWords, 1) + } + + // Copy full words from source to dest + let offset := 0 + let maxOffset := mul(0x20, lenFullWords) + for {offset := 0} lt(offset, maxOffset) {offset := add(offset, 0x20)} { + mstore(add(dest, offset), mload(add(source, offset))) + } + + // Copy remaining bytes + if gt(remainder, 0) { + // Read a full word from source, containing X bytes to copy to dest. + // We only want to keep the X bytes, zeroing out the remaining bytes. + // We accomplish this by a right shift followed by a left shift. + // Example: + // Suppose a word of 8 bits has all 1's: [11111111] + // Let X = 7 (we want to copy the first 7 bits) + // Apply a right shift of 1: [01111111] + // Apply a left shift of 1: [11111110] + let sourceShiftFactor := exp(2, mul(8, sub(0x20, remainder))) + let sourceWord := mload(add(source, offset)) + let sourceBytes := mul(div(sourceWord, sourceShiftFactor), sourceShiftFactor) + + // Read a full word from dest, containing (32-X) bytes to retain. + // We need to zero out the remaining bytes to be overwritten by source, + // while retaining the (32-X) bytes we don't want to overwrite. + // We accomplish this by a left shift followed by a right shift. + // Example: + // Suppose a word of 8 bits has all 1's: [11111111] + // Let X = 7 (we want to free the first 7 bits, and retain the last bit) + // Apply a left shift of 1: [11111110] + // Apply a right shift of 1: [01111111] + let destShiftFactor := exp(2, mul(8, remainder)) + let destWord := mload(add(dest, offset)) + let destBytes := div(mul(destWord, destShiftFactor), destShiftFactor) + + // Combine the source and dest bytes. There should be no overlap: + // The source bytes run from [0..X-1] and the dest bytes from [X..31]. + // Example: + // Following the example from above, we have [11111110] + // from the source word and [01111111] from the dest word. + // Combine these words using to get [11111111]. + let combinedDestWord := or(sourceBytes, destBytes) + + // Store the combined word into dest + mstore(add(dest, offset), combinedDestWord) + } + } + } +} -- cgit v1.2.3 From 3d65341080177bdd436e7628a76e65774b947a38 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 10:56:47 -0700 Subject: Tests for libMem --- .../current/test/TestLibMem/TestLibMem.sol | 23 ++++++++-------------- packages/contracts/src/utils/artifacts.ts | 2 ++ packages/contracts/src/utils/types.ts | 1 + 3 files changed, 11 insertions(+), 15 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 4cf62bf3a..0c6f8fbc9 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -27,8 +27,7 @@ contract TestLibMem is { function test1() - public - pure + external { // Length of array & length to copy uint256 length = 0; @@ -52,8 +51,7 @@ contract TestLibMem is } function test2() - public - pure + external { // Length of array & length to copy uint256 length = 1; @@ -78,8 +76,7 @@ contract TestLibMem is } function test3() - public - pure + external { // Length of array & length to copy uint256 length = 11; @@ -106,8 +103,7 @@ contract TestLibMem is } function test4() - public - pure + external { // Length of array & length to copy uint256 length = 32; @@ -134,8 +130,7 @@ contract TestLibMem is } function test5() - public - pure + external { // Length of array & length to copy uint256 length = 72; @@ -163,8 +158,7 @@ contract TestLibMem is function test6() - public - pure + external { // Length of arrays uint256 length1 = 72; @@ -208,8 +202,7 @@ contract TestLibMem is } function test7() - public - pure + external { // Length of array & length to copy uint256 length = 72; @@ -232,7 +225,7 @@ contract TestLibMem is // We expect this to fail require( areBytesEqual(sourceArray, destArray), - "Test #7 failed. Array contents are not the same." + "Test #7 failed. Array contents are not the same. This is expected." ); } } diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 357c66a0a..1b47f1d41 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -11,6 +11,7 @@ import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; +import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; import * as TokenRegistry from '../artifacts/TokenRegistry.json'; @@ -31,6 +32,7 @@ export const artifacts = { MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, + TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 491890fa1..cc6f00b95 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -91,6 +91,7 @@ export enum ContractName { EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', + TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', -- cgit v1.2.3 From 9b82e2df58cfd7f4dc9954fa93167450919f457f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 12:29:45 -0700 Subject: Foundation for TestLibAssetProxyDecoder --- .../TestLibAssetProxyDecoder.sol | 50 ++++++++++++++++++++++ packages/contracts/src/utils/artifacts.ts | 2 + packages/contracts/src/utils/types.ts | 1 + 3 files changed, 53 insertions(+) create mode 100644 packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol new file mode 100644 index 000000000..ac7cd63ab --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol @@ -0,0 +1,50 @@ +/* + + Copyright 2018 ZeroEx Intl. + + 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; +pragma experimental ABIEncoderV2; + +import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; + +contract TestLibAssetProxyDecoder is + LibAssetProxyDecoder +{ + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC20Data(bytes memory proxyData) + public + pure + returns (uint8, address) + { + return decodeERC20Data(proxyData); + } + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC721Data(bytes memory proxyData) + internal + pure + returns ( + uint8, + address, + uint256, + bytes memory + ) + { + return decodeERC721Data(proxyData); + } +} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 1b47f1d41..44de43a95 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -10,6 +10,7 @@ import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; +import * as TestLibAssetProxyDecoder from '../artifacts/TestLibAssetProxyDecoder.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; @@ -31,6 +32,7 @@ export const artifacts = { MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, + TestLibAssetProxyDecoder: (TestLibAssetProxyDecoder as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index cc6f00b95..70abb2643 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -90,6 +90,7 @@ export enum ContractName { AccountLevels = 'AccountLevels', EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', + TestLibAssetProxyDecoder = 'TestLibAssetProxyDecoder', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', -- cgit v1.2.3 From bc0edd40427944349ac0c3e367c9638a1c7797dd Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 13:03:17 -0700 Subject: LibAssetProxyDecoder tests --- .../current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol index ac7cd63ab..e4a7de71d 100644 --- a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol +++ b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol @@ -36,7 +36,7 @@ contract TestLibAssetProxyDecoder is /// @dev Decodes ERC721 Asset Proxy data function publicDecodeERC721Data(bytes memory proxyData) - internal + public pure returns ( uint8, -- cgit v1.2.3 From d9f9895b2bcd3cde09febbe0e1af31be5ddc80e2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 16:44:37 -0700 Subject: Test for onReceived erc721 callback --- packages/contracts/src/utils/artifacts.ts | 2 ++ packages/contracts/src/utils/types.ts | 1 + 2 files changed, 3 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 44de43a95..a1c8483d8 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -2,6 +2,7 @@ import { ContractArtifact } from '@0xproject/sol-compiler'; import * as AssetProxyOwner from '../artifacts/AssetProxyOwner.json'; import * as DummyERC20Token from '../artifacts/DummyERC20Token.json'; +import * as DummyERC721Receiver from '../artifacts/DummyERC721Receiver.json'; import * as DummyERC721Token from '../artifacts/DummyERC721Token.json'; import * as ERC20Proxy from '../artifacts/ERC20Proxy.json'; import * as ERC721Proxy from '../artifacts/ERC721Proxy.json'; @@ -23,6 +24,7 @@ import * as ZRX from '../artifacts/ZRXToken.json'; export const artifacts = { AssetProxyOwner: (AssetProxyOwner as any) as ContractArtifact, DummyERC20Token: (DummyERC20Token as any) as ContractArtifact, + DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact, DummyERC721Token: (DummyERC721Token as any) as ContractArtifact, ERC20Proxy: (ERC20Proxy as any) as ContractArtifact, ERC721Proxy: (ERC721Proxy as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 70abb2643..cccca5705 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -97,6 +97,7 @@ export enum ContractName { TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', ERC721Proxy = 'ERC721Proxy', + DummyERC721Receiver = 'DummyERC721Receiver', DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', Authorizable = 'Authorizable', -- cgit v1.2.3 From 842363200b3b8aded3b03fc8e46a329ff9534e36 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 25 May 2018 00:31:03 -0700 Subject: Tons of tests around nested byte arrays and ERC721 receiver --- .../current/test/TestLibBytes/TestLibBytes.sol | 25 ++++++++++++++++++++++ .../contracts/current/utils/LibBytes/LibBytes.sol | 17 ++++++--------- 2 files changed, 32 insertions(+), 10 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 69554605d..0bf11b03b 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -155,6 +155,7 @@ contract TestLibBytes is return b; } +======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -166,4 +167,28 @@ contract TestLibBytes is result = readFirst4(b); return result; } + + function publicReadBytes( + bytes memory b, + uint256 index) + public + pure + returns (bytes memory result) + { + result = readBytes(b, index); + return result; + } + + + function publicWriteBytes( + bytes memory b, + uint256 index, + bytes memory input) + public + pure + returns (bytes memory) + { + writeBytes(b, index, input); + return b; + } } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index fb8359462..6351f1a46 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -300,24 +300,21 @@ contract LibBytes is returns (bytes memory result) { // Read length of nested bytes - require( - b.length >= index + 32, - GTE_32_LENGTH_REQUIRED - ); uint256 nestedBytesLength = readUint256(b, index); + index += 32; // Assert length of is valid, given // length of nested bytes require( - b.length >= index + 32 + nestedBytesLength, + b.length >= index + nestedBytesLength, GTE_32_LENGTH_REQUIRED ); // Allocate memory and copy value to result result = new bytes(nestedBytesLength); memcpy( - getMemAddress(result) + 32, // +32 skips array length - getMemAddress(b) + index + 32, // +32 skips array length + getMemAddress(result) + 32, // +32 skips array length + getMemAddress(b) + index + 32, nestedBytesLength ); @@ -344,9 +341,9 @@ contract LibBytes is // Copy into memcpy( - getMemAddress(b) + index, - getMemAddress(input), - input.length + 32 /* 32 bytes to store length */ + getMemAddress(b) + index + 32, // +32 to skip length of + getMemAddress(input), // include length of byte array + input.length + 32 // +32 bytes to store length ); } } -- cgit v1.2.3 From d17e031259e11e3e37cbb837245171cbdd4d219b Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 25 May 2018 17:20:00 -0700 Subject: Fixed up wording in memcpy --- packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index b07a5da54..215a661e2 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -82,8 +82,8 @@ contract LibMem { // Example: // Suppose a word of 8 bits has all 1's: [11111111] // Let X = 7 (we want to free the first 7 bits, and retain the last bit) - // Apply a left shift of 1: [11111110] - // Apply a right shift of 1: [01111111] + // Apply a left shift of 7: [10000000] + // Apply a right shift of 7: [00000001] let destShiftFactor := exp(2, mul(8, remainder)) let destWord := mload(add(dest, offset)) let destBytes := div(mul(destWord, destShiftFactor), destShiftFactor) @@ -92,7 +92,7 @@ contract LibMem { // The source bytes run from [0..X-1] and the dest bytes from [X..31]. // Example: // Following the example from above, we have [11111110] - // from the source word and [01111111] from the dest word. + // from the source word and [00000001] from the dest word. // Combine these words using to get [11111111]. let combinedDestWord := or(sourceBytes, destBytes) -- cgit v1.2.3 From f5bc0b205c217eac8abdfee9dcdb3c4d21b5c31e Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Mon, 28 May 2018 12:50:03 +0200 Subject: Generate tests from vectors --- .../current/test/TestLibMem/TestLibMem.sol | 27 +++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 0c6f8fbc9..18deede4c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -19,12 +19,33 @@ pragma solidity ^0.4.24; import "../../utils/LibMem/LibMem.sol"; -import "../../utils/LibBytes/LibBytes.sol"; contract TestLibMem is - LibMem, - LibBytes + LibMem { + function testMemcpy( + bytes mem, ///< Memory contents we want to apply memcpy to + uint256 dest, + uint256 source, + uint256 length + ) + public // not external, we need input in memory + pure + returns (bytes) + { + // Sanity check. Overflows are not checked. + require(source + length <= mem.length); + require(dest + length <= mem.length); + + // Get pointer to memory contents + uint256 offset = getMemAddress(mem) + 32; + + // Execute memcpy adjusted for memory array location + memcpy(offset + dest, offset + source, length); + + // Return modified memory contents + return mem; + } function test1() external -- cgit v1.2.3 From 76b918d40e3bb485cdd45149888f012d9ec2b67f Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Mon, 28 May 2018 13:07:04 +0200 Subject: Convert Solidity tests to vectors --- .../current/test/TestLibMem/TestLibMem.sol | 203 --------------------- 1 file changed, 203 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 18deede4c..64bc182f4 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -46,207 +46,4 @@ contract TestLibMem is // Return modified memory contents return mem; } - - function test1() - external - { - // Length of array & length to copy - uint256 length = 0; - - // Create source array - bytes memory sourceArray = new bytes(length); - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #1 failed. Array contents are not the same." - ); - } - - function test2() - external - { - // Length of array & length to copy - uint256 length = 1; - - // Create source array - bytes memory sourceArray = new bytes(length); - sourceArray[0] = byte(1); - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #2 failed. Array contents are not the same." - ); - } - - function test3() - external - { - // Length of array & length to copy - uint256 length = 11; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #3 failed. Array contents are not the same." - ); - } - - function test4() - external - { - // Length of array & length to copy - uint256 length = 32; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #4 failed. Array contents are not the same." - ); - } - - function test5() - external - { - // Length of array & length to copy - uint256 length = 72; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #5 failed. Array contents are not the same." - ); - } - - - function test6() - external - { - // Length of arrays - uint256 length1 = 72; - uint256 length2 = 100; - - // The full source array is used for comparisons at the end - bytes memory fullSourceArray = new bytes(length1 + length2); - - // First source array - bytes memory sourceArray1 = new bytes(length1); - for(uint256 i = 0; i < length1; ++i) { - sourceArray1[i] = byte((i % 0xF) + 1); // [1..f] - fullSourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Second source array - bytes memory sourceArray2 = new bytes(length2); - for(uint256 j = 0; i < length2; ++i) { - sourceArray2[j] = byte((j % 0xF) + 1); // [1..f] - fullSourceArray[length1+j] = byte((j % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source arrays - bytes memory destArray = new bytes(length1 + length2); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray1) + 32, // skip copying array length - length1 - ); - memcpy( - getMemAddress(destArray) + 32 + length1, // skip copying array length + sourceArray1 bytes - getMemAddress(sourceArray2) + 32, // skip copying array length - length2 - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(fullSourceArray, destArray), - "Test #6 failed. Array contents are not the same." - ); - } - - function test7() - external - { - // Length of array & length to copy - uint256 length = 72; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - 8 // Copy all but last byte. - ); - - // Verify contents of source & dest arrays match - // We expect this to fail - require( - areBytesEqual(sourceArray, destArray), - "Test #7 failed. Array contents are not the same. This is expected." - ); - } } -- cgit v1.2.3 From 069b89b2084a65e6846a0722a9883af1104feb08 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Mon, 28 May 2018 15:24:09 +0200 Subject: Implement memcpy using masking and end-aligned words --- .../src/contracts/current/utils/LibMem/LibMem.sol | 144 ++++++++++++--------- 1 file changed, 85 insertions(+), 59 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index 215a661e2..f7ff4ca59 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -19,7 +19,7 @@ pragma solidity ^0.4.24; contract LibMem { - + function getMemAddress(bytes memory input) internal pure @@ -30,9 +30,11 @@ contract LibMem { } return address_; } - - /// @dev Writes a uint256 into a specific position in a byte array. - /// @param dest memory adress to copy bytes to + + /// @dev Copies `length` bytes from memory location `source` to `dest`. + /// @param dest memory address to copy bytes to + /// @param source memory address to copy bytes from + /// @param length number of bytes to copy function memcpy( uint256 dest, uint256 source, @@ -41,63 +43,87 @@ contract LibMem { internal pure { - // Base cases - if(length == 0) return; - if(source == dest) return; - - // Copy bytes from source to dest - assembly { - // Compute number of complete words to copy + remaining bytes - let lenFullWords := div(add(length, 0x1F), 0x20) - let remainder := mod(length, 0x20) - if gt(remainder, 0) { - lenFullWords := sub(lenFullWords, 1) + if (length < 32) { + // Handle a partial word by reading destination and masking + // off the bits we are interested in. + // This correctly handles overlap, zero lengths and source == dest + assembly { + let mask := sub(exp(256, sub(32, length)), 1) + let s := and(mload(source), not(mask)) + let d := and(mload(dest), mask) + mstore(dest, or(s, d)) } - - // Copy full words from source to dest - let offset := 0 - let maxOffset := mul(0x20, lenFullWords) - for {offset := 0} lt(offset, maxOffset) {offset := add(offset, 0x20)} { - mstore(add(dest, offset), mload(add(source, offset))) + } else { + // Skip the O(length) loop when source == dest. + if (source == dest) { + return; } - - // Copy remaining bytes - if gt(remainder, 0) { - // Read a full word from source, containing X bytes to copy to dest. - // We only want to keep the X bytes, zeroing out the remaining bytes. - // We accomplish this by a right shift followed by a left shift. - // Example: - // Suppose a word of 8 bits has all 1's: [11111111] - // Let X = 7 (we want to copy the first 7 bits) - // Apply a right shift of 1: [01111111] - // Apply a left shift of 1: [11111110] - let sourceShiftFactor := exp(2, mul(8, sub(0x20, remainder))) - let sourceWord := mload(add(source, offset)) - let sourceBytes := mul(div(sourceWord, sourceShiftFactor), sourceShiftFactor) - - // Read a full word from dest, containing (32-X) bytes to retain. - // We need to zero out the remaining bytes to be overwritten by source, - // while retaining the (32-X) bytes we don't want to overwrite. - // We accomplish this by a left shift followed by a right shift. - // Example: - // Suppose a word of 8 bits has all 1's: [11111111] - // Let X = 7 (we want to free the first 7 bits, and retain the last bit) - // Apply a left shift of 7: [10000000] - // Apply a right shift of 7: [00000001] - let destShiftFactor := exp(2, mul(8, remainder)) - let destWord := mload(add(dest, offset)) - let destBytes := div(mul(destWord, destShiftFactor), destShiftFactor) - - // Combine the source and dest bytes. There should be no overlap: - // The source bytes run from [0..X-1] and the dest bytes from [X..31]. - // Example: - // Following the example from above, we have [11111110] - // from the source word and [00000001] from the dest word. - // Combine these words using to get [11111111]. - let combinedDestWord := or(sourceBytes, destBytes) - - // Store the combined word into dest - mstore(add(dest, offset), combinedDestWord) + + // For large copies we copy whole words at a time. The final + // word is aligned to the end of the range (instead of after the + // previous) to handle partial words. So a copy will look like this: + // + // #### + // #### + // #### + // #### + // + // We handle overlap in the source and destination range by + // changing the copying direction. This prevents us from + // overwriting parts of source that we still need to copy. + // + // This correctly handles source == dest + // + if (source > dest) { + assembly { + // We subtract 32 from `send` and `dend` because it + // is easier to compare with in the loop, and these + // are also the addresses we need for copying the + // last bytes. + length := sub(length, 32) + let send := add(source, length) + let dend := add(dest, length) + + // Remember the last 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the last bytes in + // source already due to overlap. + let last := mload(send) + + // Copy whole words front to back + for {} lt(source, send) {} { + mstore(dest, mload(source)) + source := add(source, 32) + dest := add(dest, 32) + } + + // Write the last 32 bytes + mstore(dend, last) + } + } else { + assembly { + // We subtract 32 from `send` and `dend` because those + // are the starting points when copying a word at the end. + length := sub(length, 32) + let send := add(source, length) + let dend := add(dest, length) + + // Remember the first 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the first bytes in + // source already due to overlap. + let first := mload(source) + + // Copy whole words back to front + for {} lt(source, send) {} { + mstore(dend, mload(send)) + send := sub(send, 32) + dend := sub(dend, 32) + } + + // Write the first 32 bytes + mstore(dest, first) + } } } } -- cgit v1.2.3 From 5db15ca54cd7e1fd90bf318d2975750dcd31cddc Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 13:44:14 -0700 Subject: proxyData -> assetData --- .../LibAssetProxyDecoder/LibAssetProxyDecoder.sol | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol index ba53f2769..ec27502a8 100644 --- a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol +++ b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol @@ -29,7 +29,7 @@ contract LibAssetProxyDecoder is string constant INVALID_ERC721_METADATA_LENGTH = "Metadata must have a length of at least 53."; /// @dev Decodes ERC721 Asset Proxy data - function decodeERC20Data(bytes memory proxyData) + function decodeERC20Data(bytes memory assetData) internal pure returns ( @@ -38,17 +38,17 @@ contract LibAssetProxyDecoder is ) { require( - proxyData.length == 21, + assetData.length == 21, INVALID_ERC20_METADATA_LENGTH ); - proxyId = uint8(proxyData[0]); - token = readAddress(proxyData, 1); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); return (proxyId, token); } /// @dev Decodes ERC721 Asset Proxy data - function decodeERC721Data(bytes memory proxyData) + function decodeERC721Data(bytes memory assetData) internal pure returns ( @@ -59,14 +59,14 @@ contract LibAssetProxyDecoder is ) { require( - proxyData.length >= 53, + assetData.length >= 53, INVALID_ERC721_METADATA_LENGTH ); - proxyId = uint8(proxyData[0]); - token = readAddress(proxyData, 1); - tokenId = readUint256(proxyData, 21); - if (proxyData.length > 53) { - data = readBytes(proxyData, 53); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); + tokenId = readUint256(assetData, 21); + if (assetData.length > 53) { + data = readBytes(assetData, 53); } return (proxyId, token, tokenId, data); -- cgit v1.2.3 From e042e0ad32cd2ac9e707cb8e52961957f58314ce Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 14:06:47 -0700 Subject: Converged on naming scheme for asset data: renamed all instances of assetMetadata, proxyData, proxyMetadata to assetData --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 6 ++-- .../current/protocol/AssetProxy/ERC721Proxy.sol | 6 ++-- .../protocol/AssetProxy/MixinAssetProxy.sol | 14 +++++----- .../protocol/AssetProxy/interfaces/IAssetProxy.sol | 8 +++--- .../protocol/AssetProxy/mixins/MAssetProxy.sol | 4 +-- .../current/protocol/Exchange/Exchange.sol | 4 +-- .../Exchange/MixinAssetProxyDispatcher.sol | 10 +++---- .../current/protocol/Exchange/MixinSettlement.sol | 10 +++---- .../protocol/Exchange/MixinWrapperFunctions.sol | 32 +++++++++++----------- .../Exchange/mixins/MAssetProxyDispatcher.sol | 4 +-- .../TestAssetProxyDispatcher.sol | 4 +-- .../TestLibAssetProxyDecoder.sol | 8 +++--- packages/contracts/src/utils/match_order_tester.ts | 24 ++++++++-------- 13 files changed, 67 insertions(+), 67 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 017f94b1a..5b4367fd9 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -36,12 +36,12 @@ contract ERC20Proxy is uint8 constant PROXY_ID = 1; /// @dev Internal version of `transferFrom`. - /// @param proxyData Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory proxyData, + bytes memory assetData, address from, address to, uint256 amount @@ -52,7 +52,7 @@ contract ERC20Proxy is ( uint8 proxyId, address token - ) = decodeERC20Data(proxyData); + ) = decodeERC20Data(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index f35e48eee..e2c445463 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -38,12 +38,12 @@ contract ERC721Proxy is string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; /// @dev Internal version of `transferFrom`. - /// @param proxyData Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory proxyData, + bytes memory assetData, address from, address to, uint256 amount @@ -56,7 +56,7 @@ contract ERC721Proxy is address token, uint256 tokenId, bytes memory data - ) = decodeERC721Data(proxyData); + ) = decodeERC721Data(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol index 5fa33cbef..9032658e7 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol @@ -28,12 +28,12 @@ contract MixinAssetProxy is { /// @dev Transfers assets. Either succeeds or throws. - /// @param assetMetadata Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFrom( - bytes assetMetadata, + bytes assetData, address from, address to, uint256 amount @@ -42,7 +42,7 @@ contract MixinAssetProxy is onlyAuthorized { transferFromInternal( - assetMetadata, + assetData, from, to, amount @@ -50,12 +50,12 @@ contract MixinAssetProxy is } /// @dev Makes multiple transfers of assets. Either succeeds or throws. - /// @param assetMetadata Array of byte arrays encoded for the respective asset proxy. + /// @param assetData Array of byte arrays encoded for the respective asset proxy. /// @param from Array of addresses to transfer assets from. /// @param to Array of addresses to transfer assets to. /// @param amounts Array of amounts of assets to transfer. function batchTransferFrom( - bytes[] memory assetMetadata, + bytes[] memory assetData, address[] memory from, address[] memory to, uint256[] memory amounts @@ -63,9 +63,9 @@ contract MixinAssetProxy is public onlyAuthorized { - for (uint256 i = 0; i < assetMetadata.length; i++) { + for (uint256 i = 0; i < assetData.length; i++) { transferFromInternal( - assetMetadata[i], + assetData[i], from[i], to[i], amounts[i] diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol index 7e1848889..22f43b12f 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol @@ -26,12 +26,12 @@ contract IAssetProxy is { /// @dev Transfers assets. Either succeeds or throws. - /// @param assetMetadata Byte array encoded for the respective asset proxy. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFrom( - bytes assetMetadata, + bytes assetData, address from, address to, uint256 amount @@ -39,12 +39,12 @@ contract IAssetProxy is external; /// @dev Makes multiple transfers of assets. Either succeeds or throws. - /// @param assetMetadata Array of byte arrays encoded for the respective asset proxy. + /// @param assetData Array of byte arrays encoded for the respective asset proxy. /// @param from Array of addresses to transfer assets from. /// @param to Array of addresses to transfer assets to. /// @param amounts Array of amounts of assets to transfer. function batchTransferFrom( - bytes[] memory assetMetadata, + bytes[] memory assetData, address[] memory from, address[] memory to, uint256[] memory amounts diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol index de9d65a53..a52cb56f9 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol @@ -26,12 +26,12 @@ contract MAssetProxy is { /// @dev Internal version of `transferFrom`. - /// @param assetMetadata Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index b7b308069..51f99bafa 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -40,11 +40,11 @@ contract Exchange is string constant public VERSION = "2.0.1-alpha"; // Mixins are instantiated in the order they are inherited - constructor (bytes memory _zrxProxyData) + constructor (bytes memory _zrxAssetData) public MixinExchangeCore() MixinMatchOrders() - MixinSettlement(_zrxProxyData) + MixinSettlement(_zrxAssetData) MixinSignatureValidator() MixinTransactions() MixinAssetProxyDispatcher() diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index 8f9342739..e77d81c06 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -80,12 +80,12 @@ contract MixinAssetProxyDispatcher is } /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. - /// @param assetMetadata Byte array encoded for the respective asset proxy. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount @@ -96,16 +96,16 @@ contract MixinAssetProxyDispatcher is if (amount > 0) { // Lookup asset proxy - uint256 length = assetMetadata.length; + uint256 length = assetData.length; require( length > 0, LENGTH_GREATER_THAN_0_REQUIRED ); - uint8 assetProxyId = uint8(assetMetadata[length - 1]); + uint8 assetProxyId = uint8(assetData[length - 1]); IAssetProxy assetProxy = assetProxies[assetProxyId]; // transferFrom will either succeed or throw. - assetProxy.transferFrom(assetMetadata, from, to, amount); + assetProxy.transferFrom(assetData, from, to, amount); } } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 646d3ed58..83e9dfdf4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -40,7 +40,7 @@ contract MixinSettlement is bytes internal ZRX_PROXY_DATA; /// @dev Gets the ZRX metadata used for fee transfers. - function zrxProxyData() + function zrxAssetData() external view returns (bytes memory) @@ -48,13 +48,13 @@ contract MixinSettlement is return ZRX_PROXY_DATA; } - /// TODO: _zrxProxyData should be a constant in production. + /// TODO: _zrxAssetData should be a constant in production. /// @dev Constructor sets the metadata that will be used for paying ZRX fees. - /// @param _zrxProxyData Byte array containing ERC20 proxy id concatenated with address of ZRX. - constructor (bytes memory _zrxProxyData) + /// @param _zrxAssetData Byte array containing ERC20 proxy id concatenated with address of ZRX. + constructor (bytes memory _zrxAssetData) public { - ZRX_PROXY_DATA = _zrxProxyData; + ZRX_PROXY_DATA = _zrxAssetData; } /// @dev Settles an order by transferring assets between counterparties. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 0ad0710ce..0d9888703 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -91,12 +91,12 @@ contract MixinWrapperFunctions is // | | 0x0E0 | | 8. takerFeeAmount | // | | 0x100 | | 9. expirationTimeSeconds | // | | 0x120 | | 10. salt | - // | | 0x140 | | 11. Offset to makerAssetProxyMetadata (*) | - // | | 0x160 | | 12. Offset to takerAssetProxyMetadata (*) | - // | | 0x180 | 32 | makerAssetProxyMetadata Length | - // | | 0x1A0 | ** | makerAssetProxyMetadata Contents | - // | | 0x1C0 | 32 | takerAssetProxyMetadata Length | - // | | 0x1E0 | ** | takerAssetProxyMetadata Contents | + // | | 0x140 | | 11. Offset to makerAssetData (*) | + // | | 0x160 | | 12. Offset to takerAssetData (*) | + // | | 0x180 | 32 | makerAssetData Length | + // | | 0x1A0 | ** | makerAssetData Contents | + // | | 0x1C0 | 32 | takerAssetData Length | + // | | 0x1E0 | ** | takerAssetData Contents | // | | 0x200 | 32 | signature Length | // | | 0x220 | ** | signature Contents | @@ -163,43 +163,43 @@ contract MixinWrapperFunctions is mstore(add(dataAreaEnd, 0xE0), mload(add(sourceOffset, 0xE0))) // takerFeeAmount mstore(add(dataAreaEnd, 0x100), mload(add(sourceOffset, 0x100))) // expirationTimeSeconds mstore(add(dataAreaEnd, 0x120), mload(add(sourceOffset, 0x120))) // salt - mstore(add(dataAreaEnd, 0x140), mload(add(sourceOffset, 0x140))) // Offset to makerAssetProxyMetadata - mstore(add(dataAreaEnd, 0x160), mload(add(sourceOffset, 0x160))) // Offset to takerAssetProxyMetadata + mstore(add(dataAreaEnd, 0x140), mload(add(sourceOffset, 0x140))) // Offset to makerAssetData + mstore(add(dataAreaEnd, 0x160), mload(add(sourceOffset, 0x160))) // Offset to takerAssetData dataAreaEnd := add(dataAreaEnd, 0x180) sourceOffset := add(sourceOffset, 0x180) - // Write offset to + // Write offset to mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) - // Calculate length of + // Calculate length of arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - // Write length of + // Write length of mstore(dataAreaEnd, arrayLenBytes) dataAreaEnd := add(dataAreaEnd, 0x20) - // Write contents of + // Write contents of for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { mstore(dataAreaEnd, mload(sourceOffset)) dataAreaEnd := add(dataAreaEnd, 0x20) sourceOffset := add(sourceOffset, 0x20) } - // Write offset to + // Write offset to mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) - // Calculate length of + // Calculate length of arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - // Write length of + // Write length of mstore(dataAreaEnd, arrayLenBytes) dataAreaEnd := add(dataAreaEnd, 0x20) - // Write contents of + // Write contents of for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { mstore(dataAreaEnd, mload(sourceOffset)) dataAreaEnd := add(dataAreaEnd, 0x20) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index 87c5f6361..82eafb529 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -33,12 +33,12 @@ contract MAssetProxyDispatcher is ); /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. - /// @param assetMetadata Byte array encoded for the respective asset proxy. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol index 11ca0617d..2ae69e0ef 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -23,12 +23,12 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher { function publicDispatchTransferFrom( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount) public { - dispatchTransferFrom(assetMetadata, from, to, amount); + dispatchTransferFrom(assetData, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol index e4a7de71d..6d2866656 100644 --- a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol +++ b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol @@ -26,16 +26,16 @@ contract TestLibAssetProxyDecoder is { /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC20Data(bytes memory proxyData) + function publicDecodeERC20Data(bytes memory assetData) public pure returns (uint8, address) { - return decodeERC20Data(proxyData); + return decodeERC20Data(assetData); } /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC721Data(bytes memory proxyData) + function publicDecodeERC721Data(bytes memory assetData) public pure returns ( @@ -45,6 +45,6 @@ contract TestLibAssetProxyDecoder is bytes memory ) { - return decodeERC721Data(proxyData); + return decodeERC721Data(assetData); } } diff --git a/packages/contracts/src/utils/match_order_tester.ts b/packages/contracts/src/utils/match_order_tester.ts index f4f7f965b..fbb1b99db 100644 --- a/packages/contracts/src/utils/match_order_tester.ts +++ b/packages/contracts/src/utils/match_order_tester.ts @@ -237,11 +237,11 @@ export class MatchOrderTester { const expectedNewERC20BalancesByOwner = _.cloneDeep(erc20BalancesByOwner); const expectedNewERC721TokenIdsByOwner = _.cloneDeep(erc721TokenIdsByOwner); // Left Maker Asset (Right Taker Asset) - const makerAssetProxyIdLeft = assetProxyUtils.decodeProxyDataId(signedOrderLeft.makerAssetData); + const makerAssetProxyIdLeft = assetProxyUtils.decodeAssetDataId(signedOrderLeft.makerAssetData); if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { // Decode asset data - const erc20ProxyData = assetProxyUtils.decodeERC20ProxyData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = erc20ProxyData.tokenAddress; + const erc20AssetData = assetProxyUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = erc20AssetData.tokenAddress; const takerAssetAddressRight = makerAssetAddressLeft; // Left Maker expectedNewERC20BalancesByOwner[makerAddressLeft][makerAssetAddressLeft] = expectedNewERC20BalancesByOwner[ @@ -259,9 +259,9 @@ export class MatchOrderTester { ][makerAssetAddressLeft].add(expectedTransferAmounts.amountReceivedByTaker); } else if (makerAssetProxyIdLeft === AssetProxyId.ERC721) { // Decode asset data - const erc721ProxyData = assetProxyUtils.decodeERC721ProxyData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = erc721ProxyData.tokenAddress; - const makerAssetIdLeft = erc721ProxyData.tokenId; + const erc721AssetData = assetProxyUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = erc721AssetData.tokenAddress; + const makerAssetIdLeft = erc721AssetData.tokenId; const takerAssetAddressRight = makerAssetAddressLeft; const takerAssetIdRight = makerAssetIdLeft; // Left Maker @@ -272,11 +272,11 @@ export class MatchOrderTester { } // Left Taker Asset (Right Maker Asset) // Note: This exchange is only between the order makers: the Taker does not receive any of the left taker asset. - const takerAssetProxyIdLeft = assetProxyUtils.decodeProxyDataId(signedOrderLeft.takerAssetData); + const takerAssetProxyIdLeft = assetProxyUtils.decodeAssetDataId(signedOrderLeft.takerAssetData); if (takerAssetProxyIdLeft === AssetProxyId.ERC20) { // Decode asset data - const erc20ProxyData = assetProxyUtils.decodeERC20ProxyData(signedOrderLeft.takerAssetData); - const takerAssetAddressLeft = erc20ProxyData.tokenAddress; + const erc20AssetData = assetProxyUtils.decodeERC20AssetData(signedOrderLeft.takerAssetData); + const takerAssetAddressLeft = erc20AssetData.tokenAddress; const makerAssetAddressRight = takerAssetAddressLeft; // Left Maker expectedNewERC20BalancesByOwner[makerAddressLeft][takerAssetAddressLeft] = expectedNewERC20BalancesByOwner[ @@ -290,9 +290,9 @@ export class MatchOrderTester { ); } else if (takerAssetProxyIdLeft === AssetProxyId.ERC721) { // Decode asset data - const erc721ProxyData = assetProxyUtils.decodeERC721ProxyData(signedOrderRight.makerAssetData); - const makerAssetAddressRight = erc721ProxyData.tokenAddress; - const makerAssetIdRight = erc721ProxyData.tokenId; + const erc721AssetData = assetProxyUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = erc721AssetData.tokenAddress; + const makerAssetIdRight = erc721AssetData.tokenId; const takerAssetAddressLeft = makerAssetAddressRight; const takerAssetIdLeft = makerAssetIdRight; // Right Maker -- cgit v1.2.3 From 249a1e6d8d129d6b067a4ddadfb4b94733ddfc07 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 16:31:00 -0700 Subject: Removed the LibAssetProxyDecoder. Merged decode functions into the proxies. This way they can still be used by the forwarding contract. TestAssetDataDecoders inherits them in the same way the forwarding contract would --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 24 ++++++- .../current/protocol/AssetProxy/ERC721Proxy.sol | 29 ++++++++- .../TestAssetDataDecoders.sol | 52 +++++++++++++++ .../TestLibAssetProxyDecoder.sol | 50 --------------- .../LibAssetProxyDecoder/LibAssetProxyDecoder.sol | 74 ---------------------- packages/contracts/src/utils/artifacts.ts | 4 +- packages/contracts/src/utils/types.ts | 2 +- 7 files changed, 102 insertions(+), 133 deletions(-) create mode 100644 packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol delete mode 100644 packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol delete mode 100644 packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 5b4367fd9..11383adaf 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,14 +20,13 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; +import "../../tokens/ERC20Token/IERC20Token.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; contract ERC20Proxy is LibBytes, - LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -52,7 +51,7 @@ contract ERC20Proxy is ( uint8 proxyId, address token - ) = decodeERC20Data(assetData); + ) = decodeERC20AssetData(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; @@ -79,4 +78,23 @@ contract ERC20Proxy is { return PROXY_ID; } + + /// @dev Decodes ERC20 Asset Proxy data + function decodeERC20AssetData(bytes memory assetData) + internal + pure + returns ( + uint8 proxyId, + address token + ) + { + require( + assetData.length == 21, + INVALID_ASSET_DATA_LENGTH + ); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); + + return (proxyId, token); + } } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index e2c445463..eb23736a0 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -20,14 +20,12 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC721Token/ERC721Token.sol"; contract ERC721Proxy is LibBytes, - LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -56,7 +54,7 @@ contract ERC721Proxy is address token, uint256 tokenId, bytes memory data - ) = decodeERC721Data(assetData); + ) = decodeERC721AssetData(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; @@ -92,4 +90,29 @@ contract ERC721Proxy is { return PROXY_ID; } + + /// @dev Decodes ERC721 Asset Proxy data + function decodeERC721AssetData(bytes memory assetData) + internal + pure + returns ( + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory data + ) + { + require( + assetData.length >= 53, + INVALID_ASSET_DATA_LENGTH + ); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); + tokenId = readUint256(assetData, 21); + if (assetData.length > 53) { + data = readBytes(assetData, 53); + } + + return (proxyId, token, tokenId, data); + } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol new file mode 100644 index 000000000..45787d88b --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -0,0 +1,52 @@ +/* + + Copyright 2018 ZeroEx Intl. + + 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; +pragma experimental ABIEncoderV2; + +import "../../protocol/AssetProxy/ERC20Proxy.sol"; +import "../../protocol/AssetProxy/ERC721Proxy.sol"; + +contract TestAssetDataDecoders is + ERC20Proxy, + ERC721Proxy +{ + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC20Data(bytes memory assetData) + public + pure + returns (uint8, address) + { + return ERC20Proxy.decodeERC20AssetData(assetData); + } + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC721Data(bytes memory assetData) + public + pure + returns ( + uint8, + address, + uint256, + bytes memory + ) + { + return ERC721Proxy.decodeERC721AssetData(assetData); + } +} diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol deleted file mode 100644 index 6d2866656..000000000 --- a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol +++ /dev/null @@ -1,50 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - 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; -pragma experimental ABIEncoderV2; - -import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; - -contract TestLibAssetProxyDecoder is - LibAssetProxyDecoder -{ - - /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC20Data(bytes memory assetData) - public - pure - returns (uint8, address) - { - return decodeERC20Data(assetData); - } - - /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC721Data(bytes memory assetData) - public - pure - returns ( - uint8, - address, - uint256, - bytes memory - ) - { - return decodeERC721Data(assetData); - } -} diff --git a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol deleted file mode 100644 index ec27502a8..000000000 --- a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol +++ /dev/null @@ -1,74 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - 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; -pragma experimental ABIEncoderV2; - -import "../LibBytes/LibBytes.sol"; - -contract LibAssetProxyDecoder is - LibBytes -{ - - string constant INVALID_ERC20_METADATA_LENGTH = "Metadata must have a length of 21."; - string constant INVALID_ERC721_METADATA_LENGTH = "Metadata must have a length of at least 53."; - - /// @dev Decodes ERC721 Asset Proxy data - function decodeERC20Data(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token - ) - { - require( - assetData.length == 21, - INVALID_ERC20_METADATA_LENGTH - ); - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - - return (proxyId, token); - } - - /// @dev Decodes ERC721 Asset Proxy data - function decodeERC721Data(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token, - uint256 tokenId, - bytes memory data - ) - { - require( - assetData.length >= 53, - INVALID_ERC721_METADATA_LENGTH - ); - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - tokenId = readUint256(assetData, 21); - if (assetData.length > 53) { - data = readBytes(assetData, 53); - } - - return (proxyId, token, tokenId, data); - } -} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index a1c8483d8..42de7c921 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -11,7 +11,7 @@ import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; -import * as TestLibAssetProxyDecoder from '../artifacts/TestLibAssetProxyDecoder.json'; +import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; @@ -34,7 +34,7 @@ export const artifacts = { MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, - TestLibAssetProxyDecoder: (TestLibAssetProxyDecoder as any) as ContractArtifact, + TestAssetDataDecoders: (TestAssetDataDecoders as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index cccca5705..bb8c12088 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -90,7 +90,7 @@ export enum ContractName { AccountLevels = 'AccountLevels', EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', - TestLibAssetProxyDecoder = 'TestLibAssetProxyDecoder', + TestAssetDataDecoders = 'TestAssetDataDecoders', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', -- cgit v1.2.3 From 05f1e9e3b8f5da593d949a1a18abba70568adb9c Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 16:49:10 -0700 Subject: Resolved edge case in Memcpy where where send would eventually turn "negative" and wrap around. --- .../src/contracts/current/utils/LibMem/LibMem.sol | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index f7ff4ca59..500044500 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -19,7 +19,7 @@ pragma solidity ^0.4.24; contract LibMem { - + function getMemAddress(bytes memory input) internal pure @@ -30,7 +30,7 @@ contract LibMem { } return address_; } - + /// @dev Copies `length` bytes from memory location `source` to `dest`. /// @param dest memory address to copy bytes to /// @param source memory address to copy bytes from @@ -58,7 +58,7 @@ contract LibMem { if (source == dest) { return; } - + // For large copies we copy whole words at a time. The final // word is aligned to the end of the range (instead of after the // previous) to handle partial words. So a copy will look like this: @@ -76,6 +76,9 @@ contract LibMem { // if (source > dest) { assembly { + // Record the total number of full words to copy + let nwords := div(length, 32) + // We subtract 32 from `send` and `dend` because it // is easier to compare with in the loop, and these // are also the addresses we need for copying the @@ -83,44 +86,47 @@ contract LibMem { length := sub(length, 32) let send := add(source, length) let dend := add(dest, length) - + // Remember the last 32 bytes of source // This needs to be done here and not after the loop // because we may have overwritten the last bytes in // source already due to overlap. let last := mload(send) - + // Copy whole words front to back - for {} lt(source, send) {} { + for {let i := 0} lt(i, nwords) {i := add(i, 1)} { mstore(dest, mload(source)) source := add(source, 32) dest := add(dest, 32) } - + // Write the last 32 bytes mstore(dend, last) } } else { assembly { + // Record the total number of full words to copy + let nwords := div(length, 32) + // We subtract 32 from `send` and `dend` because those // are the starting points when copying a word at the end. length := sub(length, 32) let send := add(source, length) let dend := add(dest, length) - + // Remember the first 32 bytes of source // This needs to be done here and not after the loop // because we may have overwritten the first bytes in // source already due to overlap. let first := mload(source) - + // Copy whole words back to front - for {} lt(source, send) {} { + for {let i := 0} lt(i, nwords) {i := add(i, 1)} { mstore(dend, mload(send)) send := sub(send, 32) dend := sub(dend, 32) } - + // Write the first 32 bytes mstore(dest, first) } -- cgit v1.2.3 From 3c3851c221873baf3b7fec7213324dae0c1c3351 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 17:49:12 -0700 Subject: Fixed formatting in memory layout --- .../current/protocol/Exchange/MixinWrapperFunctions.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 0d9888703..f4822e814 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -91,12 +91,12 @@ contract MixinWrapperFunctions is // | | 0x0E0 | | 8. takerFeeAmount | // | | 0x100 | | 9. expirationTimeSeconds | // | | 0x120 | | 10. salt | - // | | 0x140 | | 11. Offset to makerAssetData (*) | - // | | 0x160 | | 12. Offset to takerAssetData (*) | - // | | 0x180 | 32 | makerAssetData Length | - // | | 0x1A0 | ** | makerAssetData Contents | - // | | 0x1C0 | 32 | takerAssetData Length | - // | | 0x1E0 | ** | takerAssetData Contents | + // | | 0x140 | | 11. Offset to makerAssetData (*) | + // | | 0x160 | | 12. Offset to takerAssetData (*) | + // | | 0x180 | 32 | makerAssetData Length | + // | | 0x1A0 | ** | makerAssetData Contents | + // | | 0x1C0 | 32 | takerAssetData Length | + // | | 0x1E0 | ** | takerAssetData Contents | // | | 0x200 | 32 | signature Length | // | | 0x220 | ** | signature Contents | -- cgit v1.2.3 From 8496c1cdd3bc477fcfe584adf8200f4ed35da2b0 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 18:59:02 -0700 Subject: Call safeTransferFrom only when there is receiver data present --- .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index eb23736a0..9dac02d87 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -77,8 +77,13 @@ contract ERC721Proxy is ); // Transfer token. + // Save gas by calling safeTransferFrom only when there is data present. // Either succeeds or throws. - ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + if(data.length > 0) { + ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + } else { + ERC721Token(token).transferFrom(from, to, tokenId); + } } /// @dev Gets the proxy id associated with the proxy address. -- cgit v1.2.3 From f03e5c6bd12c88fffbad324fd7493d3acedea0aa Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 11:34:45 -0700 Subject: Style audit proxies --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 8 +++- .../current/protocol/AssetProxy/ERC721Proxy.sol | 33 ++++++++++++----- .../TestAssetDataDecoders.sol | 43 +++++++++++++++++----- 3 files changed, 64 insertions(+), 20 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 11383adaf..54cbeb963 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -79,7 +79,10 @@ contract ERC20Proxy is return PROXY_ID; } - /// @dev Decodes ERC20 Asset Proxy data + /// @dev Decodes ERC20 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC20 proxy id. + /// @return token ERC20 token address. function decodeERC20AssetData(bytes memory assetData) internal pure @@ -88,10 +91,13 @@ contract ERC20Proxy is address token ) { + // Validate encoded data length require( assetData.length == 21, INVALID_ASSET_DATA_LENGTH ); + + // Decode data proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 9dac02d87..58c23b03b 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -53,7 +53,7 @@ contract ERC721Proxy is uint8 proxyId, address token, uint256 tokenId, - bytes memory data + bytes memory receiverData ) = decodeERC721AssetData(assetData); // Data must be intended for this proxy. @@ -76,11 +76,10 @@ contract ERC721Proxy is INVALID_AMOUNT ); - // Transfer token. - // Save gas by calling safeTransferFrom only when there is data present. - // Either succeeds or throws. - if(data.length > 0) { - ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + // Transfer token. Saves gas by calling safeTransferFrom only + // when there is receiverData present. Either succeeds or throws. + if(receiverData.length > 0) { + ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } else { ERC721Token(token).transferFrom(from, to, tokenId); } @@ -96,7 +95,13 @@ contract ERC721Proxy is return PROXY_ID; } - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC721 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC721 proxy id. + /// @return token ERC721 token address. + /// @return tokenId ERC721 token id. + /// @return receiverData Additional data with no specific format, which + /// is passed to the receiving contract's onERC721Received. function decodeERC721AssetData(bytes memory assetData) internal pure @@ -104,20 +109,28 @@ contract ERC721Proxy is uint8 proxyId, address token, uint256 tokenId, - bytes memory data + bytes memory receiverData ) { + // Validate encoded data length require( assetData.length >= 53, INVALID_ASSET_DATA_LENGTH ); + + // Decode asset data proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); tokenId = readUint256(assetData, 21); if (assetData.length > 53) { - data = readBytes(assetData, 53); + receiverData = readBytes(assetData, 53); } - return (proxyId, token, tokenId, data); + return ( + proxyId, + token, + tokenId, + receiverData + ); } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 45787d88b..2c6a8fdb0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -27,26 +27,51 @@ contract TestAssetDataDecoders is ERC721Proxy { - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC20 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC20 proxy id. + /// @return token ERC20 token address. function publicDecodeERC20Data(bytes memory assetData) public pure - returns (uint8, address) + returns ( + uint8 proxyId, + address token + ) { - return ERC20Proxy.decodeERC20AssetData(assetData); + (proxyId, token) = decodeERC20AssetData(assetData); + return (proxyId, token); } - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC721 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC721 proxy id. + /// @return token ERC721 token address. + /// @return tokenId ERC721 token id. + /// @return receiverData Additional data with no specific format, which + /// is passed to the receiving contract's onERC721Received. function publicDecodeERC721Data(bytes memory assetData) public pure returns ( - uint8, - address, - uint256, - bytes memory + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory receiverData ) { - return ERC721Proxy.decodeERC721AssetData(assetData); + ( + proxyId, + token, + tokenId, + receiverData + ) = decodeERC721AssetData(assetData); + + return ( + proxyId, + token, + tokenId, + receiverData + ); } } -- cgit v1.2.3 From 3ed13150e106c19563c8e9b06621be3d44d66b6c Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 11:54:20 -0700 Subject: Style audit for proxies + libmem + libbytes --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 2 +- .../current/protocol/AssetProxy/ERC721Proxy.sol | 4 ++-- .../current/test/TestLibBytes/TestLibBytes.sol | 11 +++++++++-- .../contracts/current/test/TestLibMem/TestLibMem.sol | 15 +++++++++++---- .../src/contracts/current/utils/LibBytes/LibBytes.sol | 13 ++++++------- .../src/contracts/current/utils/LibMem/LibMem.sol | 18 +++++++++++------- 6 files changed, 40 insertions(+), 23 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 54cbeb963..96950f1cd 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -47,7 +47,7 @@ contract ERC20Proxy is ) internal { - // Decode proxy data. + // Decode asset data. ( uint8 proxyId, address token diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 58c23b03b..102064c15 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -48,7 +48,7 @@ contract ERC721Proxy is ) internal { - // Decode proxy data. + // Decode asset data. ( uint8 proxyId, address token, @@ -118,7 +118,7 @@ contract ERC721Proxy is INVALID_ASSET_DATA_LENGTH ); - // Decode asset data + // Decode asset data. proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); tokenId = readUint256(assetData, 21); diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 0bf11b03b..f009a6a71 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -155,7 +155,6 @@ contract TestLibBytes is return b; } -======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -168,6 +167,10 @@ contract TestLibBytes is return result; } + /// @dev Reads nested bytes from a specific position. + /// @param b Byte array containing nested bytes. + /// @param index Index of nested bytes. + /// @return result Nested bytes. function publicReadBytes( bytes memory b, uint256 index) @@ -179,7 +182,11 @@ contract TestLibBytes is return result; } - + /// @dev Inserts bytes at a specific position in a byte array. + /// @param b Byte array to insert into. + /// @param index Index in byte array of . + /// @param input bytes to insert. + /// @return b Updated input byte array function publicWriteBytes( bytes memory b, uint256 index, diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 64bc182f4..076bee24c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -23,8 +23,15 @@ import "../../utils/LibMem/LibMem.sol"; contract TestLibMem is LibMem { + + /// @dev Copies a block of memory from one location to another. + /// @param mem Memory contents we want to apply memcpy to + /// @param dest Destination offset into . + /// @param source Source offset into . + /// @param length Length of bytes to copy from to + /// @return mem Memory contents after calling memcpy. function testMemcpy( - bytes mem, ///< Memory contents we want to apply memcpy to + bytes mem, uint256 dest, uint256 source, uint256 length @@ -36,13 +43,13 @@ contract TestLibMem is // Sanity check. Overflows are not checked. require(source + length <= mem.length); require(dest + length <= mem.length); - + // Get pointer to memory contents uint256 offset = getMemAddress(mem) + 32; - + // Execute memcpy adjusted for memory array location memcpy(offset + dest, offset + source, length); - + // Return modified memory contents return mem; } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 6351f1a46..5610c47b3 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -268,7 +268,6 @@ contract LibBytes is writeBytes32(b, index, bytes32(input)); } -======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -287,10 +286,10 @@ contract LibBytes is return result; } - /// @dev Reads a uint256 value from a position in a byte array. - /// @param b Byte array containing a uint256 value. - /// @param index Index in byte array of uint256 value. - /// @return uint256 value from byte array. + /// @dev Reads nested bytes from a specific position. + /// @param b Byte array containing nested bytes. + /// @param index Index of nested bytes. + /// @return result Nested bytes. function readBytes( bytes memory b, uint256 index @@ -321,10 +320,10 @@ contract LibBytes is return result; } - /// @dev Writes a uint256 into a specific position in a byte array. + /// @dev Inserts bytes at a specific position in a byte array. /// @param b Byte array to insert into. /// @param index Index in byte array of . - /// @param input uint256 to put into byte array. + /// @param input bytes to insert. function writeBytes( bytes memory b, uint256 index, diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index 500044500..960850725 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -18,23 +18,27 @@ pragma solidity ^0.4.24; -contract LibMem { +contract LibMem +{ + /// @dev Gets the memory address for a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of byte array. function getMemAddress(bytes memory input) internal pure - returns (uint256 address_) + returns (uint256 memoryAddress) { assembly { - address_ := input + memoryAddress := input } - return address_; + return memoryAddress; } /// @dev Copies `length` bytes from memory location `source` to `dest`. - /// @param dest memory address to copy bytes to - /// @param source memory address to copy bytes from - /// @param length number of bytes to copy + /// @param dest memory address to copy bytes to. + /// @param source memory address to copy bytes from. + /// @param length number of bytes to copy. function memcpy( uint256 dest, uint256 source, -- cgit v1.2.3 From e4e36760952287a84f8991df8589c183036383db Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 14:17:13 -0700 Subject: Fixed up after rebasing. Contracts build and tests pass --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 7 +++---- .../current/protocol/AssetProxy/ERC721Proxy.sol | 18 ++++++------------ 2 files changed, 9 insertions(+), 16 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 96950f1cd..4e9ae64f8 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -54,8 +54,6 @@ contract ERC20Proxy is ) = decodeERC20AssetData(assetData); // Data must be intended for this proxy. - uint256 length = assetMetadata.length; - require( proxyId == PROXY_ID, PROXY_ID_MISMATCH @@ -92,14 +90,15 @@ contract ERC20Proxy is ) { // Validate encoded data length + uint256 length = assetData.length; require( assetData.length == 21, INVALID_ASSET_DATA_LENGTH ); // Decode data - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); + token = readAddress(assetData, 0); + proxyId = uint8(assetData[length-1]); return (proxyId, token); } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 102064c15..f6c3af104 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -56,15 +56,8 @@ contract ERC721Proxy is bytes memory receiverData ) = decodeERC721AssetData(assetData); - // Data must be intended for this proxy. - uint256 length = assetMetadata.length; - require( - length == 53, - LENGTH_53_REQUIRED - ); - - // TODO: Is this too inflexible in the future? + // Data must be intended for this proxy. require( proxyId == PROXY_ID, PROXY_ID_MISMATCH @@ -113,18 +106,19 @@ contract ERC721Proxy is ) { // Validate encoded data length + uint256 length = assetData.length; require( assetData.length >= 53, INVALID_ASSET_DATA_LENGTH ); // Decode asset data. - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - tokenId = readUint256(assetData, 21); + token = readAddress(assetData, 0); + tokenId = readUint256(assetData, 20); if (assetData.length > 53) { - receiverData = readBytes(assetData, 53); + receiverData = readBytes(assetData, 52); } + proxyId = uint8(assetData[length-1]); return ( proxyId, -- cgit v1.2.3 From a1b49d8389844c9b2d62ded91b76a23deb060ab6 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 4 Jun 2018 19:40:01 -0700 Subject: Fixed after rebase --- .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 4 ++-- .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 2 +- .../current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 4e9ae64f8..c8e8f4587 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -56,7 +56,7 @@ contract ERC20Proxy is // Data must be intended for this proxy. require( proxyId == PROXY_ID, - PROXY_ID_MISMATCH + ASSET_PROXY_ID_MISMATCH ); // Transfer tokens. @@ -93,7 +93,7 @@ contract ERC20Proxy is uint256 length = assetData.length; require( assetData.length == 21, - INVALID_ASSET_DATA_LENGTH + LENGTH_21_REQUIRED ); // Decode data diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index f6c3af104..7de7968cc 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -109,7 +109,7 @@ contract ERC721Proxy is uint256 length = assetData.length; require( assetData.length >= 53, - INVALID_ASSET_DATA_LENGTH + LENGTH_AT_LEAST_53_REQUIRED ); // Decode asset data. diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol index e0c7fc796..80180a0d9 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -29,9 +29,9 @@ contract LibAssetProxyErrors { /// AssetProxy errors /// string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // Proxy id in metadata does not match this proxy id. string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. - string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. + string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. /// Length validation errors /// string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21. - string constant LENGTH_53_REQUIRED = "LENGTH_53_REQUIRED"; // Byte array must have a length of 53. + string constant LENGTH_AT_LEAST_53_REQUIRED = "LENGTH_AT_LEAST_53_REQUIRED"; // Byte array must have a length of at least 53. } -- cgit v1.2.3 From 774d831fae5809408f9ddfcf9393d579416b1bfb Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 4 Jun 2018 22:34:04 -0700 Subject: Style updates to ERC721 onReceiver --- .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 1 - .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 4 +--- .../contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 9 +++++---- 3 files changed, 6 insertions(+), 8 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index c8e8f4587..50632400e 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,7 +20,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../tokens/ERC20Token/IERC20Token.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 7de7968cc..21e5518c6 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -33,8 +33,6 @@ contract ERC721Proxy is // Id of this proxy. uint8 constant PROXY_ID = 2; - string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; - /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -60,7 +58,7 @@ contract ERC721Proxy is // Data must be intended for this proxy. require( proxyId == PROXY_ID, - PROXY_ID_MISMATCH + ASSET_PROXY_ID_MISMATCH ); // There exists only 1 of each token. diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 5610c47b3..8f6647d20 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -332,7 +332,8 @@ contract LibBytes is internal pure { - // Read length of nested bytes + // Assert length of is valid, given + // length of input require( b.length >= index + 32 /* 32 bytes to store length */ + input.length, GTE_32_LENGTH_REQUIRED @@ -340,9 +341,9 @@ contract LibBytes is // Copy into memcpy( - getMemAddress(b) + index + 32, // +32 to skip length of - getMemAddress(input), // include length of byte array - input.length + 32 // +32 bytes to store length + getMemAddress(b) + 32 + index, // +32 to skip length of + getMemAddress(input), // includes length of + input.length + 32 // +32 bytes to store length ); } } -- cgit v1.2.3 From b19276bb0f36059e67bb57b14e9f1e9e0efc17f2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 5 Jun 2018 16:44:47 -0700 Subject: Fixed merge error when rebasing wrt length variable in asset data decoders --- .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 4 ++-- .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 50632400e..dd25bf41a 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -91,13 +91,13 @@ contract ERC20Proxy is // Validate encoded data length uint256 length = assetData.length; require( - assetData.length == 21, + length == 21, LENGTH_21_REQUIRED ); // Decode data token = readAddress(assetData, 0); - proxyId = uint8(assetData[length-1]); + proxyId = uint8(assetData[length - 1]); return (proxyId, token); } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 21e5518c6..499d8d96e 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -106,14 +106,14 @@ contract ERC721Proxy is // Validate encoded data length uint256 length = assetData.length; require( - assetData.length >= 53, + length >= 53, LENGTH_AT_LEAST_53_REQUIRED ); // Decode asset data. token = readAddress(assetData, 0); tokenId = readUint256(assetData, 20); - if (assetData.length > 53) { + if (length > 53) { receiverData = readBytes(assetData, 52); } proxyId = uint8(assetData[length-1]); -- cgit v1.2.3 From 37684c6af0d2962f7c7822dd14531787bd7b4212 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 5 Jun 2018 17:30:43 -0700 Subject: Fixed a styling throughout contracts. Moved closing parenthesis for long list of function parameters to next line. --- .../protocol/Exchange/MixinWrapperFunctions.sol | 27 ++++++++++++++-------- .../Exchange/interfaces/IAssetProxyDispatcher.sol | 3 ++- .../protocol/Exchange/interfaces/ITransactions.sol | 3 ++- .../Exchange/interfaces/IWrapperFunctions.sol | 27 ++++++++++++++-------- .../test/DummyERC20Token/DummyERC20Token.sol | 3 ++- .../DummyERC721Receiver/DummyERC721Receiver.sol | 3 ++- .../test/DummyERC721Token/DummyERC721Token.sol | 3 ++- .../current/test/TestLibBytes/TestLibBytes.sol | 24 ++++++++++++------- 8 files changed, 62 insertions(+), 31 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index f4822e814..e09f80bcc 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -40,7 +40,8 @@ contract MixinWrapperFunctions is function fillOrKillOrder( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (FillResults memory fillResults) { @@ -65,7 +66,8 @@ contract MixinWrapperFunctions is function fillOrderNoThrow( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (FillResults memory fillResults) { @@ -264,7 +266,8 @@ contract MixinWrapperFunctions is function batchFillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public { for (uint256 i = 0; i < orders.length; i++) { @@ -283,7 +286,8 @@ contract MixinWrapperFunctions is function batchFillOrKillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public { for (uint256 i = 0; i < orders.length; i++) { @@ -303,7 +307,8 @@ contract MixinWrapperFunctions is function batchFillOrdersNoThrow( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public { for (uint256 i = 0; i < orders.length; i++) { @@ -323,7 +328,8 @@ contract MixinWrapperFunctions is function marketSellOrders( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { @@ -366,7 +372,8 @@ contract MixinWrapperFunctions is function marketSellOrdersNoThrow( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { @@ -408,7 +415,8 @@ contract MixinWrapperFunctions is function marketBuyOrders( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { @@ -459,7 +467,8 @@ contract MixinWrapperFunctions is function marketBuyOrdersNoThrow( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol index 3ce5ef157..2c331dc34 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol @@ -28,7 +28,8 @@ contract IAssetProxyDispatcher { function registerAssetProxy( uint8 assetProxyId, address newAssetProxy, - address oldAssetProxy) + address oldAssetProxy + ) external; /// @dev Gets an asset proxy. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol index d973bf001..2f9a5bc7c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol @@ -28,6 +28,7 @@ contract ITransactions { uint256 salt, address signer, bytes data, - bytes signature) + bytes signature + ) external; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol index 8682b394a..acd4f359c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol @@ -33,7 +33,8 @@ contract IWrapperFunctions is function fillOrKillOrder( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (LibFillResults.FillResults memory fillResults); @@ -46,7 +47,8 @@ contract IWrapperFunctions is function fillOrderNoThrow( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (LibFillResults.FillResults memory fillResults); @@ -57,7 +59,8 @@ contract IWrapperFunctions is function batchFillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public; /// @dev Synchronously executes multiple calls of fillOrKill. @@ -67,7 +70,8 @@ contract IWrapperFunctions is function batchFillOrKillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public; /// @dev Fills an order with specified parameters and ECDSA signature. @@ -78,7 +82,8 @@ contract IWrapperFunctions is function batchFillOrdersNoThrow( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public; /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. @@ -89,7 +94,8 @@ contract IWrapperFunctions is function marketSellOrders( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); @@ -102,7 +108,8 @@ contract IWrapperFunctions is function marketSellOrdersNoThrow( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); @@ -114,7 +121,8 @@ contract IWrapperFunctions is function marketBuyOrders( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); @@ -127,7 +135,8 @@ contract IWrapperFunctions is function marketBuyOrdersNoThrow( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); diff --git a/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol b/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol index 0c7b18c0c..b2fe2df06 100644 --- a/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol +++ b/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol @@ -31,7 +31,8 @@ contract DummyERC20Token is Mintable, Ownable { string _name, string _symbol, uint256 _decimals, - uint256 _totalSupply) + uint256 _totalSupply + ) public { name = _name; diff --git a/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol index 1596f3357..c584d0b54 100644 --- a/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol +++ b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol @@ -52,7 +52,8 @@ contract DummyERC721Receiver is function onERC721Received( address _from, uint256 _tokenId, - bytes _data) + bytes _data + ) public returns (bytes4) { diff --git a/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol b/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol index 369a2950d..5503eb2f2 100644 --- a/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol +++ b/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol @@ -34,7 +34,8 @@ contract DummyERC721Token is */ constructor ( string name, - string symbol) + string symbol + ) public ERC721Token(name, symbol) {} diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index f009a6a71..22c84504c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -68,7 +68,8 @@ contract TestLibBytes is /// @return address from byte array. function publicReadAddress( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (address result) @@ -84,7 +85,8 @@ contract TestLibBytes is function publicWriteAddress( bytes memory b, uint256 index, - address input) + address input + ) public pure returns (bytes memory) @@ -99,7 +101,8 @@ contract TestLibBytes is /// @return bytes32 value from byte array. function publicReadBytes32( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (bytes32 result) @@ -115,7 +118,8 @@ contract TestLibBytes is function publicWriteBytes32( bytes memory b, uint256 index, - bytes32 input) + bytes32 input + ) public pure returns (bytes memory) @@ -130,7 +134,8 @@ contract TestLibBytes is /// @return uint256 value from byte array. function publicReadUint256( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (uint256 result) @@ -146,7 +151,8 @@ contract TestLibBytes is function publicWriteUint256( bytes memory b, uint256 index, - uint256 input) + uint256 input + ) public pure returns (bytes memory) @@ -173,7 +179,8 @@ contract TestLibBytes is /// @return result Nested bytes. function publicReadBytes( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (bytes memory result) @@ -190,7 +197,8 @@ contract TestLibBytes is function publicWriteBytes( bytes memory b, uint256 index, - bytes memory input) + bytes memory input + ) public pure returns (bytes memory) -- cgit v1.2.3 From f457a56d4a0a4e5a5b5b11289f65df64cf2c7f1f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 7 Jun 2018 11:11:40 -0700 Subject: Style updates to contracts --- .../current/protocol/AssetProxy/ERC721Proxy.sol | 2 +- .../contracts/current/utils/LibBytes/LibBytes.sol | 28 +++++++++++----------- packages/contracts/src/utils/artifacts.ts | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 499d8d96e..25136133d 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -116,7 +116,7 @@ contract ERC721Proxy is if (length > 53) { receiverData = readBytes(assetData, 52); } - proxyId = uint8(assetData[length-1]); + proxyId = uint8(assetData[length - 1]); return ( proxyId, diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 8f6647d20..282455ea0 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -25,11 +25,11 @@ contract LibBytes is { // Revert reasons - string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0."; - string constant GTE_4_LENGTH_REQUIRED = "Length must be greater than or equal to 4."; - string constant GTE_20_LENGTH_REQUIRED = "Length must be greater than or equal to 20."; - string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32."; - string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds."; + string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_THAN_4_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -41,7 +41,7 @@ contract LibBytes is { require( b.length > 0, - GT_ZERO_LENGTH_REQUIRED + GREATER_THAN_ZERO_LENGTH_REQUIRED ); // Store last byte. @@ -65,7 +65,7 @@ contract LibBytes is { require( b.length >= 20, - GTE_20_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED ); // Store last 20 bytes. @@ -128,7 +128,7 @@ contract LibBytes is { require( b.length >= index + 20, // 20 is length of address - GTE_20_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED ); // Add offset to index: @@ -160,7 +160,7 @@ contract LibBytes is { require( b.length >= index + 20, // 20 is length of address - GTE_20_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED ); // Add offset to index: @@ -199,7 +199,7 @@ contract LibBytes is { require( b.length >= index + 32, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED ); // Arrays are prefixed by a 256 bit length parameter @@ -226,7 +226,7 @@ contract LibBytes is { require( b.length >= index + 32, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED ); // Arrays are prefixed by a 256 bit length parameter @@ -278,7 +278,7 @@ contract LibBytes is { require( b.length >= 4, - GTE_4_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED ); assembly { result := mload(add(b, 32)) @@ -306,7 +306,7 @@ contract LibBytes is // length of nested bytes require( b.length >= index + nestedBytesLength, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED ); // Allocate memory and copy value to result @@ -336,7 +336,7 @@ contract LibBytes is // length of input require( b.length >= index + 32 /* 32 bytes to store length */ + input.length, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED ); // Copy into diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 42de7c921..bf7221d6d 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -10,8 +10,8 @@ import * as Exchange from '../artifacts/Exchange.json'; import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; -import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; +import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; -- cgit v1.2.3 From 5bb7219f4b8d5ce22169d5139bd1c07d7b2fcafd Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 7 Jun 2018 11:43:17 -0700 Subject: Camelcase in memCopy --- .../current/test/TestLibMem/TestLibMem.sol | 8 +++--- .../contracts/current/utils/LibBytes/LibBytes.sol | 4 +-- .../src/contracts/current/utils/LibMem/LibMem.sol | 32 +++++++++++----------- 3 files changed, 22 insertions(+), 22 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 076bee24c..b7e2e06b8 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -25,11 +25,11 @@ contract TestLibMem is { /// @dev Copies a block of memory from one location to another. - /// @param mem Memory contents we want to apply memcpy to + /// @param mem Memory contents we want to apply memCopy to /// @param dest Destination offset into . /// @param source Source offset into . /// @param length Length of bytes to copy from to - /// @return mem Memory contents after calling memcpy. + /// @return mem Memory contents after calling memCopy. function testMemcpy( bytes mem, uint256 dest, @@ -47,8 +47,8 @@ contract TestLibMem is // Get pointer to memory contents uint256 offset = getMemAddress(mem) + 32; - // Execute memcpy adjusted for memory array location - memcpy(offset + dest, offset + source, length); + // Execute memCopy adjusted for memory array location + memCopy(offset + dest, offset + source, length); // Return modified memory contents return mem; diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 282455ea0..4f9e2f152 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -311,7 +311,7 @@ contract LibBytes is // Allocate memory and copy value to result result = new bytes(nestedBytesLength); - memcpy( + memCopy( getMemAddress(result) + 32, // +32 skips array length getMemAddress(b) + index + 32, nestedBytesLength @@ -340,7 +340,7 @@ contract LibBytes is ); // Copy into - memcpy( + memCopy( getMemAddress(b) + 32 + index, // +32 to skip length of getMemAddress(input), // includes length of input.length + 32 // +32 bytes to store length diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index 960850725..6afb9973a 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -39,7 +39,7 @@ contract LibMem /// @param dest memory address to copy bytes to. /// @param source memory address to copy bytes from. /// @param length number of bytes to copy. - function memcpy( + function memCopy( uint256 dest, uint256 source, uint256 length @@ -81,42 +81,42 @@ contract LibMem if (source > dest) { assembly { // Record the total number of full words to copy - let nwords := div(length, 32) + let nWords := div(length, 32) - // We subtract 32 from `send` and `dend` because it + // We subtract 32 from `sEnd` and `dEnd` because it // is easier to compare with in the loop, and these // are also the addresses we need for copying the // last bytes. length := sub(length, 32) - let send := add(source, length) - let dend := add(dest, length) + let sEnd := add(source, length) + let dEnd := add(dest, length) // Remember the last 32 bytes of source // This needs to be done here and not after the loop // because we may have overwritten the last bytes in // source already due to overlap. - let last := mload(send) + let last := mload(sEnd) // Copy whole words front to back - for {let i := 0} lt(i, nwords) {i := add(i, 1)} { + for {let i := 0} lt(i, nWords) {i := add(i, 1)} { mstore(dest, mload(source)) source := add(source, 32) dest := add(dest, 32) } // Write the last 32 bytes - mstore(dend, last) + mstore(dEnd, last) } } else { assembly { // Record the total number of full words to copy - let nwords := div(length, 32) + let nWords := div(length, 32) - // We subtract 32 from `send` and `dend` because those + // We subtract 32 from `sEnd` and `dEnd` because those // are the starting points when copying a word at the end. length := sub(length, 32) - let send := add(source, length) - let dend := add(dest, length) + let sEnd := add(source, length) + let dEnd := add(dest, length) // Remember the first 32 bytes of source // This needs to be done here and not after the loop @@ -125,10 +125,10 @@ contract LibMem let first := mload(source) // Copy whole words back to front - for {let i := 0} lt(i, nwords) {i := add(i, 1)} { - mstore(dend, mload(send)) - send := sub(send, 32) - dend := sub(dend, 32) + for {let i := 0} lt(i, nWords) {i := add(i, 1)} { + mstore(dEnd, mload(sEnd)) + sEnd := sub(sEnd, 32) + dEnd := sub(dEnd, 32) } // Write the first 32 bytes -- cgit v1.2.3 From 05123ea6f48a71ac0a53dcadcbc5924cbd1d1486 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 7 Jun 2018 16:32:42 -0700 Subject: Updated LibBytes error messages --- .../src/contracts/current/utils/LibBytes/LibBytes.sol | 2 +- packages/contracts/src/utils/constants.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 4f9e2f152..d1d10476f 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -26,7 +26,7 @@ contract LibBytes is // Revert reasons string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_THAN_4_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index fa2a4af3c..af3f26d82 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,11 +19,11 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', - LIB_BYTES_GT_ZERO_LENGTH_REQUIRED: 'Length must be greater than 0.', - LIB_BYTES_GTE_4_LENGTH_REQUIRED: 'Length must be greater than or equal to 4.', - LIB_BYTES_GTE_20_LENGTH_REQUIRED: 'Length must be greater than or equal to 20.', - LIB_BYTES_GTE_32_LENGTH_REQUIRED: 'Length must be greater than or equal to 32.', - LIB_BYTES_INDEX_OUT_OF_BOUNDS: 'Specified array index is out of bounds.', + LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED: 'GREATER_THAN_ZERO_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, -- cgit v1.2.3 From 760bab8f866ec3d5fc7627ce9bbf5c2eaaef1f36 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 8 Jun 2018 11:18:32 -0700 Subject: Implement SolidityProfiler & adapt sol-cov to work with Geth --- packages/contracts/src/utils/coverage.ts | 3 ++- packages/contracts/src/utils/profiler.ts | 27 +++++++++++++++++++++++++++ packages/contracts/src/utils/web3_wrapper.ts | 22 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 packages/contracts/src/utils/profiler.ts (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/coverage.ts b/packages/contracts/src/utils/coverage.ts index 41c83f703..de29a3ecc 100644 --- a/packages/contracts/src/utils/coverage.ts +++ b/packages/contracts/src/utils/coverage.ts @@ -14,7 +14,8 @@ export const coverage = { _getCoverageSubprovider(): CoverageSubprovider { const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); - const subprovider = new CoverageSubprovider(solCompilerArtifactAdapter, defaultFromAddress); + const isVerbose = true; + const subprovider = new CoverageSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); return subprovider; }, }; diff --git a/packages/contracts/src/utils/profiler.ts b/packages/contracts/src/utils/profiler.ts new file mode 100644 index 000000000..85ee24f22 --- /dev/null +++ b/packages/contracts/src/utils/profiler.ts @@ -0,0 +1,27 @@ +import { devConstants } from '@0xproject/dev-utils'; +import { ProfilerSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; +import * as _ from 'lodash'; + +let profilerSubprovider: ProfilerSubprovider; + +export const profiler = { + start(): void { + profiler.getProfilerSubproviderSingleton().start(); + }, + stop(): void { + profiler.getProfilerSubproviderSingleton().stop(); + }, + getProfilerSubproviderSingleton(): ProfilerSubprovider { + if (_.isUndefined(profilerSubprovider)) { + profilerSubprovider = profiler._getProfilerSubprovider(); + } + return profilerSubprovider; + }, + _getProfilerSubprovider(): ProfilerSubprovider { + const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; + const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); + const isVerbose = true; + const subprovider = new ProfilerSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); + return subprovider; + }, +}; diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index df9bf88c8..c475d96a9 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -1,8 +1,10 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; import { prependSubprovider } from '@0xproject/subproviders'; +import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { coverage } from './coverage'; +import { profiler } from './profiler'; enum ProviderType { Ganache = 'ganache', @@ -45,9 +47,29 @@ const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); +const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); +if (isCoverageEnabled && isProfilerEnabled) { + throw new Error( + `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, + ); +} if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); prependSubprovider(provider, coverageSubprovider); } +if (isProfilerEnabled) { + if (testProvider === ProviderType.Ganache) { + logUtils.warn( + "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", + ); + process.exit(1); + } + const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); + logUtils.log( + "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", + ); + profilerSubprovider.stop(); + prependSubprovider(provider, profilerSubprovider); +} export const web3Wrapper = new Web3Wrapper(provider); -- cgit v1.2.3 From 3cc30f91a99578b626d95811a26ee7b19f404455 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 11 Jun 2018 17:04:47 -0700 Subject: Speedup awaitTransactionMinedAsync and reduce polling interval in contracts tests --- packages/contracts/src/utils/constants.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index af3f26d82..5e336589f 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -27,7 +27,10 @@ export const constants = { ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, - AWAIT_TRANSACTION_MINED_MS: 100, + // Note(albrow): In practice V8 and most other engines limit the minimum + // interval for setInterval to 10ms. We still set it to 0 here in order to + // ensure we always use the minimum interval. + AWAIT_TRANSACTION_MINED_MS: 0, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, -- cgit v1.2.3 From ee8c9b764d0ee153efa91075b35f3192b72be119 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sat, 9 Jun 2018 19:01:28 -0700 Subject: Pop id from assetData before dispatching to AssetProxies --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 37 +-------------- .../current/protocol/AssetProxy/ERC721Proxy.sol | 34 ++++---------- .../AssetProxy/libs/LibAssetProxyErrors.sol | 5 --- .../Exchange/MixinAssetProxyDispatcher.sol | 14 +++--- .../protocol/Exchange/MixinExchangeCore.sol | 7 +-- .../current/protocol/Exchange/MixinMatchOrders.sol | 18 ++++---- .../current/protocol/Exchange/MixinSettlement.sol | 42 ++++++++++++----- .../protocol/Exchange/MixinWrapperFunctions.sol | 52 ++++++++++++---------- .../protocol/Exchange/libs/LibExchangeErrors.sol | 1 - .../Exchange/mixins/MAssetProxyDispatcher.sol | 2 + .../TestAssetDataDecoders.sol | 21 --------- .../TestAssetProxyDispatcher.sol | 3 +- .../contracts/current/utils/LibBytes/LibBytes.sol | 24 ++++++++++ packages/contracts/src/utils/exchange_wrapper.ts | 2 +- packages/contracts/src/utils/formatters.ts | 16 ++++++- packages/contracts/src/utils/order_utils.ts | 3 ++ 16 files changed, 132 insertions(+), 149 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index dd25bf41a..ddcd78e93 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -47,16 +47,7 @@ contract ERC20Proxy is internal { // Decode asset data. - ( - uint8 proxyId, - address token - ) = decodeERC20AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); + address token = readAddress(assetData, 0); // Transfer tokens. bool success = IERC20Token(token).transferFrom(from, to, amount); @@ -75,30 +66,4 @@ contract ERC20Proxy is { return PROXY_ID; } - - /// @dev Decodes ERC20 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC20 proxy id. - /// @return token ERC20 token address. - function decodeERC20AssetData(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token - ) - { - // Validate encoded data length - uint256 length = assetData.length; - require( - length == 21, - LENGTH_21_REQUIRED - ); - - // Decode data - token = readAddress(assetData, 0); - proxyId = uint8(assetData[length - 1]); - - return (proxyId, token); - } } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 25136133d..861fac2c1 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -46,30 +46,22 @@ contract ERC721Proxy is ) internal { + // There exists only 1 of each token. + require( + amount == 1, + INVALID_AMOUNT + ); + // Decode asset data. ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) = decodeERC721AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); - - // There exists only 1 of each token. - require( - amount == 1, - INVALID_AMOUNT - ); - // Transfer token. Saves gas by calling safeTransferFrom only // when there is receiverData present. Either succeeds or throws. - if(receiverData.length > 0) { + if (receiverData.length > 0) { ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } else { ERC721Token(token).transferFrom(from, to, tokenId); @@ -97,29 +89,19 @@ contract ERC721Proxy is internal pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { - // Validate encoded data length - uint256 length = assetData.length; - require( - length >= 53, - LENGTH_AT_LEAST_53_REQUIRED - ); - // Decode asset data. token = readAddress(assetData, 0); tokenId = readUint256(assetData, 20); - if (length > 53) { + if (assetData.length > 52) { receiverData = readBytes(assetData, 52); } - proxyId = uint8(assetData[length - 1]); return ( - proxyId, token, tokenId, receiverData diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol index 80180a0d9..65bdacdb7 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -27,11 +27,6 @@ contract LibAssetProxyErrors { string constant AUTHORIZED_ADDRESS_MISMATCH = "AUTHORIZED_ADDRESS_MISMATCH"; // Address at index does not match given target address. /// AssetProxy errors /// - string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // Proxy id in metadata does not match this proxy id. string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. - - /// Length validation errors /// - string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21. - string constant LENGTH_AT_LEAST_53_REQUIRED = "LENGTH_AT_LEAST_53_REQUIRED"; // Byte array must have a length of at least 53. } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index e77d81c06..9e0246303 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,12 +19,14 @@ pragma solidity ^0.4.24; import "../../utils/Ownable/Ownable.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, + LibBytes, LibExchangeErrors, MAssetProxyDispatcher { @@ -81,11 +83,13 @@ contract MixinAssetProxyDispatcher is /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. /// @param assetData Byte array encoded for the respective asset proxy. + /// @param assetProxyId Id of assetProxy to dispach to. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount @@ -94,16 +98,8 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { - - // Lookup asset proxy - uint256 length = assetData.length; - require( - length > 0, - LENGTH_GREATER_THAN_0_REQUIRED - ); - uint8 assetProxyId = uint8(assetData[length - 1]); + // Lookup assetProxy IAssetProxy assetProxy = assetProxies[assetProxyId]; - // transferFrom will either succeed or throw. assetProxy.transferFrom(assetData, from, to, amount); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 12b57d99f..0a0f0209a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -108,9 +108,6 @@ contract MixinExchangeCore is // Compute proportional fill amounts fillResults = calculateFillResults(order, takerAssetFilledAmount); - // Settle order - settleOrder(order, takerAddress, fillResults); - // Update exchange internal state updateFilledState( order, @@ -119,6 +116,10 @@ contract MixinExchangeCore is orderInfo.orderTakerAssetFilledAmount, fillResults ); + + // Settle order + settleOrder(order, takerAddress, fillResults); + return fillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index ed76287e0..517b743fe 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,7 +14,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -25,7 +24,6 @@ import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; contract MixinMatchOrders is - LibBytes, LibMath, LibExchangeErrors, MExchangeCore, @@ -94,14 +92,6 @@ contract MixinMatchOrders is rightSignature ); - // Settle matched orders. Succeeds or throws. - settleMatchedOrders( - leftOrder, - rightOrder, - takerAddress, - matchedFillResults - ); - // Update exchange state updateFilledState( leftOrder, @@ -117,6 +107,14 @@ contract MixinMatchOrders is rightOrderInfo.orderTakerAssetFilledAmount, matchedFillResults.right ); + + // Settle matched orders. Succeeds or throws. + settleMatchedOrders( + leftOrder, + rightOrder, + takerAddress, + matchedFillResults + ); return matchedFillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 83e9dfdf4..f0caf7446 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -19,6 +19,7 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; @@ -28,6 +29,7 @@ import "./mixins/MSettlement.sol"; import "./mixins/MAssetProxyDispatcher.sol"; contract MixinSettlement is + LibBytes, LibMath, LibExchangeErrors, MMatchOrders, @@ -37,7 +39,7 @@ contract MixinSettlement is // ZRX metadata used for fee transfers. // This will be constant throughout the life of the Exchange contract, // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_PROXY_DATA; + bytes internal ZRX_ASSET_DATA; /// @dev Gets the ZRX metadata used for fee transfers. function zrxAssetData() @@ -45,7 +47,7 @@ contract MixinSettlement is view returns (bytes memory) { - return ZRX_PROXY_DATA; + return ZRX_ASSET_DATA; } /// TODO: _zrxAssetData should be a constant in production. @@ -54,7 +56,7 @@ contract MixinSettlement is constructor (bytes memory _zrxAssetData) public { - ZRX_PROXY_DATA = _zrxAssetData; + ZRX_ASSET_DATA = _zrxAssetData; } /// @dev Settles an order by transferring assets between counterparties. @@ -68,26 +70,34 @@ contract MixinSettlement is ) internal { + uint8 makerAssetProxyId = uint8(popByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popByte(order.takerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + uint8 zrxProxyId = uint8(popByte(zrxAssetData)); dispatchTransferFrom( order.makerAssetData, + makerAssetProxyId, order.makerAddress, takerAddress, fillResults.makerAssetFilledAmount ); dispatchTransferFrom( order.takerAssetData, + takerAssetProxyId, takerAddress, order.makerAddress, fillResults.takerAssetFilledAmount ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid @@ -107,21 +117,28 @@ contract MixinSettlement is ) internal { + uint8 leftMakerAssetProxyId = uint8(popByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popByte(rightOrder.makerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + uint8 zrxProxyId = uint8(popByte(zrxAssetData)); // Order makers and taker dispatchTransferFrom( leftOrder.makerAssetData, + leftMakerAssetProxyId, leftOrder.makerAddress, rightOrder.makerAddress, matchedFillResults.right.takerAssetFilledAmount ); dispatchTransferFrom( rightOrder.makerAssetData, + rightMakerAssetProxyId, rightOrder.makerAddress, leftOrder.makerAddress, matchedFillResults.left.takerAssetFilledAmount ); dispatchTransferFrom( leftOrder.makerAssetData, + leftMakerAssetProxyId, leftOrder.makerAddress, takerAddress, matchedFillResults.leftMakerAssetSpreadAmount @@ -129,13 +146,15 @@ contract MixinSettlement is // Maker fees dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -144,7 +163,8 @@ contract MixinSettlement is // Taker fees if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -154,13 +174,15 @@ contract MixinSettlement is ); } else { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.takerFeePaid diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index e09f80bcc..cd5e26fb7 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -335,12 +335,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. + // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); + } // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -379,12 +380,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. + // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); + } // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -422,12 +424,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); + } // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); @@ -474,12 +477,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); + } // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol index 2b4bbeec4..48dd0f8be 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -25,7 +25,6 @@ contract LibExchangeErrors { string constant INVALID_TAKER = "INVALID_TAKER"; // Invalid takerAddress. string constant INVALID_SENDER = "INVALID_SENDER"; // Invalid `msg.sender`. string constant INVALID_ORDER_SIGNATURE = "INVALID_ORDER_SIGNATURE"; // Signature validation failed. - string constant ASSET_DATA_MISMATCH = "ASSET_DATA_MISMATCH"; // Asset data must be the same for each order. /// fillOrder validation errors /// string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index 82eafb529..d16a830f4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -34,11 +34,13 @@ contract MAssetProxyDispatcher is /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. /// @param assetData Byte array encoded for the respective asset proxy. + /// @param assetProxyId Id of assetProxy to dispach to. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 2c6a8fdb0..5987291d2 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -23,26 +23,8 @@ import "../../protocol/AssetProxy/ERC20Proxy.sol"; import "../../protocol/AssetProxy/ERC721Proxy.sol"; contract TestAssetDataDecoders is - ERC20Proxy, ERC721Proxy { - - /// @dev Decodes ERC20 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC20 proxy id. - /// @return token ERC20 token address. - function publicDecodeERC20Data(bytes memory assetData) - public - pure - returns ( - uint8 proxyId, - address token - ) - { - (proxyId, token) = decodeERC20AssetData(assetData); - return (proxyId, token); - } - /// @dev Decodes ERC721 Asset data. /// @param assetData Encoded byte array. /// @return proxyId Intended ERC721 proxy id. @@ -54,21 +36,18 @@ contract TestAssetDataDecoders is public pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { ( - proxyId, token, tokenId, receiverData ) = decodeERC721AssetData(assetData); return ( - proxyId, token, tokenId, receiverData diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol index 2ae69e0ef..d469a07f0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -24,11 +24,12 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher { function publicDispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount) public { - dispatchTransferFrom(assetData, from, to, amount); + dispatchTransferFrom(assetData, assetProxyId, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index d1d10476f..aac0ffc31 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -30,6 +30,7 @@ contract LibBytes is string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -114,6 +115,29 @@ contract LibBytes is return equal; } + /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. + /// @param dest Byte array that will be overwritten with source bytes. + /// @param source Byte array to copy onto dest bytes. + function deepCopyBytes( + bytes memory dest, + bytes memory source + ) + internal + pure + { + uint256 sourceLen = source.length; + // Dest length must be >= source length, or some bytes would not be copied. + require( + dest.length >= sourceLen, + GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED + ); + memCopy( + getMemAddress(dest) + 32, // +32 to skip length of + getMemAddress(source) + 32, // +32 to skip length of + sourceLen + ); + } + /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index a8ca5183e..6603538b9 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -165,7 +165,7 @@ export class ExchangeWrapper { public async marketBuyOrdersNoThrowAsync( orders: SignedOrder[], from: string, - opts: { makerAssetFillAmount: BigNumber }, + opts: { makerAssetFillAmount: BigNumber; gas?: number }, ): Promise { const params = formatters.createMarketBuyOrders(orders, opts.makerAssetFillAmount); const txHash = await this._exchange.marketBuyOrdersNoThrow.sendTransactionAsync( diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts index 1035f2d7c..b25dec27c 100644 --- a/packages/contracts/src/utils/formatters.ts +++ b/packages/contracts/src/utils/formatters.ts @@ -28,8 +28,14 @@ export const formatters = { signatures: [], takerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.takerAssetData = `0x${_.repeat( + '0', + signedOrders[0].takerAssetData.length - 2, + )}`; + } marketSellOrders.orders.push(orderWithoutExchangeAddress); marketSellOrders.signatures.push(signedOrder.signature); }); @@ -41,8 +47,14 @@ export const formatters = { signatures: [], makerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.makerAssetData = `0x${_.repeat( + '0', + signedOrders[0].makerAssetData.length - 2, + )}`; + } marketBuyOrders.orders.push(orderWithoutExchangeAddress); marketBuyOrders.signatures.push(signedOrder.signature); }); diff --git a/packages/contracts/src/utils/order_utils.ts b/packages/contracts/src/utils/order_utils.ts index 2a8791e4c..a9f994d80 100644 --- a/packages/contracts/src/utils/order_utils.ts +++ b/packages/contracts/src/utils/order_utils.ts @@ -1,6 +1,7 @@ import { OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { constants } from './constants'; import { CancelOrder, MatchOrder } from './types'; export const orderUtils = { @@ -43,6 +44,8 @@ export const orderUtils = { leftSignature: signedOrderLeft.signature, rightSignature: signedOrderRight.signature, }; + fill.right.makerAssetData = constants.NULL_BYTES; + fill.right.takerAssetData = constants.NULL_BYTES; return fill; }, }; -- cgit v1.2.3 From 764b1c35cb7bf763deeb5db34ebe36d6e973b409 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 10 Jun 2018 16:09:07 -0700 Subject: Add tests for deepCopyBytes and missing write methods from LibBytes --- .../contracts/current/test/TestLibBytes/TestLibBytes.sol | 15 +++++++++++++++ packages/contracts/src/utils/constants.ts | 1 + 2 files changed, 16 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 22c84504c..abce0cb22 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -62,6 +62,21 @@ contract TestLibBytes is return equal; } + /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. + /// @param dest Byte array that will be overwritten with source bytes. + /// @param source Byte array to copy onto dest bytes. + function publicDeepCopyBytes( + bytes memory dest, + bytes memory source + ) + public + pure + returns (bytes memory) + { + deepCopyBytes(dest, source); + return dest; + } + /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index 5e336589f..ec3c8fd36 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -24,6 +24,7 @@ export const constants = { LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, -- cgit v1.2.3 From 5910bec52e0664f70d5dc98ce8303ec5373107ba Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 10 Jun 2018 21:13:59 -0700 Subject: Make ZRX_PROXY_ID constant rather than popping it from ZRX_ASSET_DATA --- .../current/protocol/Exchange/MixinSettlement.sol | 28 +++++++--------------- 1 file changed, 9 insertions(+), 19 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index f0caf7446..69b70112f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -36,19 +36,11 @@ contract MixinSettlement is MSettlement, MAssetProxyDispatcher { - // ZRX metadata used for fee transfers. + // ZRX address encoded as a byte array. // This will be constant throughout the life of the Exchange contract, // since ZRX will always be transferred via the ERC20 AssetProxy. bytes internal ZRX_ASSET_DATA; - - /// @dev Gets the ZRX metadata used for fee transfers. - function zrxAssetData() - external - view - returns (bytes memory) - { - return ZRX_ASSET_DATA; - } + uint8 constant ZRX_PROXY_ID = 1; /// TODO: _zrxAssetData should be a constant in production. /// @dev Constructor sets the metadata that will be used for paying ZRX fees. @@ -73,7 +65,6 @@ contract MixinSettlement is uint8 makerAssetProxyId = uint8(popByte(order.makerAssetData)); uint8 takerAssetProxyId = uint8(popByte(order.takerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; - uint8 zrxProxyId = uint8(popByte(zrxAssetData)); dispatchTransferFrom( order.makerAssetData, makerAssetProxyId, @@ -90,14 +81,14 @@ contract MixinSettlement is ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid @@ -120,7 +111,6 @@ contract MixinSettlement is uint8 leftMakerAssetProxyId = uint8(popByte(leftOrder.makerAssetData)); uint8 rightMakerAssetProxyId = uint8(popByte(rightOrder.makerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; - uint8 zrxProxyId = uint8(popByte(zrxAssetData)); // Order makers and taker dispatchTransferFrom( leftOrder.makerAssetData, @@ -147,14 +137,14 @@ contract MixinSettlement is // Maker fees dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -164,7 +154,7 @@ contract MixinSettlement is if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -175,14 +165,14 @@ contract MixinSettlement is } else { dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.takerFeePaid -- cgit v1.2.3 From 2f96cb257c0f7280f8b578eed6a3c1711749c0e9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 11 Jun 2018 11:58:18 -0700 Subject: Looks up the memory location of makerAssetData/takerAssetData --- .../src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol | 2 ++ 1 file changed, 2 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index cd5e26fb7..88f916179 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -174,6 +174,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of + sourceOffset := mload(add(order, 0x140)) // makerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) @@ -193,6 +194,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of + sourceOffset := mload(add(order, 0x160)) // takerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) -- cgit v1.2.3 From a0a90afbc0962eb70b2abb3d24aef80a8d8a822d Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 11 Jun 2018 19:44:27 -0700 Subject: Pass gas in to marketBuyOrdersNoThrow --- packages/contracts/src/utils/exchange_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index 6603538b9..4cc8f0b89 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -172,7 +172,7 @@ export class ExchangeWrapper { params.orders, params.makerAssetFillAmount, params.signatures, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; -- cgit v1.2.3 From 3a5f3e8b55f4b9733ef542281ee71c4fdc9cd39d Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 12 Jun 2018 11:43:19 -0700 Subject: Unpop byte rather than making deep copy --- .../protocol/Exchange/MixinWrapperFunctions.sol | 54 +++++----- .../contracts/current/utils/LibBytes/LibBytes.sol | 116 ++++++++++----------- packages/contracts/src/utils/formatters.ts | 11 +- 3 files changed, 91 insertions(+), 90 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 88f916179..a7849f4cb 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -19,7 +19,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -27,7 +26,6 @@ import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; contract MixinWrapperFunctions is - LibBytes, LibMath, LibFillResults, LibExchangeErrors, @@ -335,15 +333,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being sold by taker is the same for each order. - // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. - // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); - } + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -355,6 +351,14 @@ contract MixinWrapperFunctions is signatures[i] ); + // HACK: the proxyId is "popped" from the byte array before a fill is settled + // by subtracting from the length of the array. Since the popped byte is + // still in memory, we can "unpop" it by incrementing the length of the byte array. + assembly { + let len := mload(takerAssetData) + mstore(takerAssetData, add(len, 1)) + } + // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -380,15 +384,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being sold by taker is the same for each order. - // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. - // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); - } + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -424,15 +426,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being bought by taker is the same for each order. // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. - // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); - } + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); @@ -452,6 +452,14 @@ contract MixinWrapperFunctions is signatures[i] ); + // HACK: the proxyId is "popped" from the byte array before a fill is settled + // by subtracting from the length of the array. Since the popped byte is + // still in memory, we can "unpop" it by incrementing the length of the byte array. + assembly { + let len := mload(makerAssetData) + mstore(makerAssetData, add(len, 1)) + } + // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -477,15 +485,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being bought by taker is the same for each order. // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. - // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); - } + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index aac0ffc31..339270a57 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -80,64 +80,6 @@ contract LibBytes is return result; } - /// @dev Tests equality of two byte arrays. - /// @param lhs First byte array to compare. - /// @param rhs Second byte array to compare. - /// @return True if arrays are the same. False otherwise. - function areBytesEqual( - bytes memory lhs, - bytes memory rhs - ) - internal - pure - returns (bool equal) - { - assembly { - // Get the number of words occupied by - let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) - - // Add 1 to the number of words, to account for the length field - lenFullWords := add(lenFullWords, 0x1) - - // Test equality word-by-word. - // Terminates early if there is a mismatch. - for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { - let lhsWord := mload(add(lhs, mul(i, 0x20))) - let rhsWord := mload(add(rhs, mul(i, 0x20))) - equal := eq(lhsWord, rhsWord) - if eq(equal, 0) { - // Break - i := lenFullWords - } - } - } - - return equal; - } - - /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. - /// @param dest Byte array that will be overwritten with source bytes. - /// @param source Byte array to copy onto dest bytes. - function deepCopyBytes( - bytes memory dest, - bytes memory source - ) - internal - pure - { - uint256 sourceLen = source.length; - // Dest length must be >= source length, or some bytes would not be copied. - require( - dest.length >= sourceLen, - GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED - ); - memCopy( - getMemAddress(dest) + 32, // +32 to skip length of - getMemAddress(source) + 32, // +32 to skip length of - sourceLen - ); - } - /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. @@ -370,4 +312,62 @@ contract LibBytes is input.length + 32 // +32 bytes to store length ); } + + /// @dev Tests equality of two byte arrays. + /// @param lhs First byte array to compare. + /// @param rhs Second byte array to compare. + /// @return True if arrays are the same. False otherwise. + function areBytesEqual( + bytes memory lhs, + bytes memory rhs + ) + internal + pure + returns (bool equal) + { + assembly { + // Get the number of words occupied by + let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) + + // Add 1 to the number of words, to account for the length field + lenFullWords := add(lenFullWords, 0x1) + + // Test equality word-by-word. + // Terminates early if there is a mismatch. + for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { + let lhsWord := mload(add(lhs, mul(i, 0x20))) + let rhsWord := mload(add(rhs, mul(i, 0x20))) + equal := eq(lhsWord, rhsWord) + if eq(equal, 0) { + // Break + i := lenFullWords + } + } + } + + return equal; + } + + /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. + /// @param dest Byte array that will be overwritten with source bytes. + /// @param source Byte array to copy onto dest bytes. + function deepCopyBytes( + bytes memory dest, + bytes memory source + ) + internal + pure + { + uint256 sourceLen = source.length; + // Dest length must be >= source length, or some bytes would not be copied. + require( + dest.length >= sourceLen, + GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED + ); + memCopy( + getMemAddress(dest) + 32, // +32 to skip length of + getMemAddress(source) + 32, // +32 to skip length of + sourceLen + ); + } } diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts index b25dec27c..32e4787d6 100644 --- a/packages/contracts/src/utils/formatters.ts +++ b/packages/contracts/src/utils/formatters.ts @@ -2,6 +2,7 @@ import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; +import { constants } from './constants'; import { orderUtils } from './order_utils'; import { BatchCancelOrders, BatchFillOrders, MarketBuyOrders, MarketSellOrders } from './types'; @@ -31,10 +32,7 @@ export const formatters = { _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); if (i !== 0) { - orderWithoutExchangeAddress.takerAssetData = `0x${_.repeat( - '0', - signedOrders[0].takerAssetData.length - 2, - )}`; + orderWithoutExchangeAddress.takerAssetData = constants.NULL_BYTES; } marketSellOrders.orders.push(orderWithoutExchangeAddress); marketSellOrders.signatures.push(signedOrder.signature); @@ -50,10 +48,7 @@ export const formatters = { _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); if (i !== 0) { - orderWithoutExchangeAddress.makerAssetData = `0x${_.repeat( - '0', - signedOrders[0].makerAssetData.length - 2, - )}`; + orderWithoutExchangeAddress.makerAssetData = constants.NULL_BYTES; } marketBuyOrders.orders.push(orderWithoutExchangeAddress); marketBuyOrders.signatures.push(signedOrder.signature); -- cgit v1.2.3 From 0917fa0d75b7328c156af2ffafa641ae09286e2e Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 12 Jun 2018 15:29:18 -0700 Subject: Rename popByte and popAddress --- .../src/contracts/current/protocol/Exchange/MixinSettlement.sol | 8 ++++---- .../current/protocol/Exchange/MixinSignatureValidator.sol | 4 ++-- .../src/contracts/current/test/TestLibBytes/TestLibBytes.sol | 8 ++++---- .../contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 69b70112f..29a9c87bd 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -62,8 +62,8 @@ contract MixinSettlement is ) internal { - uint8 makerAssetProxyId = uint8(popByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popByte(order.takerAssetData)); + uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; dispatchTransferFrom( order.makerAssetData, @@ -108,8 +108,8 @@ contract MixinSettlement is ) internal { - uint8 leftMakerAssetProxyId = uint8(popByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popByte(rightOrder.makerAssetData)); + uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; // Order makers and taker dispatchTransferFrom( diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 48a0c5552..1a556dfe2 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -93,7 +93,7 @@ contract MixinSignatureValidator is ); // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popByte(signature))); + SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); // Variables are not scoped in Solidity. uint8 v; @@ -183,7 +183,7 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validator = popAddress(signature); + address validator = popLast20Bytes(signature); // Ensure signer has approved validator. if (!allowedValidators[signer][validator]) { return false; diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index abce0cb22..6f1898acd 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -28,24 +28,24 @@ contract TestLibBytes is /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function publicPopByte(bytes memory b) + function publicPopLastByte(bytes memory b) public pure returns (bytes memory, bytes1 result) { - result = popByte(b); + result = popLastByte(b); return (b, result); } /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function publicPopAddress(bytes memory b) + function publicPopLast20Bytes(bytes memory b) public pure returns (bytes memory, address result) { - result = popAddress(b); + result = popLast20Bytes(b); return (b, result); } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 339270a57..10d7ce41a 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -35,7 +35,7 @@ contract LibBytes is /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function popByte(bytes memory b) + function popLastByte(bytes memory b) internal pure returns (bytes1 result) @@ -59,7 +59,7 @@ contract LibBytes is /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function popAddress(bytes memory b) + function popLast20Bytes(bytes memory b) internal pure returns (address result) -- cgit v1.2.3 From 2c7d6a7711e44294e64858095971a2aebc6d1829 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 14 Jun 2018 10:54:54 +0200 Subject: Handle tokens that do not return bool --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 30 +++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index ddcd78e93..eeddb9707 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -50,7 +50,35 @@ contract ERC20Proxy is address token = readAddress(assetData, 0); // Transfer tokens. - bool success = IERC20Token(token).transferFrom(from, to, amount); + // We do a raw call so we can check the success separate + // from the return data. + bool success = token.call(abi.encodeWithSelector( + IERC20Token(token).transferFrom.selector, + from, + to, + amount + )); + require( + success, + TRANSFER_FAILED + ); + + // Check return data. + // If there is no return data, we assume the token incorrectly + // does not return a bool. In this case we expect it to revert + // on failure, which was handled above. + // If the token does return data, we require that it is a single + // value that evaluates to true. + assembly { + if returndatasize { + success := 0 + if eq(returndatasize, 32) { + // First 64 bytes of memory are reserved scratch space + returndatacopy(0, 0, 32) + success := mload(0) + } + } + } require( success, TRANSFER_FAILED -- cgit v1.2.3 From 7ab921669bf52c1cb2d43350b2cccc8efe91bdbd Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 13:58:28 -0700 Subject: Introduce subprovider for printing revert stack traces --- packages/contracts/src/utils/revert_trace.ts | 21 ++++++++++++ packages/contracts/src/utils/web3_wrapper.ts | 51 ++++++++++++++++------------ 2 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 packages/contracts/src/utils/revert_trace.ts (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/revert_trace.ts b/packages/contracts/src/utils/revert_trace.ts new file mode 100644 index 000000000..0bf8384bc --- /dev/null +++ b/packages/contracts/src/utils/revert_trace.ts @@ -0,0 +1,21 @@ +import { devConstants } from '@0xproject/dev-utils'; +import { RevertTraceSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; +import * as _ from 'lodash'; + +let revertTraceSubprovider: RevertTraceSubprovider; + +export const revertTrace = { + getRevertTraceSubproviderSingleton(): RevertTraceSubprovider { + if (_.isUndefined(revertTraceSubprovider)) { + revertTraceSubprovider = revertTrace._getRevertTraceSubprovider(); + } + return revertTraceSubprovider; + }, + _getRevertTraceSubprovider(): RevertTraceSubprovider { + const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; + const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); + const isVerbose = true; + const subprovider = new RevertTraceSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); + return subprovider; + }, +}; diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index c475d96a9..b8e8ed8ce 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -5,6 +5,7 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { coverage } from './coverage'; import { profiler } from './profiler'; +import { revertTrace } from './revert_trace'; enum ProviderType { Ganache = 'ganache', @@ -48,28 +49,34 @@ const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); -if (isCoverageEnabled && isProfilerEnabled) { - throw new Error( - `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, - ); -} -if (isCoverageEnabled) { - const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); - prependSubprovider(provider, coverageSubprovider); -} -if (isProfilerEnabled) { - if (testProvider === ProviderType.Ganache) { - logUtils.warn( - "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", - ); - process.exit(1); - } - const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); - logUtils.log( - "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", - ); - profilerSubprovider.stop(); - prependSubprovider(provider, profilerSubprovider); +const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); +// TODO(albrow): Include revertTrace checks in the warnings below. +// if (isCoverageEnabled && isProfilerEnabled) { +// throw new Error( +// `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, +// ); +// } +// if (isCoverageEnabled) { +// const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); +// prependSubprovider(provider, coverageSubprovider); +// } +// if (isProfilerEnabled) { +// if (testProvider === ProviderType.Ganache) { +// logUtils.warn( +// "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", +// ); +// process.exit(1); +// } +// const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); +// logUtils.log( +// "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", +// ); +// profilerSubprovider.stop(); +// prependSubprovider(provider, profilerSubprovider); +// } +if (isRevertTraceEnabled) { + const revertTraceSubprovider = revertTrace.getRevertTraceSubproviderSingleton(); + prependSubprovider(provider, revertTraceSubprovider); } export const web3Wrapper = new Web3Wrapper(provider); -- cgit v1.2.3 From 263bfb1bdad8acdb9b534edf6e79024e8da35721 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 15:46:59 -0700 Subject: Fix a bug in revert_trace.ts --- packages/contracts/src/utils/web3_wrapper.ts | 46 ++++++++++++++-------------- 1 file changed, 23 insertions(+), 23 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index b8e8ed8ce..f51ad435b 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -51,29 +51,29 @@ const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); // TODO(albrow): Include revertTrace checks in the warnings below. -// if (isCoverageEnabled && isProfilerEnabled) { -// throw new Error( -// `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, -// ); -// } -// if (isCoverageEnabled) { -// const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); -// prependSubprovider(provider, coverageSubprovider); -// } -// if (isProfilerEnabled) { -// if (testProvider === ProviderType.Ganache) { -// logUtils.warn( -// "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", -// ); -// process.exit(1); -// } -// const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); -// logUtils.log( -// "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", -// ); -// profilerSubprovider.stop(); -// prependSubprovider(provider, profilerSubprovider); -// } +if (isCoverageEnabled && isProfilerEnabled) { + throw new Error( + `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, + ); +} +if (isCoverageEnabled) { + const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); + prependSubprovider(provider, coverageSubprovider); +} +if (isProfilerEnabled) { + if (testProvider === ProviderType.Ganache) { + logUtils.warn( + "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", + ); + process.exit(1); + } + const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); + logUtils.log( + "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", + ); + profilerSubprovider.stop(); + prependSubprovider(provider, profilerSubprovider); +} if (isRevertTraceEnabled) { const revertTraceSubprovider = revertTrace.getRevertTraceSubproviderSingleton(); prependSubprovider(provider, revertTraceSubprovider); -- cgit v1.2.3 From d9292a70bfa00715b6b007b0ecd696db02b1d042 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:00:24 -0700 Subject: Remove unused variables and other small fixes --- packages/contracts/src/utils/web3_wrapper.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index f51ad435b..772e4c613 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -7,6 +7,8 @@ import { coverage } from './coverage'; import { profiler } from './profiler'; import { revertTrace } from './revert_trace'; +import * as _ from 'lodash'; + enum ProviderType { Ganache = 'ganache', Geth = 'geth', @@ -50,11 +52,10 @@ export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); -// TODO(albrow): Include revertTrace checks in the warnings below. -if (isCoverageEnabled && isProfilerEnabled) { - throw new Error( - `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, - ); +const enabledSubproviderCount = _.filter([isCoverageEnabled, isProfilerEnabled, isRevertTraceEnabled], _.identity) + .length; +if (enabledSubproviderCount > 1) { + throw new Error(`Only one of coverage, profiler, and revert trace subproviders can be enabled at a time`); } if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); -- cgit v1.2.3 From 5a8539a1228baeb085ed7851245337f27ee1d974 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:04:08 -0700 Subject: Fix linter errors and remove coverage.json --- packages/contracts/src/utils/web3_wrapper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 772e4c613..67b71cdeb 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -2,13 +2,12 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; import { prependSubprovider } from '@0xproject/subproviders'; import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import * as _ from 'lodash'; import { coverage } from './coverage'; import { profiler } from './profiler'; import { revertTrace } from './revert_trace'; -import * as _ from 'lodash'; - enum ProviderType { Ganache = 'ganache', Geth = 'geth', -- cgit v1.2.3 From 7032825e35e825e42196d4ccc23b1a1f7fb8038a Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:53:48 -0700 Subject: Change wording of error message when you try to use more than one subprovider --- packages/contracts/src/utils/web3_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 67b71cdeb..c9d83a02d 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -54,7 +54,7 @@ const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); const enabledSubproviderCount = _.filter([isCoverageEnabled, isProfilerEnabled, isRevertTraceEnabled], _.identity) .length; if (enabledSubproviderCount > 1) { - throw new Error(`Only one of coverage, profiler, and revert trace subproviders can be enabled at a time`); + throw new Error(`Only one of coverage, profiler, or revert trace subproviders can be enabled at a time`); } if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); -- cgit v1.2.3