diff options
23 files changed, 385 insertions, 12 deletions
diff --git a/.circleci/config.yml b/.circleci/config.yml index b93f62fa7..43e542a86 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -80,6 +80,7 @@ jobs: - run: yarn wsrun test:circleci @0xproject/sra-report - run: yarn wsrun test:circleci @0xproject/subproviders - run: yarn wsrun test:circleci @0xproject/web3-wrapper + - run: yarn wsrun test:circleci @0xproject/utils - save_cache: key: coverage-0xjs-{{ .Environment.CIRCLE_SHA1 }} paths: diff --git a/packages/0x.js/CHANGELOG.json b/packages/0x.js/CHANGELOG.json index 4f39a9b8c..913b7a76e 100644 --- a/packages/0x.js/CHANGELOG.json +++ b/packages/0x.js/CHANGELOG.json @@ -1,5 +1,13 @@ [ { + "version": "1.0.1-rc.3", + "changes": [ + { + "note": "Dependencies updated" + } + ] + }, + { "version": "1.0.1-rc.2", "changes": [ { diff --git a/packages/0x.js/test/0x.js_test.ts b/packages/0x.js/test/0x.js_test.ts index b4baecb1e..be2a94482 100644 --- a/packages/0x.js/test/0x.js_test.ts +++ b/packages/0x.js/test/0x.js_test.ts @@ -48,10 +48,11 @@ describe('ZeroEx library', () => { const ethSignSignature = '0x1B61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc3340349190569279751135161d22529dc25add4f6069af05be04cacbda2ace225403'; const address = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; + const bytes32Zeros = '0x0000000000000000000000000000000000000000000000000000000000000000'; it("should return false if the data doesn't pertain to the signature & address", async () => { - return expect((zeroEx.exchange as any).isValidSignatureAsync('0x0', address, ethSignSignature)).to.become( - false, - ); + return expect( + (zeroEx.exchange as any).isValidSignatureAsync(bytes32Zeros, address, ethSignSignature), + ).to.become(false); }); it("should return false if the address doesn't pertain to the signature & data", async () => { const validUnrelatedAddress = '0x8b0292b11a196601ed2ce54b665cafeca0347d42'; diff --git a/packages/base-contract/CHANGELOG.json b/packages/base-contract/CHANGELOG.json index 9dddde18a..b041e13da 100644 --- a/packages/base-contract/CHANGELOG.json +++ b/packages/base-contract/CHANGELOG.json @@ -1,5 +1,14 @@ [ { + "version": "2.0.0-rc.1", + "changes": [ + { + "pr": 915, + "note": "Added strict encoding/decoding checks for sendTransaction and call" + } + ] + }, + { "timestamp": 1532619515, "version": "1.0.4", "changes": [ diff --git a/packages/base-contract/src/index.ts b/packages/base-contract/src/index.ts index a240fb8b6..12f974445 100644 --- a/packages/base-contract/src/index.ts +++ b/packages/base-contract/src/index.ts @@ -82,6 +82,27 @@ export class BaseContract { } return txDataWithDefaults; } + // Throws if the given arguments cannot be safely/correctly encoded based on + // the given inputAbi. An argument may not be considered safely encodeable + // if it overflows the corresponding Solidity type, there is a bug in the + // encoder, or the encoder performs unsafe type coercion. + public static strictArgumentEncodingCheck(inputAbi: DataItem[], args: any[]): void { + const coder = ethers.utils.AbiCoder.defaultCoder; + const params = abiUtils.parseEthersParams(inputAbi); + const rawEncoded = coder.encode(params.names, params.types, args); + const rawDecoded = coder.decode(params.names, params.types, rawEncoded); + for (let i = 0; i < rawDecoded.length; i++) { + const original = args[i]; + const decoded = rawDecoded[i]; + if (!abiUtils.isAbiDataEqual(params.names[i], params.types[i], original, decoded)) { + throw new Error( + `Cannot safely encode argument: ${params.names[i]} (${original}) of type ${ + params.types[i] + }. (Possible type overflow or other encoding error)`, + ); + } + } + } protected _lookupEthersInterface(functionSignature: string): ethers.Interface { const ethersInterface = this._ethersInterfacesByFunctionSignature[functionSignature]; if (_.isUndefined(ethersInterface)) { diff --git a/packages/base-contract/test/base_contract_test.ts b/packages/base-contract/test/base_contract_test.ts new file mode 100644 index 000000000..2c31d1f11 --- /dev/null +++ b/packages/base-contract/test/base_contract_test.ts @@ -0,0 +1,114 @@ +import * as chai from 'chai'; +import 'mocha'; + +import { BaseContract } from '../src'; + +const { expect } = chai; + +describe('BaseContract', () => { + describe('strictArgumentEncodingCheck', () => { + it('works for simple types', () => { + BaseContract.strictArgumentEncodingCheck( + [{ name: 'to', type: 'address' }], + ['0xe834ec434daba538cd1b9fe1582052b880bd7e63'], + ); + }); + it('works for array types', () => { + const inputAbi = [ + { + name: 'takerAssetFillAmounts', + type: 'uint256[]', + }, + ]; + const args = [ + ['9000000000000000000', '79000000000000000000', '979000000000000000000', '7979000000000000000000'], + ]; + BaseContract.strictArgumentEncodingCheck(inputAbi, args); + }); + it('works for tuple/struct types', () => { + const inputAbi = [ + { + components: [ + { + name: 'makerAddress', + type: 'address', + }, + { + name: 'takerAddress', + type: 'address', + }, + { + name: 'feeRecipientAddress', + type: 'address', + }, + { + name: 'senderAddress', + type: 'address', + }, + { + name: 'makerAssetAmount', + type: 'uint256', + }, + { + name: 'takerAssetAmount', + type: 'uint256', + }, + { + name: 'makerFee', + type: 'uint256', + }, + { + name: 'takerFee', + type: 'uint256', + }, + { + name: 'expirationTimeSeconds', + type: 'uint256', + }, + { + name: 'salt', + type: 'uint256', + }, + { + name: 'makerAssetData', + type: 'bytes', + }, + { + name: 'takerAssetData', + type: 'bytes', + }, + ], + name: 'order', + type: 'tuple', + }, + ]; + const args = [ + { + makerAddress: '0x6ecbe1db9ef729cbe972c83fb886247691fb6beb', + takerAddress: '0x0000000000000000000000000000000000000000', + feeRecipientAddress: '0xe834ec434daba538cd1b9fe1582052b880bd7e63', + senderAddress: '0x0000000000000000000000000000000000000000', + makerAssetAmount: '0', + takerAssetAmount: '200000000000000000000', + makerFee: '1000000000000000000', + takerFee: '1000000000000000000', + expirationTimeSeconds: '1532563026', + salt: '59342956082154660870994022243365949771115859664887449740907298019908621891376', + makerAssetData: '0xf47261b00000000000000000000000001dc4c1cefef38a777b15aa20260a54e584b16c48', + takerAssetData: '0xf47261b00000000000000000000000001d7022f5b17d2f8b695918fb48fa1089c9f85401', + }, + ]; + BaseContract.strictArgumentEncodingCheck(inputAbi, args); + }); + it('throws for integer overflows', () => { + expect(() => + BaseContract.strictArgumentEncodingCheck([{ name: 'amount', type: 'uint8' }], ['256']), + ).to.throw(); + }); + it('throws for fixed byte array overflows', () => { + expect(() => + BaseContract.strictArgumentEncodingCheck([{ name: 'hash', type: 'bytes8' }], ['0x001122334455667788']), + ).to.throw(); + }); + }); +}); diff --git a/packages/contract-wrappers/CHANGELOG.json b/packages/contract-wrappers/CHANGELOG.json index 02e1acf91..fb077ce73 100644 --- a/packages/contract-wrappers/CHANGELOG.json +++ b/packages/contract-wrappers/CHANGELOG.json @@ -3,6 +3,10 @@ "version": "1.0.1-rc.3", "changes": [ { + "pr": 915, + "note": "Added strict encoding/decoding checks for sendTransaction and call" + }, + { "note": "Add ForwarderWrapper", "pr": 934 }, diff --git a/packages/contract_templates/partials/callAsync.handlebars b/packages/contract_templates/partials/callAsync.handlebars index fcaae57c6..94752691d 100644 --- a/packages/contract_templates/partials/callAsync.handlebars +++ b/packages/contract_templates/partials/callAsync.handlebars @@ -7,6 +7,7 @@ async callAsync( const functionSignature = '{{this.functionSignature}}'; const inputAbi = self._lookupAbi(functionSignature).inputs; [{{> params inputs=inputs}}] = BaseContract._formatABIDataItemList(inputAbi, [{{> params inputs=inputs}}], BaseContract._bigNumberToString.bind(self)); + BaseContract.strictArgumentEncodingCheck(inputAbi, [{{> params inputs=inputs}}]); const ethersFunction = self._lookupEthersInterface(functionSignature).functions.{{this.name}}( {{> params inputs=inputs}} ) as ethers.CallDescription; diff --git a/packages/contract_templates/partials/tx.handlebars b/packages/contract_templates/partials/tx.handlebars index e297d05e6..4340d662e 100644 --- a/packages/contract_templates/partials/tx.handlebars +++ b/packages/contract_templates/partials/tx.handlebars @@ -11,6 +11,7 @@ public {{this.tsName}} = { const self = this as any as {{contractName}}Contract; const inputAbi = self._lookupAbi('{{this.functionSignature}}').inputs; [{{> params inputs=inputs}}] = BaseContract._formatABIDataItemList(inputAbi, [{{> params inputs=inputs}}], BaseContract._bigNumberToString.bind(self)); + BaseContract.strictArgumentEncodingCheck(inputAbi, [{{> params inputs=inputs}}]); const encodedData = self._lookupEthersInterface('{{this.functionSignature}}').functions.{{this.name}}( {{> params inputs=inputs}} ).data; diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index f2bb42c75..bef0547bd 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -113,7 +113,7 @@ describe('MixinSignatureValidator', () => { it('should revert when signature type is unsupported', async () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; - const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; + const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectContractCallFailed( signatureValidator.publicIsValidSignature.callAsync( @@ -126,7 +126,7 @@ describe('MixinSignatureValidator', () => { }); it('should revert when SignatureType=Illegal', async () => { - const unsupportedSignatureHex = `0x${SignatureType.Illegal}`; + const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectContractCallFailed( signatureValidator.publicIsValidSignature.callAsync( @@ -139,7 +139,7 @@ describe('MixinSignatureValidator', () => { }); it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { - const signatureHex = `0x${SignatureType.Invalid}`; + const signatureHex = '0x' + Buffer.from([SignatureType.Invalid]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, diff --git a/packages/fill-scenarios/CHANGELOG.json b/packages/fill-scenarios/CHANGELOG.json index 04e076203..1c3864da2 100644 --- a/packages/fill-scenarios/CHANGELOG.json +++ b/packages/fill-scenarios/CHANGELOG.json @@ -6,6 +6,9 @@ "note": "Updated to use latest orderFactory interface, fixed `feeRecipient` spelling error in public interface", "pr": 936 + }, + { + "note": "Dependencies updated" } ] }, diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index 776bd67ec..fa82976ad 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -10,6 +10,9 @@ { "note": "Added marketUtils", "pr": 937 + }, + { + "note": "Dependencies updated" } ] }, diff --git a/packages/order-utils/test/signature_utils_test.ts b/packages/order-utils/test/signature_utils_test.ts index 5714f9671..baae2b414 100644 --- a/packages/order-utils/test/signature_utils_test.ts +++ b/packages/order-utils/test/signature_utils_test.ts @@ -22,7 +22,8 @@ describe('Signature utils', () => { let address = '0x5409ed021d9299bf6814279a6a1411a7e866a631'; it("should return false if the data doesn't pertain to the signature & address", async () => { - expect(await isValidSignatureAsync(provider, '0x0', ethSignSignature, address)).to.be.false(); + const bytes32Zeros = '0x0000000000000000000000000000000000000000000000000000000000000000'; + expect(await isValidSignatureAsync(provider, bytes32Zeros, ethSignSignature, address)).to.be.false(); }); it("should return false if the address doesn't pertain to the signature & data", async () => { const validUnrelatedAddress = '0x8b0292b11a196601ed2ce54b665cafeca0347d42'; diff --git a/packages/order-watcher/CHANGELOG.json b/packages/order-watcher/CHANGELOG.json index 08c1f7f58..0c9ef7a8a 100644 --- a/packages/order-watcher/CHANGELOG.json +++ b/packages/order-watcher/CHANGELOG.json @@ -1,5 +1,13 @@ [ { + "version": "1.0.1-rc.3", + "changes": [ + { + "note": "Dependencies updated" + } + ] + }, + { "version": "1.0.1-rc.2", "changes": [ { diff --git a/packages/typescript-typings/types/ethers/index.d.ts b/packages/typescript-typings/types/ethers/index.d.ts index f869196e0..875563ba2 100644 --- a/packages/typescript-typings/types/ethers/index.d.ts +++ b/packages/typescript-typings/types/ethers/index.d.ts @@ -34,4 +34,22 @@ declare module 'ethers' { const enum errors { INVALID_ARGUMENT = 'INVALID_ARGUMENT', } + + export type ParamName = null | string | NestedParamName; + + export interface NestedParamName { + name: string | null; + names: ParamName[]; + } + + export const utils: { + AbiCoder: { + defaultCoder: AbiCoder; + }; + }; + + export interface AbiCoder { + encode: (names: ParamName[] | string[], types: string[] | any[], args: any[] | undefined) => string; + decode: (names: ParamName[] | string[], types: string[] | string, data: string | undefined) => any; + } } diff --git a/packages/utils/coverage/.gitkeep b/packages/utils/coverage/.gitkeep new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/packages/utils/coverage/.gitkeep diff --git a/packages/utils/package.json b/packages/utils/package.json index ee150cb0e..ee5fc264f 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -5,12 +5,17 @@ "node": ">=6.12" }, "description": "0x TS utils", - "main": "lib/index.js", - "types": "lib/index.d.ts", + "main": "lib/src/index.js", + "types": "lib/src/index.d.ts", "scripts": { "watch_without_deps": "tsc -w", "build": "tsc && copyfiles -u 2 './lib/monorepo_scripts/**/*' ./scripts", "clean": "shx rm -rf lib scripts", + "test": "yarn run_mocha", + "test:circleci": "yarn test:coverage", + "run_mocha": "mocha --require source-map-support/register --require make-promises-safe lib/test/**/*_test.js --bail --exit", + "test:coverage": "nyc npm run test --all && yarn coverage:report:lcov", + "coverage:report:lcov": "nyc report --reporter=text-lcov > coverage/lcov.info", "lint": "tslint --project .", "manual:postpublish": "yarn build; node ./scripts/postpublish.js" }, @@ -27,12 +32,15 @@ "@0xproject/monorepo-scripts": "^1.0.4", "@0xproject/tslint-config": "^1.0.4", "@types/lodash": "4.14.104", + "@types/mocha": "^2.2.42", "copyfiles": "^1.2.0", "make-promises-safe": "^1.1.0", "npm-run-all": "^4.1.2", "shx": "^0.2.2", "tslint": "5.11.0", - "typescript": "2.9.2" + "typescript": "2.9.2", + "chai": "^4.0.1", + "mocha": "^4.0.1" }, "dependencies": { "@0xproject/types": "^1.0.1-rc.3", diff --git a/packages/utils/src/abi_utils.ts b/packages/utils/src/abi_utils.ts index 421dd405c..c9b70966c 100644 --- a/packages/utils/src/abi_utils.ts +++ b/packages/utils/src/abi_utils.ts @@ -1,7 +1,160 @@ import { AbiDefinition, AbiType, ContractAbi, DataItem, MethodAbi } from 'ethereum-types'; +import * as ethers from 'ethers'; import * as _ from 'lodash'; +import { BigNumber } from './configured_bignumber'; + +// Note(albrow): This function is unexported in ethers.js. Copying it here for +// now. +// Source: https://github.com/ethers-io/ethers.js/blob/884593ab76004a808bf8097e9753fb5f8dcc3067/contracts/interface.js#L30 +function parseEthersParams(params: DataItem[]): { names: ethers.ParamName[]; types: string[] } { + const names: ethers.ParamName[] = []; + const types: string[] = []; + + params.forEach((param: DataItem) => { + if (param.components != null) { + let suffix = ''; + const arrayBracket = param.type.indexOf('['); + if (arrayBracket >= 0) { + suffix = param.type.substring(arrayBracket); + } + + const result = parseEthersParams(param.components); + names.push({ name: param.name || null, names: result.names }); + types.push('tuple(' + result.types.join(',') + ')' + suffix); + } else { + names.push(param.name || null); + types.push(param.type); + } + }); + + return { + names, + types, + }; +} + +// returns true if x is equal to y and false otherwise. Performs some minimal +// type conversion and data massaging for x and y, depending on type. name and +// type should typically be derived from parseEthersParams. +function isAbiDataEqual(name: ethers.ParamName, type: string, x: any, y: any): boolean { + if (_.isUndefined(x) && _.isUndefined(y)) { + return true; + } else if (_.isUndefined(x) && !_.isUndefined(y)) { + return false; + } else if (!_.isUndefined(x) && _.isUndefined(y)) { + return false; + } + if (_.endsWith(type, '[]')) { + // For array types, we iterate through the elements and check each one + // individually. Strangely, name does not need to be changed in this + // case. + if (x.length !== y.length) { + return false; + } + const newType = _.trimEnd(type, '[]'); + for (let i = 0; i < x.length; i++) { + if (!isAbiDataEqual(name, newType, x[i], y[i])) { + return false; + } + } + return true; + } + if (_.startsWith(type, 'tuple(')) { + if (_.isString(name)) { + throw new Error('Internal error: type was tuple but names was a string'); + } else if (_.isNull(name)) { + throw new Error('Internal error: type was tuple but names was null'); + } + // For tuples, we iterate through the underlying values and check each + // one individually. + const types = splitTupleTypes(type); + if (types.length !== name.names.length) { + throw new Error( + `Internal error: parameter types/names length mismatch (${types.length} != ${name.names.length})`, + ); + } + for (let i = 0; i < types.length; i++) { + // For tuples, name is an object with a names property that is an + // array. As an example, for orders, name looks like: + // + // { + // name: 'orders', + // names: [ + // 'makerAddress', + // // ... + // 'takerAssetData' + // ] + // } + // + const nestedName = _.isString(name.names[i]) + ? (name.names[i] as string) + : ((name.names[i] as ethers.NestedParamName).name as string); + if (!isAbiDataEqual(name.names[i], types[i], x[nestedName], y[nestedName])) { + return false; + } + } + return true; + } else if (type === 'address' || type === 'bytes') { + // HACK(albrow): ethers.js returns the checksummed address even when + // initially passed in a non-checksummed address. To account for that, + // we convert to lowercase before comparing. + return _.isEqual(_.toLower(x), _.toLower(y)); + } else if (_.startsWith(type, 'uint') || _.startsWith(type, 'int')) { + return new BigNumber(x).eq(new BigNumber(y)); + } + return _.isEqual(x, y); +} + +// splitTupleTypes splits a tuple type string (of the form `tuple(X)` where X is +// any other type or list of types) into its component types. It works with +// nested tuples, so, e.g., `tuple(tuple(uint256,address),bytes32)` will yield: +// `['tuple(uint256,address)', 'bytes32']`. It expects exactly one tuple type as +// an argument (not an array). +function splitTupleTypes(type: string): string[] { + if (_.endsWith(type, '[]')) { + throw new Error('Internal error: array types are not supported'); + } else if (!_.startsWith(type, 'tuple(')) { + throw new Error('Internal error: expected tuple type but got non-tuple type: ' + type); + } + // Trim the outtermost tuple(). + const trimmedType = type.substring('tuple('.length, type.length - 1); + const types: string[] = []; + let currToken = ''; + let parenCount = 0; + // Tokenize the type string while keeping track of parentheses. + for (const char of trimmedType) { + switch (char) { + case '(': + parenCount += 1; + currToken += char; + break; + case ')': + parenCount -= 1; + currToken += char; + break; + case ',': + if (parenCount === 0) { + types.push(currToken); + currToken = ''; + break; + } else { + currToken += char; + break; + } + default: + currToken += char; + break; + } + } + types.push(currToken); + return types; +} + export const abiUtils = { + parseEthersParams, + isAbiDataEqual, + splitTupleTypes, parseFunctionParam(param: DataItem): string { if (param.type === 'tuple') { // Parse out tuple types into {type_1, type_2, ..., type_N} diff --git a/packages/utils/test/abi_utils_test.ts b/packages/utils/test/abi_utils_test.ts new file mode 100644 index 000000000..0ebee64c4 --- /dev/null +++ b/packages/utils/test/abi_utils_test.ts @@ -0,0 +1,19 @@ +import * as chai from 'chai'; +import 'mocha'; + +import { abiUtils } from '../src'; + +const expect = chai.expect; + +describe('abiUtils', () => { + describe('splitTupleTypes', () => { + it('handles basic types', () => { + const got = abiUtils.splitTupleTypes('tuple(bytes,uint256,address)'); + expect(got).to.deep.equal(['bytes', 'uint256', 'address']); + }); + it('handles nested tuple types', () => { + const got = abiUtils.splitTupleTypes('tuple(tuple(bytes,uint256),address)'); + expect(got).to.deep.equal(['tuple(bytes,uint256)', 'address']); + }); + }); +}); diff --git a/packages/utils/tsconfig.json b/packages/utils/tsconfig.json index c56d255d5..8b4cd47a2 100644 --- a/packages/utils/tsconfig.json +++ b/packages/utils/tsconfig.json @@ -3,5 +3,5 @@ "compilerOptions": { "outDir": "lib" }, - "include": ["./src/**/*"] + "include": ["src/**/*", "test/**/*"] } diff --git a/packages/website/public/images/team/amir.jpeg b/packages/website/public/images/team/amir.jpeg Binary files differdeleted file mode 100644 index 7ee16263a..000000000 --- a/packages/website/public/images/team/amir.jpeg +++ /dev/null diff --git a/packages/website/public/images/team/amir.png b/packages/website/public/images/team/amir.png Binary files differnew file mode 100644 index 000000000..2bb795d50 --- /dev/null +++ b/packages/website/public/images/team/amir.png diff --git a/packages/website/ts/pages/about/about.tsx b/packages/website/ts/pages/about/about.tsx index b9bc906bd..1574f65e0 100644 --- a/packages/website/ts/pages/about/about.tsx +++ b/packages/website/ts/pages/about/about.tsx @@ -27,7 +27,7 @@ const teamRow1: ProfileInfo[] = [ title: 'Co-founder & CTO', description: `Smart contract R&D. Previously fixed income trader at DRW. \ Finance at University of Illinois, Urbana-Champaign.`, - image: '/images/team/amir.jpeg', + image: '/images/team/amir.png', linkedIn: 'https://www.linkedin.com/in/abandeali1/', github: 'https://github.com/abandeali1', medium: 'https://medium.com/@abandeali1', |