QA Report

[code423n4]

Seaport QA report (Low/Non-critical)

Summary: The code base is well structured and well documented. Although assembly is heavily utilized to optimize the gas spending, it also contains the reference files to compare and test on. One concern going forward is to modify and update the code. One should have a deep understanding to be able to safely change things. After apply any change, it should be thoroughly reviewed and tested.

Low

lack of owner address check in the createConduit

• contracts/conduit/ConduitController.sol:94 github link
• function createConduit

      _conduits[conduit].owner = initialOwner;

  Upon create conduit, createConduit function does not ensure whether the owner address is valid and accessible, such as zero address check. Presumably setting an address one does not have an access is not intended use, since transferring ownership is a two-step process of nominating and accepting. Upon nominating ownership with transferOwnership, zero address is checked for the potential owner (in ConduitController.sol:197) When the user creates a conduit with either zero address or a random address as the owner of the conduit by accident, the conduit is basically useless. The conduit cannot be updated to add channels, nor the ownership can be transferred.

Non-critical

misleading comment in GettersAndDerivers.sol

• contracts/lib/GettersAndDerivers.sol:131 github link
• function _deriveOrderHash

          // Iterate over the offer items (not including tips).
          // prettier-ignore
          for { let i := 0 } lt(i, originalConsiderationLength) {
              i := add(i, 1)
          } {

  This is iterating over the consideration, but the comment says Iterate over the offer items

misleading comment in FulfillmentApplier.sol

• contracts/lib/FulfillmentApplier.sol:188 github link
• function _aggregateValidFulfillmentOfferItems

   * @dev Internal pure function to aggregate a group of offer items using
   *      supplied directives on which component items are candidates for
   *      aggregation, skipping items on orders that are not available.
   *
   * @param advancedOrders  The orders to aggregate offer items from.
   * @param offerComponents An array of FulfillmentComponent structs
   *                        indicating the order index and item index of each
   *                        candidate offer item for aggregation.
   * @param execution       The execution to apply the aggregation to.
   */
  function _aggregateValidFulfillmentOfferItems(
      AdvancedOrder[] memory advancedOrders,
      FulfillmentComponent[] memory offerComponents,
      Execution memory execution
  ) internal view {

  The comment in the line 188 Internal pure function does not match with the function's actual modifier internal view (line 202). The function _aggregateValidFulfillmentOfferItems is not pure as it uses caller() in the line 297.

misleading comment in ReferenceFulfillmentApplier.sol

• reference/lib/ReferenceFulfillmentApplier.sol:252 github link
• function _aggregateValidFulfillmentOfferItems

   * @dev Internal pure function to aggregate a group of offer items using
   *      supplied directives on which component items are candidates for
   *      aggregation, skipping items on orders that are not available.
   *
   * @param ordersToExecute The orders to aggregate offer items from.
   * @param offerComponents An array of FulfillmentComponent structs
   *                        indicating the order index and item index of each
   *                        candidate offer item for aggregation.
   * @param startIndex      The initial order index to begin iteration on when
   *                        searching for offer items to aggregate.
   *
   * @return execution The aggregated offer items.
   */
  function _aggregateValidFulfillmentOfferItems(
      OrderToExecute[] memory ordersToExecute,
      FulfillmentComponent[] memory offerComponents,
      uint256 startIndex
  ) internal view returns (Execution memory execution) {

  The comment in the line 252 Internal pure function does not match the function's actual modifier internal view in the line 269. The function _aggregateValidFulfillmentOfferItems is not pure as it uses msg.sender in the line 296.

missing Natspec in function _callIsValideOrder

• contracts/lib/ZoneInteraction.sol:56 github link
• function _callIsValidOrder is missing Natspec

typo in Assertions.sol

• contracts/lib/Assertions.sol:38 github link

   * @dev Internal view function to to ensure that the supplied consideration

  to is duplicated. function to to ensure -> function to ensure

[GalloDaSballo]

lack of owner address check in the createConduit

Valid Low per #56

Typo /Comments

1 NC