fundrawtransaction: Keep change-output keys by default, make it optional #9377

1. 
   jonasschnelli commented at 8:20 am on December 19, 2016: contributor

   Alternative solution for #9370, fixes #9362.

   This adds reserveChangeKey, a new boolean option to fundrawtransaction. The default is true resulting in always removing the change output address key from the keypool.

   Includes test and documentation in the release-notes.

2. jonasschnelli added the label Wallet on Dec 19, 2016
3. 
   laanwj commented at 8:45 am on December 19, 2016: member

   Thanks! Concept ACK. We could still use #9370 in some form after this, but this is a more effective solution to the issue.
4. 
   gmaxwell commented at 2:12 am on December 20, 2016: contributor

   Concept ACK. Thanks!
5. laanwj added this to the milestone 0.14.0 on Jan 12, 2017
6. in qa/rpc-tests/fundrawtransaction.py: in 161af5b699 outdated

   678+        result3 = self.nodes[3].fundrawtransaction(rawtx)
   679+        res_dec = self.nodes[0].decoderawtransaction(result3["hex"])
   680+        changeaddress = ""
   681+        for out in res_dec['vout']:
   682+            if out['value'] > 1.0:
   683+                changeaddress += out['scriptPubKey']['addresses'][0]
   

   ---

   ---

   

   jtimon commented at 0:07 am on January 13, 2017:

   why += instead of a regular assignment? What if there’s more outputs than one with value > 1?
7. 
   jtimon commented at 0:08 am on January 13, 2017: contributor

   utACK 161af5b modulo the tests which I’m not sure I’m fully understanding. Needs rebase.
8. 
   TheBlueMatt commented at 4:25 am on January 13, 2017: member

   Needs rebase (but is a bugfix, so can go in for 0.14 after the feature freeze on Monday).
9. [Wallet] Add an option to keep the change address key, true by default 9aa4e6a6c2
10. [QA] Add test for fundrawtransactions new reserveChangeKey option 9eb325d079
11. Add fundrawtransactions new reserveChangeKey option to the release notes c9f3062d55
12. jonasschnelli force-pushed on Jan 19, 2017
13. 
    jonasschnelli commented at 7:53 pm on January 19, 2017: contributor

    Rebased.
14. 
    MarcoFalke commented at 8:36 pm on January 19, 2017: member

    utACK c9f3062d551823f006d400dddb54c12620cd29c4, but agree with the nit by @jtimon. Either you use a set/list or a single string, but please no string manipulations (concatenation) with addresses. Anyway, there is only a single coin, so you can go without the +=.
15. 
    laanwj commented at 1:31 pm on January 20, 2017: member

    Not going to block this on a test nit - that can be fixed later. I have enough confidence in this to sneak this in before the feature freeze.
16. laanwj merged this on Jan 20, 2017
17. laanwj closed this on Jan 20, 2017

    ---

18. laanwj referenced this in commit fb75cd04bb on Jan 20, 2017
19. 
    gituser commented at 2:37 pm on February 2, 2017: none

    Just a quick question to confirm: so in the latest master there is no more problem at fundrawtransaction() with address reuse?

    We hit an issue when sending transaction via fundrawtransaction change was sent to the existing user account address and thus credited our user with balance.

    sendtoaddress always creates a new change address I suppose.

20. 
    jonasschnelli commented at 3:11 pm on February 2, 2017: contributor

    We hit an issue when sending transaction via fundrawtransaction change was sent to the existing user account address and thus credited our user with balance.

    Yes. This is exactly what this PR does fix.

21. laanwj referenced this in commit 9e8d6a3fb4 on Jul 18, 2017
22. codablock referenced this in commit 80c8347082 on Jan 19, 2018
23. codablock referenced this in commit db53a4a7a4 on Jan 20, 2018
24. codablock referenced this in commit 36109e0580 on Jan 21, 2018
25. andvgal referenced this in commit 8a308b2bd5 on Jan 6, 2019
26. CryptoCentric referenced this in commit 4ed611e461 on Feb 27, 2019
27. PastaPastaPasta referenced this in commit f639a6a16f on Sep 8, 2019
28. PastaPastaPasta referenced this in commit 443b577931 on Sep 8, 2019
29. barrystyle referenced this in commit d507557171 on Jan 22, 2020
30. MarcoFalke locked this on Sep 8, 2021

Contributors
jonasschnelli laanwj gmaxwell jtimon TheBlueMatt MarcoFalke gituser

Labels
Wallet

Milestone
0.14.0