diff --git a/contracts/BokkyPooBahsRedBlackTreeLibrary.sol b/contracts/BokkyPooBahsRedBlackTreeLibrary.sol index cdb1a82..41d0936 100644 --- a/contracts/BokkyPooBahsRedBlackTreeLibrary.sol +++ b/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; diff --git a/contracts/SamWitchOrderBook.sol b/contracts/SamWitchOrderBook.sol index 388555e..072e24d 100644 --- a/contracts/SamWitchOrderBook.sol +++ b/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; diff --git a/test/SamWitchOrderBook.ts b/test/SamWitchOrderBook.ts index 541d136..04bd99d 100644 --- a/test/SamWitchOrderBook.ts +++ b/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(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(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(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);