Force distinct destinations in CWallet::CreateTransaction #12419

1. 
   promag commented at 1:20 am on February 13, 2018: member

   Check that all transaction destinations, including change address if specified, are distinct.

   The error is also raised in the UI:

2. promag renamed this:
   wallet: Force distinct destinations in CWallet::CreateTransaction
   Force distinct destinations in CWallet::CreateTransaction
   on Feb 13, 2018
3. fanquake added the label Wallet on Feb 13, 2018
4. 
   promag commented at 1:21 am on February 13, 2018: member

   A similar check exists in sendmany but at the moment the errors are different. Not sure it should change there or try to make this equal (the problem is that the errors in CWallet are translated and RPC errors aren’t).
5. 
   laanwj commented at 10:23 am on February 13, 2018: member

   Concept ACK. I think this should be enforced consistently.
6. 
   promag commented at 11:19 am on February 13, 2018: member

   Can you be more specific @laanwj?
7. 
   laanwj commented at 1:32 pm on February 13, 2018: member

   Huh? No, I mean that exactly what this PR does, checking it in CreateTransaction is good.
8. 
   promag commented at 3:13 pm on February 13, 2018: member

   Ah ok, misunderstood you.
9. 
   jonasschnelli commented at 7:46 am on February 17, 2018: contributor

   utACK e82d17991418bdc3cfe60407a66735c265f8cd21
10. 
    Sjors commented at 3:47 pm on February 20, 2018: member

    Concept ACK, but now there’s two different error messages depending on whether the duplicate is a change address or a regular destination:

11. 
    promag commented at 4:43 pm on March 6, 2018: member

    @Sjors I guess we could remove the old check?
12. 
    promag commented at 5:57 pm on March 6, 2018: member

    Should all checks for duplicate addresses be removed?
13. 
    Sjors commented at 10:13 pm on March 6, 2018: member

    @promag I suggest reusing the original warning.
14. 
    Empact commented at 6:44 am on May 21, 2018: member

    Concept ACK, agree the original warning is more clear.
15. DrahtBot closed this on Jul 20, 2018

    ---

16. 
    DrahtBot commented at 8:29 pm on July 20, 2018: member
17. DrahtBot reopened this on Jul 20, 2018

    ---

18. 
    ken2812221 commented at 1:44 pm on September 18, 2018: contributor

    Concept ACK
19. 
    meshcollider commented at 1:48 am on November 11, 2018: contributor

    This looks good but the comments above need addressing (feel free to cherry-pick from my 201811_duplicate_dest_check branch)

    EDIT: This will also prevent fundrawtransaction from funding a transaction with duplicate outputs, correct? Is that what we want? If someone has gone to the effort of creating a transaction with duplicate outputs manually, maybe we should allow it there?

20. DrahtBot added the label Needs rebase on Apr 27, 2019
21. promag force-pushed on May 21, 2019
22. wallet: Force distinct destinations in CWallet::CreateTransaction

    Check that all transaction destinations, including change address
    if specified, are distinct.

    04f45ea52c
23. promag force-pushed on May 21, 2019
24. DrahtBot removed the label Needs rebase on May 21, 2019
25. 
    promag commented at 10:59 am on June 27, 2019: member

    Closing for now, as @meshcollider mentions, this approach breaks the following case:

    https://github.com/bitcoin/bitcoin/blob/3077f11dadffbc8f2575449fc0177c91a9c3e046/test/functional/wallet_groups.py#L76-L83

26. promag closed this on Jun 27, 2019

    ---

27. promag deleted the branch on Jun 27, 2019
28. DrahtBot locked this on Dec 16, 2021

Contributors
promag laanwj jonasschnelli Sjors Empact DrahtBot ken2812221 meshcollider

Labels
Wallet