Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: refactoring the token metadata cache to become a globally accessible #1008

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

geekbrother
Copy link
Contributor

Description

This PR refactors the tokens metadata cache to be globally accessible by the TokenMetadataCacheProvider to fix the cache inconsistency between different providers and metadata caching implementations between providers.

How Has This Been Tested?

Current balance and history integration tests should pass.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@geekbrother geekbrother self-assigned this Mar 27, 2025
@nopestack nopestack requested a review from Copilot March 28, 2025 09:57
Copy link

@Copilot 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 tokens metadata caching mechanism to make it globally accessible via the TokenMetadataCacheProvider interface. Key changes include removing the token_metadata_cache field from the AppState, updating cache-related method signatures across providers and handlers, and implementing a new TokenMetadataCache that conforms to the TokenMetadataCacheProvider trait.

Reviewed Changes

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

Show a summary per file
File Description
src/state.rs Removed token_metadata_cache from AppState and its associated instantiation in new_state.
src/providers/zerion.rs Updated cache interface usage in BalanceProvider and HistoryProvider implementations to use TokenMetadataCacheProvider.
src/providers/solscan.rs Switched to the new TokenMetadataCacheProvider interface and adjusted error logging and metadata handling.
src/providers/one_inch.rs Modified the get_price signature to accept a TokenMetadataCacheProvider argument.
src/providers/mod.rs Introduced the TokenMetadataCacheProvider trait and updated provider traits and repository initialization.
src/providers/dune.rs Replaced the legacy cache functions with the new TokenMetadataCacheProvider methods and updated error handling.
src/providers/coinbase.rs Changed the get_transactions signature to use the new TokenMetadataCacheProvider interface.
src/lib.rs Removed initialization of token_metadata_cache from the bootstrap and updated dependency injections.
src/handlers/history.rs Updated transactions handler to pass token_metadata_cache from the providers repository.
src/handlers/fungible_price.rs Updated fungible price handler to pass token_metadata_cache, in line with the new cache interface.
src/handlers/balance.rs Removed legacy caching functions, updated cache TTL type, and implemented a new TokenMetadataCache struct.
Comments suppressed due to low confidence (2)

src/providers/solscan.rs:307

  • [nitpick] Consider using '{}' as the placeholder (e.g. error!("Error when setting token metadata in cache: {}", e)) for consistency with other error logs.
error!("Error when setting the token metadata to the cache: {e}");

src/handlers/balance.rs:441

  • Typo in the error message: 'seting' should be corrected to 'setting'.
cache.set_ex(key, value, ttl).await.map_err(|e| StorageError::Connection(format!("Error when seting cache: {e}")))?;

@geekbrother geekbrother force-pushed the fix/shared_token_metadata_cache branch from 1ad440b to 24bd82d Compare March 31, 2025 14:24
@geekbrother geekbrother merged commit 81a6ff3 into master Mar 31, 2025
15 checks passed
@geekbrother geekbrother deleted the fix/shared_token_metadata_cache branch March 31, 2025 14:37
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.

3 participants