[Wallet] Bugfix: Fundrawtransaction: don’t terminate when keypool is empty #9295

1. 
   jonasschnelli commented at 12:51 pm on December 6, 2016: contributor

   Right now, CWallet::CreateTransaction() has an assertion that ensures that a key could be retrieved from the keypool.

   fundrawtransaction can run on an encrypted unlocked wallet (it actually doesn’t sign). But, it can still fail because the keypool could be drained on a locked wallet.

   Current master crashes in that case.

2. [Wallet] Bugfix: FRT: don't terminate when keypool is empty c24a4f5981
3. jonasschnelli added the label Needs backport on Dec 6, 2016
4. jonasschnelli added the label Wallet on Dec 6, 2016
5. jonasschnelli added this to the milestone 0.14.0 on Dec 6, 2016
6. MarcoFalke removed this from the milestone 0.14.0 on Dec 6, 2016
7. MarcoFalke added this to the milestone 0.12.branch on Dec 6, 2016
8. jonasschnelli added this to the milestone 0.14.0 on Dec 6, 2016
9. jonasschnelli removed this from the milestone 0.12.branch on Dec 6, 2016
10. MarcoFalke added this to the milestone 0.13.2 on Dec 6, 2016
11. MarcoFalke removed this from the milestone 0.14.0 on Dec 6, 2016
12. 
    gmaxwell commented at 5:21 pm on December 6, 2016: contributor

    utACK.
13. MarcoFalke added this to the milestone 0.12.branch on Dec 6, 2016
14. MarcoFalke removed this from the milestone 0.13.2 on Dec 6, 2016
15. 
    MarcoFalke commented at 6:26 pm on December 6, 2016: member

    Tagged for 0.12 backport because I am able to reproduce on that branch:

    0bin/bitcoin-0.12.1/bin/bitcoin-qt -regtest -datadir=/tmp 
    1bitcoin-qt: wallet/wallet.cpp:2107: bool CWallet::CreateTransaction(const std::vector<CRecipient>&, CWalletTx&, CReserveKey&, CAmount&, int&, std::string&, const CCoinControl*, bool): Assertion `ret' failed.
    2Aborted (core dumped)
    

16. 
    MarcoFalke commented at 6:26 pm on December 6, 2016: member

    utACK c24a4f5
17. doublek420 approved
18. 
    paveljanik commented at 10:53 am on December 7, 2016: contributor

    ACK https://github.com/bitcoin/bitcoin/commit/c24a4f5981d47d55aa9e4eb40294832a4d38fb80

    What about new test case (failing in the master, OK here)?

19. 
    MarcoFalke commented at 11:02 am on December 7, 2016: member

    Yes, agree with paveljanik, please add the trivial test case somewhere.

    On Wed, Dec 7, 2016 at 11:53 AM, paveljanik notifications@github.com wrote:

    ACK c24a4f5 https://github.com/bitcoin/bitcoin/commit/c24a4f5981d47d55aa9e4eb40294832a4d38fb80

    What about new test case (failing in the master, OK here)?

    — You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/9295#issuecomment-265417093, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv9w4s_o7vveUUeHjEGhWl0JRtKjTks5rFpAygaJpZM4LFX5w .

20. [QA] add fundrawtransaction test on a locked wallet with empty keypool 1a6eacbf3b
21. 
    jonasschnelli commented at 2:21 pm on December 7, 2016: contributor

    Added a commit with a test that ensures the new behaviour. Test will fail on master.
22. 
    MarcoFalke commented at 8:59 pm on December 7, 2016: member

    utACK 1a6eacbf3b7e3d5941fec1154079bbc4678ce861
23. MarcoFalke referenced this in commit 8dee97f982 on Dec 7, 2016
24. MarcoFalke referenced this in commit d6098956c3 on Dec 7, 2016
25. 
    paveljanik commented at 7:04 am on December 8, 2016: contributor

    Thank you @jonasschnelli.

    Tested ACK 1a6eacb

    Fails on master with

    0Initializing test directory /var/folders/65/fn0h49r55k7779vg1b_h461r0000gn/T/testyal7ogu7/50231
    1Mining blocks...
    2Assertion failed: (ret), function CreateTransaction, file wallet/wallet.cpp, line 2392.
    3Unexpected exception caught during testing: ConnectionRefusedError(61, 'Connection refused')
    

    OK here.

26. MarcoFalke added this to the milestone 0.13.2 on Dec 8, 2016
27. MarcoFalke removed this from the milestone 0.12.branch on Dec 8, 2016
28. 
    gmaxwell commented at 0:02 am on December 10, 2016: contributor

    ACK.
29. 
    sipa commented at 0:07 am on December 10, 2016: member

    utACK 1a6eacbf3b7e3d5941fec1154079bbc4678ce861
30. sipa merged this on Dec 10, 2016
31. sipa closed this on Dec 10, 2016

    ---

32. sipa referenced this in commit 815640ec6a on Dec 10, 2016
33. MarcoFalke referenced this in commit 0cc07f82f0 on Dec 14, 2016
34. MarcoFalke referenced this in commit 43bcfca489 on Dec 14, 2016
35. MarcoFalke removed the label Needs backport on Dec 14, 2016
36. 
    MarcoFalke commented at 12:07 pm on December 14, 2016: member

    Backports:

    • for 0.13 in #9347
    • for 0.12 in #9211
37. UdjinM6 referenced this in commit 03ddd5da55 on Mar 22, 2017
38. UdjinM6 referenced this in commit 5842b23a5a on Mar 22, 2017
39. UdjinM6 referenced this in commit 9e4de7e5f9 on Mar 22, 2017
40. UdjinM6 referenced this in commit 21e288229f on Mar 22, 2017
41. UdjinM6 referenced this in commit ff30aed68f on Apr 11, 2017
42. lateminer referenced this in commit 5deab0d5fa on Jan 6, 2018
43. thokon00 referenced this in commit 602cdf3acd on Apr 17, 2018
44. DrahtBot locked this on Sep 8, 2021

Contributors
jonasschnelli gmaxwell MarcoFalke doublek420 paveljanik sipa

Labels
Wallet

Milestone
0.13.2