[VDG] Unify amount display

[SuperJMN]

This PR normalizes amount displaying by using a BtcAmount type on the ViewModel side and an AmountControl in the View side. This also centralizes how things are displayed.

Benefits are:

• We no longer have to worry about how an amount will be displayed in the UI.

• By default, the amount will include the approximate USD equivalence (when needed).

• Fee amounts will always obey user settings (either sats or BTC).

• Bonus track 😁: If a new exchange rate arrives, the USD amount will vary accordingly, in a dynamic way.

  Fixes: 11423

[SuperJMN]

OK, after discussing a bit with @ichthus1604 we've decided to integrate the ability to create amounts in UiContext.
This is because creating amount is a cross-cutting concern in several ViewModels and adding a separate factory clutters contructors too much and doesn't offer any direct benefit (apart from being more explicit).

If this approach doesn't give the desired results, we can explorer further, but I think this will satisfy our needs while making things testable and decoupled.

[soosr]

Oh, many conflicts!

[ichthus1604]

I'd rather not have this method in the UiContext class itself, but rather inside a service.

Something like:

var amount = UiContext.AmountProvider.Create(money);

Also, don't forget to rename IExchangeRateProvider to IAmountProvider, because the former already exists in the backend and we don't want to introduce a name clash. Not a compilation problem because they're in different namespaces, but still confusing for people who look for these interfaces using Ctrl .

[SuperJMN]

I didn't include a service because it's currently only a method now, but I'm OK with it.
Regarding the rename of IExchangeRateProvider to IAmountProvider. I think they're 2 different things. We still need an IExchangeRateProvider for scenarios like this:

https://github.com/zkSNACKs/WalletWasabi/blob/c2774d2af624a02796ffd6c8082ad31adbe051f5/WalletWasabi.Fluent/ViewModels/Wallets/Home/Tiles/BtcPriceTileViewModel.cs#L5C2-L5C2

We don't need an amount here, but only the exchange rate.

[ichthus1604]

@SuperJMN Also, in most cases we do have an IWalletModel, except for specific places like TransactionBroadcasterViewModel.

It's probably a good idea to add IWalletModel.CreateAmount(Money money) method as well.

[SuperJMN]

@SuperJMN Also, in most cases we do have an IWalletModel, except for specific places like TransactionBroadcasterViewModel.

It's probably a good idea to add IWalletModel.CreateAmount(Money money) method as well.

OK!

BTW, please, check BtcPriceTileViewModel.cs, where only the exchange rate is used, not a full amount.

[ichthus1604]

@SuperJMN How about moving the exchange rate observable to IAmountProvider? That way you only have 1 interface and you still keep the ability to provider exchange rate to ViewModels that need it.

[SuperJMN]

@SuperJMN How about moving the exchange rate observable to IAmountProvider? That way you only have 1 interface and you still keep the ability to provider exchange rate to ViewModels that need it.

That makes me happier, but the name might be confusing. I wonder if there's a good name for the abstraction.

• It deals with amounts in BTC and USD.
• It provides exchange rates

[ichthus1604]

@SuperJMN

I wonder if there's a good name for the abstraction

There are 2 hard things in computer science:

• naming things <- you are here
• cache misses
• off by one errors

Jokes aside, maybe ICurrencyProvider is a better name? you'd probably have to rename BtcAmount to Currency as well

[SuperJMN]

I've renamed BtcAmount to just Amount. I think it plays well. What are your feelings @zkSNACKs/ui-team ?

[SuperJMN]

I think this is pretty much done.

[ichthus1604]

I'd rather keep the HasBalance observable, because that's required in many places and now those places are duplicating the logic:

this.WhenAnyObservable(x => x._wallet.Balances).Select(x => x != Money.Zero)

since you've deleted the WalletBalancesModel, maybe just add this property to the WalletModel itself.

[SuperJMN]

I'd rather keep the HasBalance observable, because that's required in many places and now those places are duplicating the logic:

this.WhenAnyObservable(x => x._wallet.Balances).Select(x => x != Money.Zero)

since you've deleted the WalletBalancesModel, maybe just add this property to the WalletModel itself.

Done! The logic is duplicated however, because in the Model I can't have accesss to Amount (Amount is a considered a ViewModel 😁).

As a side note, as I progress in my career, I see that the distinction between VMs and Models sometimes fades so much that you end up having a Model that is crafted so well that I can be used in the View with no changes.

[soosr]

@ichthus1604 Merge this PR when you are happy with it

[ichthus1604]

Approved. There are improvements I want to make to the design, but these can be part of a future PR.

[MarnixCroes]

#2110 would now be easy to implement?

[soosr]

#2110 would now be easy to implement?

Maybe. Would be good to know why the original request got rejected.

[Kruwed]

#2110 would now be easy to implement?

Maybe. Would be good to know why the original request got rejected.

It was one of the many that were closed due to being years old. It is still a relevant/requested feature.