refactor: replace manual Satoshis-to-BTC conversions with FormatMoney() #20537

This tiny PR replaces all remaining occurences of manual conversion of Satoshi amounts (CAmount) to a currency unit string via division and modulo operation on COIN (100000000) by the utility function FormatMoney(...). The instances were found with the command

git grep "\%[ ]*COIN"

Only ToString()-functions are affected, but still I'd argue that the constants COIN should only be used:

• for money constants (like in benchmark/tests, chainparams, GetBlockSubsidy(...) etc.) and
• for utility functions that need it (like {Parse,Format}Money, ValueFromAmount etc.)

Note that strictly speaking, this is not a pure refactor, as the function FormatMoney(...) drops excess zero after two decimal places.

Not sure about this. I've been working in #20391 to move the use of COIN (or 1e8) and 1000 from CFeeRate callers all into policy/feerate, where their use is documented. This partially does the opposite without removing the other ones, seems less clear next to the btc/kvb line, and brings in a dependency. There may be a better abstraction we can do for ToString, but I think it should handle both sat/vB and BTC/kvB and also returning both with and without the units. Ideas welcome.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #20391 (wallet: introduce setfeerate (an improved settxfee, in sat/vB) by jonatack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@theStack Note that FormatMoney(n) currently has a gotcha that is described in #20402 ("Invalid integer negation in FormatMoney(CAmount n) and ValueFromAmount (CAmount n) when n = std::numeric_limits<CAmount>::min()"). See suggested fix in #20406 too.

I haven't checked if the new call sites introduced by this PR are affected, but I thought it was worth mentioning at least when considering new FormatMoney usage :)

Thanks for your feedback! @jonatack: I see your point and and agree that this is counter-productive w.r.t. to the planned fee rate improvements. Will take a deeper look at #20391 soon (put it into my evergrowing TOREVIEW file ;-)). @practicalswift: As this PR would only change the usage in ToString() methods (i.e. for creating debug strings) it is not critical, but good hint anyway.

Not sure what I should do with this PR, maybe it would still make sense to use FormatMoney in CTxOut::ToString() though.