refactor: make the fixed-point price encoding internal#823
Merged
Conversation
Price exposed pub value and pub price_type, letting a caller construct thetadatadx::Price with a price_type outside the 0..=MAX_PRICE_TYPE range that to_f64, Display, and the Ord comparison all rely on. The encoded exponent price_type - 10 indexes the 20-entry POW10 tables; a price_type of 30 or a negative price_type produced an out-of-bounds index that panicked under release array bounds-checking, and a debug_assert did not guard release builds. Make the raw fields crate-private so the range invariant can only be established through the validating constructors new and with_value_and_type; readers use the existing value and price_type accessors. As defense in depth, clamp price_type into the table range on every read path so conversion, formatting, and comparison saturate to a well-defined result instead of indexing out of bounds, even for an internally constructed odd value. In-range values are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Encode the decimal exponent as a validated PriceType newtype over 0..=MAX_PRICE_TYPE whose only constructors check the range, so an out-of-range price_type cannot be represented. The conversion and comparison paths index the POW10 tables with an exponent that is in bounds by construction, which removes the per-read clamp helper and its three call sites; no read path can silently produce a wrong-but-finite price for an exponent the type forbids. The wire decode boundary already constructs through the fallible with_value_and_type, which now threads PriceType::new and rejects out-of-range cells as before. Price::new keeps its saturating contract via PriceType::saturating so existing infallible callers stay panic-free. The public price_type() getter still returns i32, so readers are unchanged. Price is a pure-Rust internal type: it carries no repr(C), no layout assertion, and never crosses the FFI or language-binding boundary as raw fields (bindings consume the decoded f64 via to_f64), so changing the field type from i32 to PriceType breaks no ABI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A price is encoded as a (value, price_type) mantissa-and-exponent pair where the real price is value * 10^(price_type - 10). That variable-precision fixed-point encoding is a wire-transport detail: a client receives decoded prices (f64 dollars on the tick rows) and never sees, sets, or reasons about the raw pair. The encoding was leaking through the public crate surface. Remove Price, PriceType, PriceError, and MAX_PRICE_TYPE from the curated public API and from the tdbe / tdbe::types facades, gating them behind the __internal feature (doc-hidden) so they stay reachable for workspace tools, bindings, and the data-format benches but are absent from the default public surface and rendered rustdoc. The decode layer reaches them through the full tdbe::types::price leaf path, never a short facade alias, so the encoding cannot drift back onto the public surface through a convenience re-export. No public function or binding accepts or returns a Price; every public tick row already carries the decoded f64 price, so nothing public breaks. The price_type tick-schema column is a positional wire-layout field the parser reads and discards, already not a client-facing struct field in any binding. The exponent stays a validated PriceType newtype over 0..=MAX_PRICE_TYPE, so an out-of-range exponent is unrepresentable and the POW10 table indexing needs no per-read range check. The earlier panic class is eliminated because only the validated internal decode path (Price::with_value_and_type, threading PriceType::new) constructs a Price. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A price is encoded as a
(value, price_type)mantissa-and-exponent pair where the real price isvalue * 10^(price_type - 10). That variable-precision fixed-point encoding is a wire-transport detail. A client of this crate receives decoded prices (f64dollars on the tick rows, viato_f64), so it must never see, set, or reason about the raw(value, price_type)pair. This PR makes the encoding internal: it removesPrice,PriceType,PriceError, andMAX_PRICE_TYPEfrom the public API, and it hardens the exponent so only the validated internal decode path can construct aPrice.Make the encoding internal
Priceand its supporting types were re-exported on the curated public surface (thetadatadx::Price,thetadatadx::PriceType,thetadatadx::PriceError,thetadatadx::MAX_PRICE_TYPE) and on the internaltdbe/tdbe::typesfacades. None of that is needed by a client: every public tick row already carries the decodedf64price, and no public function or binding accepts or returns aPrice. The pair never crosses a binding boundary.These types are now gated behind the
__internalfeature (doc-hidden, with the same "NOT a stable public surface" contract as the other wire-internal re-exports), so they stay reachable for workspace tools, bindings, and the data-format benches but are absent from the default public API and from rendered rustdoc. The decode layer reaches them through the fulltdbe::types::priceleaf path, never a short facade alias, so the encoding cannot drift back onto the public surface through a convenience re-export. Theprice_typetick-schema column is a positional wire-layout field the parser reads and discards; it was already not a client-facing struct field in any binding, so no tick row exposes the raw exponent.Parse, don't validate: the exponent is unrepresentable out of range
The decimal exponent is a validated
PriceTypenewtype over0..=MAX_PRICE_TYPE. Its only constructors check the range:PriceType::new(i32) -> Result<_, PriceError>/TryFrom<i32>rejects out of range with a typed error.PriceType::saturating(i32)is infallible, snapping into range at the nearest boundary; it backs the lossyPrice::new.Because an out-of-range exponent cannot be constructed,
exp = price_type - 10is confined to-10..=9andexp.unsigned_abs()indexes the precomputedPOW10_I64/POW10_F64tables in bounds by construction. The per-read clamps into_f64,Display, andcompareare gone; the table access is provably in range with no runtime check and no path that can observe or fabricate a result from an out-of-range exponent. The earlier panic class is eliminated because only the validated decode boundary constructs aPrice: the wire decode path (FPSS / MDDS) constructs through the falliblePrice::with_value_and_type, which threadsPriceType::newand rejects out-of-range cells exactly as before;Price::newkeeps its saturating contract viaPriceType::saturatingso existing infallible callers stay panic-free.Why this is ABI-safe
Priceis a pure-Rust internal type, with evidence:#[repr(C)], no layout assertion, nobytemuck/zerocopytraits onPrice.Price's raw fields. The C ABI exposesPriceTick, which carries a decodeddouble price, notPrice. The layout-assert suite coversPriceTick, notPrice.f64viato_f64; the(value, price_type)pair never crosses a binding boundary.So removing the public re-export and changing the field type from
i32toPriceTypecannot break any ABI. Verified:thetadatadx-ffi,thetadatadx-py, andthetadatadx-napiall build against the default-feature crate; the tick layout-assert suite (incl.price_tick_layout) passes unchanged.Tests
price_type_rejects_out_of_rangeaccepts0..=19and rejects-1,20,99,i32::MIN,i32::MAX; out-of-range is unrepresentable.price_type_saturating_clamps_to_boundariessnaps at both ends and leaves in-range values untouched.every_price_type_indexes_tables_in_boundsandevery_valid_price_type_renders_and_converts_finitelypin that every representable exponent indexes the tables in bounds and converts finitely.in_range_conversions_are_unchangedpinsvalue=12345/type=8 -> 123.45,5/12 -> 500.0,100/10 -> 100.0for bothto_f64andDisplay.with_value_and_type_enforces_range,accessors_match_fields,is_unset_vs_is_zero_value,new_saturates_out_of_range_price_typeretained.Verification:
cargo fmt --all -- --check;cargo test -p thetadatadx --lib(815 passed); FFI + Python + TypeScript binding crates build; binding-parity check clean;git grepconfirms the default public surface no longer namesPrice/PriceType/PriceError/MAX_PRICE_TYPE.