Add event to setSignatureValidatorApproval, rename signer => signerAddress accross all contracts

14 files changed, 94 insertions, 68 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index 901e71936..c227d210f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -312,7 +312,11 @@ contract MixinExchangeCore is
         // Validate Maker signature (check only if first time seen)
         if (orderInfo.orderTakerAssetFilledAmount == 0) {
             require(
-                isValidSignature(orderInfo.orderHash, order.makerAddress, signature),
+                isValidSignature(
+                    orderInfo.orderHash,
+                    order.makerAddress,
+                    signature
+                ),
                 INVALID_ORDER_SIGNATURE
             );
         }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 5897e6416..881d6e7b3 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -43,43 +43,52 @@ contract MixinSignatureValidator is
 
     /// @dev Approves a hash on-chain using any valid signature type.
     ///      After presigning a hash, the preSign signature type will become valid for that hash and signer.
-    /// @param signer Address that should have signed the given hash.
+    /// @param signerAddress Address that should have signed the given hash.
     /// @param signature Proof that the hash has been signed by signer.
     function preSign(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes signature
     )
         external
     {
         require(
-            isValidSignature(hash, signer, signature),
+            isValidSignature(
+                hash,
+                signerAddress,
+                signature
+            ),
             INVALID_SIGNATURE
         );
-        preSigned[hash][signer] = true;
+        preSigned[hash][signerAddress] = true;
     }
 
     /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
-    /// @param validator Address of Validator contract.
+    /// @param validatorAddress Address of Validator contract.
     /// @param approval Approval or disapproval of  Validator contract.
     function setSignatureValidatorApproval(
-        address validator,
+        address validatorAddress,
         bool approval
     )
         external
     {
-        address signer = getCurrentContextAddress();
-        allowedValidators[signer][validator] = approval;
+        address signerAddress = getCurrentContextAddress();
+        allowedValidators[signerAddress][validatorAddress] = approval;
+        emit SignatureValidatorApproval(
+            signerAddress,
+            validatorAddress,
+            approval
+        );
     }
 
     /// @dev Verifies that a hash has been signed by the given signer.
     /// @param hash Any 32 byte hash.
-    /// @param signer Address that should have signed the given hash.
+    /// @param signerAddress Address that should have signed the given hash.
     /// @param signature Proof that the hash has been signed by signer.
     /// @return True if the address recovered from the provided signature matches the input signer address.
     function isValidSignature(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes memory signature
     )
         public
@@ -138,7 +147,7 @@ contract MixinSignatureValidator is
             r = readBytes32(signature, 1);
             s = readBytes32(signature, 33);
             recovered = ecrecover(hash, v, r, s);
-            isValid = signer == recovered;
+            isValid = signerAddress == recovered;
             return isValid;
 
         // Signed using web3.eth_sign
@@ -156,7 +165,7 @@ contract MixinSignatureValidator is
                 r,
                 s
             );
-            isValid = signer == recovered;
+            isValid = signerAddress == recovered;
             return isValid;
 
         // Implicitly signed by caller.
@@ -172,13 +181,13 @@ contract MixinSignatureValidator is
                 signature.length == 0,
                 LENGTH_0_REQUIRED
             );
-            isValid = signer == msg.sender;
+            isValid = signerAddress == msg.sender;
             return isValid;
 
         // Signature verified by wallet contract.
         // If used with an order, the maker of the order is the wallet contract.
         } else if (signatureType == SignatureType.Wallet) {
-            isValid = IWallet(signer).isValidSignature(hash, signature);
+            isValid = IWallet(signerAddress).isValidSignature(hash, signature);
             return isValid;
 
         // Signature verified by validator contract.
@@ -190,21 +199,21 @@ contract MixinSignatureValidator is
         // | 0x14 + x | 1      | Signature type is always "\x06" |
         } else if (signatureType == SignatureType.Validator) {
             // Pop last 20 bytes off of signature byte array.
-            address validator = popLast20Bytes(signature);
+            address validatorAddress = popLast20Bytes(signature);
             // Ensure signer has approved validator.
-            if (!allowedValidators[signer][validator]) {
+            if (!allowedValidators[signerAddress][validatorAddress]) {
                 return false;
             }
-            isValid = IValidator(validator).isValidSignature(
+            isValid = IValidator(validatorAddress).isValidSignature(
                 hash,
-                signer,
+                signerAddress,
                 signature
             );
             return isValid;
 
         // Signer signed hash previously using the preSign function.
         } else if (signatureType == SignatureType.PreSigned) {
-            isValid = preSigned[hash][signer];
+            isValid = preSigned[hash][signerAddress];
             return isValid;
 
         // Signature from Trezor hardware wallet.
@@ -229,9 +238,8 @@ contract MixinSignatureValidator is
                 r,
                 s
             );
-            isValid = signer == recovered;
+            isValid = signerAddress == recovered;
             return isValid;
-
         }
 
         // Anything else is illegal (We do not return false because
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index f1332363c..d3d22d48f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -41,17 +41,17 @@ contract MixinTransactions is
     bytes32 constant EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked(
         "ZeroExTransaction(",
         "uint256 salt,",
-        "address signer,",
+        "address signerAddress,",
         "bytes data",
         ")"
     ));
 
     /// @dev Calculates EIP712 hash of the Transaction.
     /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
-    /// @param signer Address of transaction signer.
+    /// @param signerAddress Address of transaction signer.
     /// @param data AbiV2 encoded calldata.
     /// @return EIP712 hash of the Transaction.
-    function hashZeroExTransaction(uint256 salt, address signer, bytes data)
+    function hashZeroExTransaction(uint256 salt, address signerAddress, bytes data)
         internal
         pure
         returns (bytes32)
@@ -59,19 +59,19 @@ contract MixinTransactions is
         return keccak256(abi.encode(
             EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH,
             salt,
-            signer,
+            signerAddress,
             keccak256(data)
         ));
     }
 
     /// @dev Executes an exchange method call in the context of signer.
     /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
-    /// @param signer Address of transaction signer.
+    /// @param signerAddress Address of transaction signer.
     /// @param data AbiV2 encoded calldata.
     /// @param signature Proof of signer transaction by signer.
     function executeTransaction(
         uint256 salt,
-        address signer,
+        address signerAddress,
         bytes data,
         bytes signature
     )
@@ -83,7 +83,11 @@ contract MixinTransactions is
             REENTRANCY_ILLEGAL
         );
 
-        bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(salt, signer, data));
+        bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(
+            salt,
+            signerAddress,
+            data
+        ));
 
         // Validate transaction has not been executed
         require(
@@ -92,15 +96,19 @@ contract MixinTransactions is
         );
 
         // Transaction always valid if signer is sender of transaction
-        if (signer != msg.sender) {
+        if (signerAddress != msg.sender) {
             // Validate signature
             require(
-                isValidSignature(transactionHash, signer, signature),
+                isValidSignature(
+                    transactionHash,
+                    signerAddress,
+                    signature
+                ),
                 INVALID_TX_SIGNATURE
             );
 
             // Set the current transaction signer
-            currentContextAddress = signer;
+            currentContextAddress = signerAddress;
         }
 
         // Execute transaction
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
index 02aa9776e..511463309 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol
@@ -22,32 +22,32 @@ contract ISignatureValidator {
 
     /// @dev Approves a hash on-chain using any valid signature type.
     ///      After presigning a hash, the preSign signature type will become valid for that hash and signer.
-    /// @param signer Address that should have signed the given hash.
+    /// @param signerAddress Address that should have signed the given hash.
     /// @param signature Proof that the hash has been signed by signer.
     function preSign(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes signature
     )
         external;
     
     /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf.
-    /// @param validator Address of Validator contract.
+    /// @param validatorAddress Address of Validator contract.
     /// @param approval Approval or disapproval of  Validator contract.
     function setSignatureValidatorApproval(
-        address validator,
+        address validatorAddress,
         bool approval
     )
         external;
 
     /// @dev Verifies that a signature is valid.
     /// @param hash Message hash that is signed.
-    /// @param signer Address of signer.
+    /// @param signerAddress Address of signer.
     /// @param signature Proof of signing.
     /// @return Validity of order signature.
     function isValidSignature(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes memory signature
     )
         public
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol
index 2f9a5bc7c..a7cab8f55 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol
@@ -21,12 +21,12 @@ contract ITransactions {
 
     /// @dev Executes an exchange method call in the context of signer.
     /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
-    /// @param signer Address of transaction signer.
+    /// @param signerAddress Address of transaction signer.
     /// @param data AbiV2 encoded calldata.
     /// @param signature Proof of signer transaction by signer.
     function executeTransaction(
         uint256 salt,
-        address signer,
+        address signerAddress,
         bytes data,
         bytes signature
     )
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
index 3e5ccc190..0b1796a66 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol
@@ -22,12 +22,12 @@ contract IValidator {
 
     /// @dev Verifies that a signature is valid.
     /// @param hash Message hash that is signed.
-    /// @param signer Address that should have signed the given hash.
+    /// @param signerAddress Address that should have signed the given hash.
     /// @param signature Proof of signing.
     /// @return Validity of order signature.
     function isValidSignature(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes signature
     )
         external
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
index d16a830f4..788f42c60 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
@@ -27,9 +27,9 @@ contract MAssetProxyDispatcher is
 
     // Logs registration of new asset proxy
     event AssetProxySet(
-        uint8 id,
-        address newAssetProxy,
-        address oldAssetProxy
+        uint8 id,               // Id of new registered AssetProxy.
+        address newAssetProxy,  // Address of new registered AssetProxy.
+        address oldAssetProxy   // Address of AssetProxy that was overwritten at given id (or null address).
     );
 
     /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws.
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 b0dbceff6..6e406e1c4 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -36,7 +36,7 @@ contract MExchangeCore is
         uint256 takerAssetFilledAmount,       // Amount of takerAsset sold by taker and bought by maker.
         uint256 makerFeePaid,                 // Amount of ZRX paid to feeRecipient by maker.
         uint256 takerFeePaid,                 // Amount of ZRX paid to feeRecipient by taker.
-        bytes32 indexed orderHash,            // EIP712 hash of order (see LibOrder.getOrderHash)
+        bytes32 indexed orderHash,            // EIP712 hash of order (see LibOrder.getOrderHash).
         bytes makerAssetData,                 // Encoded data specific to makerAsset. 
         bytes takerAssetData                  // Encoded data specific to takerAsset.
     );
@@ -46,7 +46,7 @@ contract MExchangeCore is
         address indexed makerAddress,         // Address that created the order.      
         address indexed feeRecipientAddress,  // Address that would have recieved fees if order was filled.   
         address senderAddress,                // Address that called the Exchange contract (msg.sender).
-        bytes32 indexed orderHash,            // EIP712 hash of order (see LibOrder.getOrderHash)
+        bytes32 indexed orderHash,            // EIP712 hash of order (see LibOrder.getOrderHash).
         bytes makerAssetData,                 // Encoded data specific to makerAsset. 
         bytes takerAssetData                  // Encoded data specific to takerAsset.
     );
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
index 9c6fbe22b..6cc1d7a10 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol
@@ -23,17 +23,23 @@ import "../interfaces/ISignatureValidator.sol";
 contract MSignatureValidator is
     ISignatureValidator
 {
+    event SignatureValidatorApproval(
+        address indexed signerAddress,     // Address that approves or disapproves a contract to verify signatures.
+        address indexed validatorAddress,  // Address of signature validator contract.
+        bool approved                      // Approval or disapproval of validator contract.
+    );
+
     // Allowed signature types.
     enum SignatureType {
-        Illegal,        // 0x00, default value
-        Invalid,        // 0x01
-        EIP712,         // 0x02
-        EthSign,        // 0x03
-        Caller,         // 0x04
-        Wallet,         // 0x05
-        Validator,      // 0x06
-        PreSigned,      // 0x07
-        Trezor,         // 0x08
-        NSignatureTypes // 0x09, number of signature types. Always leave at end.
+        Illegal,         // 0x00, default value
+        Invalid,         // 0x01
+        EIP712,          // 0x02
+        EthSign,         // 0x03
+        Caller,          // 0x04
+        Wallet,          // 0x05
+        Validator,       // 0x06
+        PreSigned,       // 0x07
+        Trezor,          // 0x08
+        NSignatureTypes  // 0x09, number of signature types. Always leave at end.
     }
 }
diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol
index 13953b482..f9271bf7a 100644
--- a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol
+++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol
@@ -35,18 +35,18 @@ contract TestValidator is
 
     /// @dev Verifies that a signature is valid. `signer` must match `VALID_SIGNER`.
     /// @param hash Message hash that is signed.
-    /// @param signer Address that should have signed the given hash.
+    /// @param signerAddress Address that should have signed the given hash.
     /// @param signature Proof of signing.
     /// @return Validity of signature.
     function isValidSignature(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes signature
     )
         external
         view
         returns (bool isValid)
     {
-        return (signer == VALID_SIGNER);
+        return (signerAddress == VALID_SIGNER);
     }
 }
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 460c9ea42..d35815474 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -116,18 +116,18 @@ contract Whitelist is
     /// @dev Verifies signer is same as signer of current Ethereum transaction.
     ///      NOTE: This function can currently be used to validate signatures coming from outside of this contract.
     ///      Extra safety checks can be added for a production contract.
-    /// @param signer Address that should have signed the given hash.
+    /// @param signerAddress Address that should have signed the given hash.
     /// @param signature Proof of signing.
     /// @return Validity of order signature.
     function isValidSignature(
         bytes32 hash,
-        address signer,
+        address signerAddress,
         bytes signature
     )
         external
         view
         returns (bool isValid)
     {
-        return signer == tx.origin;
+        return signerAddress == tx.origin;
     }
 }
diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts
index 4cc8f0b89..06eb70644 100644
--- a/packages/contracts/src/utils/exchange_wrapper.ts
+++ b/packages/contracts/src/utils/exchange_wrapper.ts
@@ -215,7 +215,7 @@ export class ExchangeWrapper {
     ): Promise<TransactionReceiptWithDecodedLogs> {
         const txHash = await this._exchange.executeTransaction.sendTransactionAsync(
             signedTx.salt,
-            signedTx.signer,
+            signedTx.signerAddress,
             signedTx.data,
             signedTx.signature,
             { from },
diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts
index 19ef4e1bf..348c0715d 100644
--- a/packages/contracts/src/utils/transaction_factory.ts
+++ b/packages/contracts/src/utils/transaction_factory.ts
@@ -9,7 +9,7 @@ const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = {
     name: 'ZeroExTransaction',
     parameters: [
         { name: 'salt', type: EIP712Types.Uint256 },
-        { name: 'signer', type: EIP712Types.Address },
+        { name: 'signerAddress', type: EIP712Types.Address },
         { name: 'data', type: EIP712Types.Bytes },
     ],
 };
@@ -25,10 +25,10 @@ export class TransactionFactory {
     }
     public newSignedTransaction(data: string, signatureType: SignatureType = SignatureType.EthSign): SignedTransaction {
         const salt = generatePseudoRandomSalt();
-        const signer = `0x${this._signerBuff.toString('hex')}`;
+        const signerAddress = `0x${this._signerBuff.toString('hex')}`;
         const executeTransactionData = {
             salt,
-            signer,
+            signerAddress,
             data,
         };
         const executeTransactionHashBuff = EIP712Utils.structHash(
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index a6d0c2a88..7a1f92afd 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -108,7 +108,7 @@ export enum ContractName {
 export interface SignedTransaction {
     exchangeAddress: string;
     salt: BigNumber;
-    signer: string;
+    signerAddress: string;
     data: string;
     signature: string;
 }