Age | Commit message (Collapse) | Author |
|
The method checks the oldest key time for key pools and returns the oldest. It does no modifications.
|
|
CWallet::CanGetAddresses() is used to check whether the wallet has available or is able to produce keys for addresses. It uses the ScriptPubKeyMan::CanGetAddresses(), which in turn uses the const KeypoolCountExternalKeys() method, all which do counting and no modifications.
|
|
This method returns the sum of the key pool sizes. It does no modification.
|
|
This method simply checks if HD is or can be enabled and does not require mutability.
|
|
Make LegacyScriptPubKeyMan::CanProvide method able to recognize p2sh scripts
when the redeem script is present in the mapScripts map without the p2sh script
also having to be added to the mapScripts map. This restores behavior prior to
https://github.com/bitcoin/bitcoin/pull/17261, which I think broke backwards
compatibility with old wallet files by no longer treating addresses created by
`addmultisigaddress` calls before #17261 as solvable.
The reason why tests didn't fail with the CanProvide implementation in #17261
is because of a workaround added in 4a7e43e8460127a40a7895519587399feff3b682
"Store p2sh scripts in AddAndGetDestinationForScript", which masked the problem
for new `addmultisigaddress` RPC calls without fixing it for multisig addresses
already created in old wallet files.
This change adds a lot of comments and allows reverting commit
4a7e43e8460127a40a7895519587399feff3b682 "Store p2sh scripts in
AddAndGetDestinationForScript", so the AddAndGetDestinationForScript() function,
CanProvide() method, and mapScripts map should all be more comprehensible
|
|
Needed for future ScriptPubKeyMans which may need to create
SigningProviders dynamically and thus a normal pointer is not enough
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
Add wallet logic for dealing with multiple ScriptPubKeyMan instances. This
doesn't change current behavior because there is still only a single
LegacyScriptPubKeyMan. But in the future the new logic will be used to support
descriptor wallets.
|
|
This avoids repeaded upgrades when support for more multiple keyman references
is added in the next commit:
https://github.com/bitcoin/bitcoin/pull/16341#discussion_r322370108
|
|
This commit only affects locking behavior and doesn't have other changes.
|
|
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
|
|
LegacyScriptPubKeyMan::HasEncryptionKeys
|
|
Removes SetCrypted() and fUseCrypto as we don't need them anymore.
SetCrypted calls in LegacyScriptPubKeyMan are replaced with mapKeys.empty()
IsCrypted() is changed to just call HasEncryptionKeys()
|
|
|
|
Does not change behavior. Needed to make AddCryptedKeyInner() work
with SetCrypted() being gone.
|
|
|
|
CWallet::Unlock is changed to call ScriptPubKeyMan::CheckDecryptionKey
and the original implementation of Unlock is renamed to CheckDecryptionKey.
|
|
|
|
This is a bugfix: https://github.com/bitcoin/bitcoin/pull/16341#discussion_r330669214
|
|
In order for ScriptPubKeyMan to be generic and work with future
ScriptPubKeyMans, ScriptPubKeyMan::ReturnDestination is changed to
take a CTxDestination instead of a CPubKey. Since LegacyScriptPubKeyMan
still deals with keys internally, a new map m_reserved_key_to_index is
added in order to track the keypool indexes that have been reserved.
The CPubKey argument of KeepDestination is also removed so that it is
more generic. Instead of taking a CPubKey or a CTxDestination, we just use
the nIndex given to find the pubkey.
|
|
Addresses are determined by LegacyScriptPubKeyMan::GetReservedDestination
instead of ReserveDestination::GetReservedDestination as other ScriptPubKeyMan
implementations may construct addresses differently
This does not change behavior.
|
|
These need to be added so that LearnRelatedScripts can be called
from within KeepDestination later.
|
|
There is no reason to have Keep/ReturnDestination to be a wrapper for
Keep/ReturnKey. Instead just make them the same function.
|
|
ReserveDestination
An opportunistic TopUp is moved from LegacyScriptPubKeyMan::GetNewDestination
to CWallet::GetNewDestination. Another opportunistic TopUp is moved from
LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination)
to ReserveDestination::GetReservedDestination.
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.
As such, it is also unnecessary to keep the TopUp calls in the
LegacyScriptPubKeyMan functions so they are moved.
This does not change behavior as TopUp calls are moved up the call stack.
|
|
Call LegacyScriptPubKeyMan::CanGetAddresses directly instead of calling
CWallet::CanGetAddresses to only query the relevant key manager
This is a minor change in behavior: call now only happens if a new key needs to
be reserved, since if a key is already reserved it might fail unnecessarily.
This change also serves as a sanity check
https://github.com/bitcoin/bitcoin/pull/16341#discussion_r331238394
|
|
Suggested https://github.com/bitcoin/bitcoin/pull/17304#discussion_r341194391
by Gregory Sanders <gsanders87@gmail.com>
Reason for keeping the `return true` `return false` verbosity is that more code
will be added after the ReserveKeyFromKeyPool() call before returning.
|
|
Previous discussion https://github.com/bitcoin/bitcoin/pull/17304#discussion_r340307903
|
|
Pass CTxDestination instead of more ambiguous uint160 hash value. This is more
type safe and more efficient since it avoids doing map lookups that will always
fail and were not done previously before
a18edd7b383d667b15b6d4b87aa3a055a9fa5051 from
https://github.com/bitcoin/bitcoin/pull/17304
Change suggested by Andrew Chow <achow101-github@achow101.com> in
https://github.com/bitcoin/bitcoin/pull/17304#discussion_r340345745 and
https://github.com/bitcoin/bitcoin/pull/17381#issuecomment-549994944
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
Easier to review ignoring whitespace:
git log -p -n1 -w
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
LegacyScriptPubKeyMan::ImportScriptPubKeys
This commit does not change behavior.
|
|
ScriptPubKeyMan
ScriptPubKeyMan is only using UnsetWalletFlagWithDB to unset the blank
wallet flag. Just make that it's own function and not expose the flag
writing directly.
This does not change behavior.
|
|
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
LegacyScriptPubKeyMan::GetNewDestination
This commit does not change behavior.
|
|
This commit does not change behavior.
|
|
Suggestion from MarcoFalke <falke.marco@gmail.com>
https://github.com/bitcoin/bitcoin/pull/17260#discussion_r340029481
|
|
This moves CWallet members and methods dealing with keys to a new
LegacyScriptPubKeyMan class, and updates calling code to reference the new
class instead of CWallet.
Most of the changes are simple text replacements and variable substitutions
easily verified with:
git log -p -n1 -U0 --word-diff-regex=.
The only nontrivial chunk of code added is the new LegacyScriptPubKeyMan class
declaration, but this code isn't new and is just selectively copied and moved
from the previous CWallet class declaration. This can be verified with:
git log -p -n1 --color-moved=dimmed_zebra src/wallet/scriptpubkeyman.h src/wallet/wallet.h
or
git diff HEAD~1:src/wallet/wallet.h HEAD:src/wallet/scriptpubkeyman.h
This commit does not change behavior.
|
|
Start moving wallet and ismine code to scriptpubkeyman.h, scriptpubkeyman.cpp
The easiest way to review this commit is to run:
git log -p -n1 --color-moved=dimmed_zebra
And check that everything is a move (other than includes and copyrights comments).
This commit is move-only and doesn't change code or affect behavior.
|