-
Notifications
You must be signed in to change notification settings - Fork 37.7k
[wallet] Keypool topup cleanups #11044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Tagged for backport if there is an rc2, as this fixes a confusing warning when building from source. |
Warnings removed. ACK 61aa92d |
utACK 61aa92d9a854677cafefb15117c3d83d6bab6af6 |
Log message is consistent with others related. Nit, I would reword 063aab4 to ACK 61aa92d. |
utACK 61aa92d9a854677cafefb15117c3d83d6bab6af6 |
61aa92d
to
67ceff4
Compare
@@ -3659,15 +3659,11 @@ void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id) | |||
m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); | |||
} | |||
walletdb.ErasePool(index); | |||
LogPrintf("keypool index %d removed\n", index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to put this in the wallet debug category - it's just an info message, and not very useful for most end-users I guess? (it could even be vaguely worrying if you don't know what it's about)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Followup PR to change for all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches the existing 'keypool keep' logs, which are not wallet debug category.
This PR already has a bunch of ACKs and it's wanted for rc2, so I'd rather not change this now. I'll happily open a PR to make this (and other wallet logs) wallet debug category.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok...
ACK 67ceff4. |
67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery) 1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery) Pull request description: A couple of minor cleanups suggested by @ryanofsky here: #11022 (review) Does not affect functionality. Not required for v0.15. Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec
Summary: 67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery) 1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery) Pull request description: A couple of minor cleanups suggested by @ryanofsky here: bitcoin/bitcoin#11022 (review) Does not affect functionality. Not required for v0.15. Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec Backport of Core PR11044 bitcoin/bitcoin#11044 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D3998
67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery) 1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery) Pull request description: A couple of minor cleanups suggested by @ryanofsky here: bitcoin#11022 (review) Does not affect functionality. Not required for v0.15. Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec
67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery) 1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery) Pull request description: A couple of minor cleanups suggested by @ryanofsky here: bitcoin#11022 (review) Does not affect functionality. Not required for v0.15. Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec
67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery) 1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery) Pull request description: A couple of minor cleanups suggested by @ryanofsky here: bitcoin#11022 (review) Does not affect functionality. Not required for v0.15. Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec
67ceff4 [wallet] Add logging to MarkReserveKeysAsUsed (John Newbery) 1221f60 [wallet] Remove keypool_topup_cleanups (John Newbery) Pull request description: A couple of minor cleanups suggested by @ryanofsky here: bitcoin#11022 (review) Does not affect functionality. Not required for v0.15. Tree-SHA512: d8d0698fd26ea49a4157e68669d5511095760c3a1ecfa3f917e3f273efbafb55c51a202d677614216eae3f796b6e8d17506b2ec2d4799a94f18981b396e65eec
A couple of minor cleanups suggested by @ryanofsky here: #11022 (review)
Does not affect functionality. Not required for v0.15.