fix(coinmetrics): filter invalid assets in lwba ws transport

[FionnL]

Summary

• Added invalidBaseAssets caching and filtering to the CoinMetrics LWBA WebSocket transport.
• Prevents unsupported assets (e.g., OHMV2) from causing infinite reconnect loops.
• Added unit tests for URL generation and message handling.

Test plan

• Unit tests pass (yarn jest packages/sources/coinmetrics/test/unit/lwba-ws.test.ts).
• Verified locally against live CoinMetrics API (valid and invalid pairs).

Made with Cursor

[changeset-bot]

🦋 Changeset detected

Latest commit: 68cd74c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@chainlink/coinmetrics-adapter	Patch
@chainlink/token-allocation-adapter	Patch
@chainlink/bsol-price-adapter	Patch
@chainlink/crypto-volatility-index-adapter	Patch
@chainlink/savax-price-adapter	Patch
@chainlink/xsushi-price-adapter	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

This PR updates the CoinMetrics LWBA WebSocket transport to cache and filter out unsupported base assets so they don’t repeatedly trigger websocket errors/reconnect loops.

Changes:

• Added a shared invalidBaseAssets cache and filtered invalid bases out during LWBA WS URL generation.
• Extended LWBA WS message handling to detect bad_parameter errors and record the unsupported asset.
• Added unit tests for LWBA WS URL generation and message handling, plus a changeset for a patch release.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/sources/coinmetrics/src/transport/lwba.ts	Adds invalid-asset caching/filtering and parses bad_parameter errors to populate the cache.
packages/sources/coinmetrics/test/unit/lwba-ws.test.ts	Adds Jest unit tests covering URL generation and message handling for LWBA WS.
.changeset/fix-coinmetrics-lwba-invalid-assets.md	Publishes a patch changeset for the adapter update.

---

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

[danwilliams-cll]

🤖 Automated review. This was generated by an experimental AI code-review tool (ea-review + a Chainlink EA PR-review skill) and has not been vetted by a human. Treat it as a first pass — verify each finding before acting, and expect some false positives. Not a substitute for maintainer review.

---

Good instinct — an infinite reconnect loop from one unsupported asset is a real problem worth fixing. But as written the fix has a crash path and, more importantly, may not actually fix the loop depending on the API's casing.

Blocking

1. Crash on a malformed bad_parameter message — src/transport/lwba.ts (~L96)

const matches = [...message.error.message.matchAll(findBaseCurrenciesRegex)]
if (matches && !invalidBaseAssets.includes(matches[1][1])) {

matches is an array, so matches && … is always true — a no-op guard. If the error message has fewer than two quoted substrings, matches[1] is undefined and matches[1][1] throws TypeError inside the WS message handler. (Copilot flagged this too.) Guard on shape instead: if (matches.length >= 2 && !invalidBaseAssets.includes(asset)), reading the value via matches[1]?.[1].

2. Pushed asset isn't lowercased — the fix may silently no-op — src/transport/lwba.ts (~L97)

invalidBaseAssets.push(matches[1][1])          // raw case from the API message
// …
desiredSubs.filter(({ base }) => !invalidBaseAssets.includes(base.toLowerCase()))  // compares lowercase

The filter compares against base.toLowerCase(), but the stored value keeps whatever case the API used. The test uses lowercase 'ohmv2' so it passes — but if CoinMetrics returns 'OHMV2', the stored value never matches, the asset is never filtered, and the reconnect loop persists in production. Fix: .push(matches[1][1].toLowerCase()).

3. Fragile positional parse — src/transport/lwba.ts (~L94–97)
The code assumes the asset is always the 2nd quoted token (matches[1]). That only holds for today's exact string. If the message is ever reworded/reordered (Value 'x' … for parameter 'assets'), it caches the wrong token and never filters the real asset. Anchor the parse to the value rather than a positional index, and add a test pinning the message shape.

Non-blocking (worth addressing)

4. Module-level mutable global with no TTL — src/transport/lwba.ts (L15). export const invalidBaseAssets: string[] = [] is process-global, shared across all connections, and never cleared. A requested feed is permanently suppressed until process restart — even if the rejection was transient, or CoinMetrics later adds the asset — backed only by an error log. Recommend: (a) logger.warn when an asset is added, for visibility/alerting; (b) a TTL/expiry so it retries rather than suppressing forever.

5. No empty-assets guard (~L70). If every desired sub gets filtered out, assets becomes '' and the transport opens a WS connection with ?assets=. Guard the empty case.

6. Test gaps (each would have caught a blocker above).

• A bad_parameter message with only one quoted value → proves it doesn't throw (covers Add linting Github action #1; also Copilot's suggestion).
• A bad_parameter with a non-lowercase asset (Value 'OHMV2') asserting invalidBaseAssets contains 'ohmv2' → covers Testing #2.

7. Minor. Compile the regex once at module scope, not per message; comment typo (bad_paramter); inconsistent access style (message['error']['type'] vs message.error.message); desiredSubs = desiredSubs.filter(...) reassigns the parameter.

Not an issue

Changeset is fine — one changeset, patch for a fix; the 6-package bump is normal dependency propagation, not an error.

Verdict: Request changes

#1 (crash) and especially #2 (fix silently ineffective on non-lowercase input) are blocking; #3/#4 are the difference between a band-aid and a durable fix.

[danwilliams-cll]

Automated review — refactor is a net improvement over main (old matches[1][1] code crashed on a malformed single-quote message and mis-stored the param name on the Value 'X' ... for parameter 'assets' word order; new regex is format-independent and lowercases to match the filter). Two low-severity notes inline.

[danwilliams-cll]

Re-review after the latest push. The three blockers from the previous review have all been addressed by the refactor to a shared getUnsupportedAssetFromBadParameterError helper — good work. Remaining items are non-blocking.

Resolved since last review

• Crash on malformed bad_parameter message — fixed. Old matches[1][1] threw TypeError on a message with <2 quoted tokens; helper now returns undefined via .match(...)?.[1], and a "malformed bad parameter error does not throw" test pins it.
• Pushed asset isn't lowercased — fixed. Helper now .toLowerCase()s the value, matching the base.toLowerCase() filter; an uppercase-'OHMV2' test covers it.
• Fragile positional parse — fixed. Regex /\bValue\s+'([^']+)'/i anchors to the value token instead of a positional index, so a reworded message (Value 'x' … for parameter 'assets') still parses correctly.

Non-blocking (worth addressing)

1. Helper assumes every bad_parameter is about an asset — src/transport/error-handling.ts (~L31). A bad_parameter about a different param (e.g. metrics) extracts that token (referenceratexyz) and pushes it into invalidBaseAssets; the base-filter never matches it, so the offending param is never dropped and the reconnect loop persists while the cache accumulates junk. Mitigant: metrics are built as ReferenceRate${quote.toUpperCase()} from VALID_QUOTES, so a bad-metric error is unlikely by construction. Consider gating on the param name ('assets') in the message.

2. No recovery for the all-invalid case — src/transport/price-ws.ts (~L78), same shape in lwba.ts (~L83). If every desired sub's base is cached invalid, the URL is emitted with assets='' (and metrics=''). CoinMetrics rejects empty assets with a bad_parameter that has no Value '...' token → helper returns undefined → nothing cached → reconnect with the same empty URL → loop resumes. Only a logger.warn guards it. Rare (needs all bases invalid), but it's a gap against the PR's stated goal.

3. Module-level mutable global with no TTL — invalidBaseAssets. Process-global, shared across all connections, never cleared. A requested feed is suppressed until process restart even if the rejection was transient or CoinMetrics later re-adds the asset. The added logger.warn on insert covers visibility; a TTL/expiry so it retries rather than suppressing forever would be more robust. (Non-blocking — acceptable for the immediate fix.)

Not an issue

Changeset is fine — one changeset, patch for a fix.

Verdict: Approve (non-blocking nits)

All previously-blocking issues are resolved. Items 1–3 are robustness improvements, not correctness blockers on the common path — safe to merge; addressing them (esp. #1) would harden against edge cases.