Skip to content

Conversation

@promag
Copy link
Contributor

@promag promag commented Mar 8, 2016

CWallet::CreateTransaction goes through several iterations to select the best coins and fee to fulfil the request. In each iteration the set of available coins is computed.

This PR moves the computation of available coins to the begin and reuses it in each iteration.

@jonasschnelli
Copy link
Contributor

Nice!
utACK d9668e7b87a34c7b8100a565a6ebd991cf03ef26.

Makes sense because in master, the current repetitive calls to SelectCoins() are locked with cs_main and cs_wallet. During the loops its impossible to have a different AvailableCoins set.

@maflcko
Copy link
Member

maflcko commented Mar 8, 2016

utACK d9668e7

@sipa
Copy link
Member

sipa commented Mar 9, 2016

ACK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please use std::vector for consistency and because we're striving to remove using namespace std at some point

@jtimon
Copy link
Contributor

jtimon commented Mar 16, 2016

utACK besides @laanwj 's nit.

@promag promag force-pushed the enhancement/prevent-multiple-calls-availablecoins branch from d9668e7 to bb16c88 Compare March 22, 2016 08:44
@promag
Copy link
Contributor Author

promag commented Mar 22, 2016

@laanwj fixed nit and rebased.

@morcos
Copy link
Contributor

morcos commented Mar 22, 2016

utACK bb16c88

i like it!

@laanwj laanwj merged commit bb16c88 into bitcoin:master Mar 23, 2016
laanwj added a commit that referenced this pull request Mar 23, 2016
bb16c88 Prevent multiple calls to CWallet::AvailableCoins (João Barbosa)
@ruimarinho ruimarinho deleted the enhancement/prevent-multiple-calls-availablecoins branch March 28, 2016 00:18
codablock pushed a commit to codablock/dash that referenced this pull request Dec 19, 2017
bb16c88 Prevent multiple calls to CWallet::AvailableCoins (João Barbosa)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants