Feedback on Ch 4. Wallet

[josibake]

Overall, great doc. I'll capture general feedback here and also open a PR for nits/spelling.

Overview

• BerkeleyDB is unmaintained and on a deprecation path from Core. Might be worth linking to the proposed deprecation plan: Proposed Timeline for Legacy Wallet and BDB removal bitcoin/bitcoin#20160

• IIRC, sqlite wallets cannot create legacy wallets, only descriptors (it's enforced when creating wallets BDB is on a deprecation path - might be worth mentioning and also
  adding a link to the deprecation timeline

Wallet Architecture

• in wallet/ doc, perhaps mention bech32m? i believe it will be a supported descriptor in the next release (23.0). Ping @lsilva01

Coin Selection (AvailableCoins)

• There are three coin selection algos, no? BnB, Knapsack, SRD
• When explaining coin selection, I think it's more accurate to say that BnB, Knapsack, and SRD are all run, and then the most optimal one is chosen based on the WasteMetric - in simple terms, wasteMetric calculates an opportunity cost of spending now vs how expensive this would be to spend later at 10 s/vb (paraphrashing)
• Might also be worth explaining the waste metric in this section
• It is inaccurate to say Knapsack is a fallback, I think knapsack gets chosen 60% of the time over BnB (sauces)
• Might be worth explaining output groups, as this is a pretty big section of the coinselection.cpp / spend.cpp codebase and it can be very confusing reading through the code if you don't know what an outputGroup is or what they are solving for

CWallet

• re: birthday, you can also force a wallet to rescan the whole chain
• how do birthdays work with imported wallets?
• not sure which is the most "correct" terminology, but id avoid referencing "addresses". perhaps output types, or script types?
• even better, write a section about how these concepts map to each other w.r.t the wallet (address <-> output type <-> scripts)

Constructing Transactions

• I have some notes on how AvailableCoins works, ill send them. Yours are mostly correct mod a few nits
• interestingly, CWallet::mapWallet operates on TXs, not Coins. it first checks of the TX is eligible, then loops through each Output of the tx and adds them to available coins
• AvailableCoins also takes a coincontrol object and uses it while deciding which coins are available, might be worth mentioning here

HWI

hmu when you get to this section, happy to help/review! I use it in my setup and have also dug into the internals for the 22.0 testing guide

[josibake]

a few spelling suggestions in #15

[willcl-ark]

@lsilva01 pinged you on a potential improvement from Josie above :)