From bac6833436960d2a7eb50d89e94fed226a16008b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 5 Jun 2017 16:22:56 +0200 Subject: [PATCH 01/18] Make methods accept senderAccount --- src/0x.js.ts | 20 +++------ src/contract_wrappers/exchange_wrapper.ts | 35 ++++++--------- src/contract_wrappers/token_wrapper.ts | 11 +++-- src/types.ts | 4 +- src/utils/assert.ts | 12 +++-- src/web3_wrapper.ts | 24 +--------- test/0x.js_test.ts | 22 +++++---- test/exchange_wrapper_test.ts | 54 +++++++++-------------- test/utils/fill_scenarios.ts | 13 +----- test/utils/order_factory.ts | 2 +- 10 files changed, 72 insertions(+), 125 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 850827fee0..5e2cd9ed94 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -122,17 +122,11 @@ export class ZeroEx { this.token.invalidateContractInstances(); } /** - * Sets default account for sending transactions. + * Gets accounts available for sending transactions. */ - public setTransactionSenderAccount(account: string): void { - this.web3Wrapper.setDefaultAccount(account); - } - /** - * Get the default account set for sending transactions. - */ - public async getTransactionSenderAccountIfExistsAsync(): Promise { - const senderAccountIfExists = await this.web3Wrapper.getSenderAddressIfExistsAsync(); - return senderAccountIfExists; + public async getAvailableAccountsAsync(): Promise { + const availableAccounts = await this.web3Wrapper.getAvailableAccountsAsync(); + return availableAccounts; } /** * Computes the orderHash for a given order and returns it as a hex encoded string. @@ -167,10 +161,10 @@ export class ZeroEx { * Signs an orderHash and returns it's elliptic curve signature * This method currently supports TestRPC, Geth and Parity above and below V1.6.6 */ - public async signOrderHashAsync(orderHashHex: string): Promise { + public async signOrderHashAsync(orderHashHex: string, senderAccount: string): Promise { assert.isHexString('orderHashHex', orderHashHex); - - const makerAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); + await assert.isSenderAccountHexAsync(this.web3Wrapper, senderAccount); + const makerAddress = senderAccount; let msgHashHex; const nodeVersion = await this.web3Wrapper.getNodeVersionAsync(); diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ee0b2696f2..459e24608e 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -57,7 +57,6 @@ export class ExchangeWrapper extends ContractWrapper { assert.doesConformToSchema('ecSignature', ecSignature, ecSignatureSchema); assert.isETHAddressHex('signerAddressHex', signerAddressHex); - const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); const isValidSignature = await exchangeInstance.isValidSignature.call( @@ -66,9 +65,6 @@ export class ExchangeWrapper extends ContractWrapper { ecSignature.v, ecSignature.r, ecSignature.s, - { - from: senderAddress, - }, ); return isValidSignature; } @@ -119,16 +115,16 @@ export class ExchangeWrapper extends ContractWrapper { * false forgoes this check and causes the smart contract to throw instead. */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - shouldCheckTransfer: boolean): Promise { + shouldCheckTransfer: boolean, senderAccount: string): Promise { assert.doesConformToSchema('signedOrder', SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); assert.isBigNumber('fillTakerAmount', fillTakerAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); + await assert.isSenderAccountHexAsync(this.web3Wrapper, senderAccount); - const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAccount); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -154,7 +150,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.r, signedOrder.ecSignature.s, { - from: senderAddress, + from: senderAccount, }, ); const response: ContractResponse = await exchangeInstance.fill( @@ -166,7 +162,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.r, signedOrder.ecSignature.s, { - from: senderAddress, + from: senderAccount, gas, }, ); @@ -207,11 +203,11 @@ export class ExchangeWrapper extends ContractWrapper { } private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string): Promise { + senderAccount: string): Promise { if (fillTakerAmount.eq(0)) { throw new Error(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); } - if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { + if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAccount) { throw new Error(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } const currentUnixTimestampSec = Date.now() / 1000; @@ -220,7 +216,7 @@ export class ExchangeWrapper extends ContractWrapper { } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, - senderAddress, zrxTokenAddress); + senderAccount, zrxTokenAddress); const wouldRoundingErrorOccur = await this.isRoundingErrorAsync( signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, @@ -241,16 +237,16 @@ export class ExchangeWrapper extends ContractWrapper { */ private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, + senderAccount: string, zrxTokenAddress: string): Promise { const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); - const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); + const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAccount); const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAddress); + senderAccount); // exchangeRate is the price of one maker token denominated in taker tokens const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); @@ -271,11 +267,11 @@ export class ExchangeWrapper extends ContractWrapper { const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); - const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAccount); const makerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, signedOrder.maker); const takerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAddress); + senderAccount); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); @@ -302,11 +298,8 @@ export class ExchangeWrapper extends ContractWrapper { fillTakerAmount: BigNumber.BigNumber, makerTokenAmount: BigNumber.BigNumber): Promise { const exchangeInstance = await this.getExchangeContractAsync(); - const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const isRoundingError = await exchangeInstance.isRoundingError.call( - takerTokenAmount, fillTakerAmount, makerTokenAmount, { - from: senderAddress, - }, + takerTokenAmount, fillTakerAmount, makerTokenAmount, ); return isRoundingError; } diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index c8b557d0d5..55ad9be294 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -115,21 +115,20 @@ export class TokenWrapper extends ContractWrapper { /** * Transfers `amountInBaseUnits` ERC20 tokens from `fromAddress` to `toAddress`. * Requires the fromAddress to have sufficient funds and to have approved an allowance of - * `amountInBaseUnits` for senderAddress. + * `amountInBaseUnits` for senderAccount. */ public async transferFromAsync(tokenAddress: string, fromAddress: string, toAddress: string, - senderAddress: string, amountInBaseUnits: BigNumber.BigNumber): + senderAccount: string, amountInBaseUnits: BigNumber.BigNumber): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isETHAddressHex('fromAddress', fromAddress); assert.isETHAddressHex('toAddress', toAddress); - assert.isETHAddressHex('senderAddress', senderAddress); + await assert.isSenderAccountHexAsync(this.web3Wrapper, senderAccount); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); - await assert.isSenderAddressAvailableAsync(this.web3Wrapper, senderAddress); const tokenContract = await this.getTokenContractAsync(tokenAddress); - const fromAddressAllowance = await this.getAllowanceAsync(tokenAddress, fromAddress, senderAddress); + const fromAddressAllowance = await this.getAllowanceAsync(tokenAddress, fromAddress, senderAccount); if (fromAddressAllowance.lessThan(amountInBaseUnits)) { throw new Error(ZeroExError.INSUFFICIENT_ALLOWANCE_FOR_TRANSFER); } @@ -140,7 +139,7 @@ export class TokenWrapper extends ContractWrapper { } await tokenContract.transferFrom(fromAddress, toAddress, amountInBaseUnits, { - from: senderAddress, + from: senderAccount, }); } private async getTokenContractAsync(tokenAddress: string): Promise { diff --git a/src/types.ts b/src/types.ts index 68194f5481..a02bd02527 100644 --- a/src/types.ts +++ b/src/types.ts @@ -47,7 +47,7 @@ export type CreateContractEvent = (indexFilterValues: IndexFilterValues, export interface ExchangeContract { isValidSignature: { call: (signerAddressHex: string, dataHex: string, v: number, r: string, s: string, - txOpts: TxOpts) => Promise; + txOpts?: TxOpts) => Promise; }; LogFill: CreateContractEvent; LogCancel: CreateContractEvent; @@ -60,7 +60,7 @@ export interface ExchangeContract { }; isRoundingError: { call: (takerTokenAmount: BigNumber.BigNumber, fillTakerAmount: BigNumber.BigNumber, - makerTokenAmount: BigNumber.BigNumber, txOpts: TxOpts) => Promise; + makerTokenAmount: BigNumber.BigNumber, txOpts?: TxOpts) => Promise; }; fill: { (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber, diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 5a31e1b163..2396b8534e 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -26,10 +26,14 @@ export const assert = { const web3 = new Web3(); this.assert(web3.isAddress(value), this.typeAssertionMessage(variableName, 'ETHAddressHex', value)); }, - async isSenderAddressAvailableAsync(web3Wrapper: Web3Wrapper, senderAddress: string) { - const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAddress); - assert.assert(isSenderAddressAvailable, 'Specified senderAddress isn\'t available through the \ - supplied web3 instance'); + async isSenderAccountHexAsync(web3Wrapper: Web3Wrapper, senderAccount: string): Promise { + assert.isETHAddressHex('senderAccount', senderAccount); + await assert.isSenderAccountAvailableAsync(web3Wrapper, senderAccount); + }, + async isSenderAccountAvailableAsync(web3Wrapper: Web3Wrapper, senderAccount: string): Promise { + const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAccount); + assert.assert(isSenderAddressAvailable, `Specified senderAccount ${senderAccount} isn't available through the \ + supplied web3 instance`); }, isNumber(variableName: string, value: number): void { this.assert(_.isFinite(value), this.typeAssertionMessage(variableName, 'number', value)); diff --git a/src/web3_wrapper.ts b/src/web3_wrapper.ts index 9892abcb8e..2508c51169 100644 --- a/src/web3_wrapper.ts +++ b/src/web3_wrapper.ts @@ -23,20 +23,8 @@ export class Web3Wrapper { public setDefaultAccount(address: string): void { this.web3.eth.defaultAccount = address; } - public async getSenderAddressOrThrowAsync(): Promise { - const senderAddressIfExists = await this.getSenderAddressIfExistsAsync(); - assert.assert(!_.isUndefined(senderAddressIfExists), ZeroExError.USER_HAS_NO_ASSOCIATED_ADDRESSES); - return senderAddressIfExists as string; - } - public async getFirstAddressIfExistsAsync(): Promise { - const addresses = await this.getAvailableSenderAddressesAsync(); - if (_.isEmpty(addresses)) { - return undefined; - } - return addresses[0]; - } public async isSenderAddressAvailableAsync(senderAddress: string): Promise { - const addresses = await this.getAvailableSenderAddressesAsync(); + const addresses = await this.getAvailableAccountsAsync(); return _.includes(addresses, senderAddress); } public async getNodeVersionAsync(): Promise { @@ -73,15 +61,7 @@ export class Web3Wrapper { const {timestamp} = await promisify(this.web3.eth.getBlock)(blockHash); return timestamp; } - public async getSenderAddressIfExistsAsync(): Promise { - const defaultAccount = this.web3.eth.defaultAccount; - if (!_.isUndefined(defaultAccount)) { - return defaultAccount; - } - const firstAccount = await this.getFirstAddressIfExistsAsync(); - return firstAccount; - } - private async getAvailableSenderAddressesAsync(): Promise { + public async getAvailableAccountsAsync(): Promise { const addresses: string[] = await promisify(this.web3.eth.getAccounts)(); return addresses; } diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index 8686b42eb5..a82ec45ba0 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -191,6 +191,13 @@ describe('ZeroEx library', () => { }); describe('#signOrderHashAsync', () => { let stubs: Sinon.SinonStub[] = []; + let makerAccount: string; + const web3 = web3Factory.create(); + const zeroEx = new ZeroEx(web3); + before('get maker account', async () => { + const availableAccounts = await zeroEx.getAvailableAccountsAsync(); + makerAccount = availableAccounts[0]; + }); afterEach(() => { // clean up any stubs after the test has completed _.each(stubs, s => s.restore()); @@ -203,10 +210,7 @@ describe('ZeroEx library', () => { r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33', s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254', }; - - const web3 = web3Factory.create(); - const zeroEx = new ZeroEx(web3); - const ecSignature = await zeroEx.signOrderHashAsync(orderHash); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAccount); expect(ecSignature).to.deep.equal(expectedECSignature); }); it ('should return the correct ECSignature on Parity > V1.6.6', async () => { @@ -219,9 +223,6 @@ describe('ZeroEx library', () => { r: '0x22109d11d79cb8bf96ed88625e1cd9558800c4073332a9a02857499883ee5ce3', s: '0x050aa3cc1f2c435e67e114cdce54b9527b4f50548342401bc5d2b77adbdacb02', }; - - const web3 = web3Factory.create(); - const zeroEx = new ZeroEx(web3); stubs = [ Sinon.stub((zeroEx as any).web3Wrapper, 'getNodeVersionAsync') .returns(Promise.resolve(newParityNodeVersion)), @@ -230,7 +231,7 @@ describe('ZeroEx library', () => { Sinon.stub(ZeroEx, 'isValidSignature').returns(true), ]; - const ecSignature = await zeroEx.signOrderHashAsync(orderHash); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAccount); expect(ecSignature).to.deep.equal(expectedECSignature); }); it ('should return the correct ECSignature on Parity < V1.6.6', async () => { @@ -243,9 +244,6 @@ describe('ZeroEx library', () => { r: '0xc80bedc6756722672753413efdd749b5adbd4fd552595f59c13427407ee9aee0', s: '0x2dea66f25a608bbae457e020fb6decb763deb8b7192abab624997242da248960', }; - - const web3 = web3Factory.create(); - const zeroEx = new ZeroEx(web3); stubs = [ Sinon.stub((zeroEx as any).web3Wrapper, 'getNodeVersionAsync') .returns(Promise.resolve(newParityNodeVersion)), @@ -254,7 +252,7 @@ describe('ZeroEx library', () => { Sinon.stub(ZeroEx, 'isValidSignature').returns(true), ]; - const ecSignature = await zeroEx.signOrderHashAsync(orderHash); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAccount); expect(ecSignature).to.deep.equal(expectedECSignature); }); }); diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index 4f3a48b26a..a841f82ec9 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -138,9 +138,6 @@ describe('ExchangeWrapper', () => { makerTokenAddress = makerToken.address; takerTokenAddress = takerToken.address; }); - afterEach('reset default account', () => { - zeroEx.setTransactionSenderAccount(userAddresses[0]); - }); describe('failed fills', () => { it('should throw when the fill amount is zero', async () => { const fillableAmount = new BigNumber(5); @@ -148,9 +145,8 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const zeroFillAmount = new BigNumber(0); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, zeroFillAmount, shouldCheckTransfer, + signedOrder, zeroFillAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); }); it('should throw when sender is not a taker', async () => { @@ -159,7 +155,7 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); it('should throw when order is expired', async () => { @@ -168,9 +164,8 @@ describe('ExchangeWrapper', () => { const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, ); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_EXPIRED); }); describe('should throw when not enough balance or allowance to fulfill the order', () => { @@ -187,36 +182,32 @@ describe('ExchangeWrapper', () => { await zeroEx.token.transferAsync( takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker, ); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); }); it('should throw when taker allowance is less than fill amount', async () => { const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, newAllowanceWhichIsLessThanFillAmount); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); }); it('should throw when maker balance is less than maker fill amount', async () => { await zeroEx.token.transferAsync( makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, ); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); }); it('should throw when maker allowance is less than maker fill amount', async () => { const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, newAllowanceWhichIsLessThanFillAmount); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); }); }); @@ -228,9 +219,8 @@ describe('ExchangeWrapper', () => { makerAmount, takerAmount, ); const fillTakerAmountThatCausesRoundingError = new BigNumber(3); - zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, + signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); }); describe('should throw when not enough balance or allowance to pay fees', () => { @@ -243,7 +233,6 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, fillableAmount, feeRecipient, ); - zeroEx.setTransactionSenderAccount(takerAddress); }); it('should throw when maker doesn\'t have enough balance to pay fees', async () => { const balanceToSubtractFromMaker = new BigNumber(1); @@ -251,7 +240,7 @@ describe('ExchangeWrapper', () => { zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, ); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); }); it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { @@ -259,7 +248,7 @@ describe('ExchangeWrapper', () => { await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, makerAddress, newAllowanceWhichIsLessThanFees); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); }); it('should throw when taker doesn\'t have enough balance to pay fees', async () => { @@ -268,7 +257,7 @@ describe('ExchangeWrapper', () => { zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, ); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); }); it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { @@ -276,7 +265,7 @@ describe('ExchangeWrapper', () => { await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, newAllowanceWhichIsLessThanFees); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmount, shouldCheckTransfer, + signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress, )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); }); }); @@ -295,8 +284,7 @@ describe('ExchangeWrapper', () => { .to.be.bignumber.equal(0); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) .to.be.bignumber.equal(fillableAmount); - zeroEx.setTransactionSenderAccount(takerAddress); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer); + await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) @@ -312,8 +300,7 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const partialFillAmount = new BigNumber(3); - zeroEx.setTransactionSenderAccount(takerAddress); - await zeroEx.exchange.fillOrderAsync(signedOrder, partialFillAmount, shouldCheckTransfer); + await zeroEx.exchange.fillOrderAsync(signedOrder, partialFillAmount, shouldCheckTransfer, takerAddress); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) @@ -331,8 +318,7 @@ describe('ExchangeWrapper', () => { makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, fillableAmount, feeRecipient, ); - zeroEx.setTransactionSenderAccount(takerAddress); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer); + await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer, takerAddress); expect(await zeroEx.token.getBalanceAsync(zrxTokenAddress, feeRecipient)) .to.be.bignumber.equal(makerFee.plus(takerFee)); }); @@ -447,8 +433,9 @@ describe('ExchangeWrapper', () => { done(); }); const fillTakerAmountInBaseUnits = new BigNumber(1); - zeroEx.setTransactionSenderAccount(takerAddress); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer); + await zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, takerAddress, + ); })(); }); it('Outstanding subscriptions are cancelled when zeroEx.setProviderAsync called', (done: DoneCallback) => { @@ -473,8 +460,9 @@ describe('ExchangeWrapper', () => { }); const fillTakerAmountInBaseUnits = new BigNumber(1); - zeroEx.setTransactionSenderAccount(takerAddress); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer); + await zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, takerAddress, + ); })(); }); }); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index d186593b98..075906eaa5 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -54,19 +54,14 @@ export class FillScenarios { public async createPartiallyFilledSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, takerAddress: string, fillableAmount: BigNumber.BigNumber, partialFillAmount: BigNumber.BigNumber) { - const prevSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync(); + const prevSenderAccount = await this.zeroEx.getAvailableAccountsAsync(); const [makerAddress] = this.userAddresses; const signedOrder = await this.createAsymmetricFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, fillableAmount, ); - - this.zeroEx.setTransactionSenderAccount(takerAddress); const shouldCheckTransfer = false; - await this.zeroEx.exchange.fillOrderAsync(signedOrder, partialFillAmount, shouldCheckTransfer); - - // Re-set sender account so as to avoid introducing side-effects - this.zeroEx.setTransactionSenderAccount(prevSenderAccount as string); + await this.zeroEx.exchange.fillOrderAsync(signedOrder, partialFillAmount, shouldCheckTransfer, takerAddress); return signedOrder; } private async createAsymmetricFillableSignedOrderWithFeesAsync( @@ -89,14 +84,10 @@ export class FillScenarios { await this.zeroEx.token.setProxyAllowanceAsync(this.zrxTokenAddress, takerAddress, takerFee); } - const prevTransactionSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync(); - this.zeroEx.setTransactionSenderAccount(makerAddress); const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress, takerAddress, makerFee, takerFee, makerFillableAmount, makerTokenAddress, takerFillableAmount, takerTokenAddress, feeRecepient, expirationUnixTimestampSec); - // We re-set the transactionSender to avoid introducing side-effects - this.zeroEx.setTransactionSenderAccount(prevTransactionSenderAccount as string); return signedOrder; } } diff --git a/test/utils/order_factory.ts b/test/utils/order_factory.ts index 373dbddc60..6f5fa7286d 100644 --- a/test/utils/order_factory.ts +++ b/test/utils/order_factory.ts @@ -36,7 +36,7 @@ export const orderFactory = { expirationUnixTimestampSec, }; const orderHash = await zeroEx.getOrderHashHexAsync(order); - const ecSignature = await zeroEx.signOrderHashAsync(orderHash); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, maker); const signedOrder: SignedOrder = _.assign(order, {ecSignature}); return signedOrder; }, From 2a080427278d040d2e978ceb806e61818f13ae2f Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 10:25:28 +0200 Subject: [PATCH 02/18] Add variableName to isSenderAddcountHexAsync --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/contract_wrappers/token_wrapper.ts | 6 +++--- src/utils/assert.ts | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 459e24608e..35ba7770c4 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -121,7 +121,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrderSchema); assert.isBigNumber('fillTakerAmount', fillTakerAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); - await assert.isSenderAccountHexAsync(this.web3Wrapper, senderAccount); + await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAccount); diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index 55ad9be294..758eb32f7a 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -38,7 +38,7 @@ export class TokenWrapper extends ContractWrapper { */ public async setAllowanceAsync(tokenAddress: string, ownerAddress: string, spenderAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { - assert.isETHAddressHex('ownerAddress', ownerAddress); + await assert.isSenderAccountHexAsync('ownerAddress', ownerAddress, this.web3Wrapper); assert.isETHAddressHex('spenderAddress', spenderAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); @@ -97,7 +97,7 @@ export class TokenWrapper extends ContractWrapper { public async transferAsync(tokenAddress: string, fromAddress: string, toAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); - assert.isETHAddressHex('fromAddress', fromAddress); + await assert.isSenderAccountHexAsync('fromAddress', fromAddress, this.web3Wrapper); assert.isETHAddressHex('toAddress', toAddress); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); @@ -123,7 +123,7 @@ export class TokenWrapper extends ContractWrapper { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isETHAddressHex('fromAddress', fromAddress); assert.isETHAddressHex('toAddress', toAddress); - await assert.isSenderAccountHexAsync(this.web3Wrapper, senderAccount); + await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); const tokenContract = await this.getTokenContractAsync(tokenAddress); diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 2396b8534e..0f45a62ff9 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -26,13 +26,14 @@ export const assert = { const web3 = new Web3(); this.assert(web3.isAddress(value), this.typeAssertionMessage(variableName, 'ETHAddressHex', value)); }, - async isSenderAccountHexAsync(web3Wrapper: Web3Wrapper, senderAccount: string): Promise { - assert.isETHAddressHex('senderAccount', senderAccount); + async isSenderAccountHexAsync(variableName: string, senderAccount: string, + web3Wrapper: Web3Wrapper): Promise { + assert.isETHAddressHex(variableName, senderAccount); await assert.isSenderAccountAvailableAsync(web3Wrapper, senderAccount); }, async isSenderAccountAvailableAsync(web3Wrapper: Web3Wrapper, senderAccount: string): Promise { const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAccount); - assert.assert(isSenderAddressAvailable, `Specified senderAccount ${senderAccount} isn't available through the \ + assert.assert(isSenderAddressAvailable, `Specified sender account ${senderAccount} isn't available through the \ supplied web3 instance`); }, isNumber(variableName: string, value: number): void { From fc912aff68479d099f2fc1a0671226503ac8e03c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 10:53:57 +0200 Subject: [PATCH 03/18] Fix tests --- src/0x.js.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 5e2cd9ed94..31d56fca55 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -163,7 +163,7 @@ export class ZeroEx { */ public async signOrderHashAsync(orderHashHex: string, senderAccount: string): Promise { assert.isHexString('orderHashHex', orderHashHex); - await assert.isSenderAccountHexAsync(this.web3Wrapper, senderAccount); + await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); const makerAddress = senderAccount; let msgHashHex; From d36fa171533e5c2dd5f511e5c213c27fd7cf3e4d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 11:08:48 +0200 Subject: [PATCH 04/18] Add tests for isSenderAccountHexAsync --- test/assert_test.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/assert_test.ts diff --git a/test/assert_test.ts b/test/assert_test.ts new file mode 100644 index 0000000000..2100611e94 --- /dev/null +++ b/test/assert_test.ts @@ -0,0 +1,33 @@ +import * as chai from 'chai'; +import 'mocha'; +import {ZeroEx} from '../src/0x.js'; +import {assert} from '../src/utils/assert'; +import {web3Factory} from './utils/web3_factory'; + +const expect = chai.expect; + +describe('Assertion library', () => { + const web3 = web3Factory.create(); + const zeroEx = new ZeroEx(web3); + describe('#isSenderAccountHexAsync', () => { + it('throws when address is invalid', async () => { + const address = '0xdeadbeef'; + const varName = 'account'; + return expect(assert.isSenderAccountHexAsync(varName, address, (zeroEx as any).web3Wrapper)) + .to.be.rejectedWith(`Expected ${varName} to be of type ETHAddressHex, encountered: ${address}`); + }); + it('throws when address is unavailable', async () => { + const validUnrelatedAddress = '0x8b0292b11a196601eddce54b665cafeca0347d42'; + const varName = 'account'; + return expect(assert.isSenderAccountHexAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) + .to.be.rejectedWith(`Specified sender account ${validUnrelatedAddress} \ + isn't available through the supplied web3 instance`); + }); + it('doesn\'t throw if account is available', async () => { + const availableAccount = (await zeroEx.getAvailableAccountsAsync())[0]; + const varName = 'account'; + return expect(assert.isSenderAccountHexAsync(varName, availableAccount, (zeroEx as any).web3Wrapper)) + .to.become(undefined); + }); + }); +}); From 8bf9e4167a91c3e6437606ee8799574ebc10eec2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:33:27 +0200 Subject: [PATCH 05/18] Fix comments --- src/0x.js.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 31d56fca55..d0b060ffb9 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -122,7 +122,7 @@ export class ZeroEx { this.token.invalidateContractInstances(); } /** - * Gets accounts available for sending transactions. + * Get accounts via the supplied web3 instance available for sending transactions. */ public async getAvailableAccountsAsync(): Promise { const availableAccounts = await this.web3Wrapper.getAvailableAccountsAsync(); From ce203d9e9d58c41c737ea3b392c253c0da4714ad Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:35:17 +0200 Subject: [PATCH 06/18] Remove redundant reassignment --- src/0x.js.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index d0b060ffb9..3020649718 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -164,7 +164,6 @@ export class ZeroEx { public async signOrderHashAsync(orderHashHex: string, senderAccount: string): Promise { assert.isHexString('orderHashHex', orderHashHex); await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); - const makerAddress = senderAccount; let msgHashHex; const nodeVersion = await this.web3Wrapper.getNodeVersionAsync(); @@ -178,7 +177,7 @@ export class ZeroEx { msgHashHex = ethUtil.bufferToHex(msgHashBuff); } - const signature = await this.web3Wrapper.signTransactionAsync(makerAddress, msgHashHex); + const signature = await this.web3Wrapper.signTransactionAsync(senderAccount, msgHashHex); let signatureData; const [nodeVersionNumber] = findVersions(nodeVersion); @@ -208,7 +207,7 @@ export class ZeroEx { r: ethUtil.bufferToHex(r), s: ethUtil.bufferToHex(s), }; - const isValidSignature = ZeroEx.isValidSignature(orderHashHex, ecSignature, makerAddress); + const isValidSignature = ZeroEx.isValidSignature(orderHashHex, ecSignature, senderAccount); if (!isValidSignature) { throw new Error(ZeroExError.INVALID_SIGNATURE); } From c9fe2b16b9efcef123e8402d38f84a966f49f4f7 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:37:14 +0200 Subject: [PATCH 07/18] Rename senderAccount to takerAddress --- src/contract_wrappers/exchange_wrapper.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 35ba7770c4..7b0394391f 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -115,16 +115,16 @@ export class ExchangeWrapper extends ContractWrapper { * false forgoes this check and causes the smart contract to throw instead. */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - shouldCheckTransfer: boolean, senderAccount: string): Promise { + shouldCheckTransfer: boolean, takerAddress: string): Promise { assert.doesConformToSchema('signedOrder', SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); assert.isBigNumber('fillTakerAmount', fillTakerAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); - await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); + await assert.isSenderAccountHexAsync('takerAddress', takerAddress, this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); - await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAccount); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -150,7 +150,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.r, signedOrder.ecSignature.s, { - from: senderAccount, + from: takerAddress, }, ); const response: ContractResponse = await exchangeInstance.fill( @@ -162,7 +162,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.r, signedOrder.ecSignature.s, { - from: senderAccount, + from: takerAddress, gas, }, ); From ed3d1a8142e6f65480f95df7c3991ae00c757e38 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:37:52 +0200 Subject: [PATCH 08/18] Rename assert.isSenderAccountHexAsync to assert.isSenderAddressHexAsync --- src/0x.js.ts | 2 +- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/contract_wrappers/token_wrapper.ts | 6 +++--- src/utils/assert.ts | 2 +- test/assert_test.ts | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 3020649718..65d363218f 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -163,7 +163,7 @@ export class ZeroEx { */ public async signOrderHashAsync(orderHashHex: string, senderAccount: string): Promise { assert.isHexString('orderHashHex', orderHashHex); - await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); + await assert.isSenderAddressHexAsync('senderAccount', senderAccount, this.web3Wrapper); let msgHashHex; const nodeVersion = await this.web3Wrapper.getNodeVersionAsync(); diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 7b0394391f..5c5a46a036 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -121,7 +121,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrderSchema); assert.isBigNumber('fillTakerAmount', fillTakerAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); - await assert.isSenderAccountHexAsync('takerAddress', takerAddress, this.web3Wrapper); + await assert.isSenderAddressHexAsync('takerAddress', takerAddress, this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index 758eb32f7a..326bca1895 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -38,7 +38,7 @@ export class TokenWrapper extends ContractWrapper { */ public async setAllowanceAsync(tokenAddress: string, ownerAddress: string, spenderAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { - await assert.isSenderAccountHexAsync('ownerAddress', ownerAddress, this.web3Wrapper); + await assert.isSenderAddressHexAsync('ownerAddress', ownerAddress, this.web3Wrapper); assert.isETHAddressHex('spenderAddress', spenderAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); @@ -97,7 +97,7 @@ export class TokenWrapper extends ContractWrapper { public async transferAsync(tokenAddress: string, fromAddress: string, toAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); - await assert.isSenderAccountHexAsync('fromAddress', fromAddress, this.web3Wrapper); + await assert.isSenderAddressHexAsync('fromAddress', fromAddress, this.web3Wrapper); assert.isETHAddressHex('toAddress', toAddress); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); @@ -123,7 +123,7 @@ export class TokenWrapper extends ContractWrapper { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isETHAddressHex('fromAddress', fromAddress); assert.isETHAddressHex('toAddress', toAddress); - await assert.isSenderAccountHexAsync('senderAccount', senderAccount, this.web3Wrapper); + await assert.isSenderAddressHexAsync('senderAccount', senderAccount, this.web3Wrapper); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); const tokenContract = await this.getTokenContractAsync(tokenAddress); diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 0f45a62ff9..d9cf57d1c3 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -26,7 +26,7 @@ export const assert = { const web3 = new Web3(); this.assert(web3.isAddress(value), this.typeAssertionMessage(variableName, 'ETHAddressHex', value)); }, - async isSenderAccountHexAsync(variableName: string, senderAccount: string, + async isSenderAddressHexAsync(variableName: string, senderAccount: string, web3Wrapper: Web3Wrapper): Promise { assert.isETHAddressHex(variableName, senderAccount); await assert.isSenderAccountAvailableAsync(web3Wrapper, senderAccount); diff --git a/test/assert_test.ts b/test/assert_test.ts index 2100611e94..f72956eb37 100644 --- a/test/assert_test.ts +++ b/test/assert_test.ts @@ -13,20 +13,20 @@ describe('Assertion library', () => { it('throws when address is invalid', async () => { const address = '0xdeadbeef'; const varName = 'account'; - return expect(assert.isSenderAccountHexAsync(varName, address, (zeroEx as any).web3Wrapper)) + return expect(assert.isSenderAddressHexAsync(varName, address, (zeroEx as any).web3Wrapper)) .to.be.rejectedWith(`Expected ${varName} to be of type ETHAddressHex, encountered: ${address}`); }); it('throws when address is unavailable', async () => { const validUnrelatedAddress = '0x8b0292b11a196601eddce54b665cafeca0347d42'; const varName = 'account'; - return expect(assert.isSenderAccountHexAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) + return expect(assert.isSenderAddressHexAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) .to.be.rejectedWith(`Specified sender account ${validUnrelatedAddress} \ isn't available through the supplied web3 instance`); }); it('doesn\'t throw if account is available', async () => { const availableAccount = (await zeroEx.getAvailableAccountsAsync())[0]; const varName = 'account'; - return expect(assert.isSenderAccountHexAsync(varName, availableAccount, (zeroEx as any).web3Wrapper)) + return expect(assert.isSenderAddressHexAsync(varName, availableAccount, (zeroEx as any).web3Wrapper)) .to.become(undefined); }); }); From f7236e7f499f59d883603798cda11d3636b26f4a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:40:58 +0200 Subject: [PATCH 09/18] Rename senderAccount to senderAddress --- src/0x.js.ts | 8 ++++---- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++-------- src/contract_wrappers/token_wrapper.ts | 10 +++++----- src/utils/assert.ts | 12 ++++++------ 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 65d363218f..0d8e30d8cc 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -161,9 +161,9 @@ export class ZeroEx { * Signs an orderHash and returns it's elliptic curve signature * This method currently supports TestRPC, Geth and Parity above and below V1.6.6 */ - public async signOrderHashAsync(orderHashHex: string, senderAccount: string): Promise { + public async signOrderHashAsync(orderHashHex: string, senderAddress: string): Promise { assert.isHexString('orderHashHex', orderHashHex); - await assert.isSenderAddressHexAsync('senderAccount', senderAccount, this.web3Wrapper); + await assert.isSenderAddressHexAsync('senderAddress', senderAddress, this.web3Wrapper); let msgHashHex; const nodeVersion = await this.web3Wrapper.getNodeVersionAsync(); @@ -177,7 +177,7 @@ export class ZeroEx { msgHashHex = ethUtil.bufferToHex(msgHashBuff); } - const signature = await this.web3Wrapper.signTransactionAsync(senderAccount, msgHashHex); + const signature = await this.web3Wrapper.signTransactionAsync(senderAddress, msgHashHex); let signatureData; const [nodeVersionNumber] = findVersions(nodeVersion); @@ -207,7 +207,7 @@ export class ZeroEx { r: ethUtil.bufferToHex(r), s: ethUtil.bufferToHex(s), }; - const isValidSignature = ZeroEx.isValidSignature(orderHashHex, ecSignature, senderAccount); + const isValidSignature = ZeroEx.isValidSignature(orderHashHex, ecSignature, senderAddress); if (!isValidSignature) { throw new Error(ZeroExError.INVALID_SIGNATURE); } diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 5c5a46a036..dd06830aa1 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -203,11 +203,11 @@ export class ExchangeWrapper extends ContractWrapper { } private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAccount: string): Promise { + senderAddress: string): Promise { if (fillTakerAmount.eq(0)) { throw new Error(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); } - if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAccount) { + if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } const currentUnixTimestampSec = Date.now() / 1000; @@ -216,7 +216,7 @@ export class ExchangeWrapper extends ContractWrapper { } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, - senderAccount, zrxTokenAddress); + senderAddress, zrxTokenAddress); const wouldRoundingErrorOccur = await this.isRoundingErrorAsync( signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, @@ -237,16 +237,16 @@ export class ExchangeWrapper extends ContractWrapper { */ private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAccount: string, + senderAddress: string, zrxTokenAddress: string): Promise { const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); - const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAccount); + const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAccount); + senderAddress); // exchangeRate is the price of one maker token denominated in taker tokens const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); @@ -267,11 +267,11 @@ export class ExchangeWrapper extends ContractWrapper { const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); - const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAccount); + const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); const makerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, signedOrder.maker); const takerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAccount); + senderAddress); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index 326bca1895..34697d97cd 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -115,20 +115,20 @@ export class TokenWrapper extends ContractWrapper { /** * Transfers `amountInBaseUnits` ERC20 tokens from `fromAddress` to `toAddress`. * Requires the fromAddress to have sufficient funds and to have approved an allowance of - * `amountInBaseUnits` for senderAccount. + * `amountInBaseUnits` for senderAddress. */ public async transferFromAsync(tokenAddress: string, fromAddress: string, toAddress: string, - senderAccount: string, amountInBaseUnits: BigNumber.BigNumber): + senderAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isETHAddressHex('fromAddress', fromAddress); assert.isETHAddressHex('toAddress', toAddress); - await assert.isSenderAddressHexAsync('senderAccount', senderAccount, this.web3Wrapper); + await assert.isSenderAddressHexAsync('senderAddress', senderAddress, this.web3Wrapper); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); const tokenContract = await this.getTokenContractAsync(tokenAddress); - const fromAddressAllowance = await this.getAllowanceAsync(tokenAddress, fromAddress, senderAccount); + const fromAddressAllowance = await this.getAllowanceAsync(tokenAddress, fromAddress, senderAddress); if (fromAddressAllowance.lessThan(amountInBaseUnits)) { throw new Error(ZeroExError.INSUFFICIENT_ALLOWANCE_FOR_TRANSFER); } @@ -139,7 +139,7 @@ export class TokenWrapper extends ContractWrapper { } await tokenContract.transferFrom(fromAddress, toAddress, amountInBaseUnits, { - from: senderAccount, + from: senderAddress, }); } private async getTokenContractAsync(tokenAddress: string): Promise { diff --git a/src/utils/assert.ts b/src/utils/assert.ts index d9cf57d1c3..6e10c5d40e 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -26,14 +26,14 @@ export const assert = { const web3 = new Web3(); this.assert(web3.isAddress(value), this.typeAssertionMessage(variableName, 'ETHAddressHex', value)); }, - async isSenderAddressHexAsync(variableName: string, senderAccount: string, + async isSenderAddressHexAsync(variableName: string, senderAddress: string, web3Wrapper: Web3Wrapper): Promise { - assert.isETHAddressHex(variableName, senderAccount); - await assert.isSenderAccountAvailableAsync(web3Wrapper, senderAccount); + assert.isETHAddressHex(variableName, senderAddress); + await assert.isSenderAccountAvailableAsync(web3Wrapper, senderAddress); }, - async isSenderAccountAvailableAsync(web3Wrapper: Web3Wrapper, senderAccount: string): Promise { - const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAccount); - assert.assert(isSenderAddressAvailable, `Specified sender account ${senderAccount} isn't available through the \ + async isSenderAccountAvailableAsync(web3Wrapper: Web3Wrapper, senderAddress: string): Promise { + const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAddress); + assert.assert(isSenderAddressAvailable, `Specified sender account ${senderAddress} isn't available through the \ supplied web3 instance`); }, isNumber(variableName: string, value: number): void { From 279790117be882186685c04099e3d46ec68991b6 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:43:20 +0200 Subject: [PATCH 10/18] Remove message param from before --- test/0x.js_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index a82ec45ba0..061e0cbf4a 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -194,7 +194,7 @@ describe('ZeroEx library', () => { let makerAccount: string; const web3 = web3Factory.create(); const zeroEx = new ZeroEx(web3); - before('get maker account', async () => { + before(async () => { const availableAccounts = await zeroEx.getAvailableAccountsAsync(); makerAccount = availableAccounts[0]; }); From 67e2208a1822732ea125af0c4cd757b9f735ce86 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:48:00 +0200 Subject: [PATCH 11/18] Rename accounts to addreses --- src/0x.js.ts | 8 ++++---- src/utils/assert.ts | 4 ++-- src/web3_wrapper.ts | 8 ++++---- test/0x.js_test.ts | 2 +- test/assert_test.ts | 2 +- test/utils/fill_scenarios.ts | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 0d8e30d8cc..84362bd9ac 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -122,11 +122,11 @@ export class ZeroEx { this.token.invalidateContractInstances(); } /** - * Get accounts via the supplied web3 instance available for sending transactions. + * Get addresses via the supplied web3 instance available for sending transactions. */ - public async getAvailableAccountsAsync(): Promise { - const availableAccounts = await this.web3Wrapper.getAvailableAccountsAsync(); - return availableAccounts; + public async getAvailableAddressesAsync(): Promise { + const availableAddresses = await this.web3Wrapper.getAvailableAddressesAsync(); + return availableAddresses; } /** * Computes the orderHash for a given order and returns it as a hex encoded string. diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 6e10c5d40e..5b4a6fca69 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -29,9 +29,9 @@ export const assert = { async isSenderAddressHexAsync(variableName: string, senderAddress: string, web3Wrapper: Web3Wrapper): Promise { assert.isETHAddressHex(variableName, senderAddress); - await assert.isSenderAccountAvailableAsync(web3Wrapper, senderAddress); + await assert.isSenderAddressAvailableAsync(web3Wrapper, senderAddress); }, - async isSenderAccountAvailableAsync(web3Wrapper: Web3Wrapper, senderAddress: string): Promise { + async isSenderAddressAvailableAsync(web3Wrapper: Web3Wrapper, senderAddress: string): Promise { const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAddress); assert.assert(isSenderAddressAvailable, `Specified sender account ${senderAddress} isn't available through the \ supplied web3 instance`); diff --git a/src/web3_wrapper.ts b/src/web3_wrapper.ts index 2508c51169..e1bb29f852 100644 --- a/src/web3_wrapper.ts +++ b/src/web3_wrapper.ts @@ -17,14 +17,14 @@ export class Web3Wrapper { public isAddress(address: string): boolean { return this.web3.isAddress(address); } - public getDefaultAccount(): string { + public getDefaultAddress(): string { return this.web3.eth.defaultAccount; } - public setDefaultAccount(address: string): void { + public setDefaultAddress(address: string): void { this.web3.eth.defaultAccount = address; } public async isSenderAddressAvailableAsync(senderAddress: string): Promise { - const addresses = await this.getAvailableAccountsAsync(); + const addresses = await this.getAvailableAddressesAsync(); return _.includes(addresses, senderAddress); } public async getNodeVersionAsync(): Promise { @@ -61,7 +61,7 @@ export class Web3Wrapper { const {timestamp} = await promisify(this.web3.eth.getBlock)(blockHash); return timestamp; } - public async getAvailableAccountsAsync(): Promise { + public async getAvailableAddressesAsync(): Promise { const addresses: string[] = await promisify(this.web3.eth.getAccounts)(); return addresses; } diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index 061e0cbf4a..edc2a3493d 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -195,7 +195,7 @@ describe('ZeroEx library', () => { const web3 = web3Factory.create(); const zeroEx = new ZeroEx(web3); before(async () => { - const availableAccounts = await zeroEx.getAvailableAccountsAsync(); + const availableAccounts = await zeroEx.getAvailableAddressesAsync(); makerAccount = availableAccounts[0]; }); afterEach(() => { diff --git a/test/assert_test.ts b/test/assert_test.ts index f72956eb37..c8f6987301 100644 --- a/test/assert_test.ts +++ b/test/assert_test.ts @@ -24,7 +24,7 @@ describe('Assertion library', () => { isn't available through the supplied web3 instance`); }); it('doesn\'t throw if account is available', async () => { - const availableAccount = (await zeroEx.getAvailableAccountsAsync())[0]; + const availableAccount = (await zeroEx.getAvailableAddressesAsync())[0]; const varName = 'account'; return expect(assert.isSenderAddressHexAsync(varName, availableAccount, (zeroEx as any).web3Wrapper)) .to.become(undefined); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 075906eaa5..f2daac6237 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -54,7 +54,7 @@ export class FillScenarios { public async createPartiallyFilledSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, takerAddress: string, fillableAmount: BigNumber.BigNumber, partialFillAmount: BigNumber.BigNumber) { - const prevSenderAccount = await this.zeroEx.getAvailableAccountsAsync(); + const prevSenderAccount = await this.zeroEx.getAvailableAddressesAsync(); const [makerAddress] = this.userAddresses; const signedOrder = await this.createAsymmetricFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, From 33cf004b2927f1ddb68e370f211264a132e64544 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:48:40 +0200 Subject: [PATCH 12/18] Remove unused variable --- test/utils/fill_scenarios.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index f2daac6237..d8d6cd0b90 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -54,7 +54,6 @@ export class FillScenarios { public async createPartiallyFilledSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, takerAddress: string, fillableAmount: BigNumber.BigNumber, partialFillAmount: BigNumber.BigNumber) { - const prevSenderAccount = await this.zeroEx.getAvailableAddressesAsync(); const [makerAddress] = this.userAddresses; const signedOrder = await this.createAsymmetricFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, From abd6fbfa648a540342d9c183fc8f5b36248b6055 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 12:50:18 +0200 Subject: [PATCH 13/18] Use variable name in assertin message --- src/utils/assert.ts | 7 ++++--- test/assert_test.ts | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 5b4a6fca69..aee53f30c9 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -29,11 +29,12 @@ export const assert = { async isSenderAddressHexAsync(variableName: string, senderAddress: string, web3Wrapper: Web3Wrapper): Promise { assert.isETHAddressHex(variableName, senderAddress); - await assert.isSenderAddressAvailableAsync(web3Wrapper, senderAddress); + await assert.isSenderAddressAvailableAsync(web3Wrapper, variableName, senderAddress); }, - async isSenderAddressAvailableAsync(web3Wrapper: Web3Wrapper, senderAddress: string): Promise { + async isSenderAddressAvailableAsync(web3Wrapper: Web3Wrapper, variableName: string, + senderAddress: string): Promise { const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAddress); - assert.assert(isSenderAddressAvailable, `Specified sender account ${senderAddress} isn't available through the \ + assert.assert(isSenderAddressAvailable, `Specified ${variableName} ${senderAddress} isn't available through the \ supplied web3 instance`); }, isNumber(variableName: string, value: number): void { diff --git a/test/assert_test.ts b/test/assert_test.ts index c8f6987301..147c9832f8 100644 --- a/test/assert_test.ts +++ b/test/assert_test.ts @@ -20,7 +20,7 @@ describe('Assertion library', () => { const validUnrelatedAddress = '0x8b0292b11a196601eddce54b665cafeca0347d42'; const varName = 'account'; return expect(assert.isSenderAddressHexAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) - .to.be.rejectedWith(`Specified sender account ${validUnrelatedAddress} \ + .to.be.rejectedWith(`Specified ${varName} ${validUnrelatedAddress} \ isn't available through the supplied web3 instance`); }); it('doesn\'t throw if account is available', async () => { From 80bbfc3f0ff6d000e628d6db4baddae94c4911a0 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 14:10:36 +0200 Subject: [PATCH 14/18] Last round of renamings --- test/0x.js_test.ts | 12 ++++++------ test/assert_test.ts | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/0x.js_test.ts b/test/0x.js_test.ts index edc2a3493d..43c50ef346 100644 --- a/test/0x.js_test.ts +++ b/test/0x.js_test.ts @@ -191,12 +191,12 @@ describe('ZeroEx library', () => { }); describe('#signOrderHashAsync', () => { let stubs: Sinon.SinonStub[] = []; - let makerAccount: string; + let makerAddress: string; const web3 = web3Factory.create(); const zeroEx = new ZeroEx(web3); before(async () => { - const availableAccounts = await zeroEx.getAvailableAddressesAsync(); - makerAccount = availableAccounts[0]; + const availableAddreses = await zeroEx.getAvailableAddressesAsync(); + makerAddress = availableAddreses[0]; }); afterEach(() => { // clean up any stubs after the test has completed @@ -210,7 +210,7 @@ describe('ZeroEx library', () => { r: '0x61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc33', s: '0x40349190569279751135161d22529dc25add4f6069af05be04cacbda2ace2254', }; - const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAccount); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress); expect(ecSignature).to.deep.equal(expectedECSignature); }); it ('should return the correct ECSignature on Parity > V1.6.6', async () => { @@ -231,7 +231,7 @@ describe('ZeroEx library', () => { Sinon.stub(ZeroEx, 'isValidSignature').returns(true), ]; - const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAccount); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress); expect(ecSignature).to.deep.equal(expectedECSignature); }); it ('should return the correct ECSignature on Parity < V1.6.6', async () => { @@ -252,7 +252,7 @@ describe('ZeroEx library', () => { Sinon.stub(ZeroEx, 'isValidSignature').returns(true), ]; - const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAccount); + const ecSignature = await zeroEx.signOrderHashAsync(orderHash, makerAddress); expect(ecSignature).to.deep.equal(expectedECSignature); }); }); diff --git a/test/assert_test.ts b/test/assert_test.ts index 147c9832f8..ff15d6cf6f 100644 --- a/test/assert_test.ts +++ b/test/assert_test.ts @@ -9,24 +9,24 @@ const expect = chai.expect; describe('Assertion library', () => { const web3 = web3Factory.create(); const zeroEx = new ZeroEx(web3); - describe('#isSenderAccountHexAsync', () => { + describe('#isSenderAddressHexAsync', () => { it('throws when address is invalid', async () => { const address = '0xdeadbeef'; - const varName = 'account'; + const varName = 'address'; return expect(assert.isSenderAddressHexAsync(varName, address, (zeroEx as any).web3Wrapper)) .to.be.rejectedWith(`Expected ${varName} to be of type ETHAddressHex, encountered: ${address}`); }); it('throws when address is unavailable', async () => { const validUnrelatedAddress = '0x8b0292b11a196601eddce54b665cafeca0347d42'; - const varName = 'account'; + const varName = 'address'; return expect(assert.isSenderAddressHexAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) .to.be.rejectedWith(`Specified ${varName} ${validUnrelatedAddress} \ isn't available through the supplied web3 instance`); }); - it('doesn\'t throw if account is available', async () => { - const availableAccount = (await zeroEx.getAvailableAddressesAsync())[0]; - const varName = 'account'; - return expect(assert.isSenderAddressHexAsync(varName, availableAccount, (zeroEx as any).web3Wrapper)) + it('doesn\'t throw if address is available', async () => { + const availableAddress = (await zeroEx.getAvailableAddressesAsync())[0]; + const varName = 'address'; + return expect(assert.isSenderAddressHexAsync(varName, availableAddress, (zeroEx as any).web3Wrapper)) .to.become(undefined); }); }); From 8103a6cedda2646405c8cfd644c609cf3e4a84cb Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 14:27:09 +0200 Subject: [PATCH 15/18] Rename senderAddress to signerAddress --- src/0x.js.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 84362bd9ac..19e74b4848 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -161,9 +161,9 @@ export class ZeroEx { * Signs an orderHash and returns it's elliptic curve signature * This method currently supports TestRPC, Geth and Parity above and below V1.6.6 */ - public async signOrderHashAsync(orderHashHex: string, senderAddress: string): Promise { + public async signOrderHashAsync(orderHashHex: string, signerAddress: string): Promise { assert.isHexString('orderHashHex', orderHashHex); - await assert.isSenderAddressHexAsync('senderAddress', senderAddress, this.web3Wrapper); + await assert.isSenderAddressHexAsync('signerAddress', signerAddress, this.web3Wrapper); let msgHashHex; const nodeVersion = await this.web3Wrapper.getNodeVersionAsync(); @@ -177,7 +177,7 @@ export class ZeroEx { msgHashHex = ethUtil.bufferToHex(msgHashBuff); } - const signature = await this.web3Wrapper.signTransactionAsync(senderAddress, msgHashHex); + const signature = await this.web3Wrapper.signTransactionAsync(signerAddress, msgHashHex); let signatureData; const [nodeVersionNumber] = findVersions(nodeVersion); @@ -207,7 +207,7 @@ export class ZeroEx { r: ethUtil.bufferToHex(r), s: ethUtil.bufferToHex(s), }; - const isValidSignature = ZeroEx.isValidSignature(orderHashHex, ecSignature, senderAddress); + const isValidSignature = ZeroEx.isValidSignature(orderHashHex, ecSignature, signerAddress); if (!isValidSignature) { throw new Error(ZeroExError.INVALID_SIGNATURE); } From ef57dd22e29057e5bd84b9a0ad36a123126695df Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 14:29:07 +0200 Subject: [PATCH 16/18] Rename isSenderAddressHexAsync to isSenderAddressAsync --- src/0x.js.ts | 2 +- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/contract_wrappers/token_wrapper.ts | 6 +++--- src/utils/assert.ts | 16 ++++++++-------- test/assert_test.ts | 6 +++--- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/0x.js.ts b/src/0x.js.ts index 19e74b4848..0f437e0390 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -163,7 +163,7 @@ export class ZeroEx { */ public async signOrderHashAsync(orderHashHex: string, signerAddress: string): Promise { assert.isHexString('orderHashHex', orderHashHex); - await assert.isSenderAddressHexAsync('signerAddress', signerAddress, this.web3Wrapper); + await assert.isSenderAddressAsync('signerAddress', signerAddress, this.web3Wrapper); let msgHashHex; const nodeVersion = await this.web3Wrapper.getNodeVersionAsync(); diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dd06830aa1..08e5927294 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -121,7 +121,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrderSchema); assert.isBigNumber('fillTakerAmount', fillTakerAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); - await assert.isSenderAddressHexAsync('takerAddress', takerAddress, this.web3Wrapper); + await assert.isSenderAddressAsync('takerAddress', takerAddress, this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index 34697d97cd..d30645548e 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -38,7 +38,7 @@ export class TokenWrapper extends ContractWrapper { */ public async setAllowanceAsync(tokenAddress: string, ownerAddress: string, spenderAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { - await assert.isSenderAddressHexAsync('ownerAddress', ownerAddress, this.web3Wrapper); + await assert.isSenderAddressAsync('ownerAddress', ownerAddress, this.web3Wrapper); assert.isETHAddressHex('spenderAddress', spenderAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); @@ -97,7 +97,7 @@ export class TokenWrapper extends ContractWrapper { public async transferAsync(tokenAddress: string, fromAddress: string, toAddress: string, amountInBaseUnits: BigNumber.BigNumber): Promise { assert.isETHAddressHex('tokenAddress', tokenAddress); - await assert.isSenderAddressHexAsync('fromAddress', fromAddress, this.web3Wrapper); + await assert.isSenderAddressAsync('fromAddress', fromAddress, this.web3Wrapper); assert.isETHAddressHex('toAddress', toAddress); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); @@ -123,7 +123,7 @@ export class TokenWrapper extends ContractWrapper { assert.isETHAddressHex('tokenAddress', tokenAddress); assert.isETHAddressHex('fromAddress', fromAddress); assert.isETHAddressHex('toAddress', toAddress); - await assert.isSenderAddressHexAsync('senderAddress', senderAddress, this.web3Wrapper); + await assert.isSenderAddressAsync('senderAddress', senderAddress, this.web3Wrapper); assert.isBigNumber('amountInBaseUnits', amountInBaseUnits); const tokenContract = await this.getTokenContractAsync(tokenAddress); diff --git a/src/utils/assert.ts b/src/utils/assert.ts index aee53f30c9..621c11edae 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -26,16 +26,16 @@ export const assert = { const web3 = new Web3(); this.assert(web3.isAddress(value), this.typeAssertionMessage(variableName, 'ETHAddressHex', value)); }, - async isSenderAddressHexAsync(variableName: string, senderAddress: string, - web3Wrapper: Web3Wrapper): Promise { - assert.isETHAddressHex(variableName, senderAddress); - await assert.isSenderAddressAvailableAsync(web3Wrapper, variableName, senderAddress); + async isSenderAddressAsync(variableName: string, senderAddressHex: string, + web3Wrapper: Web3Wrapper): Promise { + assert.isETHAddressHex(variableName, senderAddressHex); + await assert.isSenderAddressAvailableAsync(web3Wrapper, variableName, senderAddressHex); }, async isSenderAddressAvailableAsync(web3Wrapper: Web3Wrapper, variableName: string, - senderAddress: string): Promise { - const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAddress); - assert.assert(isSenderAddressAvailable, `Specified ${variableName} ${senderAddress} isn't available through the \ - supplied web3 instance`); + senderAddressHex: string): Promise { + const isSenderAddressAvailable = await web3Wrapper.isSenderAddressAvailableAsync(senderAddressHex); + assert.assert(isSenderAddressAvailable, `Specified ${variableName} ${senderAddressHex} isn't available \ + through the supplied web3 instance`); }, isNumber(variableName: string, value: number): void { this.assert(_.isFinite(value), this.typeAssertionMessage(variableName, 'number', value)); diff --git a/test/assert_test.ts b/test/assert_test.ts index ff15d6cf6f..560ef06d40 100644 --- a/test/assert_test.ts +++ b/test/assert_test.ts @@ -13,20 +13,20 @@ describe('Assertion library', () => { it('throws when address is invalid', async () => { const address = '0xdeadbeef'; const varName = 'address'; - return expect(assert.isSenderAddressHexAsync(varName, address, (zeroEx as any).web3Wrapper)) + return expect(assert.isSenderAddressAsync(varName, address, (zeroEx as any).web3Wrapper)) .to.be.rejectedWith(`Expected ${varName} to be of type ETHAddressHex, encountered: ${address}`); }); it('throws when address is unavailable', async () => { const validUnrelatedAddress = '0x8b0292b11a196601eddce54b665cafeca0347d42'; const varName = 'address'; - return expect(assert.isSenderAddressHexAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) + return expect(assert.isSenderAddressAsync(varName, validUnrelatedAddress, (zeroEx as any).web3Wrapper)) .to.be.rejectedWith(`Specified ${varName} ${validUnrelatedAddress} \ isn't available through the supplied web3 instance`); }); it('doesn\'t throw if address is available', async () => { const availableAddress = (await zeroEx.getAvailableAddressesAsync())[0]; const varName = 'address'; - return expect(assert.isSenderAddressHexAsync(varName, availableAddress, (zeroEx as any).web3Wrapper)) + return expect(assert.isSenderAddressAsync(varName, availableAddress, (zeroEx as any).web3Wrapper)) .to.become(undefined); }); }); From 0dfc2b5f3b81e034a9c939f667383eb03c9a5561 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 14:56:54 +0200 Subject: [PATCH 17/18] Add isAnyAddressAvailableAsync --- src/contract_wrappers/exchange_wrapper.ts | 1 + src/contract_wrappers/token_wrapper.ts | 2 ++ src/utils/assert.ts | 4 ++++ src/web3_wrapper.ts | 6 ------ 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 08e5927294..e885b0e078 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -297,6 +297,7 @@ export class ExchangeWrapper extends ContractWrapper { private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber, fillTakerAmount: BigNumber.BigNumber, makerTokenAmount: BigNumber.BigNumber): Promise { + await assert.isAnyAddressAvailableAsync(this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); const isRoundingError = await exchangeInstance.isRoundingError.call( takerTokenAmount, fillTakerAmount, makerTokenAmount, diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index d30645548e..945f5caf17 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -25,6 +25,7 @@ export class TokenWrapper extends ContractWrapper { public async getBalanceAsync(tokenAddress: string, ownerAddress: string): Promise { assert.isETHAddressHex('ownerAddress', ownerAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); + await assert.isAnyAddressAvailableAsync(this.web3Wrapper); const tokenContract = await this.getTokenContractAsync(tokenAddress); let balance = await tokenContract.balanceOf.call(ownerAddress); @@ -60,6 +61,7 @@ export class TokenWrapper extends ContractWrapper { public async getAllowanceAsync(tokenAddress: string, ownerAddress: string, spenderAddress: string) { assert.isETHAddressHex('ownerAddress', ownerAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); + await assert.isAnyAddressAvailableAsync(this.web3Wrapper); const tokenContract = await this.getTokenContractAsync(tokenAddress); let allowanceInBaseUnits = await tokenContract.allowance.call(ownerAddress, spenderAddress); diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 621c11edae..7ab1b7c62d 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -37,6 +37,10 @@ export const assert = { assert.assert(isSenderAddressAvailable, `Specified ${variableName} ${senderAddressHex} isn't available \ through the supplied web3 instance`); }, + async isAnyAddressAvailableAsync(web3Wrapper: Web3Wrapper): Promise { + const availableAddresses = await web3Wrapper.getAvailableAddressesAsync(); + this.assert(!_.isEmpty(availableAddresses), 'No addresses are available on the provided web3 instance'); + }, isNumber(variableName: string, value: number): void { this.assert(_.isFinite(value), this.typeAssertionMessage(variableName, 'number', value)); }, diff --git a/src/web3_wrapper.ts b/src/web3_wrapper.ts index e1bb29f852..05a8dc0631 100644 --- a/src/web3_wrapper.ts +++ b/src/web3_wrapper.ts @@ -17,12 +17,6 @@ export class Web3Wrapper { public isAddress(address: string): boolean { return this.web3.isAddress(address); } - public getDefaultAddress(): string { - return this.web3.eth.defaultAccount; - } - public setDefaultAddress(address: string): void { - this.web3.eth.defaultAccount = address; - } public async isSenderAddressAvailableAsync(senderAddress: string): Promise { const addresses = await this.getAvailableAddressesAsync(); return _.includes(addresses, senderAddress); From f54b513935dbba0dd1922566ed2fd4b4acbf6459 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 15:13:03 +0200 Subject: [PATCH 18/18] Address feedback --- src/contract_wrappers/exchange_wrapper.ts | 2 +- src/contract_wrappers/token_wrapper.ts | 4 ++-- src/utils/assert.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e885b0e078..d3a53a9f74 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -297,7 +297,7 @@ export class ExchangeWrapper extends ContractWrapper { private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber, fillTakerAmount: BigNumber.BigNumber, makerTokenAmount: BigNumber.BigNumber): Promise { - await assert.isAnyAddressAvailableAsync(this.web3Wrapper); + await assert.isUserAddressAvailableAsync(this.web3Wrapper); const exchangeInstance = await this.getExchangeContractAsync(); const isRoundingError = await exchangeInstance.isRoundingError.call( takerTokenAmount, fillTakerAmount, makerTokenAmount, diff --git a/src/contract_wrappers/token_wrapper.ts b/src/contract_wrappers/token_wrapper.ts index 945f5caf17..4412b12992 100644 --- a/src/contract_wrappers/token_wrapper.ts +++ b/src/contract_wrappers/token_wrapper.ts @@ -25,7 +25,7 @@ export class TokenWrapper extends ContractWrapper { public async getBalanceAsync(tokenAddress: string, ownerAddress: string): Promise { assert.isETHAddressHex('ownerAddress', ownerAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); - await assert.isAnyAddressAvailableAsync(this.web3Wrapper); + await assert.isUserAddressAvailableAsync(this.web3Wrapper); const tokenContract = await this.getTokenContractAsync(tokenAddress); let balance = await tokenContract.balanceOf.call(ownerAddress); @@ -61,7 +61,7 @@ export class TokenWrapper extends ContractWrapper { public async getAllowanceAsync(tokenAddress: string, ownerAddress: string, spenderAddress: string) { assert.isETHAddressHex('ownerAddress', ownerAddress); assert.isETHAddressHex('tokenAddress', tokenAddress); - await assert.isAnyAddressAvailableAsync(this.web3Wrapper); + await assert.isUserAddressAvailableAsync(this.web3Wrapper); const tokenContract = await this.getTokenContractAsync(tokenAddress); let allowanceInBaseUnits = await tokenContract.allowance.call(ownerAddress, spenderAddress); diff --git a/src/utils/assert.ts b/src/utils/assert.ts index 7ab1b7c62d..f4d653ffbe 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -37,9 +37,9 @@ export const assert = { assert.assert(isSenderAddressAvailable, `Specified ${variableName} ${senderAddressHex} isn't available \ through the supplied web3 instance`); }, - async isAnyAddressAvailableAsync(web3Wrapper: Web3Wrapper): Promise { + async isUserAddressAvailableAsync(web3Wrapper: Web3Wrapper): Promise { const availableAddresses = await web3Wrapper.getAvailableAddressesAsync(); - this.assert(!_.isEmpty(availableAddresses), 'No addresses are available on the provided web3 instance'); + this.assert(!_.isEmpty(availableAddresses), 'No addresses were available on the provided web3 instance'); }, isNumber(variableName: string, value: number): void { this.assert(_.isFinite(value), this.typeAssertionMessage(variableName, 'number', value));