Return domain newtypes from accessors (Quantity/TimestampMs/Price) (#71)#99
Conversation
…s/Price) Raw u64/u128 no longer leak across the accessor boundary: - OrderType: visible_quantity()/hidden_quantity() -> Quantity, timestamp() -> TimestampMs - MatchResult: new/with_capacity/finalize take Quantity; remaining_quantity() -> Quantity; executed_quantity() -> Result<Quantity,_> (executed_value/total_value stay u128 — no monetary newtype) - PriceLevelSnapshot: price -> Price, visible/hidden/total_quantity -> Quantity Snapshot JSON wire format is UNCHANGED (Price/Quantity are #[serde(transparent)]); SHA-256 checksum + round-trip tests pass unchanged, no format-version bump. match_order's incoming_quantity stays u64 (converted at the MatchResult boundary) to avoid re-churning the call sites changed in #65. BREAKING (pre-0.9). Migration note in lib.rs.
There was a problem hiding this comment.
Pull request overview
This PR updates the public API to return the crate’s domain newtypes (Quantity, TimestampMs, Price) from accessors/constructors instead of raw primitives, enforcing the “newtypes at domain boundaries” rule while keeping snapshot JSON wire format byte-identical via #[serde(transparent)].
Changes:
- Updated
OrderTypeaccessors to returnQuantity/TimestampMsinstead ofu64. - Updated
MatchResultconstructors and quantity-returning accessors to useQuantity(and adjusted tests/examples/benches accordingly). - Updated
PriceLevelSnapshotfields + accessors to usePrice/Quantity, with call sites migrated.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/price_level/tests/snapshot.rs | Updates snapshot tests to construct/compare Price/Quantity/TimestampMs newtypes. |
| src/price_level/tests/order_queue.rs | Adjusts order-queue tests to compare against Quantity newtypes. |
| src/price_level/tests/level.rs | Updates level tests for MatchResult/order/snapshot quantity accessors returning newtypes. |
| src/price_level/snapshot.rs | Changes snapshot DTO/storage and accessors to Price/Quantity, preserving serialization behavior. |
| src/price_level/level.rs | Adapts snapshot reconstruction and match-result construction/finalization to the new quantity newtype boundary. |
| src/orders/tests/order_type.rs | Updates OrderType tests for newtype-returning accessors. |
| src/orders/order_type.rs | Changes visible_quantity/hidden_quantity/timestamp accessors to return Quantity/TimestampMs. |
| src/lib.rs | Adds migration guide section documenting the breaking API changes. |
| src/execution/tests/match_result_trade.rs | Updates match-result trade tests for newtype-based constructors/accessors. |
| src/execution/match_result.rs | Updates MatchResult APIs to accept/return Quantity where appropriate. |
| README.md | Regenerates documentation to reflect the breaking newtype boundary changes. |
| examples/src/bin/simple.rs | Updates example to handle executed_quantity() returning Quantity. |
| examples/src/bin/integration_trade_roundtrip.rs | Updates example assertions for Quantity return values. |
| examples/src/bin/integration_special_orders.rs | Updates special-order examples for Quantity return values. |
| examples/src/bin/integration_snapshot_recovery.rs | Updates snapshot recovery example for Price/Quantity accessors. |
| examples/src/bin/integration_checked_arithmetic.rs | Updates checked-arithmetic example for Quantity accessors. |
| examples/src/bin/integration_basic_lifecycle.rs | Updates lifecycle example for Quantity accessors. |
| examples/src/bin/hft_simulation.rs | Updates simulation example to treat executed quantity as Quantity. |
| benches/price_level/iter_orders.rs | Updates benchmark to sum Quantity values via .as_u64(). |
| benches/price_level/checked_arithmetic.rs | Updates benchmark to construct MatchResult with Quantity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (73.24%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
The newtype migration table listed PriceLevelSnapshot::with_orders but omitted with_orders_and_stats, whose price param also changed u128 -> Price. Added it.
|
Thanks, added the missing with_orders_and_stats row to the breaking-change migration table. |
Summary
Return the crate's domain newtypes from accessors instead of raw
u64/u128, soraw primitives don't leak across the accessor boundary (the "newtypes at every
domain boundary" rule). User-approved breaking change for the 0.9 window.
Changes
OrderType:visible_quantity()/hidden_quantity()→Quantity,timestamp()→TimestampMs(already stored as newtypes internally).MatchResult:new/with_capacity/finalizetakeQuantity;remaining_quantity()→Quantity;executed_quantity()→Result<Quantity, _>.executed_value()/total_value()stayu128(nomonetary newtype invented).
PriceLevelSnapshot:price→Price,visible_quantity/hidden_quantity/total_quantity→Quantity(fields + accessors).Technical decisions
Price/Quantityare#[serde(transparent)], so the hand-writtenSerialize/Deserialize/FromStremit/parse the same numbers. No format-version bump. Proof: the checksum +
round-trip tests (
test_snapshot_package_*,..._preserves_statistics,max_values) pass unchanged.match_order'sincoming_quantitystaysu64(converted toQuantityat theMatchResultboundary) to avoid re-churning the ~124 call sites just touched inDecide and document taker TIF / PostOnly / MarketToLimit responsibility at the level #65. Documented in the migration note.
Public API impact
Breaking (accessor/
MatchResultreturn + param types). Migration-guide sectionadded to
src/lib.rs(before/after table);README.mdregenerated. Snapshotwire unchanged. ~205 call sites updated.
Testing
identical)
make pre-pushcleancargo test: 406 (unit) + 1 (integration) + 9 (doctests), 0 failed.cargo clippy --all-targets --all-features -- -D warnings,cargo fmt --all --check,cargo build --release: clean.Checklist
u64/u128leak on the changed surface.unwrap();# Errors/#[must_use]retainedsrc/lib.rsmigration guide +README.mdupdatedCloses #71