Get full code coverage with new tests

PaintSwap · Jan 31, 2024 · 8a962e3 · 8a962e3
1 parent 7f64252
commit 8a962e3
Show file tree

Hide file tree

Showing 6 changed files with 448 additions and 171 deletions.
diff --git a/README.md b/README.md
@@ -4,6 +4,8 @@
 
 ![swob](https://github.com/PaintSwap/samwitch-orderbook/assets/84033732/977c060f-e6e7-418f-9d44-1012599f41c6)
 
+![Code coverage](https://github.com/PaintSwap/samwitch-orderbook/assets/84033732/34d71d0b-45c4-4871-a5ed-67922b417a95)
+
 This efficient order book utilises the `BokkyPooBahsRedBlackTreeLibrary` library for sorting prices allowing `O(log n)` for tree segment insertion, traversal, and deletion. It supports batch orders and batch cancelling, `ERC2981` royalties, and a dev and burn fee on each trade.
 
 It is kept gas efficient by packing data in many areas:
@@ -16,7 +18,7 @@ The order book is kept healthy by requiring a minimum quantity that can be added
 
 Constraints:
 
-- The order quantity is limited to ~16mil
+- The order quantity to be added to the book is limited to ~16mil
 - The maximum number of orders in the book that can ever be added is limited to 1 trillion
 - The maximum number of orders that can be added to a specific price level in its lifetime is 16 billion
 

diff --git a/contracts/SamWitchOrderBook.sol b/contracts/SamWitchOrderBook.sol
@@ -15,6 +15,8 @@ import {BokkyPooBahsRedBlackTreeLibrary} from "./BokkyPooBahsRedBlackTreeLibrary
 import {IBrushToken} from "./interfaces/IBrushToken.sol";
 import {ISamWitchOrderBook} from "./interfaces/ISamWitchOrderBook.sol";
 
+import "hardhat/console.sol";
+
 /// @title SamWitchOrderBook (SWOB)
 /// @author Sam Witch (PaintSwap & Estfor Kingdom)
 /// @author 0xDoubleSharp
@@ -43,7 +45,7 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
   uint8 private burntFee;
   uint16 private royaltyFee;
   uint16 private maxOrdersPerPrice;
-  uint40 public nextOrderId;
+  uint40 private nextOrderId;
   address private royaltyRecipient;
 
   mapping(uint tokenId => TokenIdInfo tokenIdInfo) public tokenIdInfos;
@@ -79,17 +81,7 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
     __UUPSUpgradeable_init();
     __Ownable_init(_msgSender());
 
-    // make sure dev address/fee is set appropriately
-    if (_devFee != 0) {
-      if (_devAddr == address(0)) {
-        revert ZeroAddress();
-      } else if (_devFee > 1000) {
-        revert DevFeeTooHigh();
-      }
-    } else if (_devAddr != address(0)) {
-      revert DevFeeNotSet();
-    }
-
+    setFees(_devAddr, _devFee, _burntFee);
     // nft must be an ERC1155 via ERC165
     if (!_nft.supportsInterface(type(IERC1155).interfaceId)) {
       revert NotERC1155();
@@ -99,9 +91,6 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
     token = IBrushToken(_token);
     updateRoyaltyFee();
 
-    devFee = _devFee; // 30 = 0.3% fee,
-    devAddr = _devAddr;
-    burntFee = _burntFee; // 30 = 0.3% fee,
     setMaxOrdersPerPrice(_maxOrdersPerPrice); // This includes inside segments, so num segments = maxOrdersPrice / NUM_ORDERS_PER_SEGMENT
     nextOrderId = 1;
   }
@@ -256,7 +245,7 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
     }
   }
 
-  /// @notice Claim NFTs associated with filled or partially filled orders.
+  /// @notice Claim tokens associated with filled or partially filled orders.
   ///         Must be the maker of these orders.
   /// @param _orderIds Array of order IDs from which to claim NFTs
   function claimTokens(uint[] calldata _orderIds) public override {
@@ -276,22 +265,10 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
       brushClaimable[orderId] = 0;
     }
 
-    if (amount == 0) {
-      revert NothingToClaim();
-    }
-
     (uint royalty, uint dev, uint burn) = _calcFees(amount);
     uint fees = royalty.add(dev).add(burn);
-    uint amountExclFees = 0;
-    if (amount > fees) {
-      amountExclFees = amount.sub(fees);
-    }
-
+    token.safeTransfer(_msgSender(), amount.sub(fees));
     emit ClaimedTokens(_msgSender(), _orderIds, amount, fees);
-
-    if (amountExclFees != 0) {
-      token.safeTransfer(_msgSender(), amountExclFees);
-    }
   }
 
   /// @notice Claim NFTs associated with filled or partially filled orders
@@ -316,9 +293,8 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
       tokenIdsClaimableForOrder[tokenId] = 0;
     }
 
-    emit ClaimedNFTs(_msgSender(), _orderIds, _tokenIds, nftAmountsFromUs);
-
     _safeBatchTransferNFTsFromUs(_msgSender(), _tokenIds, nftAmountsFromUs);
+    emit ClaimedNFTs(_msgSender(), _orderIds, _tokenIds, nftAmountsFromUs);
   }
 
   /// @notice Convience function to claim both tokens and nfts in filled or partially filled orders.
@@ -337,15 +313,15 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
 
   /// @notice Get the amount of tokens claimable for these orders
   /// @param _orderIds The order IDs to get the claimable tokens for
-  /// @param takeAwayFees Whether to take away the fees from the claimable amount
+  /// @param _takeAwayFees Whether to take away the fees from the claimable amount
   function tokensClaimable(
     uint40[] calldata _orderIds,
-    bool takeAwayFees
+    bool _takeAwayFees
   ) external view override returns (uint amount_) {
     for (uint i = 0; i < _orderIds.length; ++i) {
       amount_ += brushClaimable[_orderIds[i]];
     }
-    if (takeAwayFees) {
+    if (_takeAwayFees) {
       (uint royalty, uint dev, uint burn) = _calcFees(amount_);
       amount_ = amount_.sub(royalty).sub(dev).sub(burn);
     }
@@ -404,18 +380,6 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
     }
   }
 
-  /// @notice Get the tick size for a specific token ID
-  /// @param _tokenId The token ID to get the tick size for
-  function getTick(uint _tokenId) external view override returns (uint) {
-    return tokenIdInfos[_tokenId].tick;
-  }
-
-  /// @notice The minimum amount that can be added to the order book for a specific token ID, to keep the order book healthy
-  /// @param _tokenId The token ID to get the minimum quantity for
-  function getMinAmount(uint _tokenId) external view override returns (uint) {
-    return tokenIdInfos[_tokenId].minQuantity;
-  }
-
   /// @notice Get all orders at a specific price level
   /// @param _side The side of the order book to get orders from
   /// @param _tokenId The token ID to get orders for
@@ -439,6 +403,9 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
       (address _royaltyRecipient, uint _royaltyFee) = IERC2981(address(nft)).royaltyInfo(1, 10000);
       royaltyRecipient = _royaltyRecipient;
       royaltyFee = uint16(_royaltyFee);
+    } else {
+      royaltyRecipient = address(0);
+      royaltyFee = 0;
     }
   }
 
@@ -465,7 +432,25 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
     emit SetTokenIdInfos(_tokenIds, _tokenIdInfos);
   }
 
-  // TODO: editOrder
+  /// @notice Set the fees for the contract
+  /// @param _devAddr The address to receive trade fees
+  /// @param _devFee The fee to send to the dev address (max 10%)
+  /// @param _burntFee The fee to burn (max 2%)
+  function setFees(address _devAddr, uint16 _devFee, uint8 _burntFee) public onlyOwner {
+    if (_devFee != 0) {
+      if (_devAddr == address(0)) {
+        revert ZeroAddress();
+      } else if (_devFee > 1000) {
+        revert DevFeeTooHigh();
+      }
+    } else if (_devAddr != address(0)) {
+      revert DevFeeNotSet();
+    }
+    devFee = _devFee; // 30 = 0.3% fee
+    devAddr = _devAddr;
+    burntFee = _burntFee;
+    emit SetFees(_devAddr, _devFee, _burntFee);
+  }
 
   function _takeFromOrderBookSide(
     uint _tokenId,
@@ -704,27 +689,14 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
 
     BokkyPooBahsRedBlackTreeLibrary.Node storage node = _tree.getNode(_price);
     uint tombstoneOffset = node.tombstoneOffset;
-    uint numInSegmentDeleted;
-    {
-      uint packed = uint(_packedOrderBookEntries[tombstoneOffset]);
-      for (uint offset; offset < NUM_ORDERS_PER_SEGMENT; ++offset) {
-        uint remainingSegment = uint64(packed >> offset.mul(64));
-        uint64 order = uint64(remainingSegment);
-        if (order == 0) {
-          numInSegmentDeleted = numInSegmentDeleted.inc();
-        } else {
-          break;
-        }
-      }
-    }
 
     (uint index, uint offset) = _find(
       _packedOrderBookEntries,
       tombstoneOffset,
       _packedOrderBookEntries.length,
       _orderId
     );
-    if (index == type(uint).max || (index == tombstoneOffset && offset < numInSegmentDeleted)) {
+    if (index == type(uint).max) {
       revert OrderNotFound(_orderId, _price);
     }
     quantity_ = uint24(uint(_packedOrderBookEntries[index]) >> offset.mul(64).add(40));
@@ -1001,7 +973,5 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
   }
 
   // solhint-disable-next-line no-empty-blocks
-  function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
-    // upgradeable by owner
-  }
+  function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
 }
diff --git a/contracts/interfaces/ISamWitchOrderBook.sol b/contracts/interfaces/ISamWitchOrderBook.sol
@@ -42,6 +42,7 @@ interface ISamWitchOrderBook is IERC1155Receiver {
   event ClaimedNFTs(address user, uint[] orderIds, uint[] tokenIds, uint[] amounts);
   event SetTokenIdInfos(uint[] tokenIds, TokenIdInfo[] tokenIdInfos);
   event SetMaxOrdersPerPriceLevel(uint maxOrdersPerPrice);
+  event SetFees(address devAddr, uint devFee, uint burntFee);
 
   error ZeroAddress();
   error DevFeeNotSet();
@@ -62,42 +63,38 @@ interface ISamWitchOrderBook is IERC1155Receiver {
   error InvalidNonce(uint invalid, uint nonce);
   error InvalidSignature(address sender, address recoveredAddress);
 
-  function limitOrders(LimitOrder[] calldata _orders) external;
+  function limitOrders(LimitOrder[] calldata orders) external;
 
-  function cancelOrders(uint[] calldata _orderIds, CancelOrder[] calldata _cancelOrderInfos) external;
+  function cancelOrders(uint[] calldata orderIds, CancelOrder[] calldata cancelOrderInfos) external;
 
   function claimTokens(uint[] calldata _orderIds) external;
 
-  function claimNFTs(uint[] calldata _orderIds, uint[] calldata _tokenIds) external;
+  function claimNFTs(uint[] calldata orderIds, uint[] calldata tokenIds) external;
 
-  function claimAll(uint[] calldata _brushOrderIds, uint[] calldata _nftOrderIds, uint[] calldata _tokenIds) external;
+  function claimAll(uint[] calldata brushOrderIds, uint[] calldata nftOrderIds, uint[] calldata tokenIds) external;
 
-  function tokensClaimable(uint40[] calldata _orderIds, bool takeAwayFees) external view returns (uint amount);
+  function tokensClaimable(uint40[] calldata orderIds, bool takeAwayFees) external view returns (uint amount);
 
   function nftsClaimable(
-    uint40[] calldata _orderIds,
-    uint[] calldata _tokenIds
+    uint40[] calldata orderIds,
+    uint[] calldata tokenIds
   ) external view returns (uint[] memory amounts);
 
-  function getHighestBid(uint _tokenId) external view returns (uint72);
+  function getHighestBid(uint tokenId) external view returns (uint72);
 
-  function getLowestAsk(uint _tokenId) external view returns (uint72);
+  function getLowestAsk(uint tokenId) external view returns (uint72);
 
   function getNode(
-    OrderSide _side,
-    uint _tokenId,
-    uint72 _price
+    OrderSide side,
+    uint tokenId,
+    uint72 price
   ) external view returns (BokkyPooBahsRedBlackTreeLibrary.Node memory);
 
-  function nodeExists(OrderSide _side, uint _tokenId, uint72 _price) external view returns (bool);
-
-  function getTick(uint _tokenId) external view returns (uint);
-
-  function getMinAmount(uint _tokenId) external view returns (uint);
+  function nodeExists(OrderSide side, uint tokenId, uint72 price) external view returns (bool);
 
   function allOrdersAtPrice(
-    OrderSide _side,
-    uint _tokenId,
-    uint72 _price
+    OrderSide side,
+    uint tokenId,
+    uint72 price
   ) external view returns (OrderBookEntryHelper[] memory orderBookEntries);
 }
diff --git a/contracts/test/MockERC1155NoRoyalty.sol b/contracts/test/MockERC1155NoRoyalty.sol
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.23;
+
+import {ERC1155} from "@openzeppelin/contracts/token/ERC1155/ERC1155.sol";
+import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";
+
+contract MockERC1155NoRoyalty is ERC1155 {
+  uint64 public nextId = 1;
+
+  constructor() ERC1155("") {}
+
+  function mint(uint _quantity) external {
+    _mint(_msgSender(), nextId++, _quantity, "");
+  }
+
+  function mintSpecificId(uint _id, uint _quantity) external {
+    _mint(_msgSender(), _id, _quantity, "");
+  }
+
+  function mintBatch(uint[] memory _amounts) external {
+    uint[] memory ids = new uint[](_amounts.length);
+    for (uint i = 0; i < _amounts.length; ++i) {
+      ids[i] = nextId++;
+    }
+    _mintBatch(_msgSender(), ids, _amounts, "");
+  }
+}
diff --git a/hardhat.config.ts b/hardhat.config.ts
@@ -1,4 +1,4 @@
-import {HardhatUserConfig, task} from "hardhat/config";
+import {HardhatUserConfig} from "hardhat/config";
 import "@nomicfoundation/hardhat-toolbox";
 import "@openzeppelin/hardhat-upgrades";
 import "hardhat-abi-exporter";
@@ -65,7 +65,7 @@ const config: HardhatUserConfig = {
   solidity: {
     compilers: [defaultConfig, lowRunsConfig, mediumRunsConfig, highRunsConfig],
     overrides: {
-      "contracts/SamWitchOrderBook.sol": highRunsConfig,
+      "contracts/SamWitchOrderBook.sol": mediumRunsConfig,
     },
   },
   gasReporter: {