Feedback wanted: Rust port of my Node.js order book

code review
1

Hi all,

I just published a Rust crate called rust-order-book, which is a port of one of my Node.js projects nodejs-order-book. I built it mainly to learn Rust while implementing a real-world trading engine.

The crate provides a high-performance limit order book with market and limit orders, price-time priority, order modification/cancellation, journaling, and snapshot/replay support. Some features like conditional orders are not yet ported, but will be added over time.

I’d love to get your feedback on:

  • API design
  • Code structure and idiomatic Rust usage
  • Performance considerations
  • Anything that could be improved

Repo link: https://github.com/fasenderos/rust-order-book

Thanks in advance! :slightly_smiling_face:

2

  1. You define an alias Quanity but don't use it for all quantities

  2. I'd go with a new-type for OrderId instead of an alias. IDs should almost always be new-types.

    Quantity and Price are also candidates for new-types, but I'm not familiar enough with your problem space to be certain.

  3. Having orig_qty, executed_qty and remaining_qty seems redundant. I'd calculate one of these in an accessor

  4. I'd have fewer pub fields, even in internal types. That makes it harder to enforce invariants

3

Thanks a lot for your detailed feedback! :folded_hands:
I’ll start refactoring based on your suggestions. If you don’t mind, I’d like to ping you once I implement the changes

4

I’ve made the changes as per your feedback, happy if you can take another look.

5 
pub struct OrderId(pub u64);

For your wrapper types, like Price and OrderId, I would say it is more idiomatic to implement Deref and DerefMut for the wrapper, so that you do not have to reimplement methods for the wrapped type, such as Add or Mul.

You can even be lazy and use derive_more:

#[derive(derive_more::Deref, derive_more::DerefMut)]
pub struct OrderId(i64);
6

Thanks for your feedback, I'll take a closer look to Deref

7

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.