wallet: Log when Wallet::SetMinVersion sets a different minversion #25896
Conversation
|
This is incorrect. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Yeah, obviously I had to know that since the section that updates the wallet version is in Notice how the new message is only printed during an RPC invocation of |
|
You misunderstand me. It is incorrect to have the logline occur during Furthermore, what you have done is log the default value just before the actual minversion is read from the wallet file. This is completely useless information. |
ZenulAbidin
force-pushed
the
wallet-minversion-debugprint
branch
from
72b0e63
to
29b6cec
Compare
Changes have been implemented. Now the logging only prints the read minversion. |
|
Perhaps it would be better to have the log line in Please also change the PR title to reflect what the code does. |
ZenulAbidin
force-pushed
the
wallet-minversion-debugprint
branch
from
29b6cec
to
d330480
Compare
ZenulAbidin
changed the title
Both of these adjustments have been made. |
src/wallet/wallet.cpp
Outdated
| @@ -542,6 +542,7 @@ void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in) | |||
| LOCK(cs_wallet); | |||
| if (nWalletVersion >= nVersion) | |||
| return; | |||
| this->WalletLogPrintf("Setting minversion to %d\n", nVersion); | |||
There was a problem hiding this comment.
@achow101 I pushed a revision without it (I actually had the patch ready since this afternoon, but I forgot to push it until now).
|
Please also update the OP text to reflect the code. |
ZenulAbidin
force-pushed
the
wallet-minversion-debugprint
branch
from
d330480
to
48e36c7
Compare
ZenulAbidin
force-pushed
the
wallet-minversion-debugprint
branch
from
48e36c7
to
835bd27
Compare
|
Just a question, since I haven't done this kind of thing before: Must I keep rebasing the FETCH_HEAD and force-pushing before this PR can be merged? I'm asking because apparently other merges are being made and CONTRIBUTING.md says that maintainers might request to rebase the FETCH_HEAD first. |
|
@ZenulAbidin No, you do not need to keep rebasing. You only need to rebase if there are conflicts with master. Otherwise you shouldn't rebase as doing so invalidates acks. Other PRs are merged because they are ready - contributors have reviewed those. Of course reviewers can review anything they wish and so it is probable that there are other things that are higher priority that are getting review. |
|
ACK 835bd27 |
…fferent minversion 835bd27 Wallet::SetMinVersion - Log the new minversion (Ali Sherief) Pull request description: This change prints a single additional line in the debug.log when bitcoin-cli loads a wallet using `loadwallet` (*not* `createwallet`). When Bitcoin Core creates a wallet, it's `minversion` is set to `FEATURE_BASE`, which is 10500. However, once the wallet is unloaded using `unloadwallet` or through program termination, and subsequently loaded again, `loadwallet` updates the `minversion` in the wallet.dat file to `FEATURE_LATEST`, currently 169900. The current logging format prints the very old wallet version during `createwallet`, and then the actual version in calls to `loadwallet`. This has confused at least one person ([reference](https://bitcointalk.org/index.php?topic=5410650.0) - I was the one who asked there if there were plans to change that behavior, and was subsequently redirected here by achow), so it will be very helpful to users to explicitly specify in the logs what the walletdb is doing. ACKs for top commit: achow101: ACK 835bd27 Tree-SHA512: 967c8c617e06a84915ddb147378ec3c8b0343e45f43145ec78df9cbc0201867f49c8e11cd068c403eb5ec06e07d38c3c0d3864dad8edc5efbb134a3fb30be41f
This change prints a single additional line in the debug.log when bitcoin-cli loads a wallet using
loadwallet(notcreatewallet).When Bitcoin Core creates a wallet, it's
minversionis set toFEATURE_BASE, which is 10500. However, once the wallet is unloaded usingunloadwalletor through program termination, and subsequently loaded again,loadwalletupdates theminversionin the wallet.dat file toFEATURE_LATEST, currently 169900.The current logging format prints the very old wallet version during
createwallet, and then the actual version in calls toloadwallet. This has confused at least one person (reference - I was the one who asked there if there were plans to change that behavior, and was subsequently redirected here by achow), so it will be very helpful to users to explicitly specify in the logs what the walletdb is doing.