adf934c6 - fix(custody): carry the saving position and its interest through the value history - #4451
Conversation
…value history The Safe's value history dropped any holding whose asset had no asset_price row for a given day, and left accrued interest out of both the history and the balance total. Ethereum/sZCHF was created long after the deposits it represents, so it has no price series before its creation date. The daily valuation iterated over the prices and looked up a balance for each, which meant a holding without a row that day contributed nothing and vanished from the series without a trace. In production a position booked in January only appeared in the chart six months later, on the day its first price was written, as a vertical jump. Valuation is now driven by the holdings instead, and an asset without a row of its own is priced from an asset sharing its price rule -- identical by definition, not an estimate. A holding that cannot be priced at all is reported once per request rather than silently skipped. Accrued interest is now part of both figures. It was previously excluded from totalValue to keep it equal to the history, which had no notion of interest; the history accrues it per day now, so both sides agree and the customer sees what the position is actually worth.
Tranches accrue with their own sign, so a fully paid out position leaves a frozen remainder -- the interest it earned while it was held. That figure is never booked and never paid out. Carrying it into totalValue and the value history, as this branch newly does, left a Safe holding nothing showing a residue forever. Interest is now only reported while the position is actually open, in both the balance total and the history. Partial payouts are unaffected: the remaining principal keeps accruing, which the existing negative-tranche test pins down. Also replaces the exact zero-balance comparison in the daily valuation with a tolerance. Balances are plain floating point sums, so a closed position rarely lands on exact zero, and the dust left behind would be reported as an unpriced holding on any day without a price.
The closed-position guards compared against exact zero while the daily valuation used a tolerance, so floating point residue could pass one and not the other -- and reviving the interest of a position that is in fact closed is exactly what the guard exists to prevent. All three now share one documented bound.
|
Three review passes on the full diff, covering conformance and correctness separately. Pass 1 found the one defect this branch introduced: because interest now counts towards the Pass 2 found the closed-position guards comparing against exact zero while the daily valuation Pass 3 raised no blocking issues. The remaining points are tracked in #4456: each is either Also verified against production while reviewing: no completed order pays out |
1 participant
What
The Safe's value history silently dropped any holding whose asset had no
asset_pricerow for agiven day, and accrued interest was part of neither the history nor the balance total.
Why
Ethereum/sZCHFwas created on 2026-07-28, long after the deposits it represents (booked2026-01-28). It therefore has no price series before that date.
calculateDailyPortfolioValue()iterated over the prices and looked up a balance for each — so a holding whose asset carried
no row that day contributed nothing and disappeared from the series without a trace.
In production this is visible: the affected Safe reads 628'115.62 CHF on 27.07. and 727'515.31 CHF
on 28.07. — a vertical jump of exactly the position that had been held since January. For a Safe
whose only holding is the saving position, the entire history reads zero until that day.
Accrued interest had a related gap: it was shown per position but excluded from
totalValue,deliberately, so the number would not disagree with a chart that knew nothing about interest. That
left the total understated by an amount that grows every day.
How
recorded as unpriced; one that cannot be priced at all is logged once per request instead of
being skipped in silence.
sZCHFthat isZCHF— the same price by definition, not an estimate, and no reconstructedprice rows are written. New:
AssetService.getAssetsByPriceRules()/getAssetsByIdWith().it is valued by the same code path as every other holding. It is computed once for all days
(
accrueInterestByDay), so a data error is reported a single time and costs the customer theinterest, not their whole history — the trade-off
getUserCustodyBalance()already makes.totalValuenow includes accrued interest. The reason it was excluded — divergence from thehistory — no longer holds.
a fully paid out position leaves a frozen remainder: the interest earned while it was held. That
figure is never booked and never paid out, so carrying it into the total and the chart would
leave a Safe holding nothing showing a residue forever. Partial payouts are unaffected — the
remaining principal keeps accruing.
BALANCE_DUST) for every "is this still a holding" decision.Balances are plain floating point sums, so a closed position rarely lands on exact zero; exact
comparisons would let that residue revive a closed position's interest or report it as an
unpriced holding.
Trade-offs
describe. The balance endpoint accrues to now, so the newest point can trail it by up to one
day's interest. That is the same class of gap the series already has against the balance's spot
price, and it is recorded at the call site.
accrueInterestByDaydrops the whole interest series when the calculation fails on any day,rather than serving the days computed so far. A partial series would bend the chart downwards at
the point of failure and read as a real loss of value; a consistently interest-free series is the
honest degradation. The failure conditions in
accrueTrancheare day-independent anyway, so apartial failure is the exception, not the rule.
dropped in the new loop: that is a data fault and belongs where it is already handled.
Known limitation (pre-existing, not introduced here)
accrueInterest()sums tranches over the entire order history and is not reset when a position isclosed. If a saving position were fully paid out and later refilled, the interest of the earlier,
closed period would reappear alongside the new one. This PR does not change that calculation — it
only stops reporting interest while a position is closed. The behaviour is unreachable in
production today (no
sZCHFwithdrawal exists, andSAVING_WITHDRAWALhas no wired order path),and whether a reopened position should keep its earlier earnings is a product question rather than
a defect. Tracked for a follow-up.
Tests
months on, matching what the balance reports.
price series of its own, so peer pricing and interest have to work on the same asset at once.
totalValueexpectation now includes interest (the old assertion pinned thebehaviour this PR changes). The failure case is unchanged — a broken interest figure is left out
of the total rather than guessed at.