Use CCoinControl selection in CWallet::FundTransaction #7506

pull promag wants to merge 1 commits into bitcoin:master from uphold:enhancement/use-coin-control-selection changing 3 files +5 −15

promag commented at 1:09 AM on February 11, 2016: member

Trivial improvement.
laanwj added the label Wallet on Feb 11, 2016
promag commented at 9:23 AM on February 11, 2016: member

@laanwj is this acceptable for the 0.12 release cycle?
laanwj commented at 9:30 AM on February 11, 2016: member

Unless it fixes a critical bug, no. This will be reviewed and merged for 0.13.
dcousens commented at 2:03 PM on February 11, 2016: contributor

utACK 8310baf
TheBlueMatt commented at 9:23 PM on February 11, 2016: member

Matt was here
MarcoFalke commented at 2:13 PM on February 13, 2016: member

utACK 8310baf
in src/wallet/wallet.cpp:None in 8310bafdf8 outdated
```
1937 | -                found = true;
1938 | -                break;
1939 | -            }
1940 | -        }
1941 | -        if (!found)
1942 | +        if (!coinControl.IsSelected(txin.prevout))
```
laanwj commented at 9:11 AM on February 17, 2016:

Is the old and new code equivalent here? To me it looks like coinControl.IsSelected and the old code check for different things: IsSelected checks coincontrol.setSelected, whereas the old code just looks at wtx and tx. Can you explain?

promag commented at 9:33 AM on February 17, 2016:

Yes they are. The only difference is that the old code performs a linear search and reimplements the COutPoint::operator== inline.

coinControl.setSelected is filled with the current inputs, which are the ones to search when the inputs are added, otherwise the same input would be duplicated.

laanwj commented at 10:07 AM on February 17, 2016:

Thanks for the explanation, makes sense to me. I missed the part where setSelected was prepopulated.
jonasschnelli added the label GUI on Feb 19, 2016
jonasschnelli commented at 4:21 PM on February 19, 2016: contributor

Do I understand this correctly: this only make sense if you use the GUI (CoinControl enabled) and fundrawtransaction (over RPC with the -server option or over the GUI console)?

Although looking at this PR from an angle where CCoinControl is a core-class (not tied to the GUI) I think this would make sense.

What real-world use cases would solve this?
promag commented at 4:49 PM on February 19, 2016: member

@jonasschnelli this is only an enhancement of the CWallet::FundTransaction implementation.
jonasschnelli commented at 8:10 AM on February 20, 2016: contributor

@promag: yes. I saw that this only affects fundrawtransaction. But IIRC, CoinControl can only be used over the GUI. So does that mean, that this change only affects users who uses the GUI together with fundrawtransaction (console/RPC)? What usecases would be possible with this change?
promag commented at 9:36 AM on February 20, 2016: member

fundrawtransactionwas already using CCoinControl.
promag commented at 1:15 AM on February 25, 2016: member

@jonasschnelli maybe CCoinControl should be considered core/wallet and not gui?
jonasschnelli commented at 7:53 AM on February 25, 2016: contributor

@promag: but there is no functionality/possibility to use CoinControl in core/wallet (non gui)?
promag commented at 10:17 AM on February 25, 2016: member

Maybe I'm not understanding what you're saying. I'm just saying fundrawtransaction is a wallet RPC command and it uses CCoinControl.
jonasschnelli commented at 10:32 AM on February 25, 2016: contributor

utACK
jonasschnelli commented at 12:25 PM on March 8, 2016: contributor

@promag: needs a (force)push for a travis re-trigger.
Use CCoinControl selection in CWallet::FundTransaction d6cc6a1830
promag force-pushed on Mar 8, 2016
jtimon commented at 6:03 PM on March 16, 2016: contributor

utACK
promag commented at 12:08 PM on March 24, 2016: member

@laanwj bump.
laanwj merged this on Mar 24, 2016
laanwj closed this on Mar 24, 2016
laanwj referenced this in commit b88e0b0c61 on Mar 24, 2016
codablock referenced this in commit 2593f7fd43 on Sep 7, 2017
UdjinM6 referenced this in commit 9707ca5cea on Sep 9, 2017
furszy referenced this in commit 0724bbbad2 on Jun 28, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels