Handle overflow for price if incrementing it at max uint72

contracts/BokkyPooBahsRedBlackTreeLibrary.sol

@@ -14,6 +14,9 @@ pragma solidity >=0.8.20;
 // Enjoy. (c) BokkyPooBah / Bok Consulting Pty Ltd 2020. The MIT Licence.
 // ----------------------------------------------------------------------------
 library BokkyPooBahsRedBlackTreeLibrary {
+
+  error KeyCannotBeZero();
+
   struct Node {
     uint72 parent;
     uint72 left;
@@ -96,7 +99,9 @@ library BokkyPooBahsRedBlackTreeLibrary {
   }
 
   function insert(Tree storage self, uint72 key) internal {
-    require(key != EMPTY);
+    if (key == EMPTY) {
+      revert KeyCannotBeZero();
+    }
     require(!exists(self, key));
     uint72 cursor = EMPTY;
     uint72 probe = self.root;

contracts/SamWitchOrderBook.sol

@@ -6,6 +6,7 @@ import {UnsafeMath} from "@0xdoublesharp/unsafe-math/contracts/UnsafeMath.sol";
 import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import {ERC1155Holder} from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
+import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 import {IERC1155} from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {IERC2981} from "@openzeppelin/contracts/interfaces/IERC2981.sol";
@@ -923,7 +924,7 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
       ) {
         // Loop until we find a suitable place to put this
         while (true) {
-          price_ = uint72(uint128(int72(price_) + _tick));
+          price_ = SafeCast.toUint72(uint128(int128(uint128(price_)) + _tick));
           if (!_tree.exists(price_)) {
             _tree.insert(price_);
             break;

test/SamWitchOrderBook.ts

@@ -1807,6 +1807,59 @@ describe("SamWitchOrderBook", function () {
     expect(orders.length).to.eq(1);
   });
 
+  it("Max number of orders for a price should increment it by the tick, sell orders, check just exceeding the extreme reverts", async function () {
+    const {orderBook, alice, tokenId, maxOrdersPerPrice} = await loadFixture(deployContractsFixture);
+
+    // Set up order book
+    const price = 4722366482869645213695n; // Max price for uint72
+    const quantity = 1;
+
+    const limitOrder: ISamWitchOrderBook.LimitOrderStruct = {
+      side: OrderSide.Sell,
+      tokenId: tokenId,
+      price,
+      quantity,
+    };
+
+    const limitOrders = new Array<ISamWitchOrderBook.LimitOrderStruct>(maxOrdersPerPrice).fill(limitOrder);
+    await orderBook.limitOrders(limitOrders);
+
+    // Try to add one more and it will be reverted
+    await expect(orderBook.connect(alice).limitOrders([limitOrder]))
+      .to.be.revertedWithCustomError(orderBook, "SafeCastOverflowedUintDowncast")
+      .withArgs(72, 4722366482869645213696n);
+  });
+
+  // Similiar to other test, but makes it exceed more
+  it("Max number of orders for a price should increment it by the tick, sell orders, check extreme reverts", async function () {
+    const {orderBook, alice, tokenId, erc1155, maxOrdersPerPrice} = await loadFixture(deployContractsFixture);
+
+    const tick = ethers.parseEther("1");
+    const minQuantity = 1;
+    await orderBook.setTokenIdInfos([tokenId + 1], [{tick, minQuantity}]);
+
+    // Set up order book
+    const price = ethers.parseEther("4722"); // At the extreme end of uint72
+    const quantity = 1;
+    await erc1155.mintSpecificId(tokenId + 1, 100000);
+    await erc1155.connect(alice).mintSpecificId(tokenId + 1, 1);
+
+    const limitOrder: ISamWitchOrderBook.LimitOrderStruct = {
+      side: OrderSide.Sell,
+      tokenId: tokenId + 1,
+      price,
+      quantity,
+    };
+
+    const limitOrders = new Array<ISamWitchOrderBook.LimitOrderStruct>(maxOrdersPerPrice).fill(limitOrder);
+    await orderBook.limitOrders(limitOrders);
+
+    // Try to add one more and it will be reverted
+    await expect(orderBook.connect(alice).limitOrders([limitOrder]))
+      .to.be.revertedWithCustomError(orderBook, "SafeCastOverflowedUintDowncast")
+      .withArgs(72, price + tick);
+  });
+
   it("Max number of orders for a price should increment it by the tick, buy orders", async function () {
     const {orderBook, alice, tokenId, maxOrdersPerPrice, tick} = await loadFixture(deployContractsFixture);
 
@@ -1834,6 +1887,30 @@ describe("SamWitchOrderBook", function () {
     expect(orders.length).to.eq(1);
   });
 
+  it("Max number of orders for a price should increment it by the tick, buy orders, check extreme reverts", async function () {
+    const {orderBook, alice, tokenId, maxOrdersPerPrice, tick} = await loadFixture(deployContractsFixture);
+
+    // Set up order book
+    const price = tick; // Minimum price
+    const quantity = 1;
+
+    const limitOrder: ISamWitchOrderBook.LimitOrderStruct = {
+      side: OrderSide.Buy,
+      tokenId,
+      price,
+      quantity,
+    };
+
+    const limitOrders = new Array<ISamWitchOrderBook.LimitOrderStruct>(maxOrdersPerPrice).fill(limitOrder);
+    await orderBook.limitOrders(limitOrders);
+
+    // Try to add one more and it will be reverted
+    await expect(orderBook.connect(alice).limitOrders([limitOrder])).to.be.revertedWithCustomError(
+      orderBook,
+      "KeyCannotBeZero",
+    );
+  });
+
   it("Max number of orders for a price should increment it by the tick, where the price level exists already and has spare segments", async function () {
     const {orderBook, alice, tokenId, maxOrdersPerPrice, tick} = await loadFixture(deployContractsFixture);