Move CWallet::setKeyPool to private section of CWallet. #8445

pull pstratem wants to merge 2 commits into bitcoin:master from pstratem:2016-07-01-cwallet-api-cleanup changing 4 files +26 −18
  1. pstratem commented at 12:18 AM on August 2, 2016: contributor

    Further encapsulation.

  2. in src/init.cpp:None in 674b2dec4a outdated 
    1438 | @@ -1439,7 +1439,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
1439 |      LogPrintf("mapBlockIndex.size() = %u\n",   mapBlockIndex.size());
1440 |      LogPrintf("nBestHeight = %d\n",                   chainActive.Height());
1441 |  #ifdef ENABLE_WALLET
1442 | -    LogPrintf("setKeyPool.size() = %u\n",      pwalletMain ? pwalletMain->setKeyPool.size() : 0);
1443 | +    LogPrintf("setKeyPool.size() = %u\n",      pwalletMain ? pwalletMain->GetKeyPool().size() : 0);
    jonasschnelli commented at 7:44 AM on August 2, 2016:

    does copy the std::set<int64_t> but no keys are copied around. Seems to be okay.

    sipa commented at 11:24 PM on August 2, 2016:

    This will copy the entire set. Perhaps a GetKeyPoolSize()?

  3. jonasschnelli commented at 7:45 AM on August 2, 2016: contributor

    utACK 674b2dec4a9a1ebbdb03d6eac0f4c858ca8793ba

  4. jonasschnelli added the label Refactoring on Aug 2, 2016
  5. jonasschnelli added the label Wallet on Aug 2, 2016
  6. in src/wallet/wallet.h:None in 674b2dec4a outdated 
     600 | +    std::set<int64_t> GetKeyPool()
 601 | +    {
 602 | +        return setKeyPool;
 603 | +    }
 604 | +
 605 | +    void LoadKeyPool(int nIndex, CKeyPool &keypool)
    paveljanik commented at 7:46 AM on August 2, 2016:

    const keypool?

    phantomcircuit commented at 1:07 AM on August 3, 2016:

    Indeed this should be const

  7. pstratem force-pushed on Aug 8, 2016
  8. pstratem commented at 2:59 AM on August 8, 2016: contributor

    nits picked

  9. MarcoFalke commented at 7:20 AM on August 8, 2016: member

    Fails with

    Assertion failed: lock cs_wallet not held in ../../src/wallet/wallet.h:849; locks held:
  10. phantomcircuit commented at 10:39 PM on August 9, 2016: none

    Indeed it does, the logic in master should be acquiring that lock but is not...

    I will correct this oversight.

    On Aug 8, 2016 12:21 AM, "MarcoFalke" notifications@github.com wrote:

    Fails with

    Assertion failed: lock cs_wallet not held in ../../src/wallet/wallet.h:849; locks held:

    — You are receiving this because you commented. Reply to this email directly, view it on GitHub #8445 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AAlvMVibAOtkoEciHx4xar3TKzTbnD44ks5qdtjdgaJpZM4JaH5I .

  11. pstratem force-pushed on Aug 17, 2016
  12. pstratem force-pushed on Aug 17, 2016
  13. Move CWallet::setKeyPool to private section of CWallet e86eb71604
  14. pstratem force-pushed on Aug 17, 2016
  15. pstratem commented at 5:10 AM on August 18, 2016: contributor

    @MarcoFalke I've fixed that (indeed we were accessing the private class members without the proper lock before).

    Ignore the travis error that's a travis error but a pr error.

  16. in src/init.cpp:None in e86eb71604 outdated 
    1438 | @@ -1439,9 +1439,12 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
1439 |      LogPrintf("mapBlockIndex.size() = %u\n",   mapBlockIndex.size());
1440 |      LogPrintf("nBestHeight = %d\n",                   chainActive.Height());
1441 |  #ifdef ENABLE_WALLET
1442 | -    LogPrintf("setKeyPool.size() = %u\n",      pwalletMain ? pwalletMain->setKeyPool.size() : 0);
1443 | -    LogPrintf("mapWallet.size() = %u\n",       pwalletMain ? pwalletMain->mapWallet.size() : 0);
1444 | -    LogPrintf("mapAddressBook.size() = %u\n",  pwalletMain ? pwalletMain->mapAddressBook.size() : 0);
1445 | +    if (pwalletMain) {
    MarcoFalke commented at 6:21 PM on August 18, 2016:

    Nit: if (!fDisableWallet) works as well. No strong opinion.

    Weak NACK: The goal was to move them into the wallet module. See #7965

  17. MarcoFalke commented at 6:22 PM on August 18, 2016: member

    Concept ACK e86eb71604e73ad35b8b1f59d73af22e353a156e

  18. pstratem commented at 10:54 PM on August 19, 2016: contributor

    @MarcoFalke This should mostly address your concern.

  19. pstratem force-pushed on Aug 20, 2016
  20. MarcoFalke commented at 6:34 AM on August 20, 2016: member

    Missing lock:

    Assertion failed: lock cs_wallet not held in ../../src/wallet/wallet.h:849; locks held:
  21. pstratem force-pushed on Aug 20, 2016
  22. Move wallet initialization logic from AppInit2 to CWallet::InitLoadWallet 8680d3aa80
  23. pstratem force-pushed on Aug 20, 2016
  24. pstratem commented at 10:00 PM on August 20, 2016: contributor

    @MarcoFalke something weird and broken was happening there, I've reduced the scope in which the lock is held and now it works fine...

  25. MarcoFalke commented at 11:37 AM on August 21, 2016: member

    utACK 8680d3aa800aa4fab8c321b92cc7f3fa97a8a0a3

  26. sipa commented at 11:52 AM on August 23, 2016: member

    utACK 8680d3aa800aa4fab8c321b92cc7f3fa97a8a0a3

  28. laanwj merged this on Aug 24, 2016
  29. laanwj closed this on Aug 24, 2016
  30. laanwj referenced this in commit f9167003d9 on Aug 24, 2016
  31. codablock referenced this in commit 5efc15dffe on Sep 19, 2017
  32. codablock referenced this in commit 52cbe60f94 on Jan 9, 2018
  33. codablock referenced this in commit d97fa0402f on Jan 9, 2018
  34. andvgal referenced this in commit e0aa5d802f on Jan 6, 2019
  35. DrahtBot locked this on Sep 8, 2021
Contributors
pstratemjonasschnellisipapaveljanikphantomcircuitMarcoFalke
Labels
RefactoringWallet
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-19 00:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me