From 6db0b2e39821f27f6f31a8247159864dacd1d2bd Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 9 Mar 2018 15:47:21 -0800 Subject: Address feedback and lint --- .../current/protocol/Exchange/ISigner.sol | 2 +- .../current/protocol/Exchange/LibErrors.sol | 4 +- .../current/protocol/Exchange/LibOrder.sol | 7 +- .../current/protocol/Exchange/LibPartialAmount.sol | 2 +- .../protocol/Exchange/MixinExchangeCore.sol | 26 +++--- .../protocol/Exchange/MixinSettlementProxy.sol | 11 ++- .../protocol/Exchange/MixinSignatureValidator.sol | 28 +++---- .../protocol/Exchange/MixinWrapperFunctions.sol | 18 +++-- .../protocol/Exchange/mixins/MExchangeCore.sol | 2 +- .../protocol/Exchange/mixins/MSettlement.sol | 2 +- packages/contracts/src/utils/crypto.ts | 2 - packages/contracts/src/utils/exchange_wrapper.ts | 94 ++++------------------ packages/contracts/src/utils/log_decoder.ts | 13 ++- packages/contracts/src/utils/order_factory.ts | 8 +- packages/contracts/src/utils/signing_utils.ts | 6 +- packages/contracts/src/utils/types.ts | 27 +------ packages/contracts/src/utils/utils.ts | 7 -- packages/contracts/test/exchange/core.ts | 7 +- packages/contracts/test/exchange/wrapper.ts | 2 +- 19 files changed, 92 insertions(+), 176 deletions(-) delete mode 100644 packages/contracts/src/utils/utils.ts (limited to 'packages') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol index 51d7e9b6b..a7ca68e09 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/ISigner.sol @@ -20,7 +20,7 @@ pragma solidity ^0.4.21; pragma experimental ABIEncoderV2; contract ISigner { - + function isValidSignature( bytes32 hash, bytes signature) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol index 69ef346fb..2bb47cfed 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol @@ -20,7 +20,7 @@ pragma solidity ^0.4.21; pragma experimental ABIEncoderV2; contract LibErrors { - + // Error Codes enum Errors { ORDER_EXPIRED, // Order has already expired @@ -28,7 +28,7 @@ contract LibErrors { ROUNDING_ERROR_TOO_LARGE, // Rounding error too large INSUFFICIENT_BALANCE_OR_ALLOWANCE // Insufficient balance or allowance for token transfer } - + event LogError(uint8 indexed errorId, bytes32 indexed orderHash); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol index a9fa20bee..3e438b3e8 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/LibOrder.sol @@ -20,7 +20,7 @@ pragma solidity ^0.4.21; pragma experimental ABIEncoderV2; contract LibOrder { - + bytes32 constant ORDER_SCHEMA_HASH = keccak256( "address exchangeAddress", "address makerAddress", @@ -35,7 +35,7 @@ contract LibOrder { "uint256 expirationTimeSeconds", "uint256 salt" ); - + struct Order { address makerAddress; address takerAddress; @@ -49,7 +49,7 @@ contract LibOrder { uint256 expirationTimeSeconds; uint256 salt; } - + /// @dev Calculates Keccak-256 hash of the order. /// @param order The order structure. /// @return Keccak-256 EIP712 hash of the order. @@ -58,6 +58,7 @@ contract LibOrder { returns (bytes32 orderHash) { // TODO: EIP712 is not finalized yet + // Source: https://github.com/ethereum/EIPs/pull/712 orderHash = keccak256( ORDER_SCHEMA_HASH, keccak256( diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol index ece672748..998ca593f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/LibPartialAmount.sol @@ -22,7 +22,7 @@ pragma experimental ABIEncoderV2; import "../../utils/SafeMath/SafeMath.sol"; contract LibPartialAmount is SafeMath { - + /// @dev Calculates partial value given a numerator and denominator. /// @param numerator Numerator. /// @param denominator Denominator. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 3a4362583..548c69606 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -42,7 +42,7 @@ contract MixinExchangeCore is // Mappings of orderHash => amounts of takerTokenAmount filled or cancelled. mapping (bytes32 => uint256) public filled; mapping (bytes32 => uint256) public cancelled; - + event LogFill( address indexed makerAddress, address takerAddress, @@ -84,7 +84,7 @@ contract MixinExchangeCore is { // Compute the order hash bytes32 orderHash = getOrderHash(order); - + // Validate order and maker only if first time seen // TODO: Read filled and cancelled only once if (filled[orderHash] == 0 && cancelled[orderHash] == 0) { @@ -92,7 +92,7 @@ contract MixinExchangeCore is require(order.takerTokenAmount > 0); require(isValidSignature(orderHash, order.makerAddress, signature)); } - + // Validate taker if (order.takerAddress != address(0)) { require(order.takerAddress == msg.sender); @@ -104,7 +104,7 @@ contract MixinExchangeCore is LogError(uint8(Errors.ORDER_EXPIRED), orderHash); return 0; } - + // Validate order availability uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(orderHash)); takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount); @@ -112,7 +112,7 @@ contract MixinExchangeCore is LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash); return 0; } - + // Validate fill order rounding if (isRoundingError(takerTokenFilledAmount, order.takerTokenAmount, order.makerTokenAmount)) { LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash); @@ -121,11 +121,11 @@ contract MixinExchangeCore is // Update state filled[orderHash] = safeAdd(filled[orderHash], takerTokenFilledAmount); - + // Settle order var (makerTokenFilledAmount, makerFeeAmountPaid, takerFeeAmountPaid) = settleOrder(order, msg.sender, takerTokenFilledAmount); - + // Log order LogFill( order.makerAddress, @@ -160,21 +160,21 @@ contract MixinExchangeCore is require(order.takerTokenAmount > 0); require(takerTokenCancelAmount > 0); require(order.makerAddress == msg.sender); - + if (block.timestamp >= order.expirationTimeSeconds) { LogError(uint8(Errors.ORDER_EXPIRED), orderHash); return 0; } - + uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(orderHash)); takerTokenCancelledAmount = min256(takerTokenCancelAmount, remainingTakerTokenAmount); if (takerTokenCancelledAmount == 0) { LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash); return 0; } - + cancelled[orderHash] = safeAdd(cancelled[orderHash], takerTokenCancelledAmount); - + LogCancel( order.makerAddress, order.feeRecipientAddress, @@ -197,7 +197,9 @@ contract MixinExchangeCore is returns (bool isError) { uint256 remainder = mulmod(target, numerator, denominator); - if (remainder == 0) return false; // No rounding error. + if (remainder == 0) { + return false; // No rounding error. + } uint256 errPercentageTimes1000000 = safeDiv( safeMul(remainder, 1000000), diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlementProxy.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlementProxy.sol index f3a9f1f9b..a9d30e08f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlementProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlementProxy.sol @@ -32,31 +32,30 @@ contract MixinSettlementProxy is ITokenTransferProxy TRANSFER_PROXY; IToken ZRX_TOKEN; - + function transferProxy() external view returns (ITokenTransferProxy) { return TRANSFER_PROXY; } - + function zrxToken() external view returns (IToken) { return ZRX_TOKEN; } - + function MixinSettlementProxy( ITokenTransferProxy proxyContract, - IToken zrxToken - ) + IToken zrxToken) public { ZRX_TOKEN = zrxToken; TRANSFER_PROXY = proxyContract; } - + function settleOrder( Order order, address takerAddress, diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index c567b1d34..c70a6b9bf 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -35,7 +35,7 @@ contract MixinSignatureValidator is Trezor, Contract } - + function isValidSignature( bytes32 hash, address signer, @@ -44,16 +44,16 @@ contract MixinSignatureValidator is returns (bool isValid) { // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.) - + require(signature.length >= 1); SignatureType signatureType = SignatureType(uint8(signature[0])); - + // Variables are not scoped in Solidity uint8 v; bytes32 r; bytes32 s; address recovered; - + // Always illegal signature // This is always an implicit option since a signer can create a // signature array with invalid type or length. We may as well make @@ -61,7 +61,7 @@ contract MixinSignatureValidator is // also the initialization value for the enum type. if (signatureType == SignatureType.Illegal) { revert(); - + // Always invalid signature // Like Illegal, this is always implicitly available and therefore // offered explicitly. It can be implicitly created by providing @@ -70,7 +70,7 @@ contract MixinSignatureValidator is require(signature.length == 1); isValid = false; return isValid; - + // Implicitly signed by caller // The signer has initiated the call. In the case of non-contract // accounts it means the transaction itself was signed. @@ -83,7 +83,7 @@ contract MixinSignatureValidator is require(signature.length == 1); isValid = signer == msg.sender; return isValid; - + // Signed using web3.eth_sign } else if (signatureType == SignatureType.Ecrecover) { require(signature.length == 66); @@ -98,7 +98,7 @@ contract MixinSignatureValidator is ); isValid = signer == recovered; return isValid; - + // Signature using EIP712 } else if (signatureType == SignatureType.EIP712) { require(signature.length == 66); @@ -108,7 +108,7 @@ contract MixinSignatureValidator is recovered = ecrecover(hash, v, r, s); isValid = signer == recovered; return isValid; - + // Signature from Trezor hardware wallet // It differs from web3.eth_sign in the encoding of message length // (Bitcoin varint encoding vs ascii-decimal, the latter is not @@ -130,13 +130,13 @@ contract MixinSignatureValidator is ); isValid = signer == recovered; return isValid; - + // Signature verified by signer contract } else if (signatureType == SignatureType.Contract) { isValid = ISigner(signer).isValidSignature(hash, signature); return isValid; } - + // Anything else is illegal (We do not return false because // the signature may actually be valid, just not in a format // that we currently support. In this case returning false @@ -144,16 +144,16 @@ contract MixinSignatureValidator is // signature was invalid.) revert(); } - + function get32(bytes b, uint256 index) private pure returns (bytes32 result) { require(b.length >= index + 32); - + // Arrays are prefixed by a 256 bit length parameter index += 32; - + // Read the bytes32 from array memory assembly { result := mload(add(b, index)) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 96ebee45d..67d4fe589 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -74,11 +74,11 @@ contract MixinWrapperFunctions is // (1): len(signature) // (2): 452 + len(signature) - // (3): 32 - (len(signature) mod 32) - // (4): 452 + len(signature) + 32 - (len(signature) mod 32) + // (3): (32 - len(signature)) mod 32 + // (4): 452 + len(signature) + (32 - len(signature)) mod 32 // [1]: https://solidity.readthedocs.io/en/develop/abi-spec.html - + bytes4 fillOrderSelector = this.fillOrder.selector; assembly { @@ -112,7 +112,7 @@ contract MixinWrapperFunctions is mstore(add(start, 420), sigLen) // Calculate signature length with padding - let paddingLen := sub(32, mod(sigLen, 32)) + let paddingLen := mod(sub(0, sigLen), 32) let sigLenWithPadding := add(sigLen, paddingLen) // Write signature @@ -120,11 +120,11 @@ contract MixinWrapperFunctions is for { let curr := 0 } lt(curr, sigLenWithPadding) { curr := add(curr, 32) } - { mstore(add(start, add(452, curr)), mload(add(sigStart, curr))) } + { mstore(add(start, add(452, curr)), mload(add(sigStart, curr))) } // Note: we assume that padding consists of only 0's // Execute delegatecall let success := delegatecall( - gas, // forward all gas + gas, // forward all gas, TODO: look into gas consumption of assert/throw address, // call address of this contract start, // pointer to start of input add(452, sigLenWithPadding), // input length is 420 + signature length + padding length @@ -214,11 +214,12 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { require(orders[i].takerTokenAddress == orders[0].takerTokenAddress); + uint256 remainingTakerTokenFillAmount = safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount); totalTakerTokenFilledAmount = safeAdd( totalTakerTokenFilledAmount, fillOrder( orders[i], - safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount), + remainingTakerTokenFillAmount, signatures[i] ) ); @@ -243,11 +244,12 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { require(orders[i].takerTokenAddress == orders[0].takerTokenAddress); + uint256 remainingTakerTokenFillAmount = safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount); totalTakerTokenFilledAmount = safeAdd( totalTakerTokenFilledAmount, fillOrderNoThrow( orders[i], - safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount), + remainingTakerTokenFillAmount, signatures[i] ) ); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index 1fc6cb855..d01609aed 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -22,7 +22,7 @@ pragma experimental ABIEncoderV2; import "../LibOrder.sol"; contract MExchangeCore is LibOrder { - + function fillOrder( Order order, uint256 takerTokenFillAmount, diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol index 618bee673..3e5b7e647 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol @@ -22,7 +22,7 @@ pragma experimental ABIEncoderV2; import "../LibOrder.sol"; contract MSettlement is LibOrder { - + function settleOrder( Order order, address taker, diff --git a/packages/contracts/src/utils/crypto.ts b/packages/contracts/src/utils/crypto.ts index 4126b8ff4..810072d2f 100644 --- a/packages/contracts/src/utils/crypto.ts +++ b/packages/contracts/src/utils/crypto.ts @@ -3,8 +3,6 @@ import ABI = require('ethereumjs-abi'); import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import { SignedOrder } from './types'; - export const crypto = { /** * We convert types from JS to Solidity as follows: diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index b53519871..5996867cb 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -31,13 +31,7 @@ export class ExchangeWrapper { params.signature, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async cancelOrderAsync( @@ -51,13 +45,7 @@ export class ExchangeWrapper { params.takerTokenCancelAmount, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async fillOrKillOrderAsync( @@ -72,13 +60,7 @@ export class ExchangeWrapper { params.signature, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async fillOrderNoThrowAsync( @@ -93,13 +75,7 @@ export class ExchangeWrapper { params.signature, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async batchFillOrdersAsync( @@ -114,13 +90,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async batchFillOrKillOrdersAsync( @@ -135,13 +105,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async batchFillOrdersNoThrowAsync( @@ -156,13 +120,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async marketFillOrdersAsync( @@ -177,13 +135,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async marketFillOrdersNoThrowAsync( @@ -198,13 +150,7 @@ export class ExchangeWrapper { params.signatures, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async batchCancelOrdersAsync( @@ -218,13 +164,7 @@ export class ExchangeWrapper { params.takerTokenCancelAmounts, { from }, ); - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); - tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => { - const logWithDecodedArgs = this._logDecoder.decodeLogOrThrow(log); - wrapLogBigNumbers(logWithDecodedArgs); - return logWithDecodedArgs; - }); + const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash); return tx; } public async getOrderHashAsync(signedOrder: SignedOrder): Promise { @@ -262,14 +202,10 @@ export class ExchangeWrapper { const filledAmount = new BigNumber(await this._exchange.filled.callAsync(orderHashHex)); return filledAmount; } -} - -function wrapLogBigNumbers(log: any): any { - const argNames = _.keys(log.args); - for (const argName of argNames) { - const isWeb3BigNumber = _.startsWith(log.args[argName].constructor.toString(), 'function BigNumber('); - if (isWeb3BigNumber) { - log.args[argName] = new BigNumber(log.args[argName]); - } + private async _getTxWithDecodedExchangeLogsAsync(txHash: string) { + const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); + tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); + tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log)); + return tx; } } diff --git a/packages/contracts/src/utils/log_decoder.ts b/packages/contracts/src/utils/log_decoder.ts index d8685e3d7..845e025fe 100644 --- a/packages/contracts/src/utils/log_decoder.ts +++ b/packages/contracts/src/utils/log_decoder.ts @@ -1,5 +1,5 @@ import { LogWithDecodedArgs, RawLog } from '@0xproject/types'; -import { AbiDecoder } from '@0xproject/utils'; +import { AbiDecoder, BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; import * as Web3 from 'web3'; @@ -27,6 +27,17 @@ export class LogDecoder { if (_.isUndefined((logWithDecodedArgsOrLog as LogWithDecodedArgs).args)) { throw new Error(`Unable to decode log: ${JSON.stringify(log)}`); } + wrapLogBigNumbers(logWithDecodedArgsOrLog); return logWithDecodedArgsOrLog; } } + +function wrapLogBigNumbers(log: any): any { + const argNames = _.keys(log.args); + for (const argName of argNames) { + const isWeb3BigNumber = _.startsWith(log.args[argName].constructor.toString(), 'function BigNumber('); + if (isWeb3BigNumber) { + log.args[argName] = new BigNumber(log.args[argName]); + } + } +} diff --git a/packages/contracts/src/utils/order_factory.ts b/packages/contracts/src/utils/order_factory.ts index 3f09cedf3..bdf5f9abe 100644 --- a/packages/contracts/src/utils/order_factory.ts +++ b/packages/contracts/src/utils/order_factory.ts @@ -8,10 +8,10 @@ import { DefaultOrderParams, SignatureType, SignedOrder, UnsignedOrder } from '. export class OrderFactory { private _defaultOrderParams: Partial; - private _secretKey: Buffer; - constructor(secretKey: Buffer, defaultOrderParams: Partial) { + private _privateKey: Buffer; + constructor(privateKey: Buffer, defaultOrderParams: Partial) { this._defaultOrderParams = defaultOrderParams; - this._secretKey = secretKey; + this._privateKey = privateKey; } public newSignedOrder( customOrderParams: Partial = {}, @@ -26,7 +26,7 @@ export class OrderFactory { ...customOrderParams, } as any) as UnsignedOrder; const orderHashBuff = orderUtils.getOrderHashBuff(order); - const signature = signingUtils.signMessage(orderHashBuff, this._secretKey, signatureType); + const signature = signingUtils.signMessage(orderHashBuff, this._privateKey, signatureType); const signedOrder = { ...order, signature: `0x${signature.toString('hex')}`, diff --git a/packages/contracts/src/utils/signing_utils.ts b/packages/contracts/src/utils/signing_utils.ts index 21b69619c..61ab1f138 100644 --- a/packages/contracts/src/utils/signing_utils.ts +++ b/packages/contracts/src/utils/signing_utils.ts @@ -3,10 +3,10 @@ import * as ethUtil from 'ethereumjs-util'; import { SignatureType } from './types'; export const signingUtils = { - signMessage(message: Buffer, secretKey: Buffer, signatureType: SignatureType): Buffer { + signMessage(message: Buffer, privateKey: Buffer, signatureType: SignatureType): Buffer { if (signatureType === SignatureType.Ecrecover) { const prefixedMessage = ethUtil.hashPersonalMessage(message); - const ecSignature = ethUtil.ecsign(prefixedMessage, secretKey); + const ecSignature = ethUtil.ecsign(prefixedMessage, privateKey); const signature = Buffer.concat([ ethUtil.toBuffer(signatureType), ethUtil.toBuffer(ecSignature.v), @@ -15,7 +15,7 @@ export const signingUtils = { ]); return signature; } else if (signatureType === SignatureType.EIP712) { - const ecSignature = ethUtil.ecsign(message, secretKey); + const ecSignature = ethUtil.ecsign(message, privateKey); const signature = Buffer.concat([ ethUtil.toBuffer(signatureType), ethUtil.toBuffer(ecSignature.v), diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index b16925825..9f874d9ec 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -108,19 +108,7 @@ export interface Artifact { }; } -export interface SignedOrder { - exchangeAddress: string; - makerAddress: string; - takerAddress: string; - makerTokenAddress: string; - takerTokenAddress: string; - feeRecipientAddress: string; - makerTokenAmount: BigNumber; - takerTokenAmount: BigNumber; - makerFeeAmount: BigNumber; - takerFeeAmount: BigNumber; - expirationTimeSeconds: BigNumber; - salt: BigNumber; +export interface SignedOrder extends UnsignedOrder { signature: string; } @@ -138,19 +126,8 @@ export interface OrderStruct { salt: BigNumber; } -export interface UnsignedOrder { +export interface UnsignedOrder extends OrderStruct { exchangeAddress: string; - makerAddress: string; - takerAddress: string; - makerTokenAddress: string; - takerTokenAddress: string; - feeRecipientAddress: string; - makerTokenAmount: BigNumber; - takerTokenAmount: BigNumber; - makerFeeAmount: BigNumber; - takerFeeAmount: BigNumber; - expirationTimeSeconds: BigNumber; - salt: BigNumber; } export enum SignatureType { diff --git a/packages/contracts/src/utils/utils.ts b/packages/contracts/src/utils/utils.ts deleted file mode 100644 index 04ac36b54..000000000 --- a/packages/contracts/src/utils/utils.ts +++ /dev/null @@ -1,7 +0,0 @@ -export const utils = { - consoleLog(message: string): void { - /* tslint:disable */ - console.log(message); - /* tslint:enable */ - }, -}; diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index c31f7ab74..d11be3726 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -502,11 +502,8 @@ describe('Exchange', () => { const invalidR = ethUtil.sha3('invalidR'); const invalidS = ethUtil.sha3('invalidS'); - const invalidSigBuff = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature.slice(0, 6)), - invalidR, - invalidS, - ]); + const signatureTypeAndV = signedOrder.signature.slice(0, 6); + const invalidSigBuff = Buffer.concat([ethUtil.toBuffer(signatureTypeAndV), invalidR, invalidS]); const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`; signedOrder.signature = invalidSigHex; return expect(exWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(constants.REVERT); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index 0ea1f2c82..0b76c0974 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -736,4 +736,4 @@ describe('Exchange', () => { }); }); }); -}); +}); // tslint:disable-line:max-file-line-count -- cgit