From cd2a6d861ff80d2582bd398d557f2c84a0f541ad Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:47:51 +0200 Subject: [PATCH] adf934c6 - fix(custody): carry the saving position and its interest through the value history (#4451) * fix(custody): carry the saving position and its interest through the 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. * style: apply prettier formatting * fix(custody): stop reporting interest for a closed saving position 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. * refactor(custody): hold the dust bound in one place 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. --- src/shared/models/asset/asset.service.ts | 26 ++ .../__tests__/custody.service.spec.ts | 214 ++++++++++++++- .../core/custody/services/custody.service.ts | 247 ++++++++++++++++-- 3 files changed, 462 insertions(+), 25 deletions(-) diff --git a/src/shared/models/asset/asset.service.ts b/src/shared/models/asset/asset.service.ts index 31d222630e..3f9e8c7981 100644 --- a/src/shared/models/asset/asset.service.ts +++ b/src/shared/models/asset/asset.service.ts @@ -61,6 +61,32 @@ export class AssetService { return this.assetRepo.findCachedBy(`${ids}`, { id: In(ids) }); } + /** + * Assets sharing a price rule, i.e. assets that are priced identically by definition. Used to + * value an asset on days it has no `asset_price` row of its own: a position introduced later + * (`Ethereum/sZCHF`, added long after the holdings it represents) has no price history before + * its creation date, while the asset it is pegged to has the full series. Reading the peer's + * price is not an estimate — the shared price rule is what defines both prices. + * + * The price rule travels with each asset so callers can map a rule to its members without a + * second round trip. Not cached: unlike the id lookups above, this is used on a read path + * that must reflect a newly added asset immediately. + */ + async getAssetsByPriceRules(priceRuleIds: number[]): Promise { + if (!priceRuleIds.length) return []; + + return this.assetRepo.find({ + where: { priceRule: { id: In(priceRuleIds) } }, + relations: { priceRule: true }, + }); + } + + async getAssetsByIdWith(ids: number[], relations: FindOptionsRelations): Promise { + if (!ids.length) return []; + + return this.assetRepo.find({ where: { id: In(ids) }, relations }); + } + async getAssetByChainId(blockchain: Blockchain, chainId: string): Promise { return this.assetRepo.findOneCachedBy(`${blockchain}-${chainId}`, { blockchain, chainId }); } diff --git a/src/subdomains/core/custody/services/__tests__/custody.service.spec.ts b/src/subdomains/core/custody/services/__tests__/custody.service.spec.ts index 91388311b2..f1c77244d5 100644 --- a/src/subdomains/core/custody/services/__tests__/custody.service.spec.ts +++ b/src/subdomains/core/custody/services/__tests__/custody.service.spec.ts @@ -27,6 +27,7 @@ describe('CustodyService', () => { let custodyOrderRepo: DeepMocked; let custodyBalanceRepo: DeepMocked; let assetPricesService: DeepMocked; + let assetService: DeepMocked; const asset = createCustomAsset({ id: 42, name: 'BTC' }); const custodyUser = createCustomUser({ id: 7, role: UserRole.CUSTODY }); @@ -39,6 +40,12 @@ describe('CustodyService', () => { custodyOrderRepo = createMock(); custodyBalanceRepo = createMock(); assetPricesService = createMock(); + assetService = createMock(); + + // No shared price rule unless a test sets one up: the substitute lookup then contributes + // nothing and each asset is valued from its own series, as before. + assetService.getAssetsByIdWith.mockResolvedValue([]); + assetService.getAssetsByPriceRules.mockResolvedValue([]); service = new CustodyService( createMock(), @@ -49,7 +56,7 @@ describe('CustodyService', () => { custodyOrderRepo, custodyBalanceRepo, assetPricesService, - createMock(), + assetService, ); userDataService.getUserData.mockResolvedValue( @@ -158,6 +165,169 @@ describe('CustodyService', () => { usd: balance * higherIdPrice.priceUsd, }); }); + + it('values a holding from a price-rule peer on days its own asset has no price row', async () => { + // The asset held has no series at all — the situation of an asset created long after the + // holdings it represents. Before this was fixed the position silently vanished from the + // series and the whole day was valued as if it were not held. + const peer = createCustomAsset({ id: 43, name: 'ZCHF', uniqueName: 'Ethereum/ZCHF' }); + const balance = 1000; + + custodyOrderRepo.find.mockResolvedValue([depositOrder(new Date('2025-11-01T08:00:00.000Z'), balance)]); + assetPricesService.getAssetPrices.mockResolvedValue([ + Object.assign(new AssetPrice(), { + id: 1, + asset: peer, + created: new Date('2025-11-01T09:00:00.000Z'), + priceChf: 1, + priceEur: 0.9, + priceUsd: 1.1, + }), + ]); + + const rule = { id: 5 } as any; + assetService.getAssetsByIdWith.mockResolvedValue([ + Object.assign(createCustomAsset({ id: asset.id }), { priceRule: rule }), + ]); + assetService.getAssetsByPriceRules.mockResolvedValue([ + Object.assign(createCustomAsset({ id: asset.id }), { priceRule: rule }), + Object.assign(peer, { priceRule: rule }), + ]); + + const result = await service.getUserCustodyHistory(accountId); + + expect(result.totalValue).toHaveLength(1); + expect(result.totalValue[0].value).toEqual({ chf: 1000, eur: 900, usd: 1100 }); + }); + + it('carries accrued interest of the saving position through the series', async () => { + const savingAsset = createCustomAsset({ id: 60, name: 'sZCHF', uniqueName: Config.custody.savingAsset }); + const deposit = 99500; + const start = new Date('2026-01-28T00:00:00.000Z'); + + custodyOrderRepo.find.mockResolvedValue([ + Object.assign(new CustodyOrder(), { + id: 1, + type: CustodyOrderType.DEPOSIT, + status: CustodyOrderStatus.COMPLETED, + inputAmount: deposit, + inputAsset: savingAsset, + user: custodyUser, + updated: start, + completedAt: start, + }), + ]); + + const price = (id: number, created: Date): AssetPrice => + Object.assign(new AssetPrice(), { id, asset: savingAsset, created, priceChf: 1, priceEur: 1, priceUsd: 1 }); + + assetPricesService.getAssetPrices.mockResolvedValue([ + price(1, new Date('2026-01-28T01:00:00.000Z')), + price(2, new Date('2026-07-28T01:00:00.000Z')), + ]); + + const result = await service.getUserCustodyHistory(accountId); + + expect(result.totalValue).toHaveLength(2); + // Day of the deposit: nothing has accrued yet. + expect(result.totalValue[0].value.chf).toBeCloseTo(deposit, 2); + // Six months on, the position is worth the deposit plus its interest — the figure the + // customer sees in the balance, so the chart no longer trails it. + expect(result.totalValue[1].value.chf).toBeCloseTo(deposit + 1726.94, 2); + }); + + it('stops carrying interest once the saving position is fully paid out', async () => { + // Tranches accrue with their own sign, so a closed position leaves a frozen remainder — + // the interest earned while it was held. That figure is never booked and never paid out, + // so a Safe that holds nothing must not keep showing it for months on end. + const savingAsset = createCustomAsset({ id: 60, name: 'sZCHF', uniqueName: Config.custody.savingAsset }); + const amount = 99500; + const depositedAt = new Date('2026-01-28T00:00:00.000Z'); + const withdrawnAt = new Date('2026-02-27T00:00:00.000Z'); + + custodyOrderRepo.find.mockResolvedValue([ + Object.assign(new CustodyOrder(), { + id: 1, + type: CustodyOrderType.DEPOSIT, + status: CustodyOrderStatus.COMPLETED, + inputAmount: amount, + inputAsset: savingAsset, + user: custodyUser, + updated: depositedAt, + completedAt: depositedAt, + }), + Object.assign(new CustodyOrder(), { + id: 2, + type: CustodyOrderType.WITHDRAWAL, + status: CustodyOrderStatus.COMPLETED, + outputAmount: amount, + outputAsset: savingAsset, + user: custodyUser, + updated: withdrawnAt, + completedAt: withdrawnAt, + }), + ]); + + const price = (id: number, created: Date): AssetPrice => + Object.assign(new AssetPrice(), { id, asset: savingAsset, created, priceChf: 1, priceEur: 1, priceUsd: 1 }); + + assetPricesService.getAssetPrices.mockResolvedValue([ + price(1, new Date('2026-01-28T01:00:00.000Z')), + price(2, new Date('2026-02-27T01:00:00.000Z')), + price(3, new Date('2026-07-28T01:00:00.000Z')), + ]); + + const result = await service.getUserCustodyHistory(accountId); + + expect(result.totalValue).toHaveLength(3); + // Day of the payout and five months later: nothing is held, so nothing is worth anything. + expect(result.totalValue[1].value.chf).toBe(0); + expect(result.totalValue[2].value.chf).toBe(0); + }); + + it('values the saving position from a price-rule peer and still accrues its interest', async () => { + // The production case in one test: sZCHF bears interest AND has no price series of its own + // before the day it was created, so both mechanisms have to work on the same asset at once. + const savingAsset = createCustomAsset({ id: 60, name: 'sZCHF', uniqueName: Config.custody.savingAsset }); + const peer = createCustomAsset({ id: 61, name: 'ZCHF', uniqueName: 'Ethereum/ZCHF' }); + const deposit = 99500; + const start = new Date('2026-01-28T00:00:00.000Z'); + + custodyOrderRepo.find.mockResolvedValue([ + Object.assign(new CustodyOrder(), { + id: 1, + type: CustodyOrderType.DEPOSIT, + status: CustodyOrderStatus.COMPLETED, + inputAmount: deposit, + inputAsset: savingAsset, + user: custodyUser, + updated: start, + completedAt: start, + }), + ]); + + // Only the peer has a series — the saving asset itself has none at all. + const peerPrice = (id: number, created: Date): AssetPrice => + Object.assign(new AssetPrice(), { id, asset: peer, created, priceChf: 1, priceEur: 1, priceUsd: 1 }); + + assetPricesService.getAssetPrices.mockResolvedValue([ + peerPrice(1, new Date('2026-01-28T01:00:00.000Z')), + peerPrice(2, new Date('2026-07-28T01:00:00.000Z')), + ]); + + const rule = { id: 5 } as any; + assetService.getAssetsByIdWith.mockResolvedValue([Object.assign(savingAsset, { priceRule: rule })]); + assetService.getAssetsByPriceRules.mockResolvedValue([ + Object.assign(savingAsset, { priceRule: rule }), + Object.assign(peer, { priceRule: rule }), + ]); + + const result = await service.getUserCustodyHistory(accountId); + + expect(result.totalValue).toHaveLength(2); + expect(result.totalValue[0].value.chf).toBeCloseTo(deposit, 2); + expect(result.totalValue[1].value.chf).toBeCloseTo(deposit + 1726.94, 2); + }); }); describe('getUserCustodyBalance', () => { @@ -167,7 +337,7 @@ describe('CustodyService', () => { await expect(service.getUserCustodyBalance(accountId)).rejects.toThrow(NotFoundException); }); - it('attaches interest and interestValue only to the saving position; totalValue stays unchanged', async () => { + it('attaches interest and interestValue only to the saving position and counts them in totalValue', async () => { jest.useFakeTimers().setSystemTime(new Date('2026-07-28T00:00:00.000Z')); const savingAsset = createCustomAsset({ @@ -222,10 +392,42 @@ describe('CustodyService', () => { expect(btcDto.interest).toBeUndefined(); expect(btcDto.interestValue).toBeUndefined(); - // totalValue must reflect only booked balances (curr.value), never the accrued interest — - // getUserCustodyHistory() has no notion of interest, so including it here would make the - // displayed figure jump against the history chart. - expect(result.totalValue).toEqual({ chf: 6000, eur: 5600, usd: 6500 }); + // Accrued interest counts towards the total: it is what the position is worth today, and + // getUserCustodyHistory() now accrues it per day too, so the figure no longer disagrees + // with the chart. Booked balances alone would be 6000 / 5600 / 6500. + expect(result.totalValue).toEqual({ + chf: expect.closeTo(7726.94, 2), + eur: expect.closeTo(7326.94, 2), + usd: expect.closeTo(8226.94, 2), + }); + }); + + it('reports no interest for a saving position that is fully paid out', async () => { + jest.useFakeTimers().setSystemTime(new Date('2026-07-28T00:00:00.000Z')); + + const savingAsset = createCustomAsset({ + id: 60, + name: 'sZCHF', + uniqueName: Config.custody.savingAsset, + approxPriceChf: 1, + approxPriceEur: 1, + approxPriceUsd: 1, + }); + + // The balance row survives a full payout with balance 0 — it is never deleted. + custodyBalanceRepo.findBy.mockResolvedValue([ + Object.assign(new CustodyBalance(), { asset: savingAsset, balance: 0, user: custodyUser }), + ]); + + const result = await service.getUserCustodyBalance(accountId); + + const szchfDto = result.balances.find((b) => b.asset.name === 'sZCHF'); + + expect(szchfDto.interest).toBeUndefined(); + expect(szchfDto.interestValue).toBeUndefined(); + expect(result.totalValue).toEqual({ chf: 0, eur: 0, usd: 0 }); + // The interest calculation is not even reached for a closed position. + expect(custodyOrderRepo.find).not.toHaveBeenCalled(); }); it('keeps all positions with correct value, drops interest, and logs when it throws', async () => { diff --git a/src/subdomains/core/custody/services/custody.service.ts b/src/subdomains/core/custody/services/custody.service.ts index ac6e93edd8..da9226e21c 100644 --- a/src/subdomains/core/custody/services/custody.service.ts +++ b/src/subdomains/core/custody/services/custody.service.ts @@ -17,7 +17,13 @@ import { In, IsNull, Not } from 'typeorm'; import { RefService } from '../../referral/process/ref.service'; import { CustodySignupDto } from '../dto/input/custody-signup.dto'; import { CustodyAuthDto } from '../dto/output/custody-auth.dto'; -import { CustodyBalanceDto, CustodyHistoryDto, CustodyHistoryEntryDto } from '../dto/output/custody-balance.dto'; +import { + CustodyAssetBalanceDto, + CustodyBalanceDto, + CustodyFiatValueDto, + CustodyHistoryDto, + CustodyHistoryEntryDto, +} from '../dto/output/custody-balance.dto'; import { CustodyBalance } from '../entities/custody-balance.entity'; import { CustodyOrder } from '../entities/custody-order.entity'; import { CustodyOrderStatus } from '../enums/custody'; @@ -40,6 +46,17 @@ interface DailyFiatValue { export class CustodyService { private readonly logger = new DfxLogger(CustodyService); + /** + * Below this, a balance is no holding. Balances are plain floating point sums over `float` + * columns, so a closed position rarely lands on exact zero — a deposit and its matching + * withdrawal can leave a residue like 1e-16. Comparing against exact zero would let that dust + * pass for a holding: it would revive the interest of a position that is in fact closed, and + * report it as unpriced on days without a price. The bound is the eight decimals a balance is + * ever displayed with (CustodyAssetBalanceDtoMapper), so nothing visible is discarded — even + * for the priciest custody asset it is worth far less than a rounded rappen. + */ + private static readonly BALANCE_DUST = 1e-8; + constructor( private readonly userService: UserService, @Inject(forwardRef(() => UserDataService)) private readonly userDataService: UserDataService, @@ -94,9 +111,16 @@ export class CustodyService { const custodyUserIds = account.users.filter((u) => u.role === UserRole.CUSTODY).map((u) => u.id); const custodyBalances = await this.custodyBalanceRepo.findBy({ user: { id: In(custodyUserIds) } }); - const savingBalance = custodyBalances.find((b) => b.asset.uniqueName === Config.custody.savingAsset); + // Summed over every row of the position, not just the first: one Safe can hold it across + // several custody users, and the guard below has to see the whole position. Interest is only + // reported while it is actually open — see withAccruedInterest() for why a closed position + // would otherwise keep showing the frozen remainder of what it once earned. + const savingBalances = custodyBalances.filter((b) => b.asset.uniqueName === Config.custody.savingAsset); + const savingPrincipal = Util.sumObjValue(savingBalances, 'balance'); + const savingBalance = savingBalances[0]; + const interestByAssetName = new Map(); - if (savingBalance) { + if (savingBalance && savingPrincipal > CustodyService.BALANCE_DUST) { try { // dueDate is a parameter on calculateAccruedInterest for deterministic tests; runtime uses now. const interest = await this.calculateAccruedInterest(custodyUserIds, savingBalance.asset, new Date()); @@ -125,9 +149,20 @@ export class CustodyService { const balances = CustodyAssetBalanceDtoMapper.mapCustodyBalances(custodyBalances, interestByAssetName); - const totalValueInEur = balances.reduce((prev, curr) => prev + curr.value.eur, 0); - const totalValueInChf = balances.reduce((prev, curr) => prev + curr.value.chf, 0); - const totalValueInUsd = balances.reduce((prev, curr) => prev + curr.value.usd, 0); + // Accrued interest counts towards the total. It is what the position is worth today, and + // leaving it out understated the Safe by a figure that grows every day. It was previously + // excluded to keep this total equal to the value history, which did not carry interest — + // getUserCustodyHistory() now accrues it per day, so both sides agree again. + // + // `interestValue` is absent on every position that bears no interest, and deliberately also + // on one whose interest could not be computed (logged above): a broken figure is left out + // of the total rather than guessed at. + const interestValue = (b: CustodyAssetBalanceDto): CustodyFiatValueDto => + b.interestValue ?? { eur: 0, chf: 0, usd: 0 }; + + const totalValueInEur = balances.reduce((prev, curr) => prev + curr.value.eur + interestValue(curr).eur, 0); + const totalValueInChf = balances.reduce((prev, curr) => prev + curr.value.chf + interestValue(curr).chf, 0); + const totalValueInUsd = balances.reduce((prev, curr) => prev + curr.value.usd + interestValue(curr).usd, 0); return { balances, @@ -238,15 +273,36 @@ export class CustodyService { .filter((a) => a); const assets = Array.from(new Map(allAssets.map((a) => [a.id, a])).values()); + // Prices are defined per price rule, but `asset_price` keeps one series per asset. An asset + // introduced long after the holdings it represents (`Ethereum/sZCHF`) therefore has no rows + // before its own creation date. Valuing only the assets that carry a row for a given day + // would drop such a position out of the series entirely and make it appear to spring into + // existence on the day its first price was written. Pulling in the assets that share its + // price rule closes that gap with the peer's series — the same price by definition, not an + // estimate. + const substituteByAsset = await this.getPriceSubstitutes(assets); + const priceAssets = Array.from( + new Map(assets.concat(Array.from(substituteByAsset.values()).flat()).map((a) => [a.id, a])).values(), + ); + // get all prices (by date) const startDate = new Date(custodyOrders[0].updated); startDate.setHours(0, 0, 0, 0); - const prices = await this.assetPricesService.getAssetPrices(assets, startDate); + const prices = await this.assetPricesService.getAssetPrices(priceAssets, startDate); const priceMap = Util.groupByAccessor(prices, (p) => Util.isoDate(p.created)); + // The interest-bearing position grows every day and is part of what the Safe is worth, so + // the series has to carry it — otherwise the chart and the balance disagree by an amount + // that widens daily. + const savingAsset = assets.find((a) => a.uniqueName === Config.custody.savingAsset); + const interestByDay = savingAsset + ? this.accrueInterestByDay(savingAsset, custodyOrders, [...priceMap.keys()], custodyUserIds) + : new Map(); + // process by day: apply order volumes before calculating value const assetBalancesMap = new Map(); + const unpricedAssetIds = new Set(); const totalValue: CustodyHistoryEntryDto[] = []; const sortedOrderDays = [...orderMap.keys()].sort(); let orderDayIndex = 0; @@ -264,8 +320,14 @@ export class CustodyService { orderDayIndex++; } + // Interest as of this day, valued like any other holding of the same asset. It is accrued + // to the start of the day, the instant the day's balances describe; the live balance + // endpoint accrues to now, so the newest point can trail it by up to one day's interest — + // the same class of gap the series already has against the balance's spot price. + const dayBalances = this.withAccruedInterest(assetBalancesMap, savingAsset, interestByDay.get(day)); + // calculate daily portfolio value from current balances and available prices - const dailyValue = this.calculateDailyPortfolioValue(dayPrices, assetBalancesMap); + const dailyValue = this.calculateDailyPortfolioValue(dayPrices, dayBalances, substituteByAsset, unpricedAssetIds); totalValue.push({ date: new Date(day), @@ -277,9 +339,104 @@ export class CustodyService { }); } + // A holding that could not be valued on any day is silently absent from the series — the + // exact failure this path was fixed for, so it must not fail quietly a second time. Report + // it once for the whole request rather than per day. + if (unpricedAssetIds.size) { + this.logger.error( + `Custody history for account ${accountId}: no price series found for asset(s) ` + + `${[...unpricedAssetIds].join(', ')} — those holdings are missing from the value history`, + ); + } + return { totalValue }; } + /** + * Peers that can price an asset on days it has no series of its own, keyed by asset id. Only + * assets that actually share a price rule appear; an asset with no peer is simply absent. + */ + private async getPriceSubstitutes(assets: Asset[]): Promise> { + const assetsWithRule = await this.assetService.getAssetsByIdWith( + assets.map((a) => a.id), + { priceRule: true }, + ); + + const ruleIds = [...new Set(assetsWithRule.map((a) => a.priceRule?.id).filter((id) => id != null))]; + const peers = await this.assetService.getAssetsByPriceRules(ruleIds); + + const peersByRule = Util.groupByAccessor(peers, (a) => a.priceRule.id); + + const substitutes = new Map(); + for (const asset of assetsWithRule) { + const ruleId = asset.priceRule?.id; + if (ruleId == null) continue; + + const others = (peersByRule.get(ruleId) ?? []).filter((p) => p.id !== asset.id); + if (others.length) substitutes.set(asset.id, others); + } + + return substitutes; + } + + /** + * Accrued interest per day of the series. Computed once for all days rather than per day so a + * broken figure is reported a single time: interest is a display add-on, and a data error in + * it must not cost the customer their entire value history — the same trade-off + * getUserCustodyBalance() already makes for the balance response. + */ + private accrueInterestByDay( + savingAsset: Asset, + orders: CustodyOrder[], + days: string[], + userIds: number[], + ): Map { + const interestByDay = new Map(); + + try { + for (const day of days) { + interestByDay.set(day, this.accrueInterest(orders, savingAsset, new Date(day), userIds)); + } + } catch (e) { + this.logger.error( + `Failed to calculate accrued interest for the value history of user(s) ${userIds.join(', ')}, asset ` + + `${savingAsset.uniqueName} — history is served without interest:`, + e, + ); + return new Map(); + } + + return interestByDay; + } + + /** + * The day's balances with accrued interest folded into the interest-bearing position, so it is + * valued by exactly the same code path as every other holding. Returns the input untouched + * when there is nothing to add, which keeps the common case free of a copy. + * + * Only added while the position is actually open. accrueInterest() sums tranches — a deposit + * accrues, a withdrawal accrues negatively from its own value date — so a position that is + * fully paid out leaves a frozen remainder: the interest earned over the time it was held. + * That figure is never booked and never paid out, so carrying it once the position is closed + * would leave a Safe showing a residue forever, for holdings it no longer has. The same guard + * applies to the balance total in getUserCustodyBalance(). + */ + private withAccruedInterest( + balances: Map, + savingAsset: Asset | undefined, + interest: number | undefined, + ): Map { + if (!savingAsset || !interest) return balances; + + const principal = balances.get(savingAsset.id) ?? 0; + if (principal <= CustodyService.BALANCE_DUST) return balances; + + const withInterest = new Map(balances); + withInterest.set(savingAsset.id, principal + interest); + + return withInterest; + } + /** * Accrued simple interest for an interest-bearing custody position. * dueDate is a parameter (not new Date() inside) so the method is deterministically testable. @@ -316,6 +473,19 @@ export class CustodyService { ], }); + return this.accrueInterest(orders, asset, dueDate, userIds); + } + + /** + * The interest calculation itself, over an already-loaded order set. Split out so the value + * history can accrue interest for every day of the series without a database round trip per + * day — it already holds every completed order it needs. + * + * Callers may pass orders that do not touch `asset` at all: the loop below selects by asset + * id, and the NULL-amount checks mirror the `Not(IsNull())` filters of the query above, so a + * wider order set yields the same result as the narrow one. + */ + private accrueInterest(orders: CustodyOrder[], asset: Asset, dueDate: Date, userIds: number[]): number { let interest = 0; const rate = Config.custody.savingInterestRate; @@ -404,7 +574,12 @@ export class CustodyService { * (e.g. local post-midnight). Use the latest price per asset for that day. * On equal `created`, the higher `id` wins (later insert), independent of list order. */ - private calculateDailyPortfolioValue(dayPrices: AssetPrice[], assetBalancesMap: Map): DailyFiatValue { + private calculateDailyPortfolioValue( + dayPrices: AssetPrice[], + assetBalancesMap: Map, + substituteByAsset: Map, + unpricedAssetIds: Set, + ): DailyFiatValue { const latestPriceByAsset = new Map(); for (const price of dayPrices) { @@ -418,16 +593,50 @@ export class CustodyService { } } - return [...latestPriceByAsset.values()].reduce( - (value, price) => { - const balance = assetBalancesMap.get(price.asset.id) ?? 0; - value.chf += balance * price.priceChf; - value.eur += balance * price.priceEur; - value.usd += balance * price.priceUsd; - return value; - }, - { chf: 0, eur: 0, usd: 0 }, - ); + // Driven by the holdings, not by the prices. Iterating the prices instead made a holding + // whose asset had no row for that day vanish from the sum without a trace — which is + // precisely how a position introduced later (Ethereum/sZCHF) went missing from six months + // of history. Every holding is now either valued or recorded as unpriced. + const value = { chf: 0, eur: 0, usd: 0 }; + + for (const [assetId, balance] of assetBalancesMap.entries()) { + // Skips what the customer sees as no holding. Balances are accumulated as plain floating + // point sums, so a fully closed position rarely lands on exact zero — a deposit and its + // matching withdrawal can leave a residue like 1e-16, which is below the eight decimals + // the balance is ever displayed with. Comparing against exact zero would carry that dust + // into the loop and, on a day without a price, report a closed position as an unpriced + // holding. A non-finite balance keeps poisoning the sum exactly as before rather than + // being quietly dropped here — that is a data fault and belongs where it is already + // handled, not hidden behind this loop. + if (Math.abs(balance) < CustodyService.BALANCE_DUST) continue; + + const price = + latestPriceByAsset.get(assetId) ?? this.findSubstitutePrice(assetId, substituteByAsset, latestPriceByAsset); + if (!price) { + unpricedAssetIds.add(assetId); + continue; + } + + value.chf += balance * price.priceChf; + value.eur += balance * price.priceEur; + value.usd += balance * price.priceUsd; + } + + return value; + } + + /** The day's price from an asset sharing the price rule — identical by definition, not inferred. */ + private findSubstitutePrice( + assetId: number, + substituteByAsset: Map, + latestPriceByAsset: Map, + ): AssetPrice | undefined { + for (const peer of substituteByAsset.get(assetId) ?? []) { + const price = latestPriceByAsset.get(peer.id); + if (price) return price; + } + + return undefined; } async getUserTotalBalancesChf(date: Date): Promise> {