GetBalance fix

[iFoggz]

Here is a simple fix to issue #1175

What happened in GUI is we check if IsTrusted() which in the case where if the input fFromMe its automatically trusted as it in a in wallet transaction only allowing u to use those inputs without the minimum required confirmations.

With sendtoaddress it pulls data from GetBalance()

In GetBalance() it does use IsTrusted() but also checks is IsConfirmed() (requires 10 confirmations to be true) this breaks the rule allowance from IsTrusted() resulting in the issue quez had in #1175 . The fix is to use (pcoin->IsConfirmed() || pcoin->fFromMe) which respects the 10 confirmation required from received transactions from outside the wallet while respecting the in wallet -> in wallet transactions.

This could also explain the occasional erroneous balance numbers when the case of available balance + unconfirmed balance != Total Balance. As unconfirmed does not include fFromMe as the rule considers it trusted as it is in-wallet->in-wallet transaction. and Available Balance did not show the right value as it did not include the in-wallet->in-wallet transaction.

Ive tested both scenarios with complete success.

1. sent a tx to my test wallet and tried to sendtoaddress with that being less then 10 confirmations and resulted in a fail.
2. sent a tx in-wallet -> in-wallet with less the 3 confirmations and worked correctly as it should
3. getbalance rpc now shows the available balance in wallet properly to include if it is a in-wallet->in-wallet transaction while respecting incoming transactions from foreign addresses as unconfirmed. and gui balance shows correctly as well now.

[iFoggz]

note IsTrusted() would of returned false if a in-wallet->in-wallet transaction ended up conflicted as confirms -1 are no trusted so no worries about that side

[jamescowens]

@denravonska I believe this applies to more than just the GUI and also fixes one of the three core issues remaining with Bittrex. I just tested it and it appears to fix the short term balance hole problem. (The tests Paul did above and my tests cover the situation that would have affected Bittrex and caused the balance trip.) We need to merge this to staging.

[denravonska]

It looks like they moved these conditions into IsTrusted in the Bitcoin tree. That might have a larger implication but it combines all the conditions into one place. Blackcoin does the same thing.

[iFoggz]

Will look into this also they use different enum as well

…

On Sat, Jul 7, 2018, 5:43 AM Marco Nilsson, ***@***.***> wrote: It looks like they moved these conditions into IsTrusted in the Bitcoin tree <https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2058>. That might have a larger implication but it combines all the conditions into one place. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1192 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMrVQNu73ev51XgXJJ7YkuGqG2L2o-jIks5uEKzvgaJpZM4U4yia> .

[iFoggz]

ive looked at the tree we would have to add a condition thou since with bitcoin its one 1 confirm to be confirmed period. so likely
if (nDepth >= 1)
return true;
would become something around:

`if (nDepth >= 1 && IsFromMe)
return true;

if (nDepth >= 10)
return true;
`

the rest of bitcoins code there evaluates when confirms is 0

[jamescowens]

Let's stick to a narrow change for this PR. We need to get this in production to fix one of the Bittrex issues. This really needs to go in Annie. We can align Gridcoin to Bitcoin's enum and other changes in a later milestone, where we have more time to test the implications.

[denravonska]

Can you rebase this to staging?

[TheCharlatan]

Would like to see it in IsTrusted as well though.

[iFoggz]

Crap was gonna look into that today more but got distracted by msi

…

On Tue, Jul 17, 2018, 10:47 PM Marco Nilsson, ***@***.***> wrote: Merged #1192 <#1192>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1192 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMrVQNfxgI62LTqPE7FzDU-LvN8tZ4w6ks5uHswIgaJpZM4U4yia> .