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: Incorrect Currency Assignment for Stock Prices (#1623) #1798

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

saphp
Copy link
Contributor

@saphp saphp commented Feb 4, 2025

Fixes #1623

Issue Description

Stocks from non-USD markets (such as Saudi and Indian stock exchanges) were incorrectly assigned USD as their currency, which distorted portfolio calculations. The issue was due to the application extracting the currency from the prices array instead of the main response object.

Root Cause

The application was fetching stock price data from Synth Finance's "Open/Close Prices" API, but it incorrectly attempted to extract the currency from within each price entry under prices[], even though the API returns the correct currency in the main object under currency. This caused the system to default to "USD" when currency information was missing at the price level.

Solution

  • Updated the currency assignment logic to fetch it from the main response object (body.dig("currency")) instead of from the price entry.
  • Ensured that the currency field is correctly set for all fetched stock prices.

Impact

✅ Fixes incorrect currency assignment for stock prices in non-USD markets.
✅ Prevents miscalculations of portfolio value due to incorrect currency conversion.
✅ Ensures compatibility with Synth Finance’s API response structure.

Testing

  • Verified that stocks from Saudi and Indian markets now display correct currency values.
  • Confirmed that existing USD-based stocks remain unaffected.

Copy link
Collaborator

@zachgoll zachgoll left a comment

Choose a reason for hiding this comment

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

Awesome, fix makes sense! Thanks for the explanation and testing.

@zachgoll zachgoll merged commit 37aab45 into maybe-finance:main Feb 4, 2025
5 checks passed
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.

Investment trades are shown with incorrect currency when non-USD
2 participants