Skip to content

fix(ENG-570): Reconcile Price codec forks beyond the FX range#151

Open
alnoki wants to merge 2 commits into
mainfrom
eng-570
Open

fix(ENG-570): Reconcile Price codec forks beyond the FX range#151
alnoki wants to merge 2 commits into
mainfrom
eng-570

Conversation

@alnoki

@alnoki alnoki commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Makes the Rust and TS Price codecs agree for inputs above the protocol's FX target band (~1e-6 .. 1e6), closing two divergences found in the ENG-557 adversarial review (PR #140). Both live in the shared Price codec, so they land together.

1. Encode huge-value divergence

Price::from_value silently saturated the f64u64 cast — (1e300 * 1e7) as u64 clamps to u64::MAX, which from_scaled then normalized into a bogus finite Price (~1.8e12). TS encodePrice instead threw RangeError. So the forks rejected different input sets.

Fix: both forks now reject any finite value whose value * 1e7 would overflow u64 (the exact saturation point, ≥ 2^64). Below that boundary both truncate the same f64 toward zero and normalize with identical integer arithmetic, so the reject and accept sets are now bit-identical. This fixes the real bug (Rust's silent normalization) without imposing an arbitrary hard 1e6 cap that would reject structurally-valid prices.

2. weightedAverage fidelity (from ENG-572)

Bit-identical to Rust inside the FX band, but two out-of-band gaps in the TS fork:

  • Gap 1 — precision. fromScaled(Number(avg), 14) rounded avg (a bigint up to u64::MAX) through an f64 once avg > 2^53, flipping the 8th significand digit vs Rust's exact u64 from_scaled. fromScaled is now bigint-based and driven from the bigint directly.
  • Gap 2 — saturation. Rust uses saturating_mul/saturating_add on the w*v products and their sum; TS used plain bigint. Now clamped at u128::MAX to mirror Rust.

The shared bigint fromScaled is what makes #1 and #2 one change.

Conformance

  • price_vectors.json — huge-value rejects (1e13, 1e300) + a large-but-valid accept (1e12) pinning the boundary.
  • share_vectors.json — two out-of-band merge_entry_basis cases: a precision case (the bigint path yields significand 60_000_000; the old Number(avg) path produced 60_000_001) and a u128-saturation case.
  • docs/interface.md §6B — the "deliberately unconformed" note is replaced with the now-agreed contract.

Verification

  • cargo test -p dropset-math-core --tests — 81 unit + 5 price-conformance + 6 share-conformance, all green.
  • pnpm --filter @dropset/sdk test — 13 TS conformance tests green (both forks replay the regenerated vectors).
  • cargo fmt / cargo clippy clean.

@linear

linear Bot commented Jun 20, 2026

Copy link
Copy Markdown

ENG-570

@alnoki alnoki changed the title ENG-570 fix(ENG-570): Reconcile Price codec forks beyond the FX range Jun 20, 2026
alnoki added 2 commits June 19, 2026 18:46
Make the Rust and TS `Price` codecs agree for inputs above the
protocol's FX target band, closing two divergences found in the
ENG-557 adversarial review.

Encode huge-value divergence: `Price::from_value` silently saturated
the `f64`->`u64` cast, normalizing e.g. `1e300` into a bogus finite
`Price`, while TS `encodePrice` threw. Both forks now reject any value
whose `value * 1e7` would overflow `u64` (the exact saturation point),
so the reject and accept sets are bit-identical.

weightedAverage fidelity: the TS fork rebuilds `fromScaled` on `bigint`
so the significand normalization is exact integer math past 2^53
(Gap 1 — `Number(avg)` rounded and flipped the 8th digit), and clamps
the weighted products / sum / total at `u128::MAX` to mirror Rust's
`saturating_mul` / `saturating_add` (Gap 2).

Pin both via conformance: `price_vectors.json` adds huge-value rejects
(`1e13`, `1e300`) plus a large-but-valid accept (`1e12`), and
`share_vectors.json` adds out-of-band `merge_entry_basis` cases for the
precision and saturation paths.
@alnoki alnoki marked this pull request as ready for review June 20, 2026 01:48
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.

1 participant