Docs/move-only
Adds doxygen comments for the CKeyPool and CReserveKey objects. The way these work is pretty confusing and it’s easy to overlook details (eg #15557 (review)).
These are on the verbose side, but I think too much commenting is better than not enough. Happy to take feedback on what’s an appropriate level.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
reviewer tip: use git diff --color-moved=dimmed-zebra
181+ * serialize/deserialize the pool data to/from the database.
182+ */
183 class CKeyPool
184 {
185 public:
186+ // The time at which the key was generated. Set in AddKeypoolPubKeyWithDB
//! Description before the member syntax to describe member or variable use to be picked up by Doxygen. Is the // syntax used here with the intent to not be picked up, or for another reason?
134@@ -135,14 +135,61 @@ enum WalletFlags : uint64_t {
135
136 static constexpr uint64_t g_known_wallet_flags = WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET | WALLET_FLAG_KEY_ORIGIN_METADATA;
137
138-/** A key pool entry */
139+/** A key from a CWallet's keypool
140+ *
141+ * The wallet holds one (for pre HD-split wallets) or several keypools. These
142+ * are sets of keys that have not yet been used to give out addresses or
143+ * send change to.
143+ * send change to.
144+ *
145+ * The Bitcoin Core wallet was originally a collection of unrelated private
146+ * keys with their associated addresses. If a non-HD wallet generated a
147+ * key/address, gave that address out and then restored a backup from before
148+ * that key had been generated, then any funds sent to that address would be
166+ * However, if many addresses were used since the backup, then the wallet may
167+ * not know how far ahead in the HD chain to look for its addresses. The
168+ * keypool is used to implement a 'gap limit'. The keypool maintains a set of
169+ * keys (by default 1000) ahead of the last used key and scans for the
170+ * addresses of those keys. This avoids the risk of not seeing transactions
171+ * involving the wallet's addresses, or re-using the same address.
168+ * keypool is used to implement a 'gap limit'. The keypool maintains a set of
169+ * keys (by default 1000) ahead of the last used key and scans for the
170+ * addresses of those keys. This avoids the risk of not seeing transactions
171+ * involving the wallet's addresses, or re-using the same address.
172+ *
173+ * HD-split added a second keypool (commit: 02592f4c). There is an external
git show 02592f4c to see what “HD-split” meant more exactly. Perhaps “The wallet with HD chain split feature (HD-split) added…”
173+ * HD-split added a second keypool (commit: 02592f4c). There is an external
174+ * keypool (for addresses to hand out) and an internal keypool (for change
175+ * addresses).
176+ *
177+ * Keypool keys are stored in the wallet/keystore's keymap. The keypool data is
178+ * stored as a sets of indexes in the wallet (setInternalKeyPool,
ACK https://github.com/bitcoin/bitcoin/pull/15777/commits/292f0ee148bbe3b305aa8c1623355cee08a05867
Feel free to ignore the nits I mention. Very helpful documentation, particularly for someone new to the codebase and its history like myself. Kudos for the relevant commit hashes to dive in further.
