search

navcoin / navcoin-core

bitcoin-core 0.13 fork ported for NavCoin
MIT License
123 stars 92 forks source link

Wallet balance sometimes is wrong #436

Closed mxaddict closed 5 years ago

mxaddict commented 5 years ago

Wallet balance sometimes is wrong, usually happens right after a stake.

Restarting the wallet seems to fix the issue.

If I get more info on how to replicate, I will update this issue.

I tried searching the open issues and seems no one else has reported this.

mxaddict commented 5 years ago

PS: I'm not sure how to get more info about the current balance of the wallet without actually giving my wallets keys.

If anyone can suggest how to get more debug info, I will gladly do it, I just don't know how as of the moment.

dy5es41 commented 5 years ago

Could you give a few more heuristics on your problem? Where are you reading the balance that is incorrect? From the GUI or from the CLI? What is the incorrect balance showing? Is it a random number or is there a pattern to it?

Feel free to upload a debug.log but I suspect it is a graphical error

mxaddict commented 5 years ago

The incorrect balance is reported both on the UI as well ask the CLI

I have a wlet running on an Ubuntu box (navcoind)

As well navcoin-qt on desktop.

It happens randomly after staking.

Next time it happens I'll get some screenshots for desktop and some CLI output for navcoind instance.

On Wed, Apr 10, 2019, 04:48 dy5es41 notifications@github.com wrote:

Could you give a few more heuristics on your problem? Where are you reading the balance that is incorrect? From the GUI or from the CLI? What is the incorrect balance showing? Is it a random number or is there a pattern to it?

Feel free to upload a debug.log but I suspect it is a graphical error

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NAVCoin/navcoin-core/issues/436#issuecomment-481429674, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAwKRtjI7JS19YpDLLHU_WyKSuLvEc0ks5vfPycgaJpZM4clM0L .

mxaddict commented 5 years ago

I'll get the debug.log as soon as I can get on my desktop.

On Wed, Apr 10, 2019, 05:58 Barry Deeney mxaddict@codedmaster.com wrote:

The incorrect balance is reported both on the UI as well ask the CLI

I have a wlet running on an Ubuntu box (navcoind)

As well navcoin-qt on desktop.

It happens randomly after staking.

Next time it happens I'll get some screenshots for desktop and some CLI output for navcoind instance.

On Wed, Apr 10, 2019, 04:48 dy5es41 notifications@github.com wrote:

Could you give a few more heuristics on your problem? Where are you reading the balance that is incorrect? From the GUI or from the CLI? What is the incorrect balance showing? Is it a random number or is there a pattern to it?

Feel free to upload a debug.log but I suspect it is a graphical error

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NAVCoin/navcoin-core/issues/436#issuecomment-481429674, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAwKRtjI7JS19YpDLLHU_WyKSuLvEc0ks5vfPycgaJpZM4clM0L .

aguycalled commented 5 years ago

the debug.log file can reveal your stakes and therefor your address. does it happen after a normal stake or only after orphans? does it get fixed just restarting the wallet or do you need to repair the wallet (restarting with -zapwallettxes)?

mxaddict commented 5 years ago

No repair required, just closing and reopening the wallet fixes the problem. So I'm assuming that after reading the index it calculates and reads the correct unspent inputs.

Yes, sharing the public address is fine with me.

On Wed, Apr 10, 2019, 06:03 alex v. notifications@github.com wrote:

the debug.log file can reveal your stakes and therefor your address. does it happen after a normal stake or only after orphans? does it get fixed just restarting the wallet or do you need to repair the wallet (restarting with -zapwallettxes)?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NAVCoin/navcoin-core/issues/436#issuecomment-481457241, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAwKTBLGreGBSK3RyQhYLkxxRVNqo8Mks5vfQ5BgaJpZM4clM0L .

mxaddict commented 5 years ago

Ok, i checked the debug.log

I found something strange:


cat debug.log | grep AddToWallet
2019-04-01 10:17:38 AddToWallet 3dfe9e3728af588e9610eeed43bbba8784dff00b1f3f5ca3818858c16c818d9b  new
2019-04-01 10:18:25 AddToWallet 3dfe9e3728af588e9610eeed43bbba8784dff00b1f3f5ca3818858c16c818d9b
2019-04-01 10:18:25 AddToWallet 3dfe9e3728af588e9610eeed43bbba8784dff00b1f3f5ca3818858c16c818d9b  update
2019-04-01 20:18:08 AddToWallet 43871ea56a7e2c15ca458a9072c84b0abe1cd9226406be6b1cfdb0aa0702d223  new
2019-04-01 20:18:08 AddToWallet 43871ea56a7e2c15ca458a9072c84b0abe1cd9226406be6b1cfdb0aa0702d223
2019-04-01 20:18:40 AddToWallet 43871ea56a7e2c15ca458a9072c84b0abe1cd9226406be6b1cfdb0aa0702d223  update
2019-04-01 20:18:52 AddToWallet 89195aea43cd5a3a8d8a366dfa35a1dc706a77842e69c6696918e8060088f142  new
2019-04-01 20:18:54 AddToWallet 89195aea43cd5a3a8d8a366dfa35a1dc706a77842e69c6696918e8060088f142
2019-04-01 20:19:13 AddToWallet 89195aea43cd5a3a8d8a366dfa35a1dc706a77842e69c6696918e8060088f142  update
2019-04-02 11:55:44 AddToWallet 5a49da60fe11d620dfce858ae1799ff0666ab7724353e3cd22801274922e9783  new
2019-04-02 11:56:46 AddToWallet 5a49da60fe11d620dfce858ae1799ff0666ab7724353e3cd22801274922e9783
2019-04-02 14:39:28 AddToWallet 4f5cb885859062949873adfba0efbfdda159b7e944d74343b0b8e539d7495718  new
2019-04-03 16:06:40 AddToWallet f06c83a2576223ee3e18325d8b23c00555f8c79ccae03c921fb35e4b047fcf29  new
2019-04-03 16:06:56 AddToWallet f06c83a2576223ee3e18325d8b23c00555f8c79ccae03c921fb35e4b047fcf29
2019-04-04 20:23:12 AddToWallet 2d5da54533aaded4cdcc194f95f0895879978b078720eeeeecf70f77ab57dd05  new
2019-04-04 23:24:00 AddToWallet 26863c0f4dba092a3551ca5c3427794d7b0d18b0e6251b2c1a3bdd476d8fc350  new
2019-04-04 23:24:17 AddToWallet 26863c0f4dba092a3551ca5c3427794d7b0d18b0e6251b2c1a3bdd476d8fc350
2019-04-05 05:17:52 AddToWallet 9dae5d4c4287ec976a999c00bb4a416a1f6f110f1129462ad6337c809c31d248  new
2019-04-05 12:09:52 AddToWallet dad49a0e1a78dc99c8bbc3136397b145fb75b0d9842989d3e411a7f10108ac5b  new
2019-04-08 18:02:08 AddToWallet 39ea8febbde77006f60920847b14a4b4a142643db3eeaab5dd179f803487fdde  new
2019-04-08 20:41:04 AddToWallet 0cf90fe6879937977d70569f97a2fa73ec024e86c3a392d04df38bc60170abda  new
2019-04-08 23:42:08 AddToWallet 76ef6e290976cbe62ae2246da8625f1c9704c312144b598b8c4cc2dfae0f01fa  new
2019-04-08 23:42:41 AddToWallet 76ef6e290976cbe62ae2246da8625f1c9704c312144b598b8c4cc2dfae0f01fa
2019-04-09 00:38:08 AddToWallet 3a5e0ff2960510bc6b9d0963a9575a4db318d1a12bda8235e0db868b9caad962  new
2019-04-09 18:46:56 AddToWallet a5ba24c26d9f24fee6557eb52bebd1cb5b572119d4a55c195df16384f3ffe66c  new
``
mxaddict commented 5 years ago

As you can see, the wallet is adding the new transaction multiple times, which I think is why the balance is wrong.

mxaddict commented 5 years ago

I'm assuming this should not be the case.

aguycalled commented 5 years ago

when the balance is wrong, is it through excess or defect? could you share the whole log? if you don't want to post it here, you can send it to alex@nav.community

mxaddict commented 5 years ago

The balance is in excess, basically it doubles the balance of the last staked block, some times tipples.

For example, if the balance wa 10,000 NAV (with 10 unspent inputs of 1000 NAV each)

What will sometimes happen is the balance should become 10002, but instead it will become 12004 meaning the new input which would be 1000 + 2 staked got added to the balance twice.

On Wed, Apr 10, 2019, 06:24 alex v. notifications@github.com wrote:

when the balance is wrong, is it through excess or defect? could you share the whole log with me? alex@nav.community

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NAVCoin/navcoin-core/issues/436#issuecomment-481462701, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAwKUqbHYOPvEU1E4BT4sGtEMN5QdmBks5vfRMMgaJpZM4clM0L .

mxaddict commented 5 years ago

I did some digging, and I think I found the issue in this snippet of code:

https://github.com/NAVCoin/navcoin-core/blob/404d85f8ea65bf764d3fa681a4d1483c3e72c507/src/wallet/wallet.cpp#L1354-L1374

Somehow the new transaction is not an Insert and Is not an Update

So the code does not run if (!pwalletdb->WriteTx(wtx))

But it still runs the rest of the code below.

I suggest maybe updating the write section of the code to this:


// Check if we need to write to disk
if (!fInsertedNew && !fUpdated) {
    return false;
}

// Write to disk
if (!pwalletdb->WriteTx(wtx)) {
    return false;
}
aguycalled commented 5 years ago

there are cases where the transaction is not new nor updated (just the same tx being tried to be added again). i think the issue is somewhere else, i'll share some binaries for you to test

mxaddict commented 5 years ago

there are cases where the transaction is not new nor updated (just the same tx being tried to be added again). i think the issue is somewhere else, i'll share some binaries for you to test

This is exactly the cause, the TX is being send to NotifyTransactionChanged(this, hash, fInsertedNew ? CT_NEW : CT_UPDATED); when it should not.

As well as running wtx.MarkDirty() even when this does not need to happen, since the same TX was attempted to be added when it did not need to be.

mxaddict commented 5 years ago

Can we atleast agree that this code:

https://github.com/NAVCoin/navcoin-core/blob/404d85f8ea65bf764d3fa681a4d1483c3e72c507/src/wallet/wallet.cpp#L1361-L1374

Should not be run if the tx is not new and is not an update?

aguycalled commented 5 years ago

i don't agree with your statement. you can check how bitcoin also runs this code even if the tx is not new or updated -https://github.com/bitcoin/bitcoin/blob/0.14/src/wallet/wallet.cpp#L978

if you look at what the code does you will see it can't cause a transaction to be added twice. NotifyTransactionChanged() just forces the wallet to show notifications, and it directly depends on if it is new. regarding balance, if you have a look at GetBalance() you will see the balance is calculated iterating over mapWallet (indices are unique, so a tx can't be iterated over twice). shown balance depends on which outputs are spent/unspent and the transaction status (confirmed/unconfirmed/mempool/etc).

i'd invite you to try my patch proposal from https://github.com/NAVCoin/navcoin-core/pull/438 - i have a node running it since some time (trying to solve another issue of orphans causing wrong balance) and had successful results

mxaddict commented 5 years ago

Ok, it might not be the cause, but I still think it should not invalidate balance cache as well as run the walletnotify command.

I opened up an issue about this on BTC repo to clarify: https://github.com/bitcoin/bitcoin/issues/15781

aguycalled commented 5 years ago

@mxaddict From my tests, https://github.com/NAVCoin/navcoin-core/pull/438 should completely fix this and other issues.

mxaddict commented 5 years ago

Good to know, any ETA on merge for the next release?

On Mon, Apr 15, 2019, 16:17 alex v. notifications@github.com wrote:

@mxaddict https://github.com/mxaddict From my tests, #438 https://github.com/NAVCoin/navcoin-core/pull/438 should completely fix this and other issues.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NAVCoin/navcoin-core/issues/436#issuecomment-483153671, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAwKei6e5HXJelRW6YmslQ5xJXBWqlLks5vhDWHgaJpZM4clM0L .

aguycalled commented 5 years ago

I'd like to see it merged this week and included in the 4.6.0 release.

mxaddict commented 5 years ago

Nice!

On Mon, Apr 15, 2019, 16:28 alex v. notifications@github.com wrote:

I'd like to see it merged this week and included in the 4.6.0 release.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NAVCoin/navcoin-core/issues/436#issuecomment-483157650, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAwKTqZY51804WaOzKQLvJQyY7ssD8Lks5vhDhAgaJpZM4clM0L .