Skip to content

Conversation

@timbrinded
Copy link
Collaborator

@timbrinded timbrinded commented Oct 29, 2025

This pull request refactors the price adapter system to replace the specialized WstETHAdapter with a more general ETHAdapter, updates documentation to reflect the new pricing logic, and improves the handling of asset addresses in Chainlink price feeds. It also updates the ERC20 ABI to a more generic format and makes the base adapter configuration type explicit.

Price Adapter System Refactor

  • Replaced WstETHAdapter with ETHAdapter throughout the codebase, including imports and the PRICE_ADAPTERS list in src/tq_oracle/adapters/price_adapters/__init__.py. The __all__ export list was also updated accordingly.
  • Updated documentation in ARCHITECTURE.md to remove details about wstETH pricing assumptions and adapter, and clarified that ETH-family assets are now handled by the new ETHAdapter.

Chainlink Price Feed Improvements

  • Added support for fetching and validating wstETH price using Chainlink's wstETH/ETH price feed in src/tq_oracle/adapters/price_adapters/chainlink.py. This includes handling checksummed asset addresses and improved logic for direct feed asset selection. [1] [2] [3]

Codebase and Configuration Improvements

  • Changed the BasePriceAdapter constructor to expect a typed OracleSettings configuration object instead of a generic object, improving type safety and clarity. [1] [2]

ERC20 ABI Update

  • Replaced the previous ERC20 ABI in src/tq_oracle/abis/ERC20.json with a more generic version, removing metadata and method identifiers, and updating function and event definitions to match a standard ABI format.

Fixes

Copilot AI review requested due to automatic review settings October 29, 2025 14:32
@timbrinded timbrinded changed the title Feat/add pyth feat: ✨ Improve Pricing Logic Oct 29, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the price adapter architecture by separating ETH/WETH pricing from wstETH pricing, adds Pyth Network as a price validation source, and updates configuration to use shared tolerance settings across validators.

  • Removed WstETHAdapter and replaced it with ETHAdapter for basic ETH/WETH pricing (wstETH now handled by Chainlink/CowSwap)
  • Added PythAdapter and PythValidator for Pyth Network integration
  • Moved price deviation tolerance settings from chainlink-specific to shared configuration

Reviewed Changes

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

Show a summary per file
File Description
src/tq_oracle/adapters/price_adapters/eth.py New adapter handling only ETH and WETH (1:1 pricing)
src/tq_oracle/adapters/price_adapters/pyth.py New Pyth Network price adapter with staleness and confidence validation
src/tq_oracle/adapters/price_validators/pyth.py New Pyth validator using shared tolerance settings
src/tq_oracle/adapters/price_adapters/cow_swap.py Enhanced to dynamically fetch token decimals and handle all non-ETH/WETH assets
src/tq_oracle/adapters/price_adapters/chainlink.py Added wstETH price feed support
src/tq_oracle/settings.py Renamed tolerance settings and added Pyth-specific configuration
src/tq_oracle/constants.py Added Pyth price feed IDs and removed hardcoded TOKEN_DECIMALS
tests/adapters/price_adapters/test_eth.py Updated tests to reflect ETHAdapter scope (ETH/WETH only)
tests/adapters/price_validators/test_pyth.py New comprehensive test suite for Pyth validator
uv.lock Added resolution markers for Python version compatibility
Comments suppressed due to low confidence (3)

tests/adapters/price_adapters/test_eth.py:117

  • The test name 'test_fetch_prices_all_three_assets' is misleading because it only tests two assets (ETH and WETH). The test should be renamed to 'test_fetch_prices_eth_and_weth' or similar to accurately reflect what it tests.
    tests/adapters/price_adapters/test_eth.py:153
  • This integration test expects ETHAdapter to return a wstETH price > 10**18, but ETHAdapter only handles ETH and WETH according to its implementation. The test will fail because wstETH is not in the adapter's supported assets. Either remove this test or change it to verify that wstETH is not priced by ETHAdapter (i.e., assert len(result.prices) == 0).
    tests/adapters/price_adapters/test_eth.py:174
  • This integration test expects 4 prices including wstETH, but ETHAdapter only handles ETH and WETH. The assertion 'len(result.prices) == 4' will fail because ETHAdapter will only add 2 prices (ETH and WETH), resulting in 3 total prices including the existing '0x111'. The expected count should be 3, and the wstETH assertions (lines 174-177) should be removed.

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

Copilot AI review requested due to automatic review settings October 29, 2025 14:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

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


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

Copilot AI review requested due to automatic review settings October 29, 2025 16:26
@timbrinded timbrinded marked this pull request as ready for review October 29, 2025 16:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

tests/adapters/price_adapters/test_eth.py:156

  • The test expects ETHAdapter to price wstETH and return a value, but the ETHAdapter implementation (lines 54-65 of eth.py) only handles ETH and WETH assets. wstETH is not included in the adapter's logic, so this test will fail as it expects 1 price but will receive 0. wstETH pricing should now come from CowSwapAdapter or ChainlinkAdapter. Either update this test to use the correct adapter or mark it as testing the full pipeline.
    tests/adapters/price_adapters/test_eth.py:1
  • This test expects ETHAdapter to return 4 prices (including wstETH), but ETHAdapter only handles ETH and WETH. The test will fail with only 3 prices (0x111, ETH, WETH). Since wstETH pricing has been moved to CowSwapAdapter or ChainlinkAdapter, this integration test should either use the full adapter pipeline or be updated to remove wstETH expectations from ETHAdapter tests.

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

@timbrinded timbrinded merged commit 7785968 into master Oct 30, 2025
12 checks passed
@timbrinded timbrinded deleted the feat/add-pyth branch October 30, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add staleness validation to chainlink adapter use cowswap for wstETH price Add pyth price oracle for last look

2 participants