From 6bc784e8002f98560871dc52fcfffbce0e8f87be Mon Sep 17 00:00:00 2001 From: Joaquin Bejar Date: Tue, 14 Jul 2026 22:05:44 +0200 Subject: [PATCH] fix(execution): serialize MatchResult outcome as Option for bincode symmetry (#135) --- CHANGELOG.md | 18 +++++ Cargo.toml | 6 +- src/execution/match_result.rs | 18 +++++ src/execution/tests/match_result_trade.rs | 80 +++++++++++++++++++++-- 4 files changed, 117 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e3725..4c4066c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.9.1] - 2026-07-14 + +### Fixed + +- **`MatchResult` round-trips non-self-describing serde formats again + (#135).** 0.9.0's decode-time validation (#117) deserializes through a wire + struct whose `outcome` is `Option`, but `Serialize` still + emitted a bare `MatchOutcome`. JSON tolerated the asymmetry; a positional + decoder (bincode) read the enum variant index where the option tag was + expected and failed with `UnexpectedVariant` on every 0.9.0 payload. + `outcome` is now serialized as `Some(outcome)`: the JSON payload is + byte-identical (serde flattens `Some`), and bincode encode/decode are + symmetric. Bincode payloads written by 0.9.0 do not decode (they never + did — 0.9.0 could not decode its own output); JSON payloads from any + version are unaffected. Pinned by a bincode leg on the round-trip + property test plus deterministic shape guards (`bincode` added as a + dev-dependency only). + ## [0.9.0] - 2026-07-14 Major hardening release: ten engine-correctness issues (#111–#120, PRs diff --git a/Cargo.toml b/Cargo.toml index 6014188..d93b196 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pricelevel" -version = "0.9.0" +version = "0.9.1" edition = "2024" authors = ["Joaquin Bejar "] description = "A high-performance, lock-free price level implementation for limit order books in Rust. This library provides the building blocks for creating efficient trading systems with support for multiple order types and concurrent access patterns." @@ -54,6 +54,10 @@ criterion = { version = "0.8", default-features = false, features = ["html_repor # `rusty-fork` / `wait-timeout` deps) we do not use; `std` keeps the core # strategy API. proptest = { version = "1", default-features = false, features = ["std", "bit-set"] } +# bincode is dev-only: it pins the non-self-describing serde round-trip of +# `MatchResult` (issue #135) — the shape downstream consumers (OrderBook-rs's +# NATS bincode path) encode with. It never ships in the library surface. +bincode = { version = "2.0", default-features = false, features = ["serde", "std"] } # loom is a dev-only model checker, gated behind `--cfg loom`. It is NOT a # runtime dependency. It models the cancel-vs-partial-fill linearization (issue # #81); see `tests/loom/cancel_match.rs`. loom cannot instrument dashmap / diff --git a/src/execution/match_result.rs b/src/execution/match_result.rs index c35199a..515cbbc 100644 --- a/src/execution/match_result.rs +++ b/src/execution/match_result.rs @@ -101,9 +101,27 @@ pub struct MatchResult { filled_order_ids: Vec, /// Terminal classification of the match (filled / killed / rejected / ...). + /// + /// Serialized as `Some(outcome)` so the emitted shape is symmetric with + /// [`MatchResultWire`]'s `Option`. JSON is unaffected — + /// serde flattens `Some` away, so the payload still carries the bare + /// value — but a non-self-describing format (bincode) decodes + /// positionally and must find the option tag the wire struct reads + /// back (#135). + #[serde(serialize_with = "serialize_outcome_as_some")] outcome: MatchOutcome, } +/// Serializes `outcome` wrapped in `Some` — see the field doc on +/// [`MatchResult::outcome`] (#135). Kept as a free function because serde's +/// `serialize_with` requires this exact signature. +fn serialize_outcome_as_some(outcome: &MatchOutcome, serializer: S) -> Result +where + S: serde::Serializer, +{ + serializer.serialize_some(outcome) +} + /// Wire form of [`MatchResult`] used for validated deserialization. /// /// It mirrors `MatchResult`'s serialized shape field-for-field (identical diff --git a/src/execution/tests/match_result_trade.rs b/src/execution/tests/match_result_trade.rs index d7f31c8..2ea5bc0 100644 --- a/src/execution/tests/match_result_trade.rs +++ b/src/execution/tests/match_result_trade.rs @@ -768,13 +768,68 @@ mod tests { result } + /// #135 regression: the `Serialize` shape must be symmetric with the + /// validated wire struct. Before the fix the bare `outcome` variant index + /// landed where the wire's `Option` tag was expected and bincode decode + /// failed with `UnexpectedVariant` on every payload 0.9.0 produced. + #[test] + fn bincode_round_trip_is_symmetric_issue_135() { + // Empty result (NotFilled) — the minimal shape that 0.9.0 failed on. + let empty = MatchResult::new(Id::from_u64(10), Quantity::new(100)); + let bytes = match bincode::serde::encode_to_vec(&empty, bincode::config::standard()) { + Ok(bytes) => bytes, + Err(error) => panic!("bincode encode failed: {error}"), + }; + let decoded: MatchResult = + match bincode::serde::decode_from_slice(&bytes, bincode::config::standard()) { + Ok((value, _)) => value, + Err(error) => panic!("bincode decode failed: {error}"), + }; + assert_eq!(decoded.outcome(), MatchOutcome::NotFilled); + assert_eq!(decoded.remaining_quantity().as_u64(), 100); + + // Partially filled result with a trade — outcome variant index 1. + let mut partial = MatchResult::new(Id::from_u64(10), Quantity::new(100)); + assert!(partial.add_trade(sample_trade(25)).is_ok()); + let bytes = match bincode::serde::encode_to_vec(&partial, bincode::config::standard()) { + Ok(bytes) => bytes, + Err(error) => panic!("bincode encode failed: {error}"), + }; + let decoded: MatchResult = + match bincode::serde::decode_from_slice(&bytes, bincode::config::standard()) { + Ok((value, _)) => value, + Err(error) => panic!("bincode decode failed: {error}"), + }; + assert_eq!(decoded.outcome(), MatchOutcome::PartiallyFilled); + assert_eq!(decoded.remaining_quantity().as_u64(), 75); + assert_eq!(decoded.trades().len(), 1); + } + + /// #135 guard: wrapping `outcome` in `Some` on the serialize side must + /// not change the JSON payload — serde flattens the option away, so the + /// key still carries the bare snake_case value. + #[test] + fn json_payload_keeps_bare_outcome_issue_135() { + let result = MatchResult::new(Id::from_u64(10), Quantity::new(100)); + let json = match serde_json::to_string(&result) { + Ok(json) => json, + Err(error) => panic!("json encode failed: {error}"), + }; + assert!( + json.contains("\"outcome\":\"not_filled\""), + "outcome must stay a bare value in JSON, got: {json}" + ); + } + proptest! { #![proptest_config(ProptestConfig { cases: 256, ..ProptestConfig::default() })] - /// Any valid result built through the public API round-trips both serde - /// JSON (outcome preserved) and Display/FromStr (benign outcome - /// re-derived identically) with no field drift — the validator never - /// rejects a legitimately-constructed value. + /// Any valid result built through the public API round-trips serde + /// JSON (outcome preserved), bincode (positional, non-self-describing + /// — pins the encode/decode shape symmetry of #135), and + /// Display/FromStr (benign outcome re-derived identically) with no + /// field drift — the validator never rejects a + /// legitimately-constructed value. #[test] fn valid_result_round_trips_both_encodings( initial in 1u64..=100_000, @@ -782,6 +837,23 @@ mod tests { ) { let original = build_valid_result(initial, &specs); + // bincode is positional: it only decodes if the Serialize shape + // matches the wire struct field-for-field (#135). + let bytes = bincode::serde::encode_to_vec(&original, bincode::config::standard()) + .map_err(|e| TestCaseError::fail(format!("bincode encode: {e}")))?; + let (bin_decoded, _): (MatchResult, usize) = + bincode::serde::decode_from_slice(&bytes, bincode::config::standard()) + .map_err(|e| TestCaseError::fail(format!("valid bincode must decode: {e}")))?; + prop_assert_eq!(bin_decoded.order_id(), original.order_id()); + prop_assert_eq!( + bin_decoded.remaining_quantity().as_u64(), + original.remaining_quantity().as_u64() + ); + prop_assert_eq!(bin_decoded.is_complete(), original.is_complete()); + prop_assert_eq!(bin_decoded.outcome(), original.outcome()); + prop_assert_eq!(bin_decoded.trades().len(), original.trades().len()); + prop_assert_eq!(bin_decoded.filled_order_ids(), original.filled_order_ids()); + // serde JSON must decode and preserve every field, outcome included. let json = serde_json::to_string(&original) .map_err(|e| TestCaseError::fail(format!("serialize: {e}")))?;