Skip to content

feat: add Chainlink historical price source - #42

Merged
matheus1lva merged 12 commits into
mainfrom
matheus1lva/chainlink-historical-source
Aug 27, 2026
Merged

feat: add Chainlink historical price source#42
matheus1lva merged 12 commits into
mainfrom
matheus1lva/chainlink-historical-source

Conversation

@matheus1lva

@matheus1lva matheus1lva commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add a Chainlink historical source. It estimates the block for the requested timestamp, reads latestRoundData() and
    decimals() at that block, scales the answer, and rejects stale or non-positive prices.
  • Add a token-to-aggregator table for 8 chains (1, 10, 100, 137, 146, 8453, 42161, 747474), 42 entries.
  • Register Chainlink at priority 15 inside marketSources, so recursive on-chain pricing can resolve leaf tokens
    through it.

Behavior

  • Prices older than 24h relative to the block timestamp are rejected (src/sources/chainlink/historical.ts:67).
  • A feed that reverts at the estimated block (not deployed yet) returns null, so the next source answers instead of the
    request failing.
  • Transport failures still reject, so a flaky RPC never reads as "no price".
  • Historical reads hit old blocks, so every feed chain needs an archive RPC in RPC_URL_<chainId>. No new env vars.

Priority and ordering

Priority is 15, not the 20 the on-chain source defaults to. The registry stable-sorts ties by registration order, so
sharing 20 would have left the documented chainlink before derived order resting on array position in
createHistoricalSources. Every registered historical source now has a distinct priority: defillama 10, chainlink 15,
on-chain 20, defillama-alias 30.

chainlink also joins SOURCE_PRIORITY in src/types.ts at index 1. That list drives the SQL CASE ranking used
when the source param is omitted, so a stored chainlink row would outrank every source below it. Nothing writes
chainlink rows today.

The ?source= filter matches stored rows only. chainlink and defillama-alias are resolved live and never
persisted, so filtering on either returns 404. docs/routes.md now says so, and lists chainlink at position 2 plus
defillama-alias, which was missing.

Feed table

Every entry was checked against the configured RPC: the token key is a deployed ERC-20, the key is lowercase, and the
aggregator's description() matches the configured symbol. Chains with no verified feed are not registered.

Notable entries:

  • Katana 0x4200…0006 (WETH) points at the ETH/USD aggregator. The KAT/USD feed is not mapped to any token.
  • Base and Katana BTC entries use WBTC/USD aggregators and report the symbol WBTC.
  • Arbitrum has both native and bridged USDC keyed to the USDC/USD feed.

Only ERC-20 keys are in the table. The 0xeee…eee native-asset placeholder is not, since no code path reaches the
source with it: the recursive resolver rewrites native coins to their wrapped address before pricing.

Tests

  • npx tsc --noEmit
  • npx @biomejs/biome check
  • Full npx vitest run: 240 passed, 2 skipped. The skips are the Enso live-API tests, gated behind ENSO_API_KEY.
  • Chainlink tests: 9 passed. They cover decimal scaling, staleness, non-positive answers, unknown tokens, the revert
    fallthrough, transport failures rejecting rather than returning null, that the feed is read at the estimated
    historical block rather than latest, and two table invariants (all keys lowercase, no chain registered with an empty
    map).

Live checks against the corrected table, run outside vitest so real RPCs are reachable:

check result
Ethereum WBTC @ 1617321599 Chainlink $58,801, registry $58,830 via DefiLlama
Katana WETH, recent $2,459.89 (symbol ETH)
Arbitrum WBTC, recent $77,707.45
Arbitrum native USDC, recent $0.99997
Ethereum WBTC @ 1550000000, before the feed existed null, falls through

@matheus1lva
matheus1lva marked this pull request as ready for review August 23, 2026 22:45
- point katana WETH at ETH/USD, drop the KAT feed
- fix the dead arbitrum WBTC key and lowercase native USDC
- collapse the two feed maps into one
- return null when a feed reverts so another source can answer
- assert the feed is read at the estimated historical block
- document the chainlink and defillama-alias sources
…ority

- inline toHistoricalPrice into the source with real types; delete coin.ts
- drop chain-1 0xeee... native key: no code path produces it
- priority 15 so chainlink no longer ties onchain's 20
- test transport failure rejects instead of reading as no price
- docs: source filter only matches stored rows
@matheus1lva

Copy link
Copy Markdown
Collaborator Author

/review-workflow

@github-actions

Copy link
Copy Markdown

Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/32992605377

@github-actions

Copy link
Copy Markdown

gitconfig-mask: sentinel

Summary

Adds a Chainlink on-chain historical price source (feed table for 9 chains + ChainlinkHistoricalSource), wired into the historical registry at priority 15 between DefiLlama and DefiLlama-alias. Docs and priority-order updates are consistent with the new resolver wiring and with how live-only sources bypass DB storage.

Issues

Chainlink source crashes on every real call (high) — the feed's decimals value comes from a live contract read (uint8), which viem decodes as a bigint, but it's used directly as the exponent in 10 ** decimals. Mixing a number base with a bigint exponent throws TypeError: Cannot mix BigInt and other types, use explicit conversions in real JS. This happens outside the revert-tolerant maybe() wrapper, so it isn't swallowed — it propagates out of getHistoricalPrice, making the source unusable against any live RPC and, if no other source succeeds, surfacing a raw TypeError instead of a clean not-found. The prior commit (63c09b9) wrapped this in Number(decimals); the later inlining commit (0ef02f7) dropped the conversion.

  • Change: const price = Number(roundData[1]) / 10 ** Number(decimals) at src/sources/chainlink/historical.ts:71.
  • Done when: getHistoricalPrice succeeds when the fake client returns decimals as a bigint (e.g. 8n), and a test exercises that path — the current test fixtures pass decimals as a plain number literal, which is why this shipped green.

supports() gate has no test coverage (medium)ChainlinkHistoricalSource.supports() (src/sources/chainlink/historical.ts:34) decides whether the source participates for a chain at all, but no test calls it directly; every sibling source (defillama/alias, defillama/historical, enso/spot, onchain/source) has explicit supports() tests.

  • Change: add tests for (a) no feed table configured, (b) feed table present but no RPC client for the chain, (c) both present.
  • Done when: a regression in the hasChainlinkFeeds(chainId) && clientForChain(chainId) !== null logic fails a test.

Suggestions

  • ChainlinkClientForChain and its default-resolution line (src/sources/chainlink/historical.ts:16,31) duplicate ClientForChain and the same fallback idiom already in src/sources/onchain/context.ts / pricer.ts. Reuse the existing type instead of redeclaring it.
  • The 201-line Chainlink feed address table (src/sources/chainlink/feeds.ts) is unverified against live on-chain data — only structural invariants (lowercase keys, non-empty maps) are tested. Not verifiable in CI (no network access); worth a manual cross-check against the Chainlink feed registry before merge.
  • src/registries/historical.ts wiring (chainlink added into marketSources, feeding marketPriceResolver) has no integration test — only the isolated ChainlinkHistoricalSource class and generic registry mechanics are tested.
  • The docs/routes.md claim that chainlink and defillama-alias are "resolved live and never written to storage" was traced and holds (route only live-resolves when no explicit source; nothing writes those source values to the DB), but wasn't independently checked by any lens.

How This Was Reviewed

Reviewed with the review-pr-workflow skill
5 review lenses, each finding independently verified by claude. 0 candidate findings were refuted and dropped.

decimals() returns uint8, decoded as bigint; using it as a number exponent
threw TypeError on every live call. Reuse ClientForChain and cover supports().
@matheus1lva

Copy link
Copy Markdown
Collaborator Author

/review-workflow

@github-actions

Copy link
Copy Markdown

Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/32995119511

@github-actions

Copy link
Copy Markdown

Summary

Adds a new chainlink historical price source (mainnet + 7 L2s) with hardcoded per-chain feed address tables, wires it into the historical registry at priority 15 (between defillama and on-chain-oracle), and documents the new source value and updated priority order in docs/routes.md. Ships with unit tests covering staleness, non-positive answers, decimal scaling, historical-block selection, and revert/transport-failure handling. No new dependencies. Lint/typecheck/tests could not be run in this CI environment (node_modules absent, no network to install) — not verifiable here, not evaluated as pass/fail.

Issues

None confirmed. Five review lenses (spec, bugs, security, deps, clarity) ran with adversarial verification; nothing survived as a confirmed defect.

Suggestions

  • Repeated feed literals (low)src/sources/chainlink/feeds.ts:41 copy-pastes the same { address, symbol } object for every alias token sharing a feed (Optimism USDC/USDC.e, Gnosis DAI, Arbitrum USDC/USDC.e, Katana ETH/WETH).
    • Change: define each shared feed once and reference it from every alias key.
    • Done when: no feed address string appears more than once in the file.
  • Duplicated test mock (low)test/sources/chainlink/historical.test.ts:32 hand-rolls a historicalClient that reimplements the address/function-name revert dispatch already in test/sources/onchain/helpers.ts's fakeClient, just to add block-aware timestamps.
    • Change: extend fakeClient with the block-timestamp/read-tracking hooks instead of maintaining a second mock.
  • Missing wiring coverage (medium) — no test asserts that chainlink actually lands in createHistoricalSources()'s output, at the documented priority, or that it's now reachable from the on-chain adapter's recursive market-price resolver via marketSources. test/registries/sources-wiring.test.ts wasn't updated to cover it, so this real behavior change ships unverified end-to-end.
  • Feed address correctness unverifiable (medium) — the ~35 hardcoded feed addresses across 7 chains (including less-common ones: Gnosis, Sonic, Katana) couldn't be cross-checked against the real Chainlink registry — not verifiable in CI (no network access). A wrong address here fails silently: latestRoundData() would just succeed against whatever contract lives there and return a wrong price. Recommend confirming these against Chainlink's published feed list before merge.

gitconfig-mask: sentinel

Verdict

APPROVE


How This Was Reviewed

Reviewed with the review-pr-workflow skill
5 review lenses, each finding independently verified by claude. 0 candidate findings were refuted and dropped.

@matheus1lva

Copy link
Copy Markdown
Collaborator Author

Addressed review:

  • deduped shared feed literals into named consts (OPTIMISM_USDC, GNOSIS_DAI, ARBITRUM_USDC, KATANA_ETH) — no feed address appears twice
  • added sources-wiring.test.ts case asserting chainlink lands in createHistoricalSources() at priority order defillama, chainlink, defillama-alias, derived and is visible to the child market resolver
  • feed addresses verified on-chain: called description() on all 41 aggregators across the 8 chains; every one returns <SYMBOL> / USD matching the table (incl. Katana USDT 0xF03E1566… = "USDT / USD", which isn't in the docs listing)
  • skipped the test-mock dedupe: extending the shared fakeClient with block-aware timestamps adds coupling to unrelated on-chain tests for no behavior gain

@matheus1lva

Copy link
Copy Markdown
Collaborator Author

/review-workflow

@github-actions

Copy link
Copy Markdown

Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/32997286931

@github-actions

Copy link
Copy Markdown

Summary

Adds a Chainlink-based historical price source: a static per-chain feed registry (src/sources/chainlink/feeds.ts), a getHistoricalPrice implementation that resolves the block nearest a timestamp and reads latestRoundData/decimals with revert- and staleness-tolerant fallthrough (src/sources/chainlink/historical.ts), and wiring into the historical source registry and docs. Lint, typecheck, and tests could not be run in CI (no node_modules, no network access to install — not verifiable in CI).

Issues

None — no findings survived independent verification. One bugs-lens claim (flat 86,400s staleness cutoff routinely rejects healthy 24h-heartbeat feeds) was refuted: the check only trips when a feed is already behind its own contracted heartbeat, and the PR's only staleness test exercises a feed ~55.5h stale, well past ordinary jitter — treated as a false positive, not reported here.

Suggestions

  • getBlock failures silently read as "no price" (low) — a block lookup is bundled into the same revert-tolerant Promise.all as the feed reads, so an RPC hiccup fetching the block is treated like "feed not deployed yet" and quietly returns null instead of retrying.

    • Change: Move client.getBlock({ blockNumber }) out of the maybe()-guarded Promise.all in src/sources/chainlink/historical.ts:53, matching src/sources/onchain/context.ts's loadContractContext, so only the two feed reads stay revert-tolerant.
    • Keep: A reverting feed (not yet deployed) must still return null and fall through to the next source.
    • Done when: a non-retryable getBlock failure surfaces as a failure/retry rather than "no Chainlink price," and a test covers that case distinct from a feed revert.
  • ~40 hardcoded feed addresses across 8 chains unverified against the real registry (high)feeds.ts is only checked structurally (lowercase keys, non-empty maps); not verifiable in CI (no network access). A wrong address either silently returns null or, worse, returns a live price from an unrelated contract sharing the same ABI.

    • Done when: addresses are cross-checked against Chainlink's published feed registry before merge.
  • Historical path only exercised on chain 1 (medium) — none of the 35 feed entries on the other 7 configured chains (10, 100, 137, 146, 8453, 42161, 747474) are run through getHistoricalPrice; only the static table's shape is tested there.

    • Done when: at least one non-mainnet chain has a test exercising clientForChain/estimateBlockByTimestamp/feed lookup end-to-end.
  • New docs claim about source=chainlink/defillama-alias is untested (medium)docs/routes.md now states these sources resolve live and return 404 when filtered on directly, but no route test exercises ?source=chainlink or ?source=defillama-alias.

    • Done when: a route-level test confirms the documented 404 behavior.
  • Bigint-decimals scaling not actually exercised (low) — the one test asserting a concrete scaled price passes decimals as a plain number, not a bigint, so it can't demonstrate the bigint-conversion fix (commit 1d15b8c) is correct; the bigint cases only assert null-safety for zero/negative answers.

    • Done when: a test computes a concrete non-zero scaled price from a genuine bigint decimals value.

Verdict

APPROVE


How This Was Reviewed

Reviewed with the review-pr-workflow skill
5 review lenses, each finding independently verified by claude. 1 candidate finding was refuted and dropped.

gitconfig-mask: sentinel

@matheus1lva

Copy link
Copy Markdown
Collaborator Author

Round 2 addressed:

  • getBlock moved out of the maybe()-guarded Promise.all — only the two feed reads stay revert-tolerant; added a test that a transport failure on the block lookup rejects instead of reading as "no price"
  • added an end-to-end test on a non-mainnet chain (42161, USDC): exercises clientForChain(42161), estimateBlockByTimestamp, feed lookup, and scaling
  • added route tests asserting ?source=chainlink and ?source=defillama-alias return NOT_FOUND, matching the docs claim
  • added a test scaling a genuine bigint decimals to a concrete non-zero price (1.23456789)
  • feed addresses: verified on-chain rather than against docs — description() called on all 41 aggregators across the 8 chains, every one returns <SYMBOL> / USD matching the table

Local: 249 tests pass, tsc --noEmit clean, biome check . clean.

@matheus1lva

Copy link
Copy Markdown
Collaborator Author

/review-workflow

@github-actions

Copy link
Copy Markdown

Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/33000746538

@github-actions

Copy link
Copy Markdown

Summary

Adds a Chainlink on-chain historical price source: feed/proxy address tables (src/sources/chainlink/feeds.ts), a ChainlinkHistoricalSource reader, registry/type/route wiring, and unit tests using synthetic mock clients.

gitconfig-mask: sentinel

Dependencies

No new dependencies (package.json unchanged from base).

Issues

None confirmed by verification.

Suggestions

  • Unverified Chainlink feed addresses (medium)src/sources/chainlink/feeds.ts hardcodes ~40 feed/proxy addresses across 8 chains; nothing in this review or the CI sandbox (no network) could cross-check them against an authoritative source, and the new tests only assert structural shape via mocks, never real addresses.
    • Done when: addresses are diffed against Chainlink's official feed list before merge.
  • Missing stale/carried-over round guard (medium)getHistoricalPrice reads roundId/answeredInRound from latestRoundData but never checks answeredInRound >= roundId or startedAt > 0, the usual Chainlink guard against a carried-over round; only updatedAt staleness is checked, and no test constructs such a round.
    • Done when: the guard is added, or its omission is documented as intentional with a covering test.
  • End-to-end fallback path untested (low) — no test drives the historical route far enough for DefiLlama to miss and Chainlink to actually serve the answer; the added test only exercises the explicit ?source=chainlink filter. docs/routes.md also isn't updated to list chainlink as a fallback source.
    • Done when: an integration test covers the live-fallback path and the docs mention chainlink as a fallback.
  • SOURCE_PRIORITY change reaches stored-row ranking (low) — adding 'chainlink' to SOURCE_PRIORITY in src/types.ts also shifts src/db/queries.ts's CASE-based ranking for persisted rows, even though chainlink prices are meant to never be persisted; no test enforces that invariant.
    • Done when: a test asserts chainlink rows are never written to token_prices.

Verdict

APPROVE


How This Was Reviewed

Reviewed with the review-pr-workflow skill
5 review lenses, each finding independently verified by claude. 0 candidate findings were refuted and dropped.

@murderteeth murderteeth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Adds a chainlink historical source with a 42-entry feed table across 8 chains, registered at priority 15 between defillama and the on-chain source. I independently re-checked the whole table against live RPCs — every token key is a deployed ERC-20, all 38 aggregators are proxies whose description() matches the configured pair at 8 decimals, and the four Katana vault-bridge tokens expose no share-price interface, so pricing them off the underlying is sound.

Issues

  • src/sources/chainlink/historical.ts:49 - RPC credentials reach the logs (high) — when the archive RPC hiccups during the block lookup, the raw transport error escapes untranslated and the request handler logs its message, which embeds the full RPC_URL_<chainId> including the provider API key. The caller also gets a generic 500 where the same fault raised by the on-chain source produces a 503.

    • Done when: a transient RPC failure during the Chainlink block lookup surfaces as the same UNAVAILABLE/503 class the on-chain source produces, and no log line emitted for that request contains the RPC endpoint URL or its query string.
    • Provenance: f05edce
  • src/sources/chainlink/historical.ts:77 - Response names the wrong token (medium) — a Chainlink-served price reports the feed's asset rather than the token that was asked for, so one token key comes back labelled differently depending on which source answered: WPOL as MATIC, vbUSDC as USDC, WETH as ETH. The table disagrees with itself too — the same WBTC address is WBTC on Base and BTC on Sonic. Prices are unaffected; only the label diverges.

    • Done when: for a token whose symbol differs from its feed's base asset (polygon 0x0d50…1270, katana 0x203a…fd36), a Chainlink-served response carries that token's symbol or null rather than the feed's, and a test pins it for at least one such token.
    • Provenance: 0ef02f7

Verdict

REQUEST_CHANGES


How This Was Reviewed

Reviewed with the review-pr-workflow skill
5 review lenses, each finding independently verified by claude. 1 candidate finding was refuted and dropped.

@matheus1lva

Copy link
Copy Markdown
Collaborator Author

/review

@github-actions

Copy link
Copy Markdown

Review started (review-pr): https://github.com/yearn/yearn-prices/actions/runs/33088099862

@github-actions

Copy link
Copy Markdown

Summary

Adds a Chainlink historical price source: a static chain→token→aggregator-proxy feed table (src/sources/chainlink/feeds.ts, 8 chains, 41 token entries), a ChainlinkHistoricalSource that estimates the block for the requested timestamp and reads latestRoundData/decimals at that block, and registration in the historical registry at priority 15 (between defillama at 10 and derived at 20). chainlink is added to SOURCE_PRIORITY so ?source=chainlink validates, and docs/routes.md is updated. Reverting feeds fall through as null; transport failures are wrapped as UNAVAILABLE so the RPC URL never reaches the log.

Checks: bun run lint, bun run typecheck, and bun run test could not be executed — node_modules is absent and there is no network to install, so lint/type/test status is not verifiable in CI. No package.json change, so no npm-policy evaluation applies. Visual verification skipped (no browser; the change is server-side only). The 41 feed addresses in feeds.ts could not be checked against the Chainlink registry, and per-feed heartbeat intervals could not be looked up — not verifiable in CI; both need confirmation against docs.chain.link before merge.

gitconfig-mask: sentinel

Issues

  • docs/routes.md:69 - Documented 404 does not happen on two of the three routes (medium) — The new sentence tells callers that filtering on chainlink or defillama-alias returns 404. That is true only for the single-token route. handleBatchHistorical and handleRangeHistorical also accept source, and both always return 200 with an empty coins object when the filter matches no rows, so a client that codes to this sentence will never see the error it was told to handle. The added test only exercises handleHistorical.

    • Done when: the sentence states the actual response per route family, and a test covers the batch or range route under ?source=chainlink so the documented behaviour is pinned rather than asserted only for the exact route.
    • Provenance: 0ef02f7
  • src/sources/chainlink/historical.ts:84 - Staleness bound sits at the heartbeat, not above it (medium) — The check rejects any round whose updatedAt is more than 86,400s older than the block, and 86,400s is exactly the heartbeat of the slower USD feeds this table configures. A feed that is behaving normally reaches and crosses that age in the window just before its next heartbeat lands, so the source returns null for reasons unrelated to the token, and the fallback it exists to provide disappears intermittently — the caller sees a 404 that looks random rather than a price. Nothing in the tests distinguishes "aged past the heartbeat" from "genuinely stale": the stale case uses a 200,000s gap.

    • Done when: the bound is greater than the longest heartbeat among the feeds in feeds.ts with that maximum recorded next to the constant, and a test covers a round aged just past one heartbeat and asserts it still prices.
    • Provenance: 0ef02f7
  • test/registries/sources-wiring.test.ts:27 - Nothing pins the priority the PR chose (low) — The assertion checks the array createHistoricalSources builds, which is registration order. SourceRegistry sorts by priority before resolving, so the real try-order is defillama, chainlink, derived, defillama-alias — not the order asserted. Changing priority = 15 to any value above 20 would silently move Chainlink below the on-chain source with the whole suite still green, undoing the ordering this PR spent a commit establishing.

    • Done when: a test asserts the registry's resolved try-order (via all() or an equivalent), and it fails if ChainlinkHistoricalSource.priority is moved outside the defillamaderived gap.
    • Provenance: 0ef02f7
  • README.md:49 - Price-sources section now describes behaviour that changed (low) — The line says historical prices come from DefiLlama on DB miss. After this PR, Chainlink is consulted on DB miss too, ahead of the alias source, on eight chains. docs/routes.md was updated in this PR; this line, which is the entry point most readers hit first, was not.

    • Done when: the historical bullet names Chainlink alongside DefiLlama and reflects the actual try-order.
    • Provenance: pre-existing (d1736d8)

Verdict

REQUEST_CHANGES


How This Was Reviewed

This review was conducted using the review-pr skill.

86,400s equalled the daily USD feed heartbeat, so a healthy feed went
null in the window before its next update. Bound is now 2x the longest
heartbeat in feeds.ts.
Only the single-token route 404s on ?source=chainlink; batch and range
return 200 with an empty coins object. README now names the historical
try-order.

@murderteeth murderteeth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Adds a chainlink historical source: a 41-entry token→aggregator table across 8 chains, a reader that estimates the block for the requested timestamp and scales latestRoundData() by decimals() at that block, and registration at priority 15 between defillama and derived. I independently re-checked the whole feed table against live RPCs — all 41 aggregators resolve, every description() matches the token key (including the four Katana vault-bridge tokens priced off their underlying and Polygon WPOLMATIC / USD), and all report 8 decimals.

No defects survived verification.


How This Was Reviewed

Reviewed with the review-pr-workflow skill
5 review lenses, each finding independently verified by claude. 3 candidate findings were refuted and dropped.

@matheus1lva
matheus1lva merged commit 6ee8f66 into main Aug 27, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants