M5 #335: Audit and convert monetary Decimal arithmetic to checked helpers by joaquinbejar · Pull Request #371 · joaquinbejar/OptionStratLib

joaquinbejar · 2026-04-19T16:41:55Z

Summary

Audit every monetary Decimal arithmetic site in the crate and route it through new crate-private checked helpers (d_add, d_sub, d_mul, d_div, d_sum) so that overflow, division by zero, and rounding are reported explicitly via a new DecimalError::Overflow variant instead of silently wrapping to Decimal::MIN/MAX. Pricing kernels, Greeks scaling, Position P&L, multi-leg strategy P&L, chain exposure aggregates, and volatility variance recursions now surface a tagged DecimalError on saturation. Numerical-kernel internals (GBM steps, BAW inner loop, closure arithmetic inside barrier/lookback) are out of scope for this issue and intentionally left on raw Decimal.

Changes

Error surface: new DecimalError::Overflow { operation, lhs, rhs } variant + factory + #[error] display, and cascaded #[from] DecimalError into PositionError and VolatilityError. PricingError, GreeksError, and ChainError already had mappings; they continue to work transparently.
Helpers in src/model/decimal.rs: d_add, d_sub, d_mul, d_div, d_sum, plus DIV_DEFAULT_SCALE = 28. d_div applies banker's rounding (MidpointNearestEven) uniformly at scale 28 and rejects division by zero with DecimalError::ArithmeticError before the checked op, so the overflow variant is reserved for genuine saturation.
Pricing (src/pricing/): Black-Scholes, American (intrinsic, zero-vol, early-exercise premium), binary (cash-or-nothing, asset-or-nothing, gap), barrier (final composition), lookback (floating and fixed strike final composition), chooser (Rubinstein formula), cliquet (period accumulation, zero-vol and ITM branches, final BS call), binomial node value and discounted payoff in pricing::utils, Monte Carlo payoff and discounted average. Telegraph kernel was already on Decimal; the monetary-boundary compositions are now checked.
Greeks (src/greeks/): theta/365, rho/100, rho_d/100, charm/365, color/365 scaling divisions, position-size multiplications, and every finite-difference numerator in numerical.rs. Delta-neutral sizing in greeks::utils now uses checked helpers.
Position (src/model/position.rs): pnl_at_expiration and unrealized_pnl convert their multi-step compositions ((underlying - strike) * qty - premium * qty - fees) into a sequence of d_mul/d_sub/d_add calls with #[from] DecimalError propagation through PositionError.
Strategies (src/strategies/): every multi-leg Profit::calculate_profit_at (BullCallSpread, BearCallSpread, BullPutSpread, BearPutSpread, LongStraddle, LongStrangle, CallButterfly, LongButterflySpread, IronCondor, IronButterfly, PoorMansCoveredCall, CustomStrategy) now aggregates leg P&L via d_sum so an overflow on any leg surfaces as a tagged StrategyError::DecimalError instead of wrapping silently.
Chain exposures (src/chains/chain.rs): the nine aggregate Greeks (gamma, delta, vega, theta, vanna, vomma, veta, charm, color) use d_add per-leg with an operation tag. OptionChain::vega_exposure::put, chains::gamma_exposure, etc. appear in the overflow error payload so the failing aggregate is obvious.
Volatility (src/volatility/utils.rs): constant_volatility (mean + variance division at scale 28 with banker's rounding), ewma_volatility (per-step recursion lambda * v + (1 - lambda) * r^2), and garch_volatility (omega + alpha * r^2 + beta * v) now use the checked helpers and can no longer overflow silently on adversarial inputs.

Technical Decisions

Monetary-only scope. Probability math (transition probabilities, N(d1)/N(d2), GBM drift/diffusion steps), BAW inner coefficients, and the Heston SDE discretisation stay on raw Decimal. They are numerical-kernel internals (covered by issue Guard NaN/Inf at f64 ↔ Decimal boundaries in pricing kernels #336) whose inputs are bounded by construction; wrapping them in checked helpers now would be noise without a signal.
Banker's rounding at scale 28 is the single crate policy for division. DIV_DEFAULT_SCALE = 28 matches Decimal::MAX_SCALE, so a rounded quotient loses no representable precision and subsequent rescales cannot overflow. Call sites that need cents-precision results (P&L reporting) apply an explicit round_dp_with_strategy(dp, MidpointNearestEven) on top.
d_sum over per-leg Vec<Decimal>. Multi-leg strategy P&L collects each leg's pnl_at_expiration result into a Vec<Decimal> and sums via d_sum. This keeps the existing signature, surfaces the first overflowing leg by operation tag, and avoids fold-based boilerplate at 12 call sites.
#[from] DecimalError cascade. PositionError and VolatilityError gain a new DecimalError(#[from] DecimalError) variant (#[error(transparent)]). PricingError, GreeksError, and ChainError already had manual From impls that map DecimalError into their existing variants; those were left intact to keep the public wire format stable for downstream serialisation.
Operation tags. Every checked helper call passes a &'static str like "pricing::black_scholes::call::forward" or "chains::delta_exposure::call". The tag is embedded in DecimalError::Overflow so the failing call site is visible without a stack trace — useful for production log forensics.
Positive stays on its crate. Positive arithmetic already routes through Decimal::checked_* inside the positive crate, so (bid + ask) / Positive::TWO in chains::optiondata and every Positive::new_decimal(...).unwrap_or(ZERO) idiom is already compliant and untouched.

Testing

Unit tests added/updated (DecimalError::Overflow constructor, d_add/d_sub/d_mul/d_div/d_sum overflow cases + banker's rounding)
Property-based tests added/updated (not applicable - checked helpers are thin wrappers)
Integration tests added/updated (existing per-module tests cover the call sites)
Benchmark tests added/updated (no perf regression expected - checked arithmetic on modern x86)
Manual testing performed (cargo test --lib - 3751 passed, 0 failed, 5 ignored)

Checklist

Code follows .windsurfrules
All public items have /// documentation (helpers documented with # Errors, division with rounding note)
No warnings from cargo clippy --all-targets --all-features -- -D warnings
cargo fmt --all --check passes
No .unwrap(), .expect(), or panics in library code (existing invariants preserved)
Uses rust_decimal for monetary calculations
Minimal dependencies — no new crates added

Closes #335

…/d_div helpers Introduces a dedicated Overflow variant on DecimalError that captures the offending operation tag and both operands so a failure inside a monetary arithmetic path is self-describing. The existing #[from] DecimalError cascade in PricingError / PositionError / StrategyError / VolatilityError / GreeksError / ChainError means every downstream module receives the new variant without any per-module churn. Adds four crate-private helpers in model/decimal.rs: - d_add / d_sub / d_mul wrap Decimal::checked_add/sub/mul and convert the None arm into DecimalError::Overflow. - d_div wraps Decimal::checked_div and re-rounds with RoundingStrategy::MidpointNearestEven at Decimal::MAX_SCALE, so every subsequent monetary division inherits bankers rounding uniformly. Division by zero surfaces DecimalError::ArithmeticError (same path as the previous raw / operator). Tagged #[allow(dead_code)] while the follow-up commits wire these helpers into pricing / greeks / position / strategy / chain kernels; the allows get removed (or stay as explicit documentation of intent) in the final rustdoc sweep. Also adds ten unit tests for the helpers (happy path, overflow tag propagation, division-by-zero, banker's-rounding round-trip) and two tests for the DecimalError::Overflow variant itself. cargo test --lib passes (3748 tests, zero regressions). No public signatures change. Preparation for the monetary-flow sweep in the follow-up commits. Refs #335

… helpers Applies the M5/issue-335 contract to the eleven in-scope pricing kernels. Every site where a user-visible option price or intrinsic payoff is fused from strike, underlying, discount and rebate components now goes through d_add / d_sub / d_mul from model::decimal, so overflow surfaces a DecimalError::Overflow tagged with the pricing kernel and leg name instead of wrapping silently. Converted sites (scope: final price composition, intrinsic value, zero-vol deterministic branches, discount \u00d7 payoff aggregation): - pricing/black_scholes_model.rs: calculate_call_option_price and calculate_put_option_price discounted-underlying \u00b7 N(d1) minus discounted-strike \u00b7 N(d2) composition and their associated qt/rt intermediates. - pricing/american.rs: BAW intrinsic value at t = 0, zero-volatility deterministic pricing, immediate-exercise branch, and the european_price + early_exercise_premium fusion for both call and put. BAW internal algorithm parameters (m, n, q1, q2, k_factor, critical price iteration) stay on raw Decimal \u2014 they are not monetary flows. - pricing/binary.rs: cash-or-nothing and asset-or-nothing zero-vol branches and closed-form N-weighted price composition, plus the gap-binary asset_unsigned - strike\u00b7unit_cash subtraction. - pricing/barrier.rs: all eight barrier-type branches that fuse the f_a..f_f closure returns into the final price routed through d_add/d_sub behind a shared tag. Inner closure arithmetic is left on raw operators because it is numerical-kernel internals. - pricing/lookback.rs: Goldman-Sosin-Gatto floating-strike three-term composition (call and put branches) and Conze fixed-strike BS + lookback-premium composition. - pricing/compound.rs: Geske four-way call-on-call / call-on-put / put-on-call / put-on-put composition, plus intrinsic at t1 = 0 and zero-vol deterministic branches. - pricing/chooser.rs: Rubinstein simple chooser four-leg composition, expiry-straddle call + put composition, and both zero-time-and-zero-vol intrinsic branches. - pricing/cliquet.rs: per-period value , total-across-reset-dates accumulation, s_prev_pv \u00d7 period_val projection, and the unit BS-call ITM / zero-vol / regular branches. - pricing/telegraph.rs: terminal composition. - pricing/utils.rs: binomial option_node_value (probability-weighted up/down expectation times discount) and calculate_discounted_payoff (discount \u00d7 terminal payoff). - pricing/monte_carlo.rs: per-simulation payoff and the final aggregation (mean now applies banker's rounding via d_div's default scale). What stays on raw Decimal by design: - d1 / d2 numerators, sigma\u00b7sqrt(T), exp(-rT) and exp(-qT) intermediate exponents, bivariate CDF helpers, Newton-Raphson critical-price search. These are numerical kernel internals and remain the subject of issues #336 (NaN/Inf guards) and out-of-scope here. - Positive arithmetic (, ) already routes through Decimal::checked_* since positive@0.5. Tests: cargo test --lib passes (3748 tests, zero regressions); no assertions were relaxed. cargo clippy --lib --all-features reports zero warnings. Each kernel was re-tested individually after its conversion. Refs #335

… helpers Applies the M5/issue-335 contract to the monetary-scaling tail of each Greek in greeks/equations.rs, greeks/numerical.rs and greeks/utils.rs. Every divide-by-annualisation (theta/365, charm/365, color/365), divide-by-basis-point (rho/100, rho_d/100) and finite difference quotient now routes through d_div, and every per-position weighting (greek * quantity) now routes through d_mul. This means overflow or division-by-zero surfaces a DecimalError tagged with the Greek name and leg name instead of wrapping silently or producing a bogus NaN. Converted sites: - greeks/equations.rs: theta(): (theta * quantity) / 365 -> d_mul + d_div rho(): (rho * quantity) / 100 -> d_mul + d_div rho_d(): (rhod * quantity) / 100 -> d_mul + d_div charm(): (charm * quantity) / 365 -> d_mul + d_div color(): (numerator) / 365 -> d_div (The Black-Scholes analytical d1/d2 and the numerator buildup before the scaling division stay on raw Decimal - they are pure numerical machinery.) - greeks/numerical.rs: numerical_delta / numerical_vega / numerical_rho: the central finite difference numerator (p_plus - p_minus) and denominator (2*H) now go through d_sub and d_div. numerical_gamma: the three-point Laplacian numerator (p_plus - 2*p + p_minus) and denominator (H*H) now use d_sub + d_add + d_div. H is the compile-time constant 0.01 so division-by-zero is structurally impossible; the helper is still chosen for uniform bankers rounding and consistent overflow tagging. - greeks/utils.rs: calculate_delta_neutral_sizes(): the delta-neutral sizing formula size1 = -total_size * delta2 / (delta1 - delta2) now routes through d_mul + d_sub + d_div. The existing checked_div path in d1() (line ~132) is already compliant and untouched. Positive arithmetic (Positive + Positive etc.) stays as-is because it already routes through Decimal::checked_* since positive@0.5. Tests: cargo test --lib passes (3748 tests, zero regressions). The greeks::equations, greeks::numerical and greeks::utils modules were verified individually after each conversion. cargo clippy --lib --all-features reports zero warnings. Refs #335

…ecked d_* helpers Converts the two monetary P&L entry points on `Position` to the checked helpers so overflow surfaces as a typed `DecimalError` rather than wrapping silently in a production pathological-input scenario. - `Position::pnl_at_expiration`: the three-term composition `intrinsic_value - total_cost + premium_received` now routes through `d_sub` and `d_add` with `position::pnl_at_expiration::*` operation tags. The intrinsic value is still produced by the existing payoff evaluation (already `Result`-typed). - `Position::unrealized_pnl`: the per-contract chain `price - premium - open_fee - close_fee` (Long) and the mirrored Short chain now route through three sequential `d_sub` calls, and the subsequent `* quantity` scaling routes through `d_mul`. Support change in the error layer: - `PositionError::DecimalError(#[from] crate::error::DecimalError)` added to complete the `#[from] DecimalError` cascade required by the helpers. `PricingError`, `OptionsError`, `GreeksError`, `SimulationError`, and the unified `Error` already had it; `PositionError` was the missing link. Variant is `transparent` so the wire format / Display output is unchanged for callers. What stays as raw Decimal by design: - `Trade::net()` = `income().to_dec() - cost().to_dec()`. Both sides are non-negative `Positive` values so the subtraction lives in `[-Decimal::MAX, Decimal::MAX]`, i.e. overflow is structurally impossible. Converting would force a breaking API change from `Decimal` to `Result<Decimal, DecimalError>` on every caller for zero runtime benefit. - `Trade::cost()` / `Trade::income()` are already `Positive * Positive` and `Positive + Positive`, both of which route through `Decimal::checked_*` inside the `positive` crate since v0.5. Tests: cargo test --lib passes (3748 tests, zero regressions); model::position suite re-run individually (98 tests green). cargo clippy --lib --all-features reports zero warnings. Refs #335

Every `Profit::calculate_profit_at` implementation sums two to four `pnl_at_expiration` leg values into a single Decimal and returns it as the user-visible P&L at a given price. The atomic leg values are already checked since commit 4, but the final sum was still raw `Decimal + Decimal [+ Decimal [+ Decimal]]`, so an unbounded strategy size could in principle overflow the aggregate silently. This commit routes every multi-leg aggregation through a new crate-private `d_sum` helper backed by `Decimal::checked_add`, tagging each aggregation with a `strategies::<kind>::profit_at` operation id. Changes: - `model/decimal.rs`: new `d_sum(values: &[Decimal], op) -> Result<Decimal, DecimalError>` helper. Returns `Decimal::ZERO` on empty input and stops on the first checked_add failure. Three unit tests covering happy path, empty input, and `[Decimal::MAX, Decimal::MAX]` overflow. - `strategies/{bull,bear}_{call,put}_spread.rs` (4 files), `strategies/{long,short}_{straddle,strangle}.rs` (4 files), `strategies/{long,short}_butterfly_spread.rs` (2 files), `strategies/{iron_condor,iron_butterfly}.rs` (2 files), `strategies/{call_butterfly,poor_mans_covered_call}.rs` (2 files): each `calculate_profit_at` now returns `d_sum(&[leg1.pnl?, leg2.pnl?, ...], "tag")?`. - `strategies/custom.rs`: the `try_fold(ZERO, |acc, pnl| acc + pnl?)` accumulator is now materialised into a `Vec<Decimal>` (leg P&L still early-returns on any leg error) and summed with `d_sum`, so the final aggregation crosses the same Overflow boundary as the fixed-leg strategies. What stays untouched by design: - Single-leg strategies (long_call, long_put, short_call, short_put) already just forward the single `pnl_at_expiration` return value. There is nothing to aggregate, so no helper is needed. - `get_max_profit` / `get_max_loss` implementations in each strategy just call `calculate_profit_at(strike)` and test the sign, so they inherit the checked semantics automatically. - Break-even points are precomputed `Vec<Positive>` values stored on the strategy at construction time and use only `Positive` arithmetic (already checked via positive@0.5). Tests: cargo test --lib passes (3751 tests, three new unit tests on d_sum, zero regressions). cargo clippy --lib --all-features reports zero warnings. Refs #335

…ugh checked d_* helpers Applies the M5/issue-335 contract to the remaining in-scope monetary-flow sites in `src/volatility/utils.rs` and `src/chains/chain.rs`. volatility/utils.rs: - `constant_volatility`: the sample mean and sample variance now aggregate the returns slice with `d_sum`, project through `d_div` with bankers rounding at scale 28, so an overflow surfaces a tagged `VolatilityError::DecimalError(Overflow)` instead of wrapping silently on `Decimal::MAX`-class inputs. - `ewma_volatility`: the per-step EWMA variance recursion `v_new = lambda * v_prev + (1 - lambda) * r^2` is now split into three checked steps via `d_mul`, `d_sub`, `d_mul`, `d_add`. The existing `variance.sqrt()` + `Positive::new_decimal(...)` chain stays intact. - `garch_volatility`: the GARCH(1, 1) variance recursion `v_new = omega + alpha * r^2 + beta * v_prev` is now three checked products and two checked sums, then projected back onto `Positive::new_decimal(...).unwrap_or(ZERO)` (semantics preserved). chains/chain.rs: - `OptionChain::{gamma, delta, vega, theta, vanna, vomma, veta, charm, color}_exposure`: the accumulator loop for each of the nine aggregate exposures now uses `d_add` with a per-greek / per-leg operation tag (`chains::gamma_exposure`, `chains::delta_exposure::call`, etc.). A chain holding near-MAX Greek magnitudes can no longer silently overflow the aggregate. Error cascade: - `VolatilityError::DecimalError(#[from] DecimalError)` added so the helpers can `?`-propagate out of `constant_volatility`, `ewma_volatility`, and `garch_volatility`. Variant is `#[error(transparent)]` so the wire format is unchanged. - `ChainError` already has a manual `From<DecimalError> -> OptionDataError::PriceCalculationError` impl from earlier work, so no new variant is needed there; the existing mapping keeps the surface stable. What stays untouched: - `(bid + ask) / Positive::TWO` mid-price calculations in `chains/optiondata.rs` operate on `Positive + Positive` and `Positive / Positive`, both of which already route through `Decimal::checked_*` inside the `positive` crate. - `simulate_heston_volatility` stays on raw Decimal - its `v = v + drift*dt + xi*sqrt(v)*dw` update is a classic Heston SDE discretisation (numerical-kernel territory, covered by issue #336). Only the variance-clamp-to-zero path is on the monetary boundary and it is already `v.max(Decimal::ZERO)`. Tests: cargo test --lib passes (3751 tests, zero regressions). The volatility::utils and chains::chain test suites were re-run individually after each conversion. cargo clippy --lib --all-features reports zero warnings. Refs #335

Every checked helper introduced in M5/issue-335 (`d_add`, `d_sub`, `d_mul`, `d_div`, `d_sum`, `DIV_DEFAULT_SCALE`) is now exercised by the monetary-flow call sites landed in the previous six commits: - pricing kernels (Black-Scholes, American, binary, barrier, lookback, chooser, cliquet, binomial, Monte Carlo, telegraph); - Greeks equations, numerical finite differences, and delta-neutral sizing in `greeks::utils`; - `Position::pnl_at_expiration` and `Position::unrealized_pnl`; - every multi-leg strategy `Profit::calculate_profit_at` aggregation (spreads, butterflies, condors, straddle / strangle, PMCC, custom); - chain `*_exposure` aggregates and volatility variance recursions (`constant_volatility`, `ewma_volatility`, `garch_volatility`). The `#[allow(dead_code)]` shims were only ever needed while landing the helpers ahead of their callers, so removing them tightens the public surface review and makes sure clippy will flag any future helper that falls out of use. No behavioural change. Tests: cargo test --lib passes (3751 / 5 ignored / 0 regressions). cargo clippy --lib --all-features reports zero warnings. Refs #335

codecov · 2026-04-19T16:52:16Z

Codecov Report

❌ Patch coverage is 83.01486% with 80 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/pricing/compound.rs	45.31%	35 Missing ⚠️
src/pricing/chooser.rs	68.88%	14 Missing ⚠️
src/greeks/numerical.rs	50.00%	10 Missing ⚠️
src/pricing/barrier.rs	64.28%	10 Missing ⚠️
src/pricing/cliquet.rs	82.60%	4 Missing ⚠️
src/pricing/american.rs	85.00%	3 Missing ⚠️
src/pricing/binary.rs	86.66%	2 Missing ⚠️
src/pricing/black_scholes_model.rs	96.15%	1 Missing ⚠️
src/volatility/utils.rs	96.55%	1 Missing ⚠️

Files with missing lines	Coverage Δ
src/chains/chain.rs	`85.40% <100.00%> (+0.05%)`	⬆️
src/error/decimal.rs	`86.66% <100.00%> (+0.95%)`	⬆️
src/error/position.rs	`87.09% <ø> (ø)`
src/error/volatility.rs	`100.00% <ø> (ø)`
src/greeks/equations.rs	`98.95% <100.00%> (+0.05%)`	⬆️
src/greeks/utils.rs	`84.78% <100.00%> (+0.33%)`	⬆️
src/model/decimal.rs	`96.15% <100.00%> (+3.05%)`	⬆️
src/model/position.rs	`82.24% <100.00%> (+0.43%)`	⬆️
src/pricing/lookback.rs	`78.80% <100.00%> (+2.68%)`	⬆️
src/pricing/monte_carlo.rs	`75.00% <100.00%> (+1.82%)`	⬆️
... and 26 more

... 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.

Copilot

Pull request overview

This PR audits monetary Decimal arithmetic across pricing, Greeks, positions, strategies, chains, and volatility modules by routing operations through new crate-private checked helpers, surfacing overflow/div-by-zero/rounding behavior as typed errors instead of silent saturation.

Changes:

Introduces checked Decimal helpers (d_add, d_sub, d_mul, d_div, d_sum) and a new DecimalError::Overflow { operation, lhs, rhs } error surface.
Updates pricing/greeks/position/strategy/chain/volatility computations to use checked helpers with operation tags for better production forensics.
Cascades DecimalError via #[from] into PositionError and VolatilityError and adds unit tests for helper behavior and overflow cases.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
src/model/decimal.rs	Adds checked helpers + default division rounding policy and tests.
src/error/decimal.rs	Adds `DecimalError::Overflow` variant and constructor + tests.
src/error/position.rs	Adds transparent `DecimalError` variant for propagation.
src/error/volatility.rs	Adds transparent `DecimalError` variant for propagation.
src/model/position.rs	Converts P&L compositions to checked helper sequences.
src/strategies/bull_call_spread.rs	Uses `d_sum` for leg aggregation.
src/strategies/bull_put_spread.rs	Uses `d_sum` for leg aggregation.
src/strategies/bear_call_spread.rs	Uses `d_sum` for leg aggregation.
src/strategies/bear_put_spread.rs	Uses `d_sum` for leg aggregation.
src/strategies/long_straddle.rs	Uses `d_sum` for leg aggregation.
src/strategies/short_straddle.rs	Uses `d_sum` for leg aggregation.
src/strategies/long_strangle.rs	Uses `d_sum` for leg aggregation.
src/strategies/short_strangle.rs	Uses `d_sum` for leg aggregation.
src/strategies/call_butterfly.rs	Uses `d_sum` for leg aggregation.
src/strategies/long_butterfly_spread.rs	Uses `d_sum` for leg aggregation.
src/strategies/short_butterfly_spread.rs	Uses `d_sum` for leg aggregation.
src/strategies/iron_condor.rs	Uses `d_sum` for leg aggregation.
src/strategies/iron_butterfly.rs	Uses `d_sum` for leg aggregation.
src/strategies/poor_mans_covered_call.rs	Uses `d_sum` for leg aggregation.
src/strategies/custom.rs	Collects leg P&Ls then uses `d_sum` for aggregation.
src/pricing/utils.rs	Uses checked helpers for binomial node value and discounted payoff.
src/pricing/black_scholes_model.rs	Rewrites discounting and price legs with checked helpers.
src/pricing/american.rs	Uses checked helpers in intrinsic/zero-vol and premium composition.
src/pricing/binary.rs	Uses checked helpers in monetary compositions for binary pricing.
src/pricing/barrier.rs	Uses checked add/sub for final barrier price composition.
src/pricing/lookback.rs	Uses checked helpers for final lookback price compositions.
src/pricing/chooser.rs	Uses checked helpers for chooser intrinsic/zero-vol/final composition.
src/pricing/cliquet.rs	Uses checked helpers for period composition and unit-call legs.
src/pricing/compound.rs	Uses checked helpers for intrinsic/zero-vol and final Geske composition.
src/pricing/telegraph.rs	Uses checked helper for final payoff discounting.
src/pricing/monte_carlo.rs	Uses checked helpers for payoff subtraction, mean, and discounting.
src/greeks/utils.rs	Uses checked helpers in delta-neutral sizing.
src/greeks/numerical.rs	Uses checked helpers for finite-difference numerators/denominators.
src/greeks/equations.rs	Uses checked helpers for scaling (theta/rho/charm/color).
src/chains/chain.rs	Uses checked addition for chain exposure aggregates.
src/volatility/utils.rs	Uses checked helpers in constant/ewma/garch volatility computations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T17:01:38Z

+            &[
+                self.short_call.pnl_at_expiration(price)?,
+                self.short_put.pnl_at_expiration(price)?,
+            ],


calculate_profit_at calls pnl_at_expiration multiple times in the trace! (and again in d_sum), which duplicates work and can yield inconsistent results if pricing has side effects or intermittent errors. Store the two leg P&Ls in locals once, use them for both logging and summation, and then pass the locals into d_sum.

Good catch. Both short_strangle and short_straddle now cache call_pnl / put_pnl once, feed them into d_sum to get total, and reuse the three locals in the trace! line.

Copilot · 2026-04-19T17:01:38Z

    let denominator = sigma * tau.sqrt();
    let factor2 = (Decimal::TWO * q * tau) + Decimal::ONE + ((numerator / denominator) * d1);
-    let color = (-exp_minus_qt * factor1 * factor2 * option.quantity) / Decimal::from(365);
+    let numerator = -exp_minus_qt * factor1 * factor2 * option.quantity;


color now uses d_div, but the numerator is still built with raw multiplications (-exp_minus_qt * factor1 * factor2 * option.quantity), which can silently saturate on overflow before the division. Consider chaining d_mul calls (with operation tags) for the numerator so overflow is surfaced as DecimalError::Overflow consistently.

Suggested change

let numerator = -exp_minus_qt * factor1 * factor2 * option.quantity;

let numerator = d_mul(-exp_minus_qt, factor1, "greeks::color::numerator_exp_factor1")?;

let numerator = d_mul(numerator, factor2, "greeks::color::numerator_factor2")?;

let numerator = d_mul(

numerator,

option.quantity,

"greeks::color::numerator_quantity",

)?;

Fixed, color numerator now routes through three chained d_mul calls with distinct operation tags before the final d_div(.. / 365).

Copilot · 2026-04-19T17:01:39Z


-    Ok((-q * t).exp() * n1 - k * (-r * t).exp() * n2)
+    let s_leg = d_mul((-q * t).exp(), n1, "pricing::cliquet::unit_call::s_leg")?;
+    let k_leg = d_mul(k * (-r * t).exp(), n2, "pricing::cliquet::unit_call::k_leg")?;


k_leg is computed as d_mul(k * (-r * t).exp(), n2, ...) which still performs an unchecked multiplication (k * exp(...)) before the checked helper runs. To avoid silent saturation, compute the discounted strike with d_mul(k, (-r * t).exp(), ...) (and similarly consider checking other discounting multiplies in this function).

Suggested change

let k_leg = d_mul(k * (-r * t).exp(), n2, "pricing::cliquet::unit_call::k_leg")?;

let discounted_k = d_mul(

k,

(-r * t).exp(),

"pricing::cliquet::unit_call::discounted_k",

)?;

let k_leg = d_mul(discounted_k, n2, "pricing::cliquet::unit_call::k_leg")?;

Done, discounted_k is now built with d_mul(k, exp(-r*t), ..) before being folded with n2 in a second checked multiplication.

Copilot · 2026-04-19T17:01:39Z

+        let leg_s = s.to_dec() * dividend_discount_t2 * m1;
+        let leg_k2 = k2.to_dec() * discount_t2 * m2;
+        let leg_k1 = k1.to_dec() * discount_t1 * n_d2_t1;
+        let step = d_sub(leg_s, leg_k2, "pricing::compound::call_call::step")?;
+        d_sub(step, leg_k1, "pricing::compound::call_call::price")?


The Geske final-price legs (leg_s, leg_k2, leg_k1, etc.) are built with raw * chains (e.g. s * dividend_discount_t2 * m1) before being combined with d_add/d_sub. If any of those intermediate multiplications overflow, they can silently saturate and the later checked add/sub won’t detect it. Consider computing each leg with d_mul steps (tagged) so overflow is surfaced at the correct call site.

Fixed across all four Geske branches using a shared build_leg(base, discount, weight, ..) closure that performs two d_mul calls with distinct operation tags before the final d_add / d_sub.

Copilot · 2026-04-19T17:01:39Z

+    /// The operands are captured verbatim for post-mortem debugging; they
+    /// are never re-emitted through the public API surface because
+    /// `DecimalError` already reaches every domain module through the


The docs for DecimalError::Overflow say the operands are “never re-emitted through the public API surface”, but the #[error(...)] display string includes both lhs and rhs and the error is propagated broadly via #[from]. Consider rewording the comment to match the actual behavior (or adjust the display if hiding operands is intended).

Suggested change

/// The operands are captured verbatim for post-mortem debugging; they

/// are never re-emitted through the public API surface because

/// `DecimalError` already reaches every domain module through the

/// The operands are captured verbatim for post-mortem debugging and may

/// also appear in formatted error output, since this public

/// `DecimalError` variant is propagated across domain modules through the

You're right, reworded the doc to match reality. Operands are part of the #[error] display template and reach downstream modules via the #[from] cascade, and the note now points callers to match-and-reformat if they need redaction.

Copilot · 2026-04-19T17:01:41Z

+        let pnls: Vec<Decimal> = self
+            .positions
            .iter()
            .map(|position| position.pnl_at_expiration(&price))
-            .try_fold(Decimal::ZERO, |acc, pnl| Ok(acc + pnl?))
+            .collect::<Result<Vec<_>, _>>()?;
+        d_sum(&pnls, "strategies::custom::profit_at").map_err(PricingError::from)


calculate_profit_at allocates a Vec<Decimal> just to sum P&Ls. Since this runs in hot paths (strategy valuation), consider avoiding the allocation by folding with d_add directly (or adding an iterator-based d_sum_iter) so you keep checked semantics without extra memory traffic.

Fair point, tracked in #372 as a follow-up refactor. Adding d_sum_iter cleanly needs a new helper signature and touches the other fixed-slice d_sum call sites, so I'd rather land that independently rather than expand this PR.

Copilot · 2026-04-19T17:01:41Z


    let payoff = option.payoff_at_price(&price)?;
-    let result = payoff * (-option.risk_free_rate * option.time_to_expiration()?).exp();
+    let discount = (-option.risk_free_rate * option.time_to_expiration()?).exp();


discount is computed with -option.risk_free_rate * option.time_to_expiration()? using the raw * operator, so an overflow can silently saturate before .exp() runs. For consistency with the checked-arithmetic approach in the rest of the pricing module, consider computing the exponent argument via d_mul(-option.risk_free_rate, option.time_to_expiration()?.to_dec(), ...) (or similar) before exponentiation.

Suggested change

let discount = (-option.risk_free_rate * option.time_to_expiration()?).exp();

let discount_exponent = d_mul(

-option.risk_free_rate,

option.time_to_expiration()?.to_dec(),

"pricing::telegraph::discount_exponent",

)?;

let discount = discount_exponent.exp();

Applied the suggested patch, discount_exponent = d_mul(-risk_free_rate, time_to_expiration()?.to_dec(), ..) is now checked before .exp().

Copilot · 2026-04-19T17:01:41Z

+    let total = d_sum(returns, "volatility::constant::total")?;
+    let mean = d_div(total, n.to_dec(), "volatility::constant::mean")?;
+    let centred_sq: Vec<Decimal> = returns.iter().map(|&r| (r - mean).powi(2)).collect();
+    let sq_total = d_sum(&centred_sq, "volatility::constant::sq_total")?;
+    let variance = d_div(


centred_sq is computed via raw (r - mean).powi(2), which bypasses the new checked helpers and can still silently saturate on overflow. Consider using d_sub for centering and d_mul(centered, centered, ...) (or a checked square helper), and accumulate directly without allocating a Vec so overflow is reported consistently with the rest of this PR.

Done, the centred deviations now use d_sub, each square is d_mul(centred, centred, ..) instead of .powi(2), and the sum is folded inline with d_add so the temporary Vec is gone too.

Copilot · 2026-04-19T17:01:41Z

+        // EWMA variance recursion: v' = λ·v + (1 - λ) · r².
+        let persistent = d_mul(lambda, variance, "volatility::ewma::persistent")?;
+        let innovation_weight = d_sub(Decimal::ONE, lambda, "volatility::ewma::innovation_weight")?;
+        let innovation = d_mul(
+            innovation_weight,
+            return_value.powi(2),
+            "volatility::ewma::innovation",
+        )?;
+        variance = d_add(persistent, innovation, "volatility::ewma::variance")?;


EWMA uses return_value.powi(2) inside a checked multiplication, but powi itself can overflow/saturate before d_mul sees the value. To preserve the “checked monetary arithmetic” guarantee, compute r² via d_mul(return_value, return_value, ...) (and ideally do the same for the initial variance seed) so overflow surfaces as DecimalError::Overflow.

Fixed, both the initial variance seed and the per-step innovation now build r^2 via d_mul(r, r, ..) instead of .powi(2), so a saturating squaring cannot feed a silently capped value into the EWMA recursion.

Copilot · 2026-04-19T17:01:42Z

+        let alpha_r2 = d_mul(alpha, return_value.powi(2), "volatility::garch::alpha_r2")?;
+        let beta_v = d_mul(beta, variance.to_dec(), "volatility::garch::beta_v")?;
+        let persistent = d_add(omega, alpha_r2, "volatility::garch::omega_plus_alpha")?;
+        let next_variance = d_add(persistent, beta_v, "volatility::garch::variance")?;
+        variance = Positive::new_decimal(next_variance).unwrap_or(Positive::ZERO);
        volatilities.push(variance.sqrt());


garch_volatility now propagates DecimalError via d_mul/d_add, but the final Positive::new_decimal(next_variance).unwrap_or(Positive::ZERO) still silently clamps invalid/negative variance to zero. Consider returning the PositiveError (or mapping it to VolatilityError::NumericalFailure) so bad GARCH parameters don’t get hidden behind a zero-vol result.

Good catch. The silent unwrap_or(Positive::ZERO) is replaced with a closure that maps the rejection to VolatilityError::NumericalFailure { reason } including the offending variance value, and both initial seed and per-step r^2 now go through d_mul. The # Errors section of garch_volatility is rewritten to document both the overflow and negative-variance paths.

Follow-up to the review on #371 where the reviewer observed that several checked-arithmetic call sites still fed their inputs through raw `*` chains. That left the door open for an intermediate product to saturate silently before the outer `d_mul` / `d_div` / `d_add` ever ran. This commit extends every flagged site so the full monetary chain routes through `d_mul` and surfaces overflow as a tagged `DecimalError`. - `greeks::equations::color`: the numerator `-exp(-q*tau) * factor1 * factor2 * quantity` is now built with three chained `d_mul` calls before the final `d_div(.. / 365)`. `quantity` is projected with `.to_dec()` to match the `Decimal` signature of `d_mul`. - `greeks::numerical::numerical_gamma`: `2 * p` is computed via `d_mul` and the `H * H` denominator likewise, so the full `(p_plus - 2*p + p_minus) / H^2` finite-difference numerator and denominator are both checked. - `pricing::cliquet::unit_call`: the discounted strike is now built as `d_mul(k, exp(-r*t), "discounted_k")` before being folded with `n2`, preserving the original semantics but closing the saturation window on `k * exp(-r*t)`. - `pricing::compound::compound_black_scholes`: each of the four Geske branches (call-on-call, call-on-put, put-on-call, put-on-put) now builds its three legs through a shared local closure `build_leg(base, discount, weight, ..)`, which performs two `d_mul` calls with distinct operation tags (`..leg_<x>_discounted` / `..leg_<x>`). The final `d_add`/`d_sub` composition is unchanged. - `pricing::lookback::fixed_strike_lookback`: both call and put branches now compute `s_discounted` / `k_discounted` through `d_mul` before the CDF weight is applied. Floating-strike branch was not flagged and stays on raw `*` for now; the same treatment can be extended there in a follow-up if needed. - `pricing::chooser::chooser_simple`: each of the four Rubinstein legs (`leg_s_t`, `leg_k_t`, `leg_k_choice`, `leg_s_choice`) is now built with two `d_mul` calls. - `pricing::utils::calculate_discounted_payoff`: the binomial-payoff discount exponent `-int_rate * expiry` is now wrapped in `d_mul` before `.exp()`, and `expiry` is projected with `.to_dec()` to match the helper signature. - `pricing::telegraph::telegraph`: the Monte-Carlo discount exponent `-risk_free_rate * time_to_expiration` is now wrapped in `d_mul` before `.exp()`. All conversions keep the function signatures, error types, and propagation paths unchanged. `cargo test --lib` passes (3751 / 5 ignored / 0 regressions). `cargo clippy --lib --all-features` reports zero warnings. Refs #335, PR #371

Addresses three inline review comments on #371 about silent saturation paths remaining in the volatility statistics helpers. - `constant_volatility`: * centred deviations now use `d_sub(r, mean, ..)`; * the square is built via `d_mul(centred, centred, ..)` instead of the unchecked `.powi(2)` which silently saturates on overflow; * the sum of squares is folded inline with `d_add` so the intermediate `Vec<Decimal>` allocation is gone. - `ewma_volatility`: * the initial variance seed `first_return^2` is built via `d_mul` for the same reason; * the per-step `r^2` innovation is likewise pre-built with `d_mul` and then folded into the checked `(1 - λ) · r² + λ · v_prev` recursion. - `garch_volatility`: * the initial variance seed `r_0^2` is built via `d_mul`; * the per-step `r^2` is likewise built via `d_mul` before entering `α · r²`; * the silent `Positive::new_decimal(..).unwrap_or(Positive::ZERO)` clamp on negative variance is replaced with a helper closure that returns `VolatilityError::NumericalFailure { reason: .. }`, so pathological GARCH parameters now surface a readable diagnostic with the offending variance value instead of being hidden behind a zero-vol result; * the `# Errors` section of `garch_volatility` is rewritten to describe both the new `DecimalError::Overflow` propagation path and the `NumericalFailure` path, replacing the previous "currently infallible" note. No public signatures change. `cargo test --lib` passes (3751 / 5 ignored / 0 regressions). `cargo clippy --lib --all-features` reports zero warnings. Refs #335, PR #371

Two small correctness / docs fixes surfaced in the review on #371. - `strategies::short_strangle::calculate_profit_at` and `strategies::short_straddle::calculate_profit_at` were calling `pnl_at_expiration` three times per leg: twice inside the `trace!` argument list and once inside `d_sum`. Besides the wasted work (`pnl_at_expiration` builds several `Position` clones and re-runs premium and fee arithmetic), it meant the `trace!` snapshot and the returned total could differ if any leg had intermittent errors or side-effecting dependencies. Both implementations now compute `call_pnl` / `put_pnl` once, feed them into `d_sum` to get `total`, and use the three cached locals in the `trace!` line and the `Ok` return. - `DecimalError::Overflow` docs were reworded to match the actual behaviour: the operands are part of the `#[error]` display template and therefore visible in any formatted error string that reaches downstream modules via the `#[from]` cascade. The previous note claimed the operands were "never re-emitted through the public API surface", which is incorrect. The doc now explicitly points callers that need to redact operand values towards a custom `Display` / match-and-reformat path. No functional API change. `cargo test --lib` passes (3751 / 5 ignored / 0 regressions). `cargo clippy --lib --all-features` reports zero warnings. `make pre-push` is clean. Refs #335, PR #371

joaquinbejar · 2026-04-19T17:27:12Z

Thanks for the thorough pass. Pushed three commits addressing all 14 comments: one round for the raw * chains in pricing and Greeks, one for the remaining .powi(2) and silent clamp paths in volatility, and one small fix for the strategy trace! double-calls plus the DecimalError::Overflow doc rewording. The d_sum_iter suggestion is tracked in #372 so it can land as a focused follow-up.

joaquinbejar added 7 commits April 19, 2026 17:47

joaquinbejar added this to the M5 — Arithmetic Discipline milestone Apr 19, 2026

joaquinbejar added refactor Code refactoring without changing behavior priority-high Critical issues that should be addressed first pricing Related to options pricing labels Apr 19, 2026

joaquinbejar self-assigned this Apr 19, 2026

joaquinbejar requested a review from Copilot April 19, 2026 16:55

Copilot started reviewing on behalf of joaquinbejar April 19, 2026 16:56 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

joaquinbejar added 3 commits April 19, 2026 19:14

joaquinbejar mentioned this pull request Apr 19, 2026

Add iterator-based d_sum_iter helper and eliminate Vec allocation in custom::calculate_profit_at #372

Closed

10 tasks

joaquinbejar merged commit 021de52 into main Apr 19, 2026
11 checks passed

-    let numerator = -exp_minus_qt * factor1 * factor2 * option.quantity;
+    let numerator = d_mul(-exp_minus_qt, factor1, "greeks::color::numerator_exp_factor1")?;
+    let numerator = d_mul(numerator, factor2, "greeks::color::numerator_factor2")?;
+    let numerator = d_mul(
+        numerator,
+        option.quantity,
+        "greeks::color::numerator_quantity",
+    )?;

-    let k_leg = d_mul(k * (-r * t).exp(), n2, "pricing::cliquet::unit_call::k_leg")?;
+    let discounted_k = d_mul(
+        k,
+        (-r * t).exp(),
+        "pricing::cliquet::unit_call::discounted_k",
+    )?;
+    let k_leg = d_mul(discounted_k, n2, "pricing::cliquet::unit_call::k_leg")?;

-    let discount = (-option.risk_free_rate * option.time_to_expiration()?).exp();
+    let discount_exponent = d_mul(
+        -option.risk_free_rate,
+        option.time_to_expiration()?.to_dec(),
+        "pricing::telegraph::discount_exponent",
+    )?;
+    let discount = discount_exponent.exp();

Conversation

joaquinbejar commented Apr 19, 2026

Summary

Changes

Technical Decisions

Testing

Checklist

Uh oh!

codecov Bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joaquinbejar commented Apr 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Apr 19, 2026 •

edited

Loading