Skip to content

Commit

Permalink
implemented generic sanity checks
Browse files Browse the repository at this point in the history
  • Loading branch information
Hammad Ghazi committed Oct 10, 2023
1 parent 88f6cab commit f64c44d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
35 changes: 34 additions & 1 deletion src/AdvancedOrderEngine.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,50 @@
pragma solidity 0.8.21;

import {OrderEngine} from "./libraries/OrderEngine.sol";
import "./AdvancedOrderEngineErrors.sol";

contract AdvancedOrderEngine {
using OrderEngine for OrderEngine.Order;

/**
* @notice Fills multiple orders by processing the specified orders and clearing prices.
*
* @param orders An array of order structs representing the orders to be filled.
* @param clearingPrices An array of clearing prices that the facilitator is offering to the makers.
* @param facilitatorInteractionCalldata The calldata for the facilitator's interaction.
* @param facilitatorInteractionTargetContract The address of the facilitator's target contract.
*/
function fillOrders(
OrderEngine.Order[] calldata orders,
uint256[] calldata clearingPrices,
bytes calldata facilitatorInteractionCalldata,
address facilitatorInteractionTargetContract
) external {
// Perform sanity checks (for example, orders and clearing prices array must have same length, etc)
// TBD: max array length check needed? Considering fn will be restricted to operators only

/**
TBD: no need to check for clearingPrices length to be equal to 0 as if that's the case, txn will revert in subsequent check
but should we check for clearingPrices length to be equal to zero explicitly for better error reporting?
Also consider generic error message
*/
// Revert if the orders array length is zero.
if (orders.length == 0) {
revert EmptyOrdersArray();
}

// Revert if the length of the orders array does not match the clearing prices array.
if (orders.length != clearingPrices.length) {
revert ArraysLengthMismatch(orders.length, clearingPrices.length);
}

// Revert if the facilitator has provided calldata for its interaction but has provided null target contract address.
if (
facilitatorInteractionCalldata.length != 0 &&
facilitatorInteractionTargetContract == address(0)
) {
revert ZeroFacilitatorTargetAddress(); // TBD: use generic error message i.e. ZeroAddress()
}

// Loop start
// Perform order specific sanity checks
// Verify signatjures
Expand Down
6 changes: 6 additions & 0 deletions src/AdvancedOrderEngineErrors.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.21;

error ArraysLengthMismatch(uint256 ordersArrLen, uint256 clearingPricesArrLen);
error ZeroFacilitatorTargetAddress();
error EmptyOrdersArray();

2 comments on commit f64c44d

@gul-hameed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too long function params variable names. make it short and explain it int the netspec for clarity

@gul-hameed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Conventional Commits specification for our commit messages. This standard makes our commit history more readable and allows for automated changelog generation. By doing so, we can maintain a well-structured log of changes, making version tracking and upgrades more transparent for all users.
More about Conventional Commits: https://www.conventionalcommits.org/

Please sign in to comment.