Comments for readability in exchange core and mixin match orders

5 files changed, 11 insertions, 4 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index 7bd88a82c..d311505bd 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -169,8 +169,9 @@ contract MixinExchangeCore is
         internal
     {
         // Ensure order is valid
-        // An order can only be filled if it is Status.FILLABLE;
+        // An order can only be filled if its status is FILLABLE;
         // however, only invalid statuses result in a throw.
+        // See LibStatus for a complete description of order statuses.
         require(
             orderStatus != uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT),
             INVALID_ORDER_MAKER_ASSET_AMOUNT
@@ -323,8 +324,9 @@ contract MixinExchangeCore is
         internal
     {
         // Ensure order is valid
-        // An order can only be cancelled if it is Status.FILLABLE;
+        // An order can only be cancelled if its status is FILLABLE;
         // however, only invalid statuses result in a throw.
+        // See LibStatus for a complete description of order statuses.
         require(
             orderStatus != uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT),
             INVALID_ORDER_MAKER_ASSET_AMOUNT
@@ -366,6 +368,7 @@ contract MixinExchangeCore is
         returns (bool stateUpdated)
     {
         // Ensure order is fillable (otherwise cancelling does nothing)
+        // See LibStatus for a complete description of order statuses.
         if (orderStatus != uint8(Status.ORDER_FILLABLE)) {
             emit ExchangeStatus(uint8(orderStatus), orderHash);
             stateUpdated = false;
@@ -390,7 +393,7 @@ contract MixinExchangeCore is
 
     /// @dev Gets information about an order: status, hash, and amount filled.
     /// @param order Order to gather information on.
-    /// @return status Status of order. Statuses are defined in the LibStatus.Status struct.
+    /// @return status Status of order. See LibStatus for a complete description of order statuses.
     /// @return orderHash Keccak-256 EIP712 hash of the order.
     /// @return takerAssetFilledAmount Amount of order that has been filled.
     function getOrderInfo(Order memory order)
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index 744262da2..35d65c213 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -48,6 +48,7 @@ contract MixinMatchOrders is
     /// @param leftSignature Proof that order was created by the left maker.
     /// @param rightSignature Proof that order was created by the right maker.
     /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
+    /// TODO: Make this function external once supported by Solidity (See Solidity Issues #3199, #1603)
     function matchOrders(
         Order memory leftOrder,
         Order memory rightOrder,
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol
index d62690933..e8dbf473b 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol
@@ -51,7 +51,7 @@ contract IExchangeCore {
 
     /// @dev Gets information about an order: status, hash, and amount filled.
     /// @param order Order to gather information on.
-    /// @return status Status of order. Statuses are defined in the LibStatus.Status struct.
+    /// @return status Status of order. See LibStatus for a complete description of order statuses.
     /// @return orderHash Keccak-256 EIP712 hash of the order.
     /// @return takerAssetFilledAmount Amount of order that has been filled.
     function getOrderInfo(LibOrder.Order memory order)
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol
index fe8cee5f1..f4ad20790 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol
@@ -32,6 +32,7 @@ contract IMatchOrders {
     /// @param leftSignature Proof that order was created by the left maker.
     /// @param rightSignature Proof that order was created by the right maker.
     /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
+    /// TODO: Make this function external once supported by Solidity (See Solidity Issues #3199, #1603)
     function matchOrders(
         LibOrder.Order memory leftOrder,
         LibOrder.Order memory rightOrder,
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol
index fea58f64e..c29eb2284 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol
@@ -37,6 +37,8 @@ contract LibStatus {
         INVALID_MAKER,                          // Invalid maker
 
         /// Order State Statuses ///
+        // A valid order remains fillable until it is expired, fully filled, or cancelled.
+        // An order's state is unaffected by external factors, like account balances.
         ORDER_INVALID_MAKER_ASSET_AMOUNT,       // Order does not have a valid maker asset amount
         ORDER_INVALID_TAKER_ASSET_AMOUNT,       // Order does not have a valid taker asset amount
         ORDER_FILLABLE,                         // Order is fillable