Feature-gated binary wire protocol (#59) by joaquinbejar · Pull Request #83 · joaquinbejar/OrderBook-rs

joaquinbejar · 2026-04-25T12:58:23Z

Summary

New wire feature flag adds a length-prefixed binary protocol — [len:u32 LE | kind:u8 | payload]. Disabled by default; existing JSON / bincode paths unchanged.
MessageKind #[repr(u8)] enum with stable explicit discriminants for NewOrder, CancelOrder, CancelReplace, MassCancel, ExecReport, TradePrint, BookUpdate.
Inbound messages are #[repr(C, packed)] with zerocopy derives plus compile-time size guards. Decoding is safe; #![deny(unsafe_code)] stays on.
Outbound messages use byte-cursor encoders (extend_from_slice).
Wire ↔ domain mapping at the boundary via TryFrom<&NewOrderWire> for OrderType<()>.
WireError enum covers Truncated, UnknownKind, InvalidPayload.
doc/wire-protocol.md documents per-message layouts, discriminants, framing, and endianness.
Round-trip proptest coverage; example examples/src/bin/wire_roundtrip.rs.

Closes #59.

Test plan

cargo clippy --all-targets --all-features -- -D warnings
cargo fmt --all --check
cargo test --all-features — 1125 passed, 0 failed
cargo test --features wire — wire round-trip proptests pass
cargo build --release --all-features

Add a feature-gated `wire` flag with optional `zerocopy = "0.8"` dependency. Introduce the binary frame codec (`[len:u32 LE | kind:u8 | payload]`), the `WireError` taxonomy (manual `Display`, no `thiserror`), and the stable `MessageKind` discriminants (`#[repr(u8)] #[non_exhaustive]`). Inbound codes occupy `0x01..=0x7F`, outbound `0x80..=0xFF`. This commit ships the wire scaffolding only — the per-message inbound/outbound modules land in the next commit.

Inbound (`src/wire/inbound/`) — `NewOrderWire` (48 B), `CancelOrderWire` (24 B), `CancelReplaceWire` (40 B), and `MassCancelWire` (24 B). Each is `#[repr(C, packed)]` with `zerocopy::{FromBytes, IntoBytes, Unaligned, Immutable, KnownLayout}` derives, a `const _: () = assert!(size_of::<…>() == N)` size guard, an `as_payload_bytes()` accessor (so callers do not need to import `zerocopy`), and a `decode_*` helper that returns `WireError::InvalidPayload` on size mismatch. Decoding is safe — `#![deny(unsafe_code)]` stays on. Outbound (`src/wire/outbound/`) — `ExecReport` (44 B), `TradePrintWire` (48 B), `BookUpdateWire` (32 B). Outbound uses explicit byte-cursor encoders (`Vec<u8>::extend_from_slice`). `status_to_wire` maps `OrderStatus` to its `STATUS_*` discriminant. Wire ↔ domain mapping at the boundary — `impl TryFrom<&NewOrderWire> for OrderType<()>` copies each packed field into a stack local first (taking a reference to a packed field is UB), validates the side / TIF / order_type discriminants, and rejects negative prices. `MassCancel` rejects unknown `scope` bytes at decode time. Round-trip `proptest` tests in every new module — encode through the framer, decode back, assert byte-for-byte equality.

`examples/src/bin/wire_roundtrip.rs` builds a `NewOrderWire`, encodes it via the framer, decodes the frame, validates the `MessageKind` byte, decodes the payload, mirrors the packed fields onto stack locals (taking a reference to a packed field is UB), and converts the result into a domain `OrderType<()>`. Every step is logged via `tracing::info!`. The example is gated by `required-features = ["wire"]` in `examples/Cargo.toml`, mirroring the existing `special_orders` pattern.

Add `doc/wire-protocol.md` with per-message offset / size / field / type / notes layout tables for every inbound and outbound message, the `MessageKind` discriminant table, the framing rule (`[len:u32 LE | kind:u8 | payload]` where `len` covers `kind + payload`), and the LE-endianness statement. Append a new sub-block to the existing 0.7.0 section in `CHANGELOG.md` covering the feature flag, framing rule, endianness, discriminant table, fixed-size packed inbound layouts via `zerocopy`, byte-cursor outbound encoders, round-trip proptests, and the `wire_roundtrip` example. Note that `JSON` and `bincode` paths are unchanged — the wire protocol is additive. `README.md` is regenerated via `cargo readme` to mirror the new What's New entry that already shipped with the lib.rs scaffolding commit. The unrelated `tests/unit/replay_determinism.rs` import-grouping fix is a `cargo fmt --all` follow-up.

Copilot

Pull request overview

Adds an additive, feature-gated (wire) binary wire protocol with length-prefixed framing and fixed-layout message bodies, plus documentation, tests, and an example binary.

Changes:

Introduces src/wire/ module tree (framing, error type, MessageKind, inbound zerocopy structs, outbound encoders/decoders).
Adds wire Cargo feature (optional zerocopy) and re-exports at the crate root.
Documents the protocol (doc/wire-protocol.md), updates README/CHANGELOG, and adds a wire_roundtrip example + proptest coverage.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`Cargo.toml`	Adds optional `zerocopy` dep and `wire` feature flag.
`src/lib.rs`	Documents the new wire protocol and conditionally exposes `pub mod wire`.
`src/wire/mod.rs`	Defines `MessageKind`, module structure, and public re-exports.
`src/wire/error.rs`	Adds `WireError` for framing/decoding failures.
`src/wire/framing.rs`	Implements `[len:u32 LE][kind:u8][payload]` encode/decode helpers + tests.
`src/wire/inbound/mod.rs`	Inbound module organization and re-exports.
`src/wire/inbound/new_order.rs`	`NewOrderWire` packed layout + decode + `TryFrom` mapping to domain.
`src/wire/inbound/cancel.rs`	`CancelOrderWire` packed layout + decode + tests.
`src/wire/inbound/cancel_replace.rs`	`CancelReplaceWire` packed layout + decode + tests.
`src/wire/inbound/mass_cancel.rs`	`MassCancelWire` packed layout + decode + tests.
`src/wire/outbound/mod.rs`	Outbound module organization and re-exports.
`src/wire/outbound/exec_report.rs`	Outbound `ExecReport` encode/decode + status mapping + tests.
`src/wire/outbound/trade_print.rs`	Outbound `TradePrintWire` encode/decode + tests.
`src/wire/outbound/book_update.rs`	Outbound `BookUpdateWire` encode/decode + tests.
`doc/wire-protocol.md`	Protocol spec: framing, discriminants, layouts, endianness, tests.
`README.md`	Adds wire protocol notes to the v0.7.0 section.
`CHANGELOG.md`	Adds an entry describing the wire protocol feature.
`examples/Cargo.toml`	Adds `wire` feature and `wire_roundtrip` bin entry.
`examples/src/bin/wire_roundtrip.rs`	Demonstrates end-to-end frame encode/decode and wire→domain conversion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T13:03:59Z

+/// Propagates any [`io::Error`] returned by the underlying writer.
+///
+/// # Panics
+///
+/// Does not panic. Payloads larger than `u32::MAX - 1` bytes are not
+/// representable on this wire and would saturate to `u32::MAX`; in practice
+/// no message in this protocol is anywhere near that size, so this case is
+/// not validated at the framer level. Callers building unbounded payloads
+/// should guard before calling.
+#[inline]
+pub fn encode_frame<W: Write>(kind: u8, payload: &[u8], out: &mut W) -> io::Result<()> {
+    // `len` is the size of `kind + payload`. `KIND_SIZE` is a `u8`, so the
+    // total fits in `u32` for any payload up to `u32::MAX - 1` bytes.
+    let body_len = u32::try_from(payload.len().saturating_add(KIND_SIZE)).unwrap_or(u32::MAX);


encode_frame silently saturates body_len to u32::MAX when payload.len() + 1 doesn't fit in u32, but still writes the full payload. That produces a frame whose declared length doesn't match the bytes on the wire and will break downstream decoders. Return an io::Error (e.g. InvalidInput) when the payload is too large instead of saturating.

Suggested change

/// Propagates any [`io::Error`] returned by the underlying writer.

///

/// # Panics

///

/// Does not panic. Payloads larger than `u32::MAX - 1` bytes are not

/// representable on this wire and would saturate to `u32::MAX`; in practice

/// no message in this protocol is anywhere near that size, so this case is

/// not validated at the framer level. Callers building unbounded payloads

/// should guard before calling.

#[inline]

pub fn encode_frame<W: Write>(kind: u8, payload: &[u8], out: &mut W) -> io::Result<()> {

// `len` is the size of `kind + payload`. `KIND_SIZE` is a `u8`, so the

// total fits in `u32` for any payload up to `u32::MAX - 1` bytes.

let body_len = u32::try_from(payload.len().saturating_add(KIND_SIZE)).unwrap_or(u32::MAX);

/// Propagates any [`io::Error`] returned by the underlying writer, and

/// returns [`io::ErrorKind::InvalidInput`] if `kind + payload` does not fit

/// in the wire-format `u32` length prefix.

///

/// # Panics

///

/// Does not panic.

#[inline]

pub fn encode_frame<W: Write>(kind: u8, payload: &[u8], out: &mut W) -> io::Result<()> {

// `len` is the size of `kind + payload`. Reject payloads whose encoded

// body length cannot be represented in the wire-format `u32` prefix so we

// never emit a frame whose declared length disagrees with the bytes

// written.

let body_len_usize = payload

.len()

.checked_add(KIND_SIZE)

.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "frame payload too large"))?;

let body_len = u32::try_from(body_len_usize)

.map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame payload too large"))?;

Adopted the suggestion — encode_frame now returns io::ErrorKind::InvalidInput when the body length doesn't fit in the u32 prefix, so the declared length always matches the bytes written.

Copilot · 2026-04-25T13:03:59Z

+//!
+//! All fields are little-endian primitives. See `doc/wire-protocol.md` for
+//! the canonical layout tables.
+


The inbound wire structs use native-endian integer types (u64/i64) but the protocol specifies little-endian. On big-endian targets zerocopy::ref_from_bytes will interpret these fields incorrectly. Consider using zerocopy's endian-aware primitives (e.g. zerocopy::little_endian::*) or gate the entire wire feature with a #[cfg(target_endian = "little")]/compile_error! so the protocol can't be enabled on big-endian platforms.

Suggested change

#[cfg(target_endian = "big")]

compile_error!(

"src/wire/inbound requires a little-endian target because inbound wire \

structs are decoded directly from protocol bytes; use endian-aware \

field types before enabling this module on big-endian platforms."

);

Added a compile_error! in src/wire/inbound/mod.rs so feature = "wire" will not build on big-endian hosts. Endian-aware field types are noted in doc/wire-protocol.md as the path to forward big-endian support.

Copilot · 2026-04-25T13:04:00Z

+        // Encode the numeric account_id into the high 8 bytes of a Hash32 so
+        // it is preserved across the wire/domain boundary without colliding
+        // with `Hash32::zero()` (which is the "no STP" sentinel).
+        let mut user_bytes = [0u8; 32];
+        if let Some(slot) = user_bytes.get_mut(0..8) {
+            slot.copy_from_slice(&account_id.to_le_bytes());
+        }


The comment says account_id is encoded into the “high 8 bytes” of the Hash32, but the code writes into user_bytes[0..8] (the low 8 bytes). Either update the comment/docs or change the slice to match the intended placement so the wire↔domain mapping is unambiguous and consistent.

Comment was wrong — the code writes the low 8 bytes. Updated the comment in new_order.rs and the corresponding line in doc/wire-protocol.md to say low 8 bytes.

Copilot · 2026-04-25T13:04:00Z

+        let kind_byte = { value.order_type };
+


NewOrderWire documents _pad as “must be zero”, but neither decode_new_order nor the TryFrom<&NewOrderWire> validation checks it. If the pad is meant to be reserved/zero for forward compatibility, reject non-zero padding (or explicitly document that it is ignored).

Suggested change

let kind_byte = { value.order_type };

let kind_byte = { value.order_type };

let pad = { value._pad };

if pad.as_bytes().iter().any(|&byte| byte != 0) {

return Err(WireError::InvalidPayload("NewOrder: non-zero reserved padding"));

}

Added a non-zero check on _pad in the TryFrom<&NewOrderWire> boundary so reserved bytes are enforced.

Copilot · 2026-04-25T13:04:00Z

+    let engine_seq = read_u64(0)?;
+    let order_id = read_u64(8)?;
+    let status = *payload.get(16).ok_or(WireError::Truncated)?;
+    let filled_qty = read_u64(17)?;
+    let remaining_qty = read_u64(25)?;
+    let price = read_i64(33)?;
+    let reject_reason = read_u16(41)?;
+    let pad = *payload.get(43).ok_or(WireError::Truncated)?;
+    Ok(ExecReport {
+        engine_seq,
+        order_id,
+        status,
+        filled_qty,
+        remaining_qty,
+        price,
+        reject_reason,
+        _pad: pad,
+    })


decode_exec_report parses status and _pad but doesn't validate them. The layout docs state _pad “must be zero” and status is only defined for 0..=4; accepting other values makes it harder to detect corrupted/invalid frames. Consider rejecting unknown status values and non-zero padding in the decoder (or document that these fields are intentionally unchecked).

decode_exec_report now rejects any status byte above STATUS_REJECTED and any non-zero _pad.

Copilot · 2026-04-25T13:04:00Z

+    let engine_seq = read_u64(0)?;
+    let side = *payload.get(8).ok_or(WireError::Truncated)?;
+    let price = read_i64(9)?;
+    let qty = read_u64(17)?;
+    Ok(BookUpdateWire {
+        engine_seq,
+        side,
+        price,
+        qty,
+    })


decode_book_update doesn't validate that side is one of the documented values (0 buy / 1 sell) and it ignores the trailing 7 bytes of reserved padding (documented as “must be zero”). If the decoder is part of the public wire surface, it should reject invalid side values and non-zero reserved bytes to catch corrupted frames early.

decode_book_update now rejects unknown side bytes and any non-zero trailing 7-byte pad.

Copilot · 2026-04-25T13:04:01Z

+without copying. Decoding is safe — the crate has
+`#![deny(unsafe_code)]` on the lib root.


This doc says the crate has #![deny(unsafe_code)] at the lib root, but src/lib.rs does not currently contain that crate-level attribute. Please update the docs or enforce the attribute so the documentation matches reality.

Suggested change

without copying. Decoding is safe — the crate has

`#![deny(unsafe_code)]` on the lib root.

without copying. Decoding is safe because it relies on

`zerocopy`-based validation and typed casting rather than unchecked

memory reinterpretation.

You're right — #![deny(unsafe_code)] is currently absent from the lib root after the alloc-counters merge. Reworded both the doc and the lib.rs blurb to describe the actual guarantee, which is that the wire path itself uses no unsafe (zerocopy validates the layout).

Copilot · 2026-04-25T13:04:01Z

+layout matches the on-wire byte order on every supported target
+(little-endian only). Consumers on big-endian hosts must explicitly
+byte-swap when reading the integers — the `decode_*` helpers in this
+crate handle that for you.


The endianness section is internally inconsistent and doesn't match the inbound implementation: it says the protocol is “little-endian only” but also says decode_* helpers handle big-endian byte-swapping. Currently the inbound zerocopy structs use native-endian integers, so big-endian decoding would be incorrect. Either (a) explicitly restrict feature=wire to target_endian = "little" and remove the byte-swap claim, or (b) switch inbound fields to endian-aware types and convert to native values on read.

Suggested change

layout matches the on-wire byte order on every supported target

(little-endian only). Consumers on big-endian hosts must explicitly

byte-swap when reading the integers — the `decode_*` helpers in this

crate handle that for you.

layout matches the on-wire byte order only on supported little-endian

targets. Accordingly, `feature=wire` is currently little-endian only;

big-endian targets are not supported by the current inbound decoding

implementation.

Rewrote the endianness section to match reality — packed inbound structs use native primitives, so feature = "wire" is now restricted to little-endian targets via compile_error!. Removed the bogus byte-swap claim.

Copilot · 2026-04-25T13:04:01Z

+pub fn decode_mass_cancel(payload: &[u8]) -> Result<MassCancelWire, WireError> {
+    let view = MassCancelWire::ref_from_bytes(payload)
+        .map_err(|_| WireError::InvalidPayload("MassCancel: payload size mismatch"))?;
+    let scope = { view.scope };
+    match scope {
+        SCOPE_ALL | SCOPE_BY_ACCOUNT | SCOPE_BY_SIDE => Ok(*view),
+        _ => Err(WireError::InvalidPayload("MassCancel: unknown scope")),
+    }


decode_mass_cancel validates scope but doesn't enforce the documented padding rules (for BySide, only _pad[0] & 1 should be used and other bits must be zero; for other scopes, all padding should be zero). Add checks so malformed frames can't smuggle unexpected data in the reserved bytes.

decode_mass_cancel now enforces the documented padding rules — for BySide only the low bit of _pad[0] may be set, every other byte must be zero; for All and ByAccount all 7 padding bytes must be zero.

Copilot · 2026-04-25T13:04:01Z

+//!   `#[repr(C, packed)]` with `zerocopy::{FromBytes, IntoBytes,
+//!   Unaligned, Immutable, KnownLayout}` derives. Each ships a
+//!   `const _: () = assert!(size_of::<…>() == N)` guard. Decoding
+//!   is safe — `#![deny(unsafe_code)]` stays on.


The crate docs claim #![deny(unsafe_code)] “stays on”, but there is no crate-level #![deny(unsafe_code)] attribute in src/lib.rs (and src/utils/counting_allocator.rs contains #![allow(unsafe_code)]). Either add the crate-level deny (and adjust existing unsafe usage accordingly) or reword this section to avoid stating a guarantee the crate doesn't currently enforce.

Suggested change

//! is safe — `#![deny(unsafe_code)]` stays on.

//! is implemented safely without relying on unsafe operations in

//! this path.

Reworded the lib.rs blurb to match what is actually enforced — the wire path itself uses no unsafe. Restoring the crate-level #![deny(unsafe_code)] is its own follow-up since it interacts with the alloc-counters allocator wrapper.

…ng + restrict wire to little-endian

joaquinbejar added 4 commits April 25, 2026 14:56

joaquinbejar requested a review from Copilot April 25, 2026 12:58

Copilot started reviewing on behalf of joaquinbejar April 25, 2026 12:58 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

address review: validate pads + return InvalidInput on overflow frami…

930822b

…ng + restrict wire to little-endian

joaquinbejar merged commit 825c2a3 into main Apr 25, 2026
13 checks passed

joaquinbejar deleted the issue-59-wire-protocol branch April 25, 2026 13:09

joaquinbejar mentioned this pull request Apr 25, 2026

Implement wire protocol feature for binary order entry #69

Closed

9 tasks

+#[cfg(target_endian = "big")]
+compile_error!(
+    "src/wire/inbound requires a little-endian target because inbound wire \
+     structs are decoded directly from protocol bytes; use endian-aware \
+     field types before enabling this module on big-endian platforms."
+);

-        let kind_byte = { value.order_type };
+        let kind_byte = { value.order_type };
+        let pad = { value._pad };
+        if pad.as_bytes().iter().any(|&byte| byte != 0) {
+            return Err(WireError::InvalidPayload("NewOrder: non-zero reserved padding"));
+        }

		without copying. Decoding is safe — the crate has
		`#![deny(unsafe_code)]` on the lib root.

-without copying. Decoding is safe — the crate has
-`#![deny(unsafe_code)]` on the lib root.
+without copying. Decoding is safe because it relies on
+`zerocopy`-based validation and typed casting rather than unchecked
+memory reinterpretation.

	//! is safe — `#![deny(unsafe_code)]` stays on.
	//! is implemented safely without relying on unsafe operations in
	//! this path.

Conversation

joaquinbejar commented Apr 25, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants