Skip to content

fix(execution): MatchResult bincode round-trip — serialize outcome as Option (#135)#136

Merged
joaquinbejar merged 1 commit into
mainfrom
issue-135-matchresult-bincode-roundtrip
Jul 14, 2026
Merged

fix(execution): MatchResult bincode round-trip — serialize outcome as Option (#135)#136
joaquinbejar merged 1 commit into
mainfrom
issue-135-matchresult-bincode-roundtrip

Conversation

@joaquinbejar

Copy link
Copy Markdown
Owner

Summary

Fixes #135MatchResult could not round-trip bincode since 0.9.0.

The decode-time validation from #117 deserializes through MatchResultWire,
whose outcome is Option<MatchOutcome> (to accept legacy payloads), but
the Serialize derive still emitted a bare MatchOutcome. JSON tolerated
the asymmetry; bincode decodes positionally and read the enum variant index
where the Option tag was expected:

UnexpectedVariant { type_name: "Option<T>", allowed: Range { min: 0, max: 1 }, found: 2 }

This blocks OrderBook-rs's bincode feature (NATS zero-copy path) from
bumping to 0.9 (joaquinbejar/OrderBook-rs#205).

Changes

  • outcome is serialized via Serializer::serialize_some, making the
    emitted shape symmetric with the wire struct. JSON payload is
    byte-identical (serde flattens Some); bincode encode/decode now agree.
  • Regression coverage: a bincode leg on the existing
    valid_result_round_trips_both_encodings property test, a deterministic
    bincode_round_trip_is_symmetric_issue_135 shape test, and a JSON guard
    asserting the payload still carries the bare "outcome" value.
  • New dev-dependency: bincode = "2.0" (serde + std features only,
    dev-only) to drive the positional-format regression.
  • Version 0.9.0 → 0.9.1 + CHANGELOG entry.

Compatibility

  • JSON payloads: unchanged in both directions, any version.
  • Bincode payloads written by 0.9.0: still do not decode — they never did
    (0.9.0 could not decode its own output), so no working payload breaks.
  • Pre-0.9 bincode payloads: unchanged (positional formats cannot express
    the absent trailing field either way).

Test plan

  • cargo test --all-features — 528 tests green.
  • make pre-push clean (clippy -D warnings, fmt, doc -D warnings).

@joaquinbejar joaquinbejar self-assigned this Jul 14, 2026
@joaquinbejar
joaquinbejar merged commit 5ee60e6 into main Jul 14, 2026
13 checks passed
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/execution/match_result.rs 94.01% <100.00%> (+1.10%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

MatchResult bincode round-trip broken: Serialize emits bare outcome, wire decode expects Option

2 participants