Move events to interface and add more comments

PaimaStudios · Apr 2, 2024 · 309a593 · 309a593
1 parent 0a11c66
commit 309a593
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 20 deletions.
diff --git a/packages/contracts/evm-contracts/contracts/orderbook/IOrderbookDex.sol b/packages/contracts/evm-contracts/contracts/orderbook/IOrderbookDex.sol
@@ -5,6 +5,7 @@ pragma solidity ^0.8.20;
 import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
 
 /// @notice Facilitates base-chain trading of an asset that is living on a different app-chain.
+/// @dev The contract should never hold any ETH itself.
 interface IOrderbookDex is IERC165 {
     struct Order {
         uint256 assetAmount;
@@ -13,6 +14,21 @@ interface IOrderbookDex is IERC165 {
         bool active;
     }
 
+    event OrderCreated(
+        uint256 indexed orderId,
+        address indexed seller,
+        uint256 assetAmount,
+        uint256 price
+    );
+    event OrderFilled(
+        uint256 indexed orderId,
+        address indexed seller,
+        address indexed buyer,
+        uint256 assetAmount,
+        uint256 price
+    );
+    event OrderCancelled(uint256 indexed orderId);
+
     /// @notice Returns the current index of orders (index that a new sell order will be mapped to).
     function getOrdersIndex() external view returns (uint256);
 
@@ -21,14 +37,20 @@ interface IOrderbookDex is IERC165 {
 
     /// @notice Creates a sell order for the specified `assetAmount` at specified `price`.
     /// @dev The order is saved in a mapping from incremental ID to Order struct.
+    /// MUST emit `OrderCreated` event.
     function createSellOrder(uint256 assetAmount, uint256 price) external;
 
-    /// @notice Fills an array of orders specified by `orderIds`.
-    /// @dev Reverts if msg.value is less than the sum of orders' prices.
-    /// If msg.value is more than the sum of orders' prices, it should refund the difference back to msg.sender.
+    /// @notice Fills an array of orders specified by `orderIds`, transferring portion of msg.value
+    /// to the orders' sellers according to the price.
+    /// @dev MUST revert if `active` parameter is `false` for any of the orders.
+    /// MUST change the `active` parameter for the specified order to `false`.
+    /// MUST emit `OrderFilled` event for each order.
+    /// If msg.value is more than the sum of orders' prices, it SHOULD refund the difference back to msg.sender.
     function fillSellOrders(uint256[] memory orderIds) external payable;
 
     /// @notice Cancels the sell order specified by `orderId`, making it unfillable.
     /// @dev Reverts if the msg.sender is not the order's seller.
+    /// MUST change the `active` parameter for the specified order to `false`.
+    /// MUST emit `OrderCancelled` event.
     function cancelSellOrder(uint256 orderId) external;
 }
diff --git a/packages/contracts/evm-contracts/contracts/orderbook/OrderbookDex.sol b/packages/contracts/evm-contracts/contracts/orderbook/OrderbookDex.sol
@@ -15,21 +15,6 @@ contract OrderbookDex is IOrderbookDex, ERC165, ReentrancyGuard {
     mapping(uint256 => Order) public orders;
     uint256 public ordersIndex;
 
-    event OrderCreated(
-        uint256 indexed orderId,
-        address indexed seller,
-        uint256 assetAmount,
-        uint256 price
-    );
-    event OrderFilled(
-        uint256 indexed orderId,
-        address indexed seller,
-        address indexed buyer,
-        uint256 assetAmount,
-        uint256 price
-    );
-    event OrderCancelled(uint256 indexed orderId);
-
     error OrderIsInactive(uint256 orderId);
     error Unauthorized();
 

diff --git a/packages/contracts/evm-contracts/test/OrderbookDex.t.sol b/packages/contracts/evm-contracts/test/OrderbookDex.t.sol
@@ -42,7 +42,7 @@ contract OrderbookDexTest is CTest {
             vm.prank(seller);
 
             vm.expectEmit(true, true, true, true);
-            emit OrderbookDex.OrderCreated(orderId, seller, assetAmount, price);
+            emit IOrderbookDex.OrderCreated(orderId, seller, assetAmount, price);
             dex.createSellOrder(assetAmount, price);
 
             uint256 newOrderId = dex.getOrdersIndex();
@@ -70,7 +70,7 @@ contract OrderbookDexTest is CTest {
                 OrderbookDex.Order memory order = dex.getOrder(firstOrderId + i);
                 sellersBalancesBefore[i] = order.seller.balance;
                 vm.expectEmit(true, true, true, true);
-                emit OrderbookDex.OrderFilled(
+                emit IOrderbookDex.OrderFilled(
                     firstOrderId + i,
                     order.seller,
                     buyer,