This pr is supposed to speed up listunspent/getbalance calls.
It introduces a vector vWallet, which contains pointers to wallet transactions. Only transactions which potentially contain unspent outputs are added to the vector.
A transaction is only added to the vector if it has at least one output that is ours, and the output is either unspent, or spent but the spending tx has less than 100 confirmations.
This feels like a hack fix to me, which might be OK… but eww.
So vWallet is memory-only, added to at startup and by AddToWallet, but transactions are never removed from it– so performance will very slowly get worse until you restart. CWallet::SetBestChain could erase entries from vWallet…
I think the best fix would be to change mapWallet into a boost::multi_index_container with two or three indices: by txid, by (spendable, nOrderPos), and maybe by (spendable, account, nOrderPos).
The tricky bit would be keeping the spendable bits up-to-date during blockchain re-orgs; extensive regression tests would be needed to make sure that isn’t screwed up. If this pull request’s “ain’t never gonna be a 100-block-long re-org” assumption is OK, then a multi_index_container solution could have a similar definition of “spendable” and sidestep that work.
Well, its for sure always much faster than what we have now. Looping through a vector is like 10 times faster than looping through a map. The vector can never have more entries than the map. And the vector endlessly growing if you dont restart, is not worse than looping through our whole map all the time, what we do now. So this is for sure an improvement regarding performance.
But I agree on little hacky, and that having just one multi_index_container would be cleaner. On the other hand, the columns account, nOrderPos are not needed, if #4702 gets merged. And switching to multi_index_container just for one column spendable is not really worth it. Maybe even make our own class “wallet-container”, which encapsulates the vector and the map with iterators etc.
But closing for now. Lets see, what happens to my other wallet pull requests, then I may reconsider this.