Skip to content

Commit 6353247

Browse files
mcelikxclaude
andcommitted
docs: record the order book redesign and the hashmap reference study
ORDER-BOOK-REDESIGN.md is the analysis behind the rebuild: the Poisson arithmetic showing a 4-way set-associative table overflows at 12.5% load, the measured cuckoo load knee at 85% rather than the published 0.976 (asymptotic for an unbounded walk, unreachable with bounded kicks), and the SLR ceiling that puts 500k tracked orders out of reach on this device. Two findings there generalise beyond this project. The obvious stress test validates the broken design: with dense sequential order references, the old table shows zero overflows at every load to 90%, because the hash maps a contiguous range near-bijectively. Only realistic key sparsity exposes it. And the published cuckoo threshold is not a hardware number. The book manual is rewritten (753 -> 1308 lines) and states its own error rather than quietly correcting it -- its never-evict rule was right, but it conflated dropping an order with relocating one. REFERENCE-fpga-hashmap.md studies adamwalker/fpga-hashmap, whose systolic cuckoo ring plus forwarding is what answers the insert-path objection. That repository is AGPL-3.0 and this one is MIT, so reference/ is gitignored: adapting it into rtl/ would relicense the whole project, and AGPL section 13 would require source disclosure to network users. Ideas taken, code not. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 810cfb8 commit 6353247

4 files changed

Lines changed: 1186 additions & 141 deletions

File tree

.gitignore

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,12 @@ tmp/
9999
*.bak
100100
*.orig
101101
*.rej
102+
103+
# ── ⚠️ Third-party reference material — NOT redistributed ────────────────────
104+
# reference/ holds clones of external projects kept locally for study.
105+
# adamwalker/fpga-hashmap is AGPL-3.0, which is incompatible with this repo's
106+
# MIT licence in the direction that matters: adapting it into rtl/ would force
107+
# the whole project to AGPL, and AGPL section 13 requires source disclosure to
108+
# network users. Kept local, never committed.
109+
# Analysis in our own words: docs/REFERENCE-fpga-hashmap.md
110+
reference/

docs/ORDER-BOOK-REDESIGN.md

Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
# Order Book Redesign — Architecture Plan
2+
3+
**Trigger:** external FPGA architecture review.
4+
**Verdict:** the order book as built cannot function. Three independent fatal defects, plus a
5+
capacity model that is wrong by roughly two orders of magnitude.
6+
**Status:** plan. Nothing below is implemented yet.
7+
8+
---
9+
10+
## 1. The review, and what it meant
11+
12+
Three observations were made:
13+
14+
| Observation | What it turned out to mean |
15+
| --- | --- |
16+
| *"Order book looks well but the system will not work like it"* | Three fatal defects, any one of which stops it |
17+
| *"Hashmap is not good, insufficient feature and low quality"* | The map overflows at 12.5% load and then permanently disables the book |
18+
| *"You don't have eviction case, you can use cuckoo hashing"* | There is no handling for a full bucket. Cuckoo **relocation** fixes it without dropping orders |
19+
20+
The third point deserves care because it appears to contradict the manual.
21+
22+
[`manuals/04-system-architecture/03-order-book-in-hardware.md`](../manuals/04-system-architecture/03-order-book-in-hardware.md) §2.4 says:
23+
24+
> **Never evict.** An eviction policy in an order map is a correctness bug wearing an
25+
> optimisation costume: the evicted order still exists at the venue, you will receive its
26+
> delete, and it will resolve to nothing.
27+
28+
**That reasoning is correct and stays.** But it conflates two different operations:
29+
30+
| Operation | Effect on the order | Safe? |
31+
| --- | --- | --- |
32+
| **Eviction (drop)** — discard an entry to make room | Order is gone. Its delete resolves to nothing. Level quantity stranded forever. | ❌ Silent corruption |
33+
| **Relocation (cuckoo kick)** — move an entry to its *alternate* bucket | Order still present, still findable, quantity still tracked. | ✅ Lossless |
34+
35+
Cuckoo hashing "evicts" in the second sense only. Every item remains in the table for its
36+
whole lifetime. **The safety rule and the architect's suggestion are compatible** — the manual
37+
simply had no mechanism for a full bucket other than giving up.
38+
39+
---
40+
41+
## 2. Defect inventory
42+
43+
### 2.1 🔴 FATAL — top-of-book price is destroyed on a best-level delete
44+
45+
[`rtl/book/top_of_book.sv:196`](../rtl/book/top_of_book.sv)
46+
47+
```systemverilog
48+
best_lvl_q[upd_sym][side_i] <= new_best_lvl; // correct level found
49+
best_qty_q[upd_sym][side_i] <= '0;
50+
best_px_q [upd_sym][side_i] <= '0; // ← price set to ZERO
51+
```
52+
53+
The priority encoder finds the correct new best *level index*, and then the price is written
54+
as zero instead of being reconstructed from that index. After the first best-level delete —
55+
which happens continuously in any active book — the system publishes **bid = $0.00**.
56+
57+
The price is recoverable: `price = window_base + level × tick_size`. The reconstruction was
58+
simply never written.
59+
60+
**Why nothing caught it:** no testbench has ever run.
61+
62+
### 2.2 🔴 FATAL — the price window is 16 cents wide and effectively never re-anchors
63+
64+
[`rtl/pkg/trading_pkg.sv:58`](../rtl/pkg/trading_pkg.sv) sets `BOOK_LEVELS = 16`.
65+
66+
The manual specifies **2048 levels**, a $20.48 window ([§7 of the book manual](../manuals/04-system-architecture/03-order-book-in-hardware.md)).
67+
The RTL implements 16 — a **$0.16 window**, 128× too narrow.
68+
69+
Worse, [`price_levels.sv`](../rtl/book/price_levels.sv) anchors on the first price seen and
70+
re-anchors *only when that side is empty*, which for a liquid symbol never happens. Once the
71+
price drifts more than 8 cents from the open, every subsequent order falls outside the window
72+
and is silently discarded.
73+
74+
**The RTL diverged from its own specification.** The manual was right; the implementation
75+
did not follow it.
76+
77+
### 2.3 🔴 FATAL — the order map overflows almost immediately
78+
79+
Measured, not estimated. 4-way set-associative, 16,384 sets, uniform hashing, Poisson
80+
occupancy:
81+
82+
| Live orders | Load | P(a given set > 4) | Expected overflowing sets |
83+
| ---: | ---: | ---: | ---: |
84+
| 4,096 | 6.2% | 6.6 × 10⁻⁶ | 0.11 |
85+
| 8,192 | 12.5% | 1.7 × 10⁻⁴ | **2.8** |
86+
| 16,384 | 25.0% | 3.7 × 10⁻³ | **60** |
87+
| 32,768 | 50.0% | 5.3 × 10⁻² | **863** |
88+
| 65,536 | 100% | 3.7 × 10⁻¹ | **6,081** |
89+
90+
The current RTL has **no overflow region at all** — a full set sets `map_stale` permanently.
91+
So the book dies at the first collision, within milliseconds of the open.
92+
93+
The manual's 64-entry overflow CAM does not rescue it either:
94+
95+
| Live orders | Load | Items needing overflow | 64 entries enough? |
96+
| ---: | ---: | ---: | :--- |
97+
| 8,192 | 12.5% | 3 | yes |
98+
| 16,384 | 25% | 71 | **no** |
99+
| 32,768 | 50% | 1,231 | **no** |
100+
| 65,536 | 100% | 12,804 | **no** |
101+
102+
**Effective capacity of the specified 65,536-entry table is roughly 8,000 orders.** For 128
103+
symbols that is 62 live orders per symbol. A single liquid name carries far more than that.
104+
105+
### 2.4 🟠 Write-back skew in the order map
106+
107+
`wb_en` / `wb_set` / `wb_way` / `wb_rec` are assigned non-blocking in the stage-2 block, then
108+
consumed by a separate `always_ff` that performs the memory write. That block sees the
109+
*previous* cycle's values, so the write lands one cycle later than the forwarding comparison
110+
assumes. The read-modify-write bypass is off by one and needs re-derivation with a cycle-accurate
111+
timing diagram.
112+
113+
### 2.4b 🔴 FATAL — ITCH Replace double-counted quantity (found by R1)
114+
115+
A fourth fatal defect, missed in the original review of this document.
116+
117+
`order_id_map` emitted `res_add = qty` **at the old record's price** on a `BOOK_REPLACE`, while
118+
`book_engine` *also* injected a synthetic `BOOK_ADD` for the new order reference. So **every
119+
ITCH `U` message added the replaced quantity twice — once at the correct new level, once at the
120+
wrong old one.**
121+
122+
Replace is a common message. The book would have inflated steadily all session, and because the
123+
inflation lands at plausible prices it would have looked like ordinary depth.
124+
125+
Fixed: replace is now a pure delete in the map, with an SVA enforcing `res_add == 0`.
126+
127+
### 2.4c ⚠️ Test-methodology trap — the obvious stress test *passes* the broken design
128+
129+
The single most important finding of the redesign, and it is about testing rather than RTL.
130+
131+
With **dense sequential** order references (`ref`, `ref+1`, `ref+2`, …) the *original, broken*
132+
4-way table shows **zero overflows at every load up to 90%** — the legacy hash maps a contiguous
133+
range near-bijectively, so nothing ever collides.
134+
135+
With realistically thinned keys, the same design strands 4 orders at 12.5% load and 9,227 at
136+
90%, exactly matching the Poisson prediction in §2.3.
137+
138+
⚠️ **A stress test written the obvious way would have validated the defect.** R7 must generate
139+
order references with realistic sparsity, never a dense range. This is now called out in the
140+
module header.
141+
142+
### 2.5 🟡 Dead signals
143+
144+
`s1_valid_d` and `s1_clear_d` in [`price_levels.sv:151`](../rtl/book/price_levels.sv) are
145+
declared and never assigned. Verilator lint would have caught this; lint has never run.
146+
147+
### 2.6 🟡 Tick size is not parameter-linked
148+
149+
`book_pkg::TICK_RECIP` is hardcoded to the ÷100 reciprocal while `TICK_UNITS` is a parameter.
150+
Changing `TICK_UNITS` to 50 for a half-penny regime silently produces wrong level indices.
151+
152+
---
153+
154+
## 3. Target design
155+
156+
### 3.1 Order map — cuckoo, d=2 hashes × b=4 slots
157+
158+
```
159+
key ──┬─► h0(key) ─► bucket A (4 slots) ──┐
160+
│ ├─► 8 full-key compares ─► hit / miss
161+
└─► h1(key) ─► bucket B (4 slots) ──┘ (1 cycle, worst case)
162+
163+
stash (16-entry CAM, parallel) ─┘
164+
```
165+
166+
**Lookup is O(1) worst case** — exactly two parallel bucket reads and eight comparators, every
167+
time, for every key. That is what a fixed-latency pipeline requires; linear probing or chaining
168+
would introduce variable latency and is disqualified regardless of its average performance.
169+
170+
**Insert:** if either bucket has a free slot, place it. Otherwise pick a victim, move it to its
171+
alternate bucket, and repeat. Bounded at `MAX_KICKS` (16). If the chain does not terminate, the
172+
item goes to the stash. If the stash is full, *then* the book goes stale.
173+
174+
**Delete:** locate and invalidate. No relocation needed.
175+
176+
Published load thresholds:
177+
178+
| Configuration | Max load |
179+
| --- | ---: |
180+
| d=2, b=1 | 0.50 |
181+
| d=2, b=2 | 0.897 |
182+
| **d=2, b=4** | **0.976** |
183+
| d=2, b=8 | 0.996 |
184+
185+
At 90% load with a 16-entry stash, insertion failure is negligible. Compare against the
186+
current design, which fails at 12.5%: **a ~7× improvement in usable capacity for identical
187+
memory.**
188+
189+
> ⚠️ **Full keys are stored, never tags.** Partial-key cuckoo (storing a tag and deriving the
190+
> alternate bucket from it) halves the memory and is standard in cuckoo *filters* — but a tag
191+
> collision returns the wrong order, and updating the wrong resting order is silent book
192+
> corruption. The manual is right to insist on full keys; this does not change.
193+
194+
#### ⚠️ Correction — the 0.976 threshold is not reachable, and this section originally said it was
195+
196+
The first version of this document sized the table at 90% load on the strength of the published
197+
0.976 threshold. **That was wrong, and R1 measured it.** A Python model of the exact algorithm,
198+
65,536 slots, realistic (non-sequential) keys:
199+
200+
| Load | Insert failures |
201+
| ---: | ---: |
202+
| 50 / 70 / 80 / 85 % | **0** |
203+
| 90 % | **322** |
204+
205+
The 0.976 figure is asymptotic for an *unbounded* random-walk insert. A bounded 16-kick chain
206+
does not reach it. **Size for ≤ 80% of slots.**
207+
208+
R1 also measured that the background relocation engine is not an optimisation but a
209+
requirement. At 85% load, same memory and same lookup:
210+
211+
| Relocation engine | Insert failures |
212+
| --- | ---: |
213+
| **Off** (a static overflow CAM — i.e. the d-left arrangement) | **1,112** |
214+
| **On** | **0** |
215+
216+
Enlarging the stash instead does nothing — 15, 31, 63 and 127 entries all give identical
217+
results. **Draining the stash is what matters, not buffering more.** That is the empirical case
218+
for cuckoo over d-left here, and it is measurement rather than argument.
219+
220+
#### Sizing
221+
222+
Slots must be a power of two, which absorbs much of the 90%→80% difference in practice.
223+
Record width 138 bits.
224+
225+
| Live orders | Slots | Load | Memory | URAM288 | Fits one SLR? |
226+
| ---: | ---: | ---: | ---: | ---: | :--- |
227+
| 102,400 (manual p99) | 131,072 | 78% | 18.1 Mbit | **64** | ✅ with 128 URAM of levels = 192 of ~320 |
228+
| 250,000 | 524,288 | 48% | 72.4 Mbit | **252** | ⚠️ leaves ~68 for everything else |
229+
| 500,000 | 1,048,576 | 48% | 144.7 Mbit | **503** |**exceeds the SLR entirely** |
230+
231+
⚠️ A VU9P SLR holds ~320 URAM288 and the whole fast path must fit in one. **500k tracked orders
232+
is not achievable on this device** — that configuration forces either fewer symbols, a narrower
233+
price window, or a different part. Capacity is a measurement, not a preference; take the live
234+
order count from `tools/pcap/stats.py` against a real capture before choosing.
235+
236+
### 3.2 Population control — track only what is in the window
237+
238+
An order is in the map **if and only if** its quantity is in the level array.
239+
240+
This makes a delete for an untracked order a correct no-op rather than an error, and bounds
241+
the map population to `symbols × window levels × orders per level` instead of the venue's
242+
entire live book. It also makes out-of-window handling coherent: an order outside the window
243+
was never added to a level, so it must never enter the map.
244+
245+
⚠️ This invariant is the whole correctness argument. It must be asserted in RTL and proven in
246+
the golden-model equivalence test, not assumed.
247+
248+
### 3.3 Price levels — 2048 per side, host-anchored
249+
250+
Implement what the manual already specifies. Add:
251+
252+
- **Host-written per-symbol reference price**, refreshed from last trade or previous close.
253+
This requires a config port on `book_engine`, which the current `fpga_top` contract lacks —
254+
a deliberate two-file change.
255+
- **Out-of-window ⇒ per-symbol stale + host re-anchor + resync**, not silent discard.
256+
- Occupancy bitmap over 2048 levels ⇒ hierarchical priority encoder (64 groups × 32) to keep
257+
the new-best search inside its cycle budget.
258+
259+
### 3.4 Top of book — correct price reconstruction
260+
261+
- Reconstruct price from level index and window base on every best change.
262+
- Read the true quantity from the level array rather than zeroing it.
263+
- Cache second-best so the common delete case avoids a rescan entirely.
264+
265+
---
266+
267+
## 4. What this does not fix
268+
269+
Being explicit, because the request was for a system that "runs in production without issue":
270+
271+
- Nothing here has been compiled, simulated, synthesized, or run on hardware.
272+
- ITCH/OUCH field offsets remain unverified against the specification PDFs.
273+
- The LULD band source defect and the Rule 612 tick-size assumption are separate items.
274+
- Exchange conformance certification, broker-dealer market access, and compliance review are
275+
organisational prerequisites that no amount of code satisfies.
276+
277+
Correct RTL is necessary for production. It is nowhere near sufficient.
278+
279+
---
280+
281+
## 5. Task breakdown
282+
283+
| ID | Task | Blocking? | Where |
284+
| --- | --- | :-: | --- |
285+
| **R1** | Cuckoo order map: d=2×b=4, relocation FSM, 16-entry stash, full keys | 🔴 | `rtl/book/order_id_map.sv` |
286+
| **R2** | Price levels: 2048/side, host anchor, out-of-window ⇒ stale | 🔴 | `rtl/book/price_levels.sv` |
287+
| **R3** | Top-of-book: price reconstruction, true qty, second-best cache | 🔴 | `rtl/book/top_of_book.sv` |
288+
| **R4** | Hierarchical priority encoder for 2048-bit occupancy | 🔴 | `rtl/common/prio_encoder.sv` |
289+
| **R5** | Book engine rewire; fix write-back skew; remove dead signals | 🔴 | `rtl/book/book_engine.sv` |
290+
| **R6** | Package sizing, tick-size/reciprocal linkage, book config port | 🔴 | `rtl/pkg/`, `rtl/fpga_top.sv` |
291+
| **R7** | Golden-model equivalence + cuckoo stress (load to 95%, kick chains, stash overflow) | 🔴 | `tb/book/` |
292+
| **R8** | CDC primitive testbenches — the untested half of coverage | 🟠 | `tb/common/` |
293+
| **R9** | LULD band source fix + Rule 612 tick-size parameterisation | 🔴 | `rtl/risk/`, `rtl/pkg/` |
294+
| **R10** | Update the book manual to match: cuckoo, sizing math, invariant | 🟠 | `manuals/04-*/03-*` |
295+
296+
`R1``R6` are the ones that make the system function at all.

0 commit comments

Comments
 (0)