Skip to content

refactor: ♻️ Reduce the number of fetches in idle_balances adapter#96

Merged
timbrinded merged 2 commits intofix/audit-issues-sherlockfrom
audit/sherlock-72-unnecessary-fetches
Nov 21, 2025
Merged

refactor: ♻️ Reduce the number of fetches in idle_balances adapter#96
timbrinded merged 2 commits intofix/audit-issues-sherlockfrom
audit/sherlock-72-unnecessary-fetches

Conversation

@timbrinded
Copy link
Contributor

This pull request updates the asset fetching logic in the idle_balances adapter to improve consistency and testability. The main change is reduce the number of times to fetch supported assets from once per fetch_all call instead of the current once-per-fetch call.

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 idle_balances adapter to optimize asset fetching by reducing redundant API calls. Instead of fetching supported assets once per subvault (in each fetch_assets call), the refactored code now fetches them once per fetch_all_assets call and passes them as a parameter.

Key Changes:

  • Added optional supported_assets parameter to fetch_assets method to allow passing pre-fetched asset lists
  • Modified fetch_all_assets to fetch supported assets once and pass them to each fetch_assets call
  • Updated tests to verify the new parameter is correctly passed and used

Reviewed Changes

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

File Description
src/tq_oracle/adapters/asset_adapters/idle_balances.py Added optional supported_assets parameter to fetch_assets and modified fetch_all_assets to fetch supported assets once and pass them to each call
tests/adapters/asset_adapters/test_idle_balances.py Updated test to verify supported_assets parameter is correctly passed in fetch_all_assets flow

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

Comment on lines +109 to +111
async def fetch_assets(
self, subvault_address: str, *, supported_assets: list[str] | None = None
) -> list[AssetData]:
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The fetch_assets method signature now differs from the abstract method in BaseAssetAdapter which expects only subvault_address: str. Adding the optional supported_assets parameter is fine for backward compatibility, but it creates an inconsistency with the base class contract. Consider whether this parameter should be documented as an implementation detail or if the base class should be updated to allow implementation-specific parameters.

Copilot uses AI. Check for mistakes.
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@timbrinded timbrinded added the sherlock Sherlock auditor changes label Nov 21, 2025
@timbrinded timbrinded changed the base branch from fix/audit-issues-nov to fix/audit-issues-sherlock November 21, 2025 13:04
@timbrinded timbrinded merged commit e3b0ecb into fix/audit-issues-sherlock Nov 21, 2025
8 checks passed
@timbrinded timbrinded deleted the audit/sherlock-72-unnecessary-fetches branch December 5, 2025 14:31
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