gridcoin-community / Gridcoin-Research

Gridcoin-Research
MIT License
585 stars 173 forks source link

Wallet Total Incorrect #775

Closed Krailon closed 6 years ago

Krailon commented 6 years ago

I recently noticed that in the time between coin(s) being detected and confirmed, the wallet total is displayed incorrectly. It appears to be calculating the "Total" by adding the "Available" and "Unconfirmed" values while the "Available" value already includes the "Unconfirmed" value. Please see the attached screenshot.

Relevant math for my case: Unconfirmed = 1.8378319 Available = sum(all incoming transactions) - sum(all outgoing transactions): 3 + 3.53465605 + 1.74557384 + 1.8378319 - 0.00011 = 10.11795179

Total = Available + Unconfirmed 10.11795179 + 1.8378319 = 11.95578369

At no point did I have 11.95578369 GRC. That value is simply wrong. Once the coin(s) are confirmed, the Total is displayed correctly. I took a look through the code but I don't see anything immediately obvious that would cause this in CWallet::GetBalance() or CWallet::GetUnconfirmedBalance(). The source of the issue is likely much deeper since the unconfirmed coin(s) seem to be being counted twice somehow. It doesn't seem to be a big problem since it corrects itself eventually, hopefully it's just a UX issue.

Screenshot: gridcoin wallet possible bug total grc wrong

iFoggz commented 6 years ago

there is a bug with unconfirmed balance that will be fixed in next version. where a unconfirmed balance received on fork doesnt go away. interesting from what i see i'll have a look at it again anyway. unconfirmed balance is not added to available till it passes 10 confirms. thanks for bringing this up first occurrence regarding this issue and i haven't duplicated it but will keep trying.

I can say its not the unconfirmedbalance side as it just searches and find coins that are not from you and are in the mainchain and have more then >= 10 confirms and adds those and returns them.

but thanks again for bringing this up

Think I've found a possible cause

iFoggz commented 6 years ago

774 confirmed. fix will be with the unconfirmed balance PR

Pythonix commented 6 years ago

774 has been merged to development. This can be closed I guess?