Skip to content

Commit f80cf3b

Browse files
committed
Stop caching each chain's own asset twice
The pair cache matched entries with a linear scan comparing tokenId with ===. Pairs written without a tokenId key read back as undefined, and undefined === null is false, so every refresh appended a second pair for each chain's own asset. A real cache had 202 pairs where only 137 were distinct, inflating each rate query and pushing it over the 100-pair chunk size into an extra request. Build the pair lists from keyed maps instead, using the same key loadExchangeRateCache de-duplicates with, so a missing tokenId and an explicit null collapse to one pair. Existing duplicates are merged away on the next refresh.
1 parent 173256a commit f80cf3b

3 files changed

Lines changed: 205 additions & 52 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- added: Verbose logging for exchange rate queries: the request body, resolved/rate-less counts, and errors are captured when the Verbose Logging setting is enabled.
66
- added: Exchange-rate cache snapshot in the support log output, plus a `rates-cache-replay` script that re-runs those queries against the rates server and reports the result for each pair.
7+
- fixed: Exchange rate queries no longer request each chain's own asset twice, which had been inflating every rate query with duplicate pairs.
78

89
## 4.50.0 (2026-07-21)
910

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { describe, expect, it } from '@jest/globals'
2+
3+
import {
4+
type ExchangeRateCache,
5+
mergePairCache
6+
} from '../../actions/ExchangeRateActions'
7+
8+
type PairCache = ReturnType<typeof mergePairCache>
9+
10+
const PAIR_EXPIRATION = 5000
11+
const rateEntry = {
12+
current: 1,
13+
yesterday: 1,
14+
yesterdayTimestamp: 0,
15+
expiration: 0
16+
}
17+
18+
const noRates: ExchangeRateCache = { crypto: {}, fiat: {} }
19+
const emptyCache: PairCache = { cryptoPairs: [], fiatPairs: [] }
20+
21+
describe('mergePairCache', () => {
22+
it('collapses a pair stored without a tokenId and one stored with null', () => {
23+
// Older cache entries were written without a `tokenId` key at all, so they
24+
// read back as undefined, while newer ones carry an explicit null. Both
25+
// mean "the chain's own asset".
26+
const previous: PairCache = {
27+
cryptoPairs: [
28+
{
29+
asset: { pluginId: 'bitcoin', tokenId: undefined },
30+
targetFiat: 'iso:USD',
31+
isoDate: undefined,
32+
expiration: 1
33+
},
34+
{
35+
asset: { pluginId: 'bitcoin', tokenId: null },
36+
targetFiat: 'iso:USD',
37+
isoDate: undefined,
38+
expiration: 1
39+
}
40+
],
41+
fiatPairs: []
42+
}
43+
44+
const out = mergePairCache(noRates, previous, PAIR_EXPIRATION)
45+
46+
expect(out.cryptoPairs).toHaveLength(1)
47+
})
48+
49+
it('does not append a duplicate for a chain asset already cached', () => {
50+
const previous: PairCache = {
51+
cryptoPairs: [
52+
// Written with no tokenId key, so it reads back off disk as undefined:
53+
{
54+
asset: { pluginId: 'bitcoin', tokenId: undefined },
55+
targetFiat: 'iso:USD',
56+
isoDate: undefined,
57+
expiration: 1
58+
}
59+
],
60+
fiatPairs: []
61+
}
62+
const rates: ExchangeRateCache = {
63+
crypto: { bitcoin: { '': { 'iso:USD': rateEntry } } },
64+
fiat: {}
65+
}
66+
67+
const out = mergePairCache(rates, previous, PAIR_EXPIRATION)
68+
69+
expect(out.cryptoPairs).toHaveLength(1)
70+
expect(out.cryptoPairs[0].asset.tokenId).toBeNull()
71+
expect(out.cryptoPairs[0].expiration).toBe(PAIR_EXPIRATION)
72+
})
73+
74+
it('stays stable across repeated merges', () => {
75+
const rates: ExchangeRateCache = {
76+
crypto: { bitcoin: { '': { 'iso:USD': rateEntry } } },
77+
fiat: { 'iso:EUR': { 'iso:USD': rateEntry } }
78+
}
79+
80+
let cache = emptyCache
81+
for (let i = 0; i < 5; i++) {
82+
cache = mergePairCache(rates, cache, PAIR_EXPIRATION)
83+
}
84+
85+
expect(cache.cryptoPairs).toHaveLength(1)
86+
expect(cache.fiatPairs).toHaveLength(1)
87+
})
88+
89+
it('keeps tokens separate from their chain asset', () => {
90+
const rates: ExchangeRateCache = {
91+
crypto: {
92+
ethereum: {
93+
'': { 'iso:USD': rateEntry },
94+
a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48: { 'iso:USD': rateEntry }
95+
}
96+
},
97+
fiat: {}
98+
}
99+
100+
const out = mergePairCache(rates, emptyCache, PAIR_EXPIRATION)
101+
102+
const tokenIds = out.cryptoPairs.map(pair => pair.asset.tokenId)
103+
expect(tokenIds).toHaveLength(2)
104+
expect(tokenIds).toContain(null)
105+
expect(tokenIds).toContain('a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48')
106+
})
107+
108+
it('keeps the same asset against different target fiats separate', () => {
109+
const rates: ExchangeRateCache = {
110+
crypto: {
111+
bitcoin: { '': { 'iso:USD': rateEntry, 'iso:EUR': rateEntry } }
112+
},
113+
fiat: {}
114+
}
115+
116+
const out = mergePairCache(rates, emptyCache, PAIR_EXPIRATION)
117+
118+
expect(out.cryptoPairs).toHaveLength(2)
119+
})
120+
})

src/actions/ExchangeRateActions.ts

Lines changed: 84 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -478,58 +478,16 @@ async function fetchExchangeRates(
478478
await Promise.allSettled(promises)
479479

480480
// Merge successful rate responses into the pair cache
481-
const cryptoPairCache = [...(exchangeRateCache?.cryptoPairs ?? [])]
482-
const fiatPairCache = [...(exchangeRateCache?.fiatPairs ?? [])]
483-
for (const [pluginId, tokenObj] of Object.entries(rates.crypto)) {
484-
for (const [tokenId, rateObj] of Object.entries(tokenObj)) {
485-
for (const targetFiat of Object.keys(rateObj)) {
486-
const edgeTokenId = tokenId === '' ? null : tokenId
487-
const cryptoPairIndex = cryptoPairCache.findIndex(
488-
pair =>
489-
pair.asset.pluginId === pluginId &&
490-
pair.asset.tokenId === edgeTokenId &&
491-
pair.targetFiat === targetFiat
492-
)
493-
if (cryptoPairIndex === -1) {
494-
cryptoPairCache.push({
495-
asset: { pluginId, tokenId: edgeTokenId },
496-
targetFiat,
497-
isoDate: undefined,
498-
expiration: pairExpiration
499-
})
500-
} else {
501-
cryptoPairCache[cryptoPairIndex] = {
502-
asset: { pluginId, tokenId: edgeTokenId },
503-
targetFiat,
504-
isoDate: undefined,
505-
expiration: pairExpiration
506-
}
507-
}
508-
}
509-
}
510-
}
511-
for (const [fiatCode, fiatObj] of Object.entries(rates.fiat)) {
512-
for (const targetFiat of Object.keys(fiatObj)) {
513-
const fiatPairIndex = fiatPairCache.findIndex(
514-
pair => pair.fiatCode === fiatCode && pair.targetFiat === targetFiat
515-
)
516-
if (fiatPairIndex === -1) {
517-
fiatPairCache.push({
518-
fiatCode,
519-
targetFiat,
520-
isoDate: undefined,
521-
expiration: pairExpiration
522-
})
523-
} else {
524-
fiatPairCache[fiatPairIndex] = {
525-
fiatCode,
526-
targetFiat,
527-
isoDate: undefined,
528-
expiration: pairExpiration
529-
}
530-
}
531-
}
532-
}
481+
const { cryptoPairs: cryptoPairCache, fiatPairs: fiatPairCache } =
482+
mergePairCache(
483+
rates,
484+
{
485+
cryptoPairs: exchangeRateCache?.cryptoPairs ?? [],
486+
fiatPairs: exchangeRateCache?.fiatPairs ?? []
487+
},
488+
pairExpiration
489+
)
490+
533491
// Update the in-memory cache:
534492
exchangeRateCache = {
535493
rates,
@@ -545,6 +503,80 @@ async function fetchExchangeRates(
545503
})
546504
}
547505

506+
/**
507+
* Key for a crypto pair. A missing `tokenId` and an explicit `null` both mean
508+
* "the chain's own asset", so they must produce the same key — this matches
509+
* how `loadExchangeRateCache` de-duplicates the pairs it reads from disk.
510+
*/
511+
const cryptoPairKey = (
512+
pluginId: string,
513+
tokenId: string | null | undefined,
514+
targetFiat: string
515+
): string => `${pluginId}${tokenId != null ? `_${tokenId}` : ''}_${targetFiat}`
516+
517+
const fiatPairKey = (fiatCode: string, targetFiat: string): string =>
518+
`${fiatCode}_${targetFiat}`
519+
520+
/**
521+
* Merges the assets we just fetched rates for into the subscribed pair lists.
522+
*
523+
* Pairs are keyed rather than matched by a linear scan, so an entry stored
524+
* without a `tokenId` key and one stored with an explicit `null` collapse into
525+
* a single pair. Comparing the two with `===` treated them as different
526+
* assets, which appended a duplicate for every chain's own asset on each
527+
* refresh and inflated later rate queries.
528+
*
529+
* Exported for unit tests.
530+
*/
531+
export function mergePairCache(
532+
rates: ExchangeRateCache,
533+
previous: { cryptoPairs: CryptoFiatPair[]; fiatPairs: FiatFiatPair[] },
534+
pairExpiration: number
535+
): { cryptoPairs: CryptoFiatPair[]; fiatPairs: FiatFiatPair[] } {
536+
const cryptoPairs = new Map<string, CryptoFiatPair>()
537+
for (const pair of previous.cryptoPairs) {
538+
const key = cryptoPairKey(
539+
pair.asset.pluginId,
540+
pair.asset.tokenId,
541+
pair.targetFiat
542+
)
543+
cryptoPairs.set(key, pair)
544+
}
545+
for (const [pluginId, tokenObj] of Object.entries(rates.crypto)) {
546+
for (const [tokenId, rateObj] of Object.entries(tokenObj)) {
547+
for (const targetFiat of Object.keys(rateObj)) {
548+
const edgeTokenId = tokenId === '' ? null : tokenId
549+
cryptoPairs.set(cryptoPairKey(pluginId, edgeTokenId, targetFiat), {
550+
asset: { pluginId, tokenId: edgeTokenId },
551+
targetFiat,
552+
isoDate: undefined,
553+
expiration: pairExpiration
554+
})
555+
}
556+
}
557+
}
558+
559+
const fiatPairs = new Map<string, FiatFiatPair>()
560+
for (const pair of previous.fiatPairs) {
561+
fiatPairs.set(fiatPairKey(pair.fiatCode, pair.targetFiat), pair)
562+
}
563+
for (const [fiatCode, fiatObj] of Object.entries(rates.fiat)) {
564+
for (const targetFiat of Object.keys(fiatObj)) {
565+
fiatPairs.set(fiatPairKey(fiatCode, targetFiat), {
566+
fiatCode,
567+
targetFiat,
568+
isoDate: undefined,
569+
expiration: pairExpiration
570+
})
571+
}
572+
}
573+
574+
return {
575+
cryptoPairs: Array.from(cryptoPairs.values()),
576+
fiatPairs: Array.from(fiatPairs.values())
577+
}
578+
}
579+
548580
const getYesterdayDateRoundDownHour = (now?: Date | number): Date => {
549581
const yesterday = now == null ? new Date() : new Date(now)
550582
yesterday.setMinutes(0)

0 commit comments

Comments
 (0)