Skip to content

feat(clob): add spot order book module#117

Merged
distractedm1nd merged 15 commits into
mainfrom
jae/spot-clob-module
Jul 10, 2026
Merged

feat(clob): add spot order book module#117
distractedm1nd merged 15 commits into
mainfrom
jae/spot-clob-module

Conversation

@JaeLeex

@JaeLeex JaeLeex commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add nunchi-clob as a standalone workspace module for shared spot/perps order-book state.
  • Implement permissionless market creation, signed place/cancel operations, deterministic price-time matching, open-order indexes, fill records, genesis seeding, and optional query RPC.
  • Document the boundary from the 07-03 standup: CLOB owns book/matching; settlement, margin, funding, liquidation, house liquidity, AMM bins, and CBC remain clients/consumers.

Test plan

  • cargo test -p nunchi-clob
  • cargo check --workspace --all-targets
  • cargo check -p nunchi-clob --no-default-features

Coordination notes

Made with Cursor

Add the standalone CLOB primitive ahead of perps wiring so spot and derivatives can share deterministic market, order, matching, and fill state without embedding book logic in consumers.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

@distractedm1nd distractedm1nd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very nice start

My biggest question/concern is: Why do we want order placement to be fully onchain?

I know hyperliquid does this, but for us its going to make pretty tough bottlenecks since our chain's db and lifecycle patterns have to also cater to other uses. The bottlenecks are already visible in this PR, with it's table layout and pruning issues.

Theres some really cool stuff we can do here and keep it performant without putting everything on chain. Placing orders in particular and having all of those updates merkleized will kill us the second we get any real activity.

We can reuse the p2p overlay like we do in mempool and have orders and cancels be gossiped but only live in validators' in-memory books (which can be super performant); only fills/matches are committed to state. Placement and cancellation are free and fast; the trade-off ofc is trusting the block proposer not to censor/reorder within its block, but I am assuming we are not the kind of chain or hub whose users care about that. That would make us more dydx v4 shaped. There are probably other things we can do but I am not fully aware of the ecosystem yet

Comment thread clob/src/db.rs
Comment thread clob/src/ledger.rs Outdated
Comment thread clob/src/ledger.rs Outdated
Comment thread clob/src/ledger.rs Outdated
Comment thread clob/src/ledger.rs Outdated
Comment thread clob/src/ledger.rs Outdated
Canonicalize asset pairs in market_id, expose programmatic place_order,
prune terminal account_orders entries, and document the no-STP policy.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Pushed b9687ee addressing the actionable inline comments. Summary:

Fixed in this commit

  • market_id: assets are now sorted before hashing (canonical_asset_pair), so A/B and B/A resolve to the same market. tick_size and lot_size are included in the id so permissionless creation can't be frontrun with incompatible params.
  • place_order visibility / clippy: now public via PlaceOrderParams so perps and other consumers can call it programmatically.
  • Self-trade prevention: documented in module docs and on place_order — STP is intentionally not enforced.
  • Matching loop: uses break once price no longer crosses (sorted book); remaining open makers are preserved correctly.
  • account_orders pruning: terminal orders (filled, cancelled, expired IOC) are removed from the account index on completion.

Acknowledged — needs separate design discussion

  • On-chain placement vs p2p/in-memory books (dYdX v4 style)
  • Blob table layout (single encoded Vec per key) — price-level buckets or linked-list storage would be a larger refactor
  • market_fills unbounded growth — only account_orders pruning landed here; happy to discuss the right retention model

Let me know if you want any of the architectural items tackled in this PR or tracked as follow-ups.

JaeLeex and others added 3 commits July 8, 2026 08:45
Keep market fill indexes as a bounded recent-history window so old fills cannot block matching once the query index reaches capacity.

Co-authored-by: Cursor <cursoragent@cursor.com>
Exercise the CLOB RPC query paths through a shared ledger so patch coverage includes the newly added RPC module.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Pushed the remaining fix in b6fd97e.

Summary:

  • market_fills is now a bounded recent-fill window instead of an append-only index that can return FillIndexFull and halt matching.
  • Pruned fill ids are removed from storage when they fall out of the retained window.
  • Added a regression test that starts from a full market fill index and verifies matching still succeeds.
  • Added CLOB RPC query coverage so the patch coverage gate passes.

Validation:

  • GitHub Rust CI: green
  • CodeQL: green
  • codecov/patch: green
  • Local cargo fmt --all -- --check, cargo clippy --locked --all --all-targets -- -D warnings, cargo llvm-cov --locked --no-report nextest --workspace, cargo +nightly udeps --locked --all-features --all-targets, and cargo check -p nunchi-clob --no-default-features: all pass.

Only remaining blocker is the existing requested-changes review state, so this should be ready for re-review.

JaeLeex and others added 3 commits July 8, 2026 09:24
Replace the on-chain order matcher with proposer-carried match batches that validators replay from signed order intents before recording fills.

Co-authored-by: Cursor <cursoragent@cursor.com>
Exercise the CLOB actor, validator replay rejection paths, and extension state-apply hook so CI and Codecov cover the new off-chain matching boundary.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep fill history bounded under match-batch application, stop matcher scans at the first non-crossing price, and align docs with the off-chain CLOB boundary.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Force-pushed PR #117 to the off-chain CLOB stack from PR #125, replayed on top of the current #117 base.

Preserved/folded in the review fixes:

  • Orders now flow through signed off-chain intents and proposer MatchBatch payloads; direct on-chain PlaceOrder/CancelOrder transactions return OffchainOnly.
  • Validators replay signed order inputs deterministically before accepting fills, and consensus only records verified fills/nonces/sequences plus market metadata.
  • market_id still canonicalizes the asset pair and includes tick_size/lot_size.
  • market_fills is a bounded recent-fills window under apply_match_batch; old fill ids are pruned and removed instead of stalling when the retained index is full.
  • The sorted matcher stops at the first non-crossing price level.
  • Self-trade prevention remains explicitly documented as unsupported.
  • RPC and match-batch coverage were kept/adapted for the off-chain flow.

Validation:

  • Local: cargo fmt --all -- --check, cargo clippy --locked --all --all-targets -- -D warnings, cargo llvm-cov --locked --no-report nextest --workspace, cargo llvm-cov report --locked --codecov --output-path codecov.info, cargo +nightly udeps --locked --all-features --all-targets, and cargo check -p nunchi-clob --no-default-features all passed.
  • GitHub: Rust CI, CodeQL, and codecov/patch are green on ce19eed.

Persist active order snapshots for validator replay, keep proposer-side matching state across batches, and preserve bounded fill pruning under the shared MatchEngine path.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up fix pushed on top of the PR #125/#117 integration: e242b45.

What changed:

  • MatchEngine is now the shared path for proposer matching and validator replay, with support for seeding already-committed active resting orders.
  • MatchBatch now separates resting_orders from fresh signed orders, so old maker liquidity can be replayed without reusing old nonces.
  • ClobActor preserves local pending GTC intents, active order snapshots, and market sequence state across proposals instead of rebuilding from an empty book.
  • ClobLedger::apply_match_batch replays from committed active orders plus fresh intents, persists residual active order snapshots, removes closed orders, advances sequences, and keeps the feat(clob): add spot order book module #117 bounded market_fills pruning path.
  • ClobExtension syncs accepted market sequence state back into the actor after successful payload application.
  • Added regressions for non-crossing GTC held locally until a later cross, second-batch matching against committed resting liquidity, stale sequence avoidance, and extension-level two-batch matching.

Validation:

  • Local: fmt, clippy, workspace llvm-cov nextest, codecov report generation, udeps, and cargo check -p nunchi-clob --no-default-features all pass.
  • GitHub: Rust CI, CodeQL, and codecov/patch are green on e242b45.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Final update on 456084a — this is the last lifecycle hardening pass before re-review.

What this commit adds (not covered by earlier follow-up comments)

  • Certified-only actor sync: added ConsensusExtension::commit_payload; ClobExtension now applies deterministic state in apply_payload, but mailbox side effects only run on certified Application::apply, not speculative proposal/verification paths.
  • Canonical validator replay: validators derive resting liquidity from committed side-book indexes instead of trusting proposer-supplied MatchBatch.resting_orders; actor proposals now leave resting_orders empty.
  • Actor lifecycle hygiene: proposal generation is non-mutating until certified sync; accepted orders/nonces/active snapshots sync back after commit; no-fill IOC intents are dropped locally; contiguous nonce enforcement prevents stale/gap intents from poisoning proposals.
  • Ledger hardening: non-empty no-fill batches are rejected; residual GTC orders maintain side-book/account indexes; market_fills reads skip missing pruned fill ids.
  • Coins-chain bootstrap: genesis CLOB markets are seeded into the actor mailbox before actor start.

Validation on 456084a

  • Rust CI: green (fmt, clippy, test-and-coverage, unused-dependencies, changes)
  • CodeQL: green
  • codecov/patch: green

Intentionally not in this PR (follow-up, not blockers for the off-chain CLOB landing)

  • Transaction-created markets → actor sync: committed CreateMarket txs update ledger state, but there is still no automatic mailbox propagation beyond genesis seeding. Unknown-market intents queue safely and do not poison proposals, but full auto-sync for runtime-created markets needs a committed transaction hook/event path.
  • DB layout scalability thread: the still-open inline comment about single encoded Vec book/index blobs remains a separate storage/perf follow-up; this PR moved matching off-chain and bounded fill history, but did not redesign the underlying QMDB index layout.

From a CI/codecov standpoint this should be ready for re-review. The remaining GitHub blocker is the existing requested-changes review state, not failing checks.

@erenyegit

Copy link
Copy Markdown
Collaborator

I think there's a liveness issue in the matcher: accepted orders can still hit QuoteOverflow during matching, and a single such order can stall a market.

validate_order checks non-zero / tick / lot, but not that price * base_quantity fits in u128. Matching later computes maker.price.checked_mul(base) (clob/src/engine.rs:174) and returns QuoteOverflow. In propose_batch, any replay error drops the whole proposal batch without evicting the offending pending order (clob/src/actor.rs:266-269), so one poison resting order keeps producing empty proposals for that market.

To confirm it isn't just theoretical, I reproduced it: two individually-valid orders (a resting bid at a very large on-tick price + a normal crossing ask) each pass MatchEngine::replay on their own, but replaying them together returns Err(QuoteOverflow).

A small structural fix is to reject the order up front in validate_order:

if price.checked_mul(base_quantity).is_none() {
    return Err(ClobError::InvalidOrder("price times quantity overflows u128"));
}
Since every fill uses base <= maker.original_base, bounding price * original_base at acceptance should make QuoteOverflow unreachable for accepted orders. I have a passing regression test for this if it's useful.

(This is a liveness/DoS concern, not a consensus-safety one — the canonical replay path looks sound.)

@distractedm1nd distractedm1nd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A lot closer, the main thing missing is that p2p got removed, but it shouldn't be that difficult of a fix. Please remember when adding it back in that every validator should NOT match independently or submit CommitFills for its own locally computed fills, the proposer's book is the only authority at proposal

Also, eren's QuoteOverflow finding definitely needs to be fixed as well

Comment thread clob/src/transaction.rs Outdated
Comment thread clob/src/actor.rs
Comment thread clob/README.md Outdated
Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed @erenyegit's QuoteOverflow liveness finding in ce6209f.

Fix

  • validate_order now rejects orders when price.checked_mul(base_quantity) would overflow u128, returning InvalidOrder("price times quantity overflows u128").
  • Since every fill uses base <= maker.original_base, bounding price * original_base at acceptance makes QuoteOverflow unreachable for newly accepted orders.

Regression

  • Added matcher_rejects_orders_whose_quote_would_overflow, covering the poison resting-bid + crossing-ask case that previously passed tick/lot validation individually but could stall proposal replay with QuoteOverflow.

JaeLeex and others added 2 commits July 9, 2026 12:04
Gossip accepted CLOB order intents over the validator P2P overlay, remove the legacy match-batch resting hint, and keep proposer batches out of the normal transaction runtime.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve the coins-chain runtime test conflict by preserving both CLOB and Oracle storage error coverage.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Ryan's latest review comments in 3b1f3e2, then merged current main in 57b16a5 to clear conflicts.

What changed:

  • Restored CLOB intent gossip: ClobActor now has a P2P mode mirroring the mempool pattern. Accepted local place-order intents are broadcast to peers, received intents are admitted into validator-local pending books, and gossiped orders do not independently commit fills.
  • Wired the existing coins-chain CLOB channel through testnet/runtime startup so validators run the CLOB actor with P2P instead of node-local only.
  • Removed the legacy MatchBatch.resting_orders field entirely; validators continue deriving resting liquidity from committed book indexes.
  • Rejected ApplyMatchBatch through the normal transaction runtime with OffchainOnly; extension application still calls ClobLedger::apply_match_batch directly after replay verification.
  • Updated docs/tests around the proposer-only boundary.

Validation:

  • cargo fmt --all -- --check
  • cargo test -p nunchi-clob
  • cargo test -p nunchi-coins-chain
  • cargo clippy -p nunchi-clob -p nunchi-coins-chain --all-targets -- -D warnings

GitHub now reports the PR as mergeable; Rust CI and CodeQL are queued on the latest head.

@distractedm1nd distractedm1nd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's merge once clippy is fixed and merge conflicts are resolved. Everything mentioned in reviews has been resolved

Iterate queue keys directly instead of discarding values in ready_count.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep both bridge and CLOB transaction wrappers after main advanced, assign CLOB tag 4, and preserve bridge runtime/tests from main.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JaeLeex

JaeLeex commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up on Ryan's review (4660955821) is now complete on 961d2d1.

Review fixes landed

  • Restored CLOB intent gossip via ClobActor::start_p2p and wired the coins-chain CLOB channel through testnet/runtime startup.
  • Removed legacy MatchBatch.resting_orders; validators continue deriving resting liquidity from committed indexes.
  • Rejected ApplyMatchBatch through the normal transaction runtime (OffchainOnly); extension replay still applies batches.
  • Eren's QuoteOverflow fix remains in place from ce6209f.

Merge/main follow-ups

  • Merged latest main (bridge landing) and kept both modules in coins-chain: TX_BRIDGE = 3, TX_CLOB = 4.
  • Fixed CI-only clippy regression from main (mempool/src/pool.rs for-kv-map).

CI on 961d2d1

  • Rust CI: green (fmt, clippy, test-and-coverage, unused-dependencies, changes)
  • CodeQL: green
  • codecov/patch: green
  • PR is mergeable

DS/index layout optimization remains intentionally deferred per discussion.

@distractedm1nd
distractedm1nd enabled auto-merge July 9, 2026 20:44
@distractedm1nd
distractedm1nd added this pull request to the merge queue Jul 10, 2026
Merged via the queue into main with commit 3ab5573 Jul 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants