As no one responds in #1432, I decided to create a pull for further discussion!
- add GetImmatureCredit() function in CWalletTx
- include clientModel in overviewpage and sendcoinsdialog
- rename model to walletModel in overviewpage and sendcoinsdialog
- update displayed balances when numBlocksChanged() SIGNAL occurs (as
an immature balance matures via new blocks and not new Tx) on
overviewpage and sendcoinsdialog
- optimize handling of cached balances in walletmodell
It's hard to test when testnet isn't working that good on my computer.
I don't think this is the right solution. The view classes (Overviewpage, SendCoinsDialog, ...) should not be burdened with implementation details such as new blocks coming in. As I understand, there is a problem with WalletModel not sending the BalanceUpdated signal every time the Balance changes. Hence, this should be fixed in WalletModel.
The problem seems to be that some kind of transactions (the "mined" ones) need to be baby-sitted for at least 120 blocks until they mature (normal transactions for 6, but this is correctly handled). However, re-computing the balance and updating the UI every time a block comes in also isn't a good solution as this may go very fast. This needs a timer (invoked only a few times per second) that recomputes the balance and issues a BalanceChanged signal if needed...
Tangential: Can you try to keep 1) changes in which you rename all kinds of stuff and 2) functional changes separate? Mixing these makes it very hard to review code changes. Commits of type 1 can be verbose, whereas commits of type 2 should be as minimal as possible, especially if you make changes to the core as well as the UI.
@burger2 To fix your testnet problems try out #1477. Current master has a bug, which gives all testnet nodes on IRC the same NICK, which causes the current testnet3 troubles.
@laanwj I think the immature balance, which then matures after 120 blocks is not taken correctly into account in terms of the balanceChanged signal. The state change from immature to mature, which also changes the current balance then, is not triggered by a transaction, so the emited balanceChanged signal in WalletModel::updateTransaction() does not help here. That's why I introduced a mechanism, which uses block-changes for this. If you have a cleaner / better idea I'm fine with a simpler patch :). The re-name was a logical thing for me here, because there are now 2 models in overviepage and sendcoinsdialog, so only e.g. setModel is missleading. I guess a timer is not needed if I add a check for IsInitialBlockDownload() in the code, as with a catched-up chain the processing won't happen that often.
Yes, I'm going to implement this differently. This should really be restricted to WalletModel. It's indeed true that the balanceChanged signal in updateTransaction does not help here, but it could be emitted from different places as well. Having a non-functional balanceChanged signal and working around that means confusion for future updates, you should simply be able to connect to balanceChanged and have the most recent balance.
Then, OverviewPage and SendcoinsDialog don't need the client model and everything there can be kept the same.
That's fine, if you are currently working on a patch I'm going to stop further progress here. What about the changes to wallet and walletmodel? I'll quote myself:
I'm sure the current call to GetCredit(pcoin) can't be correct, as it does:
<pre> if (IsCoinBase() && GetBlocksToMaturity() > 0) return 0; </pre>
And this is the opposite of what we need for the immature coins, right?
I'm not particularly known with that part of the code, but what do you think is wrong? To me, it reads like "If this is a coin-base (mined), and there are more than 0 blocks left to maturity, the credit is 0". Which makes sense to me.
It does a return 0; for the conditions we want to be true for adding immature coins together. Need to double check...
The current master code uses GetCredit() from CWallet, while I added GetImmatureCredit() into CWalletTx, which I think fit's better in there, as it's similar to CWallet::GetBalance() and CWallet::GetUnconfirmedBalance(). But you are probably right and the current code works.
The change in WalletModel also makes sense (makes it somewhat more readable). The only thing I don't agree with are the changes in the views.
That's fine, I'll open a new one and you patch the signal thing :).