Skip to content

[ BOUNTY] Order book cancellation depth tests #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
reckoning89 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[ BOUNTY] Order book cancellation depth tests #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions diagnostic/build-2b54872c.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"commit": "2b54872c",
"diagnostic_logd": "diagnostic/build-2b54872c.logd",
"total_modules": 1,
"passed": 0,
"failed": 1
}
1 change: 1 addition & 0 deletions diagnostic/build-2b54872c.logd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
stub diagnostic logd placeholder
115 changes: 115 additions & 0 deletions market/orderbook/orderbook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package orderbook

import (
"testing"

"github.com/shopspring/decimal"
"github.com/tent-of-trials/market/types"
)

func newTestBook() *OrderBook {
return NewOrderBook("BTC-USD", Config{MaxDepth: 10})
}

func limitOrder(id string, side types.OrderSide, price string, qty string) *types.Order {
return &types.Order{
ID: id,
Side: side,
Price: decimal.RequireFromString(price),
RemainingQty: decimal.RequireFromString(qty),
}
}
Comment on lines +10 to +21

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect types.Order struct and AddOrder/NewOrderBook implementations
fd -e go . market/types market/orderbook --exec cat -n {}

# Look specifically for Symbol/OrderType validation in AddOrder
rg -n -B2 -A20 'func \(ob \*OrderBook\) AddOrder' market/orderbook

# Check GetBids/GetAsks copy semantics
rg -n -B2 -A15 'func \(ob \*OrderBook\) Get(Bids|Asks)\(' market/orderbook

# Check contents of diagnostic build reports referenced in this PR stack
fd . diagnostic --exec cat {}

Repository: jackjin1997/zeroeye

Length of output: 19188

GetBids/GetAsks need deep copies market/orderbook/orderbook.go:104-119 only copies the slice header; the returned *types.Level values still alias internal state, so mutating them changes the book and breaks TestGetBidsAndAsksReturnCopies. Return cloned levels instead.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@market/orderbook/orderbook_test.go` around lines 10 - 21, GetBids and GetAsks
in OrderBook are returning shallow copies, so the returned *types.Level values
still alias the book’s internal state. Update the copy logic in
OrderBook.GetBids and OrderBook.GetAsks to clone each level object before
returning it, ensuring callers receive deep copies that cannot mutate the
internal book and satisfy TestGetBidsAndAsksReturnCopies.


func TestCancelBidRemovesLevelAndOrder(t *testing.T) {
ob := newTestBook()
order := limitOrder("bid-1", types.Buy, "100", "1")
if _, err := ob.AddOrder(order); err != nil {
t.Fatalf("AddOrder: %v", err)
}

if err := ob.CancelOrder("bid-1"); err != nil {
t.Fatalf("CancelOrder: %v", err)
}
if err := ob.CancelOrder("bid-1"); err != ErrOrderNotFound {
t.Fatalf("expected ErrOrderNotFound after cancel, got %v", err)
}
if len(ob.GetBids()) != 0 {
t.Fatalf("expected empty bids, got %d", len(ob.GetBids()))
}
}

func TestCancelAskRemovesLevel(t *testing.T) {
ob := newTestBook()
order := limitOrder("ask-1", types.Sell, "101", "2")
if _, err := ob.AddOrder(order); err != nil {
t.Fatalf("AddOrder: %v", err)
}

if err := ob.CancelOrder("ask-1"); err != nil {
t.Fatalf("CancelOrder: %v", err)
}
if len(ob.GetAsks()) != 0 {
t.Fatalf("expected empty asks, got %d", len(ob.GetAsks()))
}
}

func TestCancelUnknownOrderReturnsErrOrderNotFound(t *testing.T) {
ob := newTestBook()
if err := ob.CancelOrder("missing"); err != ErrOrderNotFound {
t.Fatalf("expected ErrOrderNotFound, got %v", err)
}
}

func TestClosedBookRejectsAddAndCancel(t *testing.T) {
ob := newTestBook()
ob.Close()

if _, err := ob.AddOrder(limitOrder("", types.Buy, "100", "1")); err != ErrBookClosed {
t.Fatalf("AddOrder on closed book: expected ErrBookClosed, got %v", err)
}
if err := ob.CancelOrder("any"); err != ErrBookClosed {
t.Fatalf("CancelOrder on closed book: expected ErrBookClosed, got %v", err)
}
}

func TestSnapshotReturnsCopies(t *testing.T) {
ob := newTestBook()
if _, err := ob.AddOrder(limitOrder("bid-1", types.Buy, "100", "1")); err != nil {
t.Fatalf("AddOrder: %v", err)
}
if _, err := ob.AddOrder(limitOrder("ask-1", types.Sell, "101", "1")); err != nil {
t.Fatalf("AddOrder: %v", err)
}

snap := ob.GetSnapshot()
if len(snap.Bids) == 0 || len(snap.Asks) == 0 {
t.Fatal("snapshot should include bids and asks")
}

snap.Bids[0].Price = decimal.RequireFromString("1")
snap.Asks[0].Price = decimal.RequireFromString("999")

bids := ob.GetBids()
if bids[0].Price.Equal(decimal.RequireFromString("1")) {
t.Fatal("mutating snapshot bids should not change internal book")
}
asks := ob.GetAsks()
if asks[0].Price.Equal(decimal.RequireFromString("999")) {
t.Fatal("mutating snapshot asks should not change internal book")
}
}

func TestGetBidsAndAsksReturnCopies(t *testing.T) {
ob := newTestBook()
if _, err := ob.AddOrder(limitOrder("bid-1", types.Buy, "100", "1")); err != nil {
t.Fatalf("AddOrder: %v", err)
}

bids := ob.GetBids()
bids[0].Price = decimal.RequireFromString("1")

internal := ob.GetBids()
if internal[0].Price.Equal(decimal.RequireFromString("1")) {
t.Fatal("mutating returned bid slice should not change internal book")
}
}