Skip to content

Conversation

jnewbery
Copy link
Contributor

A couple of minor cleanups suggested by @ryanofsky here: #11022 (review)

Does not affect functionality. Not required for v0.15.

@jnewbery jnewbery mentioned this pull request Aug 14, 2017
@maflcko maflcko added this to the 0.15.0 milestone Aug 14, 2017
@maflcko
Copy link
Member

maflcko commented Aug 14, 2017

Tagged for backport if there is an rc2, as this fixes a confusing warning when building from source.

@paveljanik
Copy link
Contributor

Warnings removed.

ACK 61aa92d

@jonasschnelli
Copy link
Contributor

utACK 61aa92d9a854677cafefb15117c3d83d6bab6af6

@promag
Copy link
Contributor

promag commented Aug 16, 2017

Log message is consistent with others related.

Nit, I would reword 063aab4 to Remove unused CWallet::HasUnusedKeys since it doesn't really revert c25d90f, but keep the body.

ACK 61aa92d.

@sipa
Copy link
Member

sipa commented Aug 16, 2017

utACK 61aa92d9a854677cafefb15117c3d83d6bab6af6

Unused function. Mostly reverts c25d90f

c25d90f... was merged as part of PR 11022 but is not required.
@jnewbery jnewbery force-pushed the keypool_topup_cleanups branch from 61aa92d to 67ceff4 Compare August 16, 2017 21:23
@jnewbery
Copy link
Contributor Author

Thanks @promag . I've changed the commit message, but contents of the commit are the same. Check that 61aa92d == 67ceff4

@@ -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);
Copy link
Member

@laanwj laanwj Aug 18, 2017

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)

Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok...

@promag
Copy link
Contributor

promag commented Aug 18, 2017

ACK 67ceff4.

@laanwj laanwj merged commit 67ceff4 into bitcoin:master Aug 18, 2017
laanwj added a commit that referenced this pull request Aug 18, 2017
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
laanwj pushed a commit that referenced this pull request Aug 21, 2017
Unused function. Mostly reverts c25d90f

c25d90f... was merged as part of PR 11022 but is not required.

Github-Pull: #11044
Rebased-From: 1221f60
Tree-SHA512: da229b128bee5f124c009a1a2adfb4fa879366c81789824c426c9ce5209c835888a7e6cfeb1724551320a98cd08406a605372f84487a0d289cd6e02f9ac3ea21
laanwj pushed a commit that referenced this pull request Aug 21, 2017
Github-Pull: #11044
Rebased-From: 67ceff4
Tree-SHA512: 850c5b1010c84e164edf24a83ae36e46309b2eb7a67854bad509265ed590ba67d5f743a8416590da6ecca85fe4bda7f20e8c3152e422638eb7898db11a416af7
@jnewbery jnewbery deleted the keypool_topup_cleanups branch May 23, 2018 21:18
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 13, 2019
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 19, 2019
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 20, 2019
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 20, 2019
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
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants