Merge pull request 1inch#249 from 1inch/docs/orders

[SC-804] Natspec for limit order protocol

contracts/OrderLib.sol

@@ -10,22 +10,27 @@ import "./libraries/MakerTraitsLib.sol";
 import "./libraries/ExtensionLib.sol";
 import "./helpers/AmountCalculator.sol";
 
-/// @title OrderUtils
-/// @dev Library for common order functions.
-library OrderLib {
+/**
+ * @title OrderLib
+ * @dev The library provides common functionality for processing and manipulating limit orders. 
+ * It provides functionality to calculate and verify order hashes, calculate trade amounts, and validate 
+ * extension data associated with orders. The library also contains helper methods to get the receiver of 
+ * an order and call getter functions.
+ */
+ library OrderLib {
     using AddressLib for Address;
     using MakerTraitsLib for MakerTraits;
     using ExtensionLib for bytes;
 
-    /// @dev Error for incorrect getter function called.
+    /// @dev Error to be thrown when the incorrect getter function called.
     error WrongGetter();
-    /// @dev Error when the amount call fails.
+    /// @dev Error to be thrown when the call to get the amount fails.
     error GetAmountCallFailed();
-    /// @dev Error for missing order extension.
+    /// @dev Error to be thrown when the extension data of an order is missing.
     error MissingOrderExtension();
-    /// @dev Error for unexpected order extension.
+    /// @dev Error to be thrown when the order has an unexpected extension.
     error UnexpectedOrderExtension();
-    /// @dev Error for invalid extension.
+    /// @dev Error to be thrown when the order extension is invalid.
     error ExtensionInvalid();
 
     /// @dev The typehash of the order struct.
@@ -45,10 +50,10 @@ library OrderLib {
     uint256 constant internal _DATA_HASH_SIZE = 0x120;
 
     /**
-      * @dev Calculates the hash of an order.
-      * @param order The order to hash.
-      * @param domainSeparator The EIP-712 domain separator to use.
-      * @return result The hash of the order.
+      * @notice Calculates the hash of an order.
+      * @param order The order to be hashed.
+      * @param domainSeparator The domain separator to be used for the EIP-712 hashing.
+      * @return result The keccak256 hash of the order data.
       */
     function hash(IOrderMixin.Order calldata order, bytes32 domainSeparator) internal pure returns(bytes32 result) {
         bytes32 typehash = _LIMIT_ORDER_TYPEHASH;
@@ -64,19 +69,22 @@ library OrderLib {
     }
 
     /**
-      * @dev Gets the receiver address of an order.
+      * @notice Returns the receiver address for an order.
       * @param order The order.
-      * @return receiver The receiver address.
+      * @return receiver The address of the receiver, either explicitly defined in the order or the maker's address if not specified.
       */
-    function getReceiver(IOrderMixin.Order calldata order) internal pure returns(address) {
+    function getReceiver(IOrderMixin.Order calldata order) internal pure returns(address /*receiver*/) {
         address receiver = order.receiver.get();
         return receiver != address(0) ? receiver : order.maker.get();
     }
 
-    /** @dev Calculates the amount of the asset the maker receives in exchange for the asset they provide.
+    /** 
+      * @notice Calculates the making amount based on the requested taking amount.
+      * @dev If getter is specified in the extension data, the getter is called to calculate the making amount,
+      * otherwise the making amount is calculated linearly.
       * @param order The order.
       * @param extension The extension data associated with the order.
-      * @param requestedTakingAmount The amount of the asset the taker wants to take.
+      * @param requestedTakingAmount The amount the taker wants to take.
       * @param remainingMakingAmount The remaining amount of the asset left to fill.
       * @param orderHash The hash of the order.
       * @return makingAmount The amount of the asset the maker receives.
@@ -96,12 +104,14 @@ library OrderLib {
         return _callGetter(getter, requestedTakingAmount, remainingMakingAmount, orderHash);
     }
 
-
     /**
+      * @notice Calculates the taking amount based on the requested making amount.
+      * @dev If getter is specified in the extension data, the getter is called to calculate the taking amount,
+      * otherwise the taking amount is calculated linearly.
       * @param order The order.
       * @param extension The extension data associated with the order.
-      * @param requestedMakingAmount The amount of the asset the maker wants to receive.
-      * @param remainingMakingAmount The remaining amount of the asset left to fill.
+      * @param requestedMakingAmount The amount the maker wants to receive.
+      * @param remainingMakingAmount The remaining amount of the asset left to be filled.
       * @param orderHash The hash of the order.
       * @return takingAmount The amount of the asset the taker takes.
       */
@@ -121,7 +131,7 @@ library OrderLib {
     }
 
     /**
-      * @dev Internal function that calls a getter function to calculate an amount for a trade.
+      * @notice Calls the getter function to calculate an amount for a trade.
       * @param getter The address of the getter function.
       * @param requestedAmount The amount requested by the taker.
       * @param remainingMakingAmount The remaining amount of the asset left to fill.

contracts/OrderMixin.sol

@@ -268,6 +268,29 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
         (makingAmount, takingAmount) = _fillOrderTo(order, orderHash, extension, remainingMakingAmount, amount, takerTraits, target, interaction);
     }
 
+
+    /**
+      * @notice Fills an order and transfers making amount to a specified target.
+      * @dev If the target is zero assigns it the caller's address. 
+      * The function flow is as follows:
+      * 1. Validate order
+      * 2. Call maker pre-interaction 
+      * 3. Transfer maker asset to taker
+      * 4. Call taker interaction
+      * 5. Transfer taker asset to maker
+      * 5. Call maker post-interaction
+      * 6. Emit OrderFilled event
+      * @param order The order details.
+      * @param orderHash The hash of the order.
+      * @param extension The extension calldata of the order.
+      * @param remainingMakingAmount The remaining amount to be filled.
+      * @param amount The order amount.
+      * @param takerTraits The taker preferences for the order.
+      * @param target The address to which the order is filled.
+      * @param interaction The interaction calldata.
+      * @return makingAmount The computed amount that the maker will get.
+      * @return takingAmount The computed amount that the taker will send.
+      */
     function _fillOrderTo(
         IOrderMixin.Order calldata order,
         bytes32 orderHash,
@@ -445,6 +468,13 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
         emit OrderFilled(orderHash, makingAmount);
     }
 
+    /**
+      * @notice Checks the remaining making amount for the order.
+      * @dev If the order has been invalidated, the function will revert.
+      * @param order The order to check.
+      * @param orderHash The hash of the order.
+      * @return remainingMakingAmount The remaining amount of the order.
+      */
     function _checkRemainingMakingAmount(IOrderMixin.Order calldata order, bytes32 orderHash) private view returns(uint256 remainingMakingAmount) {
         if (order.makerTraits.useBitInvalidator()) {
             remainingMakingAmount = order.makingAmount;
@@ -454,6 +484,12 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
         if (remainingMakingAmount == 0) revert InvalidatedOrder();
     }
 
+    /**
+      * @notice Executes the maker's permit and checks for potential reentrancy attacks.
+      * @param order The order to apply the permit for.
+      * @param orderHash The hash of the order.
+      * @param extension The extension data associated with the order.
+      */
     function _applyMakerPermit(IOrderMixin.Order calldata order, bytes32 orderHash, bytes calldata extension) private {
         bytes calldata makerPermit = extension.makerPermit();
         if (makerPermit.length >= 20) {
@@ -466,6 +502,16 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
         }
     }
 
+    /**
+      * @notice Calls the transferFrom function with an arbitrary suffix.
+      * @dev The suffix is appended to the end of the standard ERC20 transferFrom function parameters.
+      * @param asset The token to be transferred.
+      * @param from The address to transfer the token from.
+      * @param to The address to transfer the token to.
+      * @param amount The amount of the token to transfer.
+      * @param suffix The suffix (additional data) to append to the end of the transferFrom call.
+      * @return success A boolean indicating whether the transfer was successful.
+      */
     function _callTransferFromWithSuffix(address asset, address from, address to, uint256 amount, bytes calldata suffix) private returns(bool success) {
         bytes4 selector = IERC20.transferFrom.selector;
         assembly ("memory-safe") { // solhint-disable-line no-inline-assembly

contracts/libraries/BitInvalidatorLib.sol

@@ -4,36 +4,38 @@ pragma solidity ^0.8.0;
 
 /**
  * @title BitInvalidatorLib
- * @dev This library provides functionality to invalidate objects based on a bit invalidator.
- * The Data struct holds a mapping where each key represents a slot number and each value contains an integer.
- * Each bit of the integer represents whether the corresponding object is still valid or has been invalidated (0 - valid, 1 - invalidated).
- * To access an object's state or invalidate it, a nonce must be passed that identifies it, and follows the following rules:
+ * @dev The library provides a mechanism to invalidate objects based on a bit invalidator.
+ * The bit invalidator holds a mapping where each key represents a slot number and each value contains an integer.
+ * Each bit of the integer represents whether the object with corresponding index is valid or has been invalidated (0 - valid, 1 - invalidated).
+ * The nonce given to access or invalidate an entity's state follows this structure:
  * - bits [0..7] represent the object state index in the slot.
  * - bits [8..255] represent the slot number (mapping key).
  */
 library BitInvalidatorLib {
-    /// @dev Error thrown when a bit has already been invalidated for an order.
+    /// @dev The error is thrown when an attempt is made to invalidate an already invalidated entity.
     error BitInvalidatedOrder();
 
     struct Data {
         mapping(uint256 => uint256) _raw;
     }
 
     /**
-     * @dev Returns the values for a specific slot.
+     * @notice Retrieves the validity status of entities in a specific slot.
+     * @dev Each bit in the returned value corresponds to the validity of an entity. 0 for valid, 1 for invalidated.
      * @param self The data structure.
-     * @param nonce The nonce specifing the slot.
-     * @return uint256 The value of the specific slot.
+     * @param nonce The nonce identifying the slot.
+     * @return result The validity status of entities in the slot as a uint256.
      */
     function checkSlot(Data storage self, uint256 nonce) internal view returns(uint256) {
         uint256 invalidatorSlot = nonce >> 8;
         return self._raw[invalidatorSlot];
     }
 
     /**
-     * @dev Invalidates the specific bit in the specific slot.
+     * @notice Checks the validity of a specific entity and invalidates it if valid.
+     * @dev Throws an error if the entity has already been invalidated.
      * @param self The data structure.
-     * @param nonce The nonce specifing the slot and the object index.
+     * @param nonce The nonce identifying the slot and the entity.
      */
     function checkAndInvalidate(Data storage self, uint256 nonce) internal {
         uint256 invalidatorSlot = nonce >> 8;
@@ -43,11 +45,11 @@ library BitInvalidatorLib {
         self._raw[invalidatorSlot] = invalidator | invalidatorBit;
     }
 
-
     /**
-     * @dev Invalidates multiple objects in a single slot.
+     * @notice Invalidates multiple entities in a single slot.
+     * @dev The entities to be invalidated are identified by setting their corresponding bits to 1 in a mask.
      * @param self The data structure.
-     * @param nonce The nonce specifing the slot.
+     * @param nonce The nonce identifying the slot.
      * @param additionalMask A mask of bits to be invalidated.
      */
     function massInvalidate(Data storage self, uint256 nonce, uint256 additionalMask) internal {

contracts/libraries/ExtensionLib.sol

@@ -5,6 +5,10 @@ pragma solidity ^0.8.0;
 import "../interfaces/IOrderMixin.sol";
 import "./OffsetsLib.sol";
 
+/**
+ * @title ExtensionLib
+ * @notice Library for retrieving extensions information for the IOrderMixin Interface.
+ */
 library ExtensionLib {
     using AddressLib for Address;
     using OffsetsLib for Offsets;
@@ -20,38 +24,86 @@ library ExtensionLib {
         PostInteractionData
     }
 
+    /**
+     * @notice Returns the MakerAssetSuffix from the provided extension calldata.
+     * @param extension The calldata from which the MakerAssetSuffix is to be retrieved.
+     * @return calldata Bytes representing the MakerAssetSuffix.
+     */
     function makerAssetSuffix(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.MakerAssetSuffix);
     }
 
+    /**
+     * @notice Returns the TakerAssetSuffix from the provided extension calldata.
+     * @param extension The calldata from which the TakerAssetSuffix is to be retrieved.
+     * @return calldata Bytes representing the TakerAssetSuffix.
+     */
     function takerAssetSuffix(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.TakerAssetSuffix);
     }
 
+    /**
+     * @notice Returns the MakingAmountGetter from the provided extension calldata.
+     * @param extension The calldata from which the MakingAmountGetter is to be retrieved.
+     * @return calldata Bytes representing the MakingAmountGetter.
+     */
     function makingAmountGetter(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.MakingAmountGetter);
     }
 
+    /**
+     * @notice Returns the TakingAmountGetter from the provided extension calldata.
+     * @param extension The calldata from which the TakingAmountGetter is to be retrieved.
+     * @return calldata Bytes representing the TakingAmountGetter.
+     */
     function takingAmountGetter(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.TakingAmountGetter);
     }
 
+    /**
+     * @notice Returns the order's predicate from the provided extension calldata.
+     * @param extension The calldata from which the predicate is to be retrieved.
+     * @return calldata Bytes representing the predicate.
+     */
     function predicate(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.Predicate);
     }
 
+    /**
+     * @notice Returns the maker's permit from the provided extension calldata.
+     * @param extension The calldata from which the maker's permit is to be retrieved.
+     * @return calldata Bytes representing the maker's permit.
+     */
     function makerPermit(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.MakerPermit);
     }
 
+
+    /**
+     * @notice Returns the pre-interaction from the provided extension calldata.
+     * @param extension The calldata from which the pre-interaction is to be retrieved.
+     * @return calldata Bytes representing the pre-interaction.
+     */
     function preInteractionTargetAndData(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.PreInteractionData);
     }
 
+    /**
+     * @notice Returns the post-interaction from the provided extension calldata.
+     * @param extension The calldata from which the post-interaction is to be retrieved.
+     * @return calldata Bytes representing the post-interaction.
+     */
     function postInteractionTargetAndData(bytes calldata extension) internal pure returns(bytes calldata) {
         return _get(extension, DynamicField.PostInteractionData);
     }
 
+    /**
+     * @notice Retrieves a specific field from the provided extension calldata.
+     * @dev The first 32 bytes of an extension calldata contain offsets to the end of each field within the calldata.
+     * @param extension The calldata from which the field is to be retrieved.
+     * @param field The specific dynamic field to retrieve from the extension.
+     * @return result A bytes calldata representing the requested field.
+     */
     function _get(bytes calldata extension, DynamicField field) private pure returns(bytes calldata result) {
         if (extension.length < 0x20) return msg.data[:0];