wallet: Add fiat pricing to Wallet View #3001

ericholguin · 2025-04-26T00:23:32Z

Summary

This PR adds the fiat price to user's wallet balance. Using Coinbase API to get BTC current price and the user's locale (region) to determine their currency.

Changelog-Added: Added fiat pricing to wallet balance

Checklist

I have read (or I am familiar with) the Contribution Guidelines
I have tested the changes in this PR
I have opened or referred to an existing github issue related to this change.
My PR is either small, or I have split it into smaller logical commits that are easier to review
I have added the signoff line to all my commits. See Signing off your work
I have added appropriate changelog entries for the changes in this PR. See Adding changelog entries
- I do not need to add a changelog entry. Reason: [Please provide a reason]
I have added appropriate Closes: or Fixes: tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patches

Test report

Please provide a test report for the changes in this PR. You can use the template below, but feel free to modify it as needed.

Device: [Please specify the device you used for testing]
iPhone 16 Pro

iOS: [Please specify the iOS version you used for testing]
18.3.2

Damus: [Please specify the Damus version or commit hash you used for testing]
1.14 (1) 54d6161

Setup: [Please provide a brief description of the setup you used for testing, if applicable]

XCode Simulator / Local Device

Steps: [Please provide a list of steps you took to test the changes in this PR]

Open Wallet Balance verify there is a fiat price below the wallet balance and verify it is in your regional currency.

Results:

PASS
Partial PASS
- Details: [Please provide details of the partial pass]

Other notes

[Please provide any other information that you think is relevant to this PR.]

jb55 · 2025-04-26T00:34:41Z

On Fri, Apr 25, 2025 at 09:23:56AM -0600, ericholguin wrote: This PR adds the fiat price to user's wallet balance. Using Coinbase API to get BTC current price and the user's locale (region) to determine their currency. Changelog-Added: Added fiat pricing to wallet balance Signed-off-by: ericholguin ***@***.***> Closes: #3001 ---

LGTM Reviewed-by: William Casarin ***@***.***>

jb55 · 2025-04-26T00:36:26Z

o1 review: From: ***@***.*** To: ***@***.*** Subject: Review for PR wallet: Add fiat pricing to Wallet View #3001 #3001 Review for "wallet: Add fiat pricing to Wallet View" #3001 by ericholguin Here are a few observations about potential issues in this PR: 1. Locale.currency force-unwrap: The code uses 'Locale.current.currency!.identifier', which could lead to a crash if the locale lacks a recognized currency. It might be safer to handle cases where this could be nil. 2. Error handling for fetch: In CoinbaseModel, if the network call fails, there's only a printed error. You may want to provide a user-facing fallback or more robust error handling.

Copilot

Pull Request Overview

This PR adds fiat pricing to the wallet balance by integrating Coinbase API pricing data and displaying the converted value in the wallet UI. Key changes include:

Adjusting the vertical spacing in WalletView for enhanced layout consistency.
Updating BalanceView to instantiate and display fiat pricing from a new CoinbaseModel.
Introducing CoinbaseModel to handle BTC price fetching, caching, and conversion.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File	Description
damus/Views/Wallet/WalletView.swift	Increased VStack spacing to improve UI layout
damus/Views/Wallet/BalanceView.swift	Added CoinbaseModel instantiation and fiat price display
damus/Models/CoinbaseModel.swift	New model to fetch, cache, and convert BTC to fiat pricing

Files not reviewed (1)

damus.xcodeproj/project.pbxproj: Language not supported

damus/Models/CoinbaseModel.swift

jb55

potential crash

damus/Models/CoinbaseModel.swift

This PR adds the fiat price to user's wallet balance. Using Coinbase API to get BTC current price and the user's locale (region) to determine their currency. Changelog-Added: Added fiat pricing to wallet balance Signed-off-by: ericholguin <[email protected]>

danieldaquino

Thanks @ericholguin! I added some comments regarding a few concerns I have with the changes.

danieldaquino · 2025-05-07T23:13:35Z

damus/Models/CoinbaseModel.swift

+        if symbol.isEmpty || symbol == currency {
+            return amount + " " + currency
+        }
+        return symbol + amount + " " + currency


This seems to be making some assumptions about currency value formatting that might not be true for some locales.

I believe Apple provides a way to format currencies in a simpler and localization-friendly way, by using numberStyle = .currency instead of numberStyle = .decimal:

https://developer.apple.com/documentation/foundation/numberformatter/style/currency

https://needone.app/format-currency-in-swift/

danieldaquino · 2025-05-07T23:20:30Z

damus/Models/CoinbaseModel.swift

+
+struct CoinbasePrice: Codable {
+    let amount: Double
+}


nitpick tip: The structs CoinbasePrice and CoinbaseResponse exist in the context of CoinbaseModel, so we can move them inside CoinbaseModel to make namespace semantics more explicit and avoid having too many names in the global namespace:

struct CoinbaseModel { ... struct Response: Codable { let data: Price struct Price: Codable { let amount: Double } } }

danieldaquino · 2025-05-07T23:29:48Z

damus/Models/CoinbaseModel.swift

+
+    func satsToFiat(input: Int64) -> String {
+        guard let btcPrice = btcPrice, input > 0 else {
+            return ""


We should use more expressive semantics of the result, and let the view choose how to handle failures.

Instead of defaulting to an empty string or value when something fails, we should:

throw errors if something unexpected happened, and then let the view choose whether to show or suppress those errors.

Return nil if there is no value available.

danieldaquino · 2025-05-08T01:01:23Z

damus/Models/CoinbaseModel.swift

+                    self.btcPrice = decoded.data.amount
+                }
+            } catch {
+                print("Failed to fetch BTC price from Coinbase: \(error)")


I am a bit worried about how this error is being handled. If something is broken or the API stops working, the interface will just keep displaying the last known value forever, which might be misleading and hard to notice.

Copilot

Pull Request Overview

This PR adds fiat pricing to the user's wallet balance by fetching BTC spot prices from Coinbase and displaying converted values in the Wallet UI.

Increased spacing in the wallet view to make room for the new fiat price line
Introduced a CoinbaseModel to fetch, cache, and convert sats to local fiat
Integrated the fiat conversion into BalanceView with on-appear load and fetch calls

Reviewed Changes

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

File	Description
damus/Views/Wallet/WalletView.swift	Adjusted `VStack` spacing to accommodate the added fiat line
damus/Views/Wallet/BalanceView.swift	Added `@StateObject` for `CoinbaseModel` and fiat price display
damus/Models/CoinbaseModel.swift	New model for fetching/caching BTC price and converting sats
damus.xcodeproj/project.pbxproj	Registered `CoinbaseModel.swift` in build settings

Comments suppressed due to low confidence (2)

damus/Views/Wallet/BalanceView.swift:12

[nitpick] The variable name coinbase is a bit ambiguous; consider renaming it to something like coinbaseViewModel or priceModel to clarify its role.

@StateObject private var coinbase = CoinbaseModel()

damus/Models/CoinbaseModel.swift:1

New logic for price fetching, caching, and conversion lacks unit tests. Consider adding tests for satsToFiat, caching behavior, and fetchFromCoinbase error handling.

//

Copilot · 2025-05-26T23:49:11Z

damus/Views/Wallet/BalanceView.swift

+                Text(coinbase.satsToFiat(input: balance))
+                    .fontWeight(.medium)
+                    .foregroundStyle(DamusColors.neutral6)


If satsToFiat returns an empty string (e.g., zero balance or missing price), this Text will render blank space. Consider conditionally showing the view only when the string is non-empty.

Suggested change

Text(coinbase.satsToFiat(input: balance))

.fontWeight(.medium)

.foregroundStyle(DamusColors.neutral6)

let fiatValue = coinbase.satsToFiat(input: balance)

if !fiatValue.isEmpty {

Text(fiatValue)

.fontWeight(.medium)

.foregroundStyle(DamusColors.neutral6)

}

danieldaquino

Some items still need addressing. Please let me know if there are any questions on the comments placed.

tyiu

Decimal should be used instead of Double to avoid precision issues.

tyiu · 2025-08-27T01:16:47Z

damus/Models/CoinbaseModel.swift

+
+class CoinbaseModel: ObservableObject {
+
+    @Published var btcPrice: Double? = nil {


Suggested change

@Published var btcPrice: Double? = nil {

@Published var btcPrice: Decimal? = nil {

tyiu · 2025-08-27T01:16:59Z

damus/Models/CoinbaseModel.swift

+        guard let btcPrice = btcPrice, input > 0 else {
+            return ""
+        }
+        let fiat = (Double(input) / 100_000_000) * btcPrice


Suggested change

let fiat = (Double(input) / 100_000_000) * btcPrice

let fiat = (Decimal(input) / 100_000_000) * btcPrice

tyiu · 2025-08-27T01:17:14Z

damus/Models/CoinbaseModel.swift

+        }
+    }
+
+    private func cachePrice(_ price: Double) {


Suggested change

private func cachePrice(_ price: Double) {

private func cachePrice(_ price: Decimal) {

tyiu · 2025-08-27T01:17:56Z

damus/Models/CoinbaseModel.swift

+}
+
+struct CoinbasePrice: Codable {
+    let amount: Double


Suggested change

let amount: Double

let amount: Decimal

ericholguin force-pushed the sats-to-fiat branch from fca8e92 to 97744fe Compare April 26, 2025 00:25

jb55 requested a review from Copilot April 26, 2025 00:37

Copilot AI reviewed Apr 26, 2025

View reviewed changes

damus/Models/CoinbaseModel.swift Outdated Show resolved Hide resolved

jb55 requested changes Apr 26, 2025

View reviewed changes

damus/Models/CoinbaseModel.swift Outdated Show resolved Hide resolved

ericholguin force-pushed the sats-to-fiat branch from 7400b0b to 463c2d1 Compare April 29, 2025 02:28

ericholguin force-pushed the sats-to-fiat branch from 463c2d1 to d3f0bff Compare April 29, 2025 02:30

ericholguin requested a review from jb55 April 29, 2025 02:31

danieldaquino requested changes May 8, 2025

View reviewed changes

ericholguin requested a review from danieldaquino May 21, 2025 00:05

danieldaquino requested a review from Copilot May 26, 2025 23:47

Copilot AI reviewed May 26, 2025

View reviewed changes

damus-io deleted a comment from Copilot AI May 26, 2025

danieldaquino requested changes May 27, 2025

View reviewed changes

tyiu suggested changes Aug 27, 2025

View reviewed changes


		class CoinbaseModel: ObservableObject {

		@Published var btcPrice: Double? = nil {

	@Published var btcPrice: Double? = nil {
	@Published var btcPrice: Decimal? = nil {

	let fiat = (Double(input) / 100_000_000) * btcPrice
	let fiat = (Decimal(input) / 100_000_000) * btcPrice

	private func cachePrice(_ price: Double) {
	private func cachePrice(_ price: Decimal) {

wallet: Add fiat pricing to Wallet View #3001

Are you sure you want to change the base?

wallet: Add fiat pricing to Wallet View #3001

Uh oh!

Conversation

ericholguin commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Test report

Other notes

Uh oh!

jb55 commented Apr 26, 2025 via email

Uh oh!

jb55 commented Apr 26, 2025 via email

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

jb55 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danieldaquino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 26, 2025

Choose a reason for hiding this comment

Uh oh!

danieldaquino left a comment

Choose a reason for hiding this comment

Uh oh!

tyiu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ericholguin commented Apr 26, 2025 •

edited

Loading