diff options
| author | fanquake <fanquake@gmail.com> | 2019-12-17 11:50:25 -0500 |
|---|---|---|
| committer | fanquake <fanquake@gmail.com> | 2019-12-17 12:01:18 -0500 |
| commit | e6acd9f72c61bf535d9413854b1434ec40633ca0 (patch) | |
| tree | 078140d54de6e2918497c5668a9477d66d12cce3 /src/wallet/scriptpubkeyman.h | |
| parent | 890eac8f8286496bbae664792cd2a1e56991c669 (diff) | |
| parent | 6e77a7b65cda1b46ce42f0c99ca91562255aeb28 (diff) | |
Merge #17537: wallet: Cleanup and move opportunistic and superfluous TopUp()s
6e77a7b65cda1b46ce42f0c99ca91562255aeb28 keypool: Add comment about TopUp and when to use it (Andrew Chow)
ea50e34b287e0da0806c1116bb55ade730e8ff6c keypool: Move opportunistic TopUps from LegacyScriptPubKeyMan to CWallet and ReserveDestination (Andrew Chow)
bb2c8ce23c9d7ba8d0e5538243e07218443c85b4 keypool: Remove superfluous topup from CWallet::GetNewChangeDestination (Andrew Chow)
Pull request description:
* The `TopUp()` in `CWallet::GetNewChangeDestination` is unnecessary as currently m_spk_man calls TopUp further down the call stack inside LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination). This also lets us prepare for future changes with multiple ScriptPubKeyMans in the wallet.
* An opportunistic `TopUp()` is moved from `LegacyScriptPubKeyMan::GetNewDestination` to `CWallet::GetNewDestination`.
* Another opportunistic `TopUp()` is moved from `LegacyScriptPubKeyMan::ReserveKeyFromKeyPool`
Moving opportunistic TopUps ensures that ScriptPubKeyMans will always be topped up before requesting Destinations from them as we cannot always rely on future ScriptPubKeyMan implementaions topping up internally.
See also: https://github.com/bitcoin/bitcoin/pull/17373#discussion_r348598174
ACKs for top commit:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/17537/commits/6e77a7b65cda1b46ce42f0c99ca91562255aeb28 only change is slight elaboration on comment
ryanofsky:
Code review ACK 6e77a7b65cda1b46ce42f0c99ca91562255aeb28. Only the comment changed since my previous review.
Tree-SHA512: bdfc8d303842c3fb7c3d40af7abfa6d9dac4ef71a24922bb92229674ee89bfe3113ebb46d3903ac48ef99f0a7d6eaac33282495844f2b31f91b8df55084c421f
Diffstat (limited to 'src/wallet/scriptpubkeyman.h')
| -rw-r--r-- | src/wallet/scriptpubkeyman.h | 4 |
1 files changed, 4 insertions, 0 deletions
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index aa5eac3a85..b78494921c 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -160,6 +160,10 @@ public: virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} + /** Fills internal address pool. Use within ScriptPubKeyMan implementations should be used sparingly and only + * when something from the address pool is removed, excluding GetNewDestination and GetReservedDestination. + * External wallet code is primarily responsible for topping up prior to fetching new addresses + */ virtual bool TopUp(unsigned int size = 0) { return false; } //! Mark unused addresses as being used |