Address feedback and lint

19 files changed, 92 insertions, 176 deletions

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<string> {
@@ -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<ArgsType>).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<UnsignedOrder>;
-    private _secretKey: Buffer;
-    constructor(secretKey: Buffer, defaultOrderParams: Partial<UnsignedOrder>) {
+    private _privateKey: Buffer;
+    constructor(privateKey: Buffer, defaultOrderParams: Partial<UnsignedOrder>) {
         this._defaultOrderParams = defaultOrderParams;
-        this._secretKey = secretKey;
+        this._privateKey = privateKey;
     }
     public newSignedOrder(
         customOrderParams: Partial<UnsignedOrder> = {},
@@ -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