Skip to content

Commit

Permalink
Disallow changing tick after it's set
Browse files Browse the repository at this point in the history
  • Loading branch information
0xSamWitch committed Feb 11, 2024
1 parent bdc4f29 commit f20cca6
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 16 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ Constraints:
- 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
- Cannot change tick for a tokenId after it's been set

While this order book was created for `ERC1155` NFTs it could be adapted for `ERC20` tokens.

> _Note: Not suitable for production until more tests are added with more code coverage._
Potential improvements:

- Use an `orderId` per price level insted of global, so that they are always sequential
- TokenInfo could use uint64 for both members and packed 2 in an array if tokenIds are sequential and limited
- Range delete of the red-black tree using split/join
- When cancelling an order some of the shifting logic can be improved to move some orders in segments in 1 go.

Expand Down
10 changes: 7 additions & 3 deletions contracts/SamWitchOrderBook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,11 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
}

for (uint i = 0; i < _tokenIds.length; ++i) {
// Cannot change tick once set
if (tokenIdInfo[_tokenIds[i]].tick != 0 && tokenIdInfo[_tokenIds[i]].tick != _tokenIdInfos[i].tick) {
revert TickCannotBeChanged();
}

tokenIdInfo[_tokenIds[i]] = _tokenIdInfos[i];
}

Expand Down Expand Up @@ -731,8 +736,7 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
revert PriceZero();
}

TokenIdInfo storage tokenIdInfo = tokenIdInfo[_limitOrder.tokenId];
uint128 tick = tokenIdInfo.tick;
uint128 tick = tokenIdInfo[_limitOrder.tokenId].tick;

if (tick == 0) {
revert TokenDoesntExist(_limitOrder.tokenId);
Expand All @@ -753,7 +757,7 @@ contract SamWitchOrderBook is ISamWitchOrderBook, ERC1155Holder, UUPSUpgradeable
);

// Add the rest to the order book if has the minimum required, in order to keep order books healthy
if (quantityRemaining >= tokenIdInfo.minQuantity) {
if (quantityRemaining >= tokenIdInfo[_limitOrder.tokenId].minQuantity) {
quantityAddedToBook_ = quantityRemaining;
_addToBook(_newOrderId, tick, _limitOrder.side, _limitOrder.tokenId, _limitOrder.price, quantityAddedToBook_);
} else if (quantityRemaining != 0) {
Expand Down
5 changes: 1 addition & 4 deletions contracts/interfaces/ISamWitchOrderBook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,7 @@ interface ISamWitchOrderBook is IERC1155Receiver {
error NothingToClaim();
error TooManyOrdersHit();
error MaxOrdersNotMultipleOfOrdersInSegment();

error DeadlineExpired(uint deadline);
error InvalidNonce(uint invalid, uint nonce);
error InvalidSignature(address sender, address recoveredAddress);
error TickCannotBeChanged();

function limitOrders(LimitOrder[] calldata orders) external;

Expand Down
81 changes: 73 additions & 8 deletions test/SamWitchOrderBook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1463,10 +1463,14 @@ describe("SamWitchOrderBook", function () {
});

it("Price must be modulus of tick quantity must be > min quantity, sell", async function () {
const {orderBook, erc1155, tokenId} = await loadFixture(deployContractsFixture);
const {orderBook, erc1155, tokenId: originalTokenId, initialQuantity} = await loadFixture(deployContractsFixture);

const tokenId = originalTokenId + 1;
await orderBook.setTokenIdInfos([tokenId], [{tick: 10, minQuantity: 20}]);

await erc1155.mintSpecificId(tokenId, initialQuantity * 2);
await erc1155.setApprovalForAll(orderBook, true);

let price = 101;
let quantity = 20;
await expect(
Expand Down Expand Up @@ -1510,10 +1514,20 @@ describe("SamWitchOrderBook", function () {
});

it("Price must be modulus of tick quantity must be > min quantity, buy", async function () {
const {orderBook, brush, tokenId} = await loadFixture(deployContractsFixture);
const {
orderBook,
brush,
tokenId: originalTokenId,
erc1155,
initialQuantity,
} = await loadFixture(deployContractsFixture);

const tokenId = originalTokenId + 1;
await orderBook.setTokenIdInfos([tokenId], [{tick: 10, minQuantity: 20}]);

await erc1155.mintSpecificId(tokenId, initialQuantity * 2);
await erc1155.setApprovalForAll(orderBook, true);

let price = 101;
let quantity = 20;
await expect(
Expand Down Expand Up @@ -1556,17 +1570,59 @@ describe("SamWitchOrderBook", function () {
expect(await brush.balanceOf(orderBook)).to.eq(quantity * price);
});

it("Change minQuantity, check other orders can still be added/taken", async function () {
const {orderBook, brush, tokenId, tick, minQuantity} = await loadFixture(deployContractsFixture);

let price = 101;
await orderBook.limitOrders([
{
side: OrderSide.Buy,
tokenId,
price,
quantity: minQuantity,
},
]);

await orderBook.setTokenIdInfos([tokenId], [{tick, minQuantity: minQuantity + 1}]);

await expect(
orderBook.limitOrders([
{
side: OrderSide.Buy,
tokenId,
price,
quantity: minQuantity,
},
]),
).to.emit(orderBook, "FailedToAddToBook");

await orderBook.limitOrders([
{
side: OrderSide.Buy,
tokenId,
price,
quantity: minQuantity + 1,
},
]);

await orderBook.limitOrders([
{
side: OrderSide.Sell,
tokenId,
price,
quantity: 1,
},
]);
});

it("Set tokenId infos can only be called by the owner", async function () {
const {orderBook, tokenId, alice} = await loadFixture(deployContractsFixture);
const {orderBook, tick, tokenId, alice} = await loadFixture(deployContractsFixture);

await expect(
orderBook.connect(alice).setTokenIdInfos([tokenId], [{tick: 10, minQuantity: 20}]),
orderBook.connect(alice).setTokenIdInfos([tokenId], [{tick, minQuantity: 20}]),
).to.be.revertedWithCustomError(orderBook, "OwnableUnauthorizedAccount");

await expect(orderBook.setTokenIdInfos([tokenId], [{tick: 10, minQuantity: 20}])).to.emit(
orderBook,
"SetTokenIdInfos",
);
await expect(orderBook.setTokenIdInfos([tokenId], [{tick, minQuantity: 20}])).to.emit(orderBook, "SetTokenIdInfos");
});

it("Set tokenId infos argument length mismatch", async function () {
Expand All @@ -1575,6 +1631,15 @@ describe("SamWitchOrderBook", function () {
await expect(orderBook.setTokenIdInfos([tokenId], [])).to.be.revertedWithCustomError(orderBook, "LengthMismatch");
});

it("Tick cannot be changed", async function () {
const {orderBook, tokenId, alice, tick, minQuantity} = await loadFixture(deployContractsFixture);

await expect(orderBook.setTokenIdInfos([tokenId], [{tick: tick + 1, minQuantity}])).to.be.revertedWithCustomError(
orderBook,
"TickCannotBeChanged",
);
});

it("Set max orders per price can only be called by the owner", async function () {
const {orderBook, alice} = await loadFixture(deployContractsFixture);
await expect(orderBook.connect(alice).setMaxOrdersPerPrice(100)).to.be.revertedWithCustomError(
Expand Down

0 comments on commit f20cca6

Please sign in to comment.