fix: 🛠️ Sherlock audit remediation #117

timbrinded · 2025-11-21T19:12:35Z

Warning

This should not be attempted to be merged until #130 updates this branch with the latest fixes.

Contains remediation for all of the issues raised by the Sherlock Auditor.

…/add-stakewise

…audit-issues-sherlock

* Replace broad exception handling with specific exceptions in price adapters * Use separate exception handlers with specific error messages for each exception type --------- Co-authored-by: Tim B <[email protected]>

* fix: 🐛 fix cowswap scaling * refactor: ♻️ Tidy code

* fix: 🔧 add timeouts added timeout to safe external api so it doesn't hang indefinitely * feat: 🩹 add cowswap timeout * increase timeout --------- Co-authored-by: timbrinded <[email protected]>

* Add Pyth API response validation with JSON and structure checks * Properly validate feed items are dicts instead of silently filtering * Refactor tests to reduce code duplication using helper functions * chore: fmt * Fix tests to use monkeypatch instead of mocker fixture * Apply ruff formatting * switch to pydantic model * fmt --------- Co-authored-by: timbrinded <[email protected]>

…ythAdapter (#107) Co-authored-by: matias-gonz <[email protected]>

* refactor: 🛠️ Change fetch_native_price return type to string to prevent float precision loss * add test * Add CowSwap API response validation with timeout and structure checks * chore: lint * Remove redundant numeric price and timeout tests * fix tests --------- Co-authored-by: timbrinded <[email protected]>

Copilot

Pull request overview

This PR addresses audit issues for the Sherlock oracle by improving robustness, error handling, and test coverage. The changes focus on fixing precision issues in price calculations, enhancing retry logic, adding comprehensive input validation, and removing unused code.

Removes unused code (units.py, safe_state.py adapter) and adds test coverage for pricing, validation, and timeout behavior
Introduces global pipeline timeout, price validation retry logic with backoff, and HTTP request retry mechanisms
Fixes decimal precision handling in price adapters (Pyth and CowSwap) to properly account for token decimals

Reviewed changes

Copilot reviewed 30 out of 31 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`uv.lock`, `pyproject.toml`	Adds pytest-mock dependency for improved testing
`tq-oracle.toml.example`	Adds configuration examples for new timeout and Pyth settings
`src/tq_oracle/settings.py`	Adds global timeout, price validation retry settings, and Pyth fail flags
`src/tq_oracle/main.py`	Adds CLI option for global timeout
`src/tq_oracle/pipeline/run.py`	Implements global timeout wrapper, removes unused address canonicalization
`src/tq_oracle/pipeline/pricing.py`	Adds retry logic with backoff for price validation
`src/tq_oracle/pipeline/assets.py`	Adds block_identifier to subvault queries
`src/tq_oracle/report/publisher.py`	Adds retry logic for Safe API calls
`src/tq_oracle/adapters/price_adapters/pyth.py`	Adds decimal-aware price calculations, input validation, and stale/missing feed tracking
`src/tq_oracle/adapters/price_adapters/cow_swap.py`	Removes unused get_token_decimals, improves error handling and precision
`src/tq_oracle/adapters/price_adapters/eth.py`	Removes redundant validate_prices call
`src/tq_oracle/adapters/price_validators/pyth.py`	Adds fail-on-stale and fail-on-missing price logic
`src/tq_oracle/adapters/asset_adapters/idle_balances.py`	Optimizes by caching supported assets query
`src/tq_oracle/processors/total_assets.py`	Refactors division to avoid precision loss
`src/tq_oracle/processors/oracle_helper.py`	Improves error handling for empty vaults
`src/tq_oracle/checks/price_validators.py`	Adds retry_recommended flag to PriceValidationError
`src/tq_oracle/adapters/check_adapters/timeout_check.py`	Adds check for suspicious reports allowing immediate replacement
`src/tq_oracle/adapters/check_adapters/safe_state.py`	Removes unused Safe state check adapter
`src/tq_oracle/adapters/check_adapters/__init__.py`	Removes SafeStateAdapter from check list
`src/tq_oracle/abi.py`	Updates get_oracle_address_from_vault to use settings object and block_identifier
`tests/test_units.py`	Removes tests for deleted scale_to_18 function
`tests/pipeline/test_run_timeout.py`	Adds tests for global timeout behavior
`tests/checks/test_price_validations.py`	Adds tests for price validation retry logic
`tests/adapters/price_validators/test_pyth.py`	Updates tests for decimal-aware pricing and adds stale/missing feed tests
`tests/adapters/price_adapters/test_pyth.py`	Adds comprehensive error handling tests
`tests/adapters/price_adapters/test_cow_swap.py`	Adds precision and error handling tests
`tests/adapters/asset_adapters/test_idle_balances.py`	Updates test to match supported_assets caching
`README.md`	Documents new global_timeout_seconds option

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

Copilot · 2025-11-21T19:15:27Z

src/tq_oracle/adapters/price_adapters/cow_swap.py

+            price_decimal = Decimal(str(price_value))
+        except Exception as e:


Catching bare Exception is too broad. Specify the expected exception types (e.g., ValueError, TypeError, decimal.InvalidOperation) to avoid masking unexpected errors.

Copilot · 2025-11-21T19:15:27Z

src/tq_oracle/pipeline/run.py

+        )
+        raise asyncio.TimeoutError(
+            "Report exceeded global timeout "
+            f"{timeout_s}s (vault={vault_address})\n N.B. This can be changed via `global_timeout_seconds` "


The error message contains a bare newline followed by 'N.B.' which may not display properly in all contexts. Consider using a space or reformatting to: f\"{timeout_s}s (vault={vault_address}). This can be changed via 'global_timeout_seconds' or CLI flag '--global-timeout-seconds'.\"

Suggested change

f"{timeout_s}s (vault={vault_address})\n N.B. This can be changed via `global_timeout_seconds` "

f"{timeout_s}s (vault={vault_address}). N.B. This can be changed via `global_timeout_seconds` "

Copilot · 2025-11-21T19:15:28Z

tests/adapters/price_validators/test_pyth.py

    assert validator.failure_tolerance == 1.0


+async def _usdc_decimals(self, _token_address: str) -> int:


The helper function _usdc_decimals is defined at module level (outside any test function or class) but takes self as a parameter. This suggests it's meant to be a method, but it's not part of any class. Either remove the self parameter or move it inside the appropriate test class/fixture.

Suggested change

async def _usdc_decimals(self, _token_address: str) -> int:

async def _usdc_decimals(_token_address: str) -> int:

Copilot · 2025-11-21T19:15:28Z

src/tq_oracle/processors/oracle_helper.py

+        logger.error("OracleHelper contract call failed: %s", str(e))
+
+        if config.ignore_empty_vault and total_assets == 0:


The error is logged before checking conditions, meaning the error will always be logged even when it's expected (empty vault with ignore flag). Consider moving the error logging inside the else block at line 81 where the exception is re-raised, or making it a warning when the vault is empty.

Copilot · 2025-11-21T19:15:28Z

src/tq_oracle/adapters/price_adapters/pyth.py

+            decimals = await self.get_token_decimals(original_address)
+            price_per_base_unit_d18 = (asset_price_18 * 10**18) // (10**decimals)


The get_token_decimals method is called for every asset inside a loop without caching. This could result in multiple RPC calls for the same token. Consider adding a decimals cache similar to what was removed from CowSwapAdapter, or calling this once per unique asset before the loop.

Copilot · 2025-11-21T19:15:28Z

tests/adapters/price_validators/test_pyth.py

        # Oracle price: USDC is ~1/3000 ETH (since ETH is $3000 and USDC is $1)
+        monkeypatch.setattr(
+            "tq_oracle.adapters.price_adapters.pyth.PythAdapter.get_token_decimals",
+            _usdc_decimals,


The _usdc_decimals function is defined with a self parameter but is being used as a standalone function via monkeypatch.setattr. This will cause a TypeError when called because the function expects self but won't receive it in this patching context. Either remove self from the function signature or use a lambda/partial to properly bind it.

Suggested change

_usdc_decimals,

lambda *args, **kwargs: _usdc_decimals(),

* fix dps * fix tests

* fix dps * fix tests * fix log

Copilot

Pull request overview

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

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

Copilot · 2025-11-23T11:53:47Z

src/tq_oracle/processors/total_assets.py

@@ -1,7 +1,10 @@
 from __future__ import annotations

+from _decimal import ROUND_DOWN


Import ROUND_DOWN from decimal instead of _decimal. The _decimal module is an internal implementation detail. Use from decimal import Decimal, ROUND_DOWN for consistency with the rest of the codebase (see oracle_helper.py line 4).

Suggested change

from _decimal import ROUND_DOWN

from decimal import ROUND_DOWN

Copilot · 2025-11-23T11:53:48Z

src/tq_oracle/processors/total_assets.py

+from _decimal import ROUND_DOWN
+
 from ..adapters.price_adapters.base import PriceData
 from ..processors.asset_aggregator import AggregatedAssets
+from decimal import Decimal




Group imports from the same module together. Both Decimal and ROUND_DOWN should be imported on the same line: from decimal import Decimal, ROUND_DOWN. This matches the pattern used in oracle_helper.py (line 4).

Suggested change

from _decimal import ROUND_DOWN

from ..adapters.price_adapters.base import PriceData

from ..processors.asset_aggregator import AggregatedAssets

from decimal import Decimal

from decimal import Decimal, ROUND_DOWN

from ..adapters.price_adapters.base import PriceData

from ..processors.asset_aggregator import AggregatedAssets

* fix dps * fix tests * fix log * fix dp rounding

timbrinded added 30 commits November 4, 2025 20:37

feat: ✨ initial attempt

78e1a56

Merge branch 'master' of github.com:Moonsong-Labs/tq-oracle into feat…

02eac5d

…/add-stakewise

Fix Tests

479a013

fixed stakewise support

5c0bcc5

minor progress

3bf8640

first attempt

5ff65e5

tidy

c8b9f02

tidy

296d558

refactor out leveraged logic

3f2bfe9

tidy

430513f

new options

58d8a35

config schema change

74263d1

tidy

df1607f

update deps

5bf20a9

pr comment fixes

f0eff94

tidy

6ca299d

tidy assets

daf1a79

Merge branch 'master' of github.com:Moonsong-Labs/tq-oracle into feat…

435eb60

…/add-stakewise

tidy

a63d765

Merge branch 'master' of github.com:Moonsong-Labs/tq-oracle into fix/…

982de6b

…audit-issues-sherlock

refactor: ♻️ Tidy pointless check (#111)

3b3fd56

feat: ✨ Add pyth failure toggles (#112)

d9de131

fix: 🐛 Make block number consistent across all RPC calls (#113)

13d049c

feat: ✨ add global timeout (#116)

93314e6

make failed confidence checks raise errors (#115)

dac25d2

chore: rm unused scale fn (#114)

f6b10ed

feat: ⚡ added backoff retries (#110)

e4e0865

fix: 🔧 tighten empty vault error handling (#106)

054285a

feat: ✨ Add post check retry (#105)

7b27b98

fix: 🐛 use proper timestamp for timeout check (#103)

db792c1

matias-gonz and others added 10 commits November 21, 2025 16:45

fix: replace broad exception handling with specific exceptions (#100)

b0a43d0

* Replace broad exception handling with specific exceptions in price adapters * Use separate exception handlers with specific error messages for each exception type --------- Co-authored-by: Tim B <[email protected]>

fix: 🐛 collect dust (#97)

682b41a

refactor: ♻️ tidy code (#96)

e3b0ecb

fix: 🛠️ Correct price scaling across adapters (#108)

3ce7240

* fix: 🐛 fix cowswap scaling * refactor: ♻️ Tidy code

refactor: ♻️ tidy code (#104)

05044d5

fix: add missing timeouts to HTTP requests (#101)

234f883

* fix: 🔧 add timeouts added timeout to safe external api so it doesn't hang indefinitely * feat: 🩹 add cowswap timeout * increase timeout --------- Co-authored-by: timbrinded <[email protected]>

refactor: ♻️ tidy code (#95)

2bb0d44

feat: confidence check fails if confidence ratio exceeds maximum in P…

2e2fd9d

…ythAdapter (#107) Co-authored-by: matias-gonz <[email protected]>

Copilot AI review requested due to automatic review settings November 21, 2025 19:12

Copilot started reviewing on behalf of timbrinded November 21, 2025 19:13 View session

Copilot finished reviewing on behalf of timbrinded November 21, 2025 19:14

Copilot AI reviewed Nov 21, 2025

View reviewed changes

timbrinded added 2 commits November 23, 2025 11:36

Fix/final report dps (#119)

eb9b801

* fix dps * fix tests

Fix/final report dps (#120)

2bb4bfe

* fix dps * fix tests * fix log

Copilot AI review requested due to automatic review settings November 23, 2025 11:51

Copilot started reviewing on behalf of timbrinded November 23, 2025 11:52 View session

Copilot finished reviewing on behalf of timbrinded November 23, 2025 11:53

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Fix/final report dps (#121)

e5f5ce6

* fix dps * fix tests * fix log * fix dp rounding

timbrinded added enhancement New feature or request FYEO FYEO auditor changes labels Dec 2, 2025

timbrinded closed this Dec 2, 2025

timbrinded reopened this Dec 2, 2025

timbrinded added sherlock Sherlock auditor changes audit and removed FYEO FYEO auditor changes enhancement New feature or request labels Dec 2, 2025

timbrinded changed the title ~~Fix/audit issues sherlock~~ fix: 🛠️ Sherlock audit remediation Dec 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: 🛠️ Sherlock audit remediation #117

fix: 🛠️ Sherlock audit remediation #117

Uh oh!

timbrinded commented Nov 21, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		price_decimal = Decimal(str(price_value))
		except Exception as e:

	f"{timeout_s}s (vault={vault_address})\n N.B. This can be changed via `global_timeout_seconds` "
	f"{timeout_s}s (vault={vault_address}). N.B. This can be changed via `global_timeout_seconds` "

		assert validator.failure_tolerance == 1.0


		async def _usdc_decimals(self, _token_address: str) -> int:

	async def _usdc_decimals(self, _token_address: str) -> int:
	async def _usdc_decimals(_token_address: str) -> int:

		logger.error("OracleHelper contract call failed: %s", str(e))

		if config.ignore_empty_vault and total_assets == 0:

		decimals = await self.get_token_decimals(original_address)
		price_per_base_unit_d18 = (asset_price_18 * 1018) // (10decimals)

		@@ -1,7 +1,10 @@
		from __future__ import annotations

		from _decimal import ROUND_DOWN

	from _decimal import ROUND_DOWN
	from decimal import ROUND_DOWN

fix: 🛠️ Sherlock audit remediation #117

Are you sure you want to change the base?

fix: 🛠️ Sherlock audit remediation #117

Uh oh!

Conversation

timbrinded commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

timbrinded commented Nov 21, 2025 •

edited

Loading